fxn/zeitwerk

New uninitialized constant exception after upgrading from 2.6.0 to 2.6.7 and using Rails concerns

feliperaul opened this issue ยท 6 comments

After a Rails upgrade from 7.0.3.1 to 7.0.4.3 (which bumped zeitwerk from 2.6.0 to 2.6.7), we started having this new uninitialized constant exception (code is simplified for conciseness, so you can copy and paste in your rails console to test):

module FooControllerConcern

  extend ActiveSupport::Concern

  include ActiveStorage::Streaming

  def stream
    DEFAULT_BLOB_STREAMING_DISPOSITION
  end

end

class FooController

  include FooControllerConcern

  def show
    stream
  end

end

So in Zeitwerk 2.6.0:

FooController.new.show
=> "inline"

In Zeitwerk 2.6.7:

FooController.new.show
NameError: uninitialized constant FooControllerConcern::DEFAULT_BLOB_STREAMING_DISPOSITION

    DEFAULT_BLOB_STREAMING_DISPOSITION
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
from (pry):6:in `stream'

As you test in the Rails console by copy and pasting, in Zeitwerk 2.6.0 (Rails 7.0.3.1) the constant DEFAULT_BLOB_STREAMING_DISPOSITION resolved without any issues; it's a constant originally defined in ActiveStorage::Streaming::DEFAULT_BLOB_STREAMING_DISPOSITION, and since FooControllerConcern includes ActiveStorage::Streaming, it appears in self.class.ancestors in FooController.

So after failing to find that constant in the lexical scope, I expected Ruby to traverse the ancestor chain and find the constant, which it did in ActiveStorage::Streaming::DEFAULT_BLOB_STREAMING_DISPOSITION.

However, in Zeitwerk 2.6.7, even tough ActiveStorage::Streaming remains to appear in self.class.ancestors in FooController, it raises uninitialized constant FooController::DEFAULT_BLOB_STREAMING_DISPOSITION in that exact same scenario.

Reading the changelog, I see changes to make Zeitwerk more strict regarding private APIs / methods, but I couldn't pinpoint if this behavior I described above is deliberate or a bug.

Lastly, even tough this now becomes a question, I'm giving it a shot because Xavier is such a nice member of the Ruby comunity and is the expert on constant resolution :)

The top google search for Ruby constant resolution echo the formula "Module.nesting + Module.ancestors"; however, it seems to be a little bit more complex than that in reality, because the code below, that doesn't involve Rails concerns at all, also raises uninitialized constant:

module FooControllerConcern
  def stream
    DEFAULT_BLOB_STREAMING_DISPOSITION
  end
end

class ParentController
  DEFAULT_BLOB_STREAMING_DISPOSITION = "inline"
end

class FooController < ParentController
  include FooControllerConcern

  def show
    stream
  end
end

FooController.new.show

I expected the above code to work, since the constant is defined in one ancestor of FooController, but it doesnt. Considering this, I find that the current behavior (raise uninitialized constant) is cohesive, but I don't understand why it didn't raise before the Zeitwerk update.

Please let me know if I'm out of bounds and this belongs to StackOverflow instead!

fxn commented

Hey! This is weird.

Rails 7 depends on Zeitwerk ~> 2.5, your app can still stick to 2.6.0. Could you pin the Zeitwerk dependency to 2.6.0 in your Gemfile in order to verify which of the two upgrades made this happen? We'll investigate in either case, just to see where should we focus.

@fxn Xavier you were right.

On Rails 7.0.3.1, even if I update Zeitwerk to 2.6.7, the constant is resolved just fine.

The problem starts at Rails 7.0.4, where the code below will raise even with Zeitwerk 2.6.0:

module FooControllerConcern

  extend ActiveSupport::Concern

  include ActiveStorage::Streaming

  def stream
    DEFAULT_BLOB_STREAMING_DISPOSITION
  end

end

class FooController

  include FooControllerConcern

  def show
    stream
  end

end

Zeitwerk::VERSION
=> "2.6.0"

FooController.new.show

=> uninitialized constant FooControllerConcern::DEFAULT_BLOB_STREAMING_DISPOSITION

    DEFAULT_BLOB_STREAMING_DISPOSITION
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
from (pry):5:in `stream'

This does not raise with Rails 7.0.3.1.

fxn commented

Awesome, we've moved forward. Let me investigate!

fxn commented

I could reproduce. It seems to be related to ActiveSupport::Concern, I do not know how yet.

If you add these two traces:

include ActiveStorage::Streaming
p ActiveStorage::Streaming.constants
p ancestors

you'll see

[:DEFAULT_BLOB_STREAMING_DISPOSITION]
[FooControllerConcern]

The first line squares, the second one does not, we just added an ancestor!

Now, if you comment out extend ActiveSupport::Concern, the output is

[:DEFAULT_BLOB_STREAMING_DISPOSITION]
[FooControllerConcern, ActiveStorage::Streaming, ActionController::Live, ActionController::DataStreaming, ActionController::Rendering]

That is the expected one, and the show method does not raise.

I'd suggest opening an issue in the Rails repo, and remove "autoloading" from the equation.

In order to reproduce it is enough to say that if we throw this one file:

module Foo
  extend ActiveSupport::Concern
  include ActiveStorage::Streaming
  p ancestors
end

and then you run

bin/rails r Foo

the ancestors are missing.

Would you be so kind as to opening that ticket and /cc me there please?

fxn commented

Narrowed it down further.

This was introduced by rails/rails#45102. The patch itself is OK as far as this problem is concerned (have not analyzed the patch itself). The issue is that it adds extend ActiveSupport::Concern to ActiveStorage::Streaming, which should be fine on paper, but creates this odd behavior.

So, we have a more specific reproduction without Active Storage. Just throw this file:

# app/models/foo.rb
module Foo
  extend ActiveSupport::Concern
end

module Bar
  extend ActiveSupport::Concern
  include Foo
  p ancestors
end

The command bin/rails r Foo prints [Bar], but it should be [Bar, Foo].

Closer!

I have created rails/rails#47699, closing here.

@fxn Thanks for digging so deep into this, and I'm happy I found an actual inconsistency. Btw, the irony is that rails/rails#45102 is actually my own commit ๐Ÿ˜… Glad there isn't anything wrong with it!

I'll follow #47699.