grape icon indicating copy to clipboard operation
grape copied to clipboard

Uninitialized constant error for nested class of Grape::API class when using bootsnap and sidekiqswarm

Open guigs opened this issue 3 years ago • 3 comments

The error only happens when booting a Rails app in a particular way (sidekiqswarm) when loading a nested class whose parent class inherits from Grape::API.

For example:

# app/api/thing.rb
class Thing < Grape::API; end

# app/api/thing/nested_thing.rb
class Thing
  class NestedThing; end
end

When started with sidekiqswarm, the application crash with:

/home/guigs/.rbenv/versions/3.0.3/lib/ruby/gems/3.0.0/gems/grape-1.6.2/lib/grape/api.rb:87:in `const_get': uninitialized constant #<Class:0x00007f5b1c6cb2d0>::NestedThing (NameError)
Did you mean?  MyApi::Thing::NestedThing
        from /home/guigs/.rbenv/versions/3.0.3/lib/ruby/gems/3.0.0/gems/grape-1.6.2/lib/grape/api.rb:87:in `const_missing'
        from /home/guigs/grapeboot/config/routes.rb:8:in `block in <main>'
        from /home/guigs/.rbenv/versions/3.0.3/lib/ruby/gems/3.0.0/gems/actionpack-7.0.3/lib/action_dispatch/routing/route_set.rb:428:in `instance_exec'
        from /home/guigs/.rbenv/versions/3.0.3/lib/ruby/gems/3.0.0/gems/actionpack-7.0.3/lib/action_dispatch/routing/route_set.rb:428:in `eval_block'
        from /home/guigs/.rbenv/versions/3.0.3/lib/ruby/gems/3.0.0/gems/actionpack-7.0.3/lib/action_dispatch/routing/route_set.rb:410:in `draw'
        from /home/guigs/grapeboot/config/routes.rb:1:in `<main>'
        from /home/guigs/.rbenv/versions/3.0.3/lib/ruby/gems/3.0.0/gems/bootsnap-1.11.1/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:39:in `load'

The problem does not happen if DISABLE_BOOTSNAP_LOAD_PATH_CACHE=1 is used.

I've created a minimum Rails app to demonstrate the problem: https://github.com/heyjobs/grapeboot

Related Sidekiq issue: https://github.com/mperham/sidekiq/issues/5346 Related Bootsnap issue: https://github.com/Shopify/bootsnap/issues/416

This has been fixed now in Bootsnap, but it was suggested that behavior should be fixed in Grape.

guigs avatar May 30 '22 16:05 guigs

To clarify, this is the constant I have a problem with: https://github.com/ruby-grape/grape/blob/a6cde5d5fb00ea7cf34f7adfbe81dcd2b3958e8d/lib/grape/api.rb#L11

If any code loaded after grape/api.rb defines some methods on Module or Class they end up being gimped. I think grape could do without that cache, just call Class.instance_methods every time.

casperisfine avatar May 30 '22 16:05 casperisfine

@casperisfine Makes sense. Try adding an integration test in Grape and fix this?

dblock avatar Jun 02 '22 13:06 dblock

I may do it at some point, but it's super low priority for me as I'm not a grape user.

casperisfine avatar Jun 02 '22 13:06 casperisfine

I came across this while looking into a Zeitwerk thing, but given the available information this seems like implementation error to me.

Here: https://github.com/heyjobs/grapeboot/tree/main/app/api

You are defining:

# app/api/thing.rb
class Thing < Grape::API
end

and

# app/api/thing/nested_thing.rb
class Thing
  class NestedThing
  end
end

Given that the inheritance from Grape::API is commented out I imagine you were trying it with that value present at some point, but without it that inner class is not inheriting from Grape::API. For example:

class Thing < Grape::API
end
class Thing
  class NestedThing
  end
end

# subclass <= superclass will evaluate to true:
Thing <= Grape::API
=> true                  
Thing::NestedThing <= Grape::API
=> nil
Thing::NestedThing <= Thing
=> nil
Thing::NestedThing <= Object
=> true

Apart from Thing::NestedThing not being defined as a subclass of Grape::API, the non-commented out content of your config/routes.rb file is referencing the class name in a void context:

Rails.application.routes.draw do
  Thing::NestedThing
end

The Grape docu will tell you to either use #compile! or to mount your top-level API class there, so I'm not sure why it's written that way but it is another reason I would expect this configuration to not work.

Your original comment mentions MyApi which isn't in your test project, and in the history it looks like you had a folder named MyApi but never actually defined it as a module? I have to wonder if this wasn't a combination of not defining the module/class/inheritance/routing correctly (as demonstrated above) and improper directory structure given the files that were defined (which is a common trigger for this issue, e.g., https://stackoverflow.com/a/29307359 and needing to have the structure app/api/api/*). While I understand a change was made in https://github.com/Shopify/bootsnap/issues/416 that may have stopped it from erroring, it seems like the implementation triggering it was not utilizing grape correctly.

allisonphillips avatar May 13 '23 08:05 allisonphillips

Thanks for the detailed call outs @allisonphillips! @guigs care to check it out?

dblock avatar May 13 '23 13:05 dblock

@allisonphillips Thank you for investigating. You're right in your analysis.

I deliberate removed the inheritance from NestedThing and removed the mounting in order to "simplify" my demonstration, to show that neither the mounting nor the inheritance in NestedThing are cause of this bug.

These were actually from of my attempts to debug this issue and a way to rule out they were not the cause.

But now I see how this can make it more confusing, as it is not doing the proper thing.

I just added back the inheritance and the mounting and the problem can still be reproduced.

guigs avatar May 15 '23 20:05 guigs

Sorry, totally forgot about this one. Proposed fix: https://github.com/ruby-grape/grape/pull/2328

casperisfine avatar May 16 '23 05:05 casperisfine