salsify/goldiloader

"NameError uninitialized constant Goldiloader" on uninstallation

syamilmj opened this issue · 7 comments

syamilmj commented

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 commented

Hmm. Do you have a stack trace?

jturkel commented

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

syamilmj commented

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 commented

@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 commented

@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.

Drowze commented

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? 😄

  • 👍1
jturkel commented

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 ;)