Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"NameError uninitialized constant Goldiloader" on uninstallation #33

Open
syamilmj opened this issue Jul 19, 2016 · 7 comments
Open

"NameError uninitialized constant Goldiloader" on uninstallation #33

syamilmj opened this issue Jul 19, 2016 · 7 comments
Assignees

Comments

@syamilmj
Copy link

Hi,

First of all, thank you for this great gem.

Some of our endpoints are being cached (using Rails.cache) and they failed with Dalli::RingError: No server available upon uninstalling goldiloader.

Checking the log, I saw this:

NameError uninitialized constant Goldiloader

Clearing up the cache brings back the server but I'm curious as to how I can prevent this in the future?

@jturkel
Copy link
Member

jturkel commented Jul 19, 2016

Hmm. Do you have a stack trace?

@jturkel jturkel self-assigned this Jul 20, 2016
@jturkel
Copy link
Member

jturkel commented Sep 7, 2016

@syamilmj - Do you have a stack trace and/or a reproducible test case?

@syamilmj
Copy link
Author

Hey jturkel, sorry for not replying earlier.

Unfortunately I no longer have the log. Here's a sample code to reproduce:

Rails.cache.fetch("mykey", expires_in: 15.minutes) do
   Post.recently_published.with_top_comments
end

We're using Amazon Elasticache for memcached, and running EC2 servers.

@jturkel
Copy link
Member

jturkel commented Sep 28, 2016

@syamilmj - Do you have a minimal Rails app that can reproduce the issue? I'm happy to look into it if I can get a reproducible test case.

@jturkel
Copy link
Member

jturkel commented Feb 7, 2017

@syamilmj - I'm closing this due to lack of activity. Please reopen if it if you can provide a reproducible test case and/or a stack trace.

@jturkel jturkel closed this as completed Feb 7, 2017
@Drowze
Copy link

Drowze commented Feb 28, 2023

I can confirm this is still an issue (at least with Rials 6.1 and redis cache store, didn't try others).

Here's a minimal repro app: https://github.com/Drowze/test-goldiloader-uninstall-issue
Error raised:

irb(main):001:0> Rails.cache.read("foo")
/Users/Drowze/.gem/ruby/3.2.0/gems/activesupport-6.1.7.2/lib/active_support/inflector/methods.rb:273:in `const_get': uninitialized constant Goldiloader (NameError)

        Object.const_get(camel_cased_word)
              ^^^^^^^^^^
Did you mean?  TestGoldiloader
<internal:marshal>:34:in `load': undefined class/module Goldiloader:: (ArgumentError)

@jturkel can we reopen this issue? 😄

@jturkel jturkel reopened this Feb 28, 2023
@jturkel
Copy link
Member

jturkel commented Feb 28, 2023

Thank you for the reproducible test case @Drowze! It looks like the problem comes from caching the results of Marshal.dump which includes the @auto_include_context variable of type Goldiloader::AutoIncludeContext defined here. Deserializing that object will fail if Goldiloader::AutoIncludeContext is no longer present. A quick workaround is to define the Goldiloader::AutoIncludeContext class after removing the goldiloader gem or just disable automatic eager loading rather than uninstalling the gem depending on your use case.

I know Rails says to avoid caching ActiveRecord instances here (and Shopify has a great post on why caching using Marshal is dangerous and alternatives) but I suspect this happens fairly frequently so I'd love to figure out an easier path for uninstalling this gem. I can think of two possibilities:

  1. Figure out how to get Marshal to not serialize @auto_include_context.
  2. Change @auto_include_context to just be an array so it can safely deserialize without Goldiloader (or really define a new instance variable to be compatible with existing cache entries).

Option 1 is definitely preferably since the data structure will bloat the size of the cache entires but I'll have to investigate the feasibility. Neither option will help with existing cache entries though. I'll investigate a bit more over the next few weeks but PRs are always welcome ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants