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

Rails 6.1 Constant autoloading not working in environment configuration files #40904

Closed
samstickland opened this issue Dec 21, 2020 · 24 comments
Closed

Comments

@samstickland
Copy link

Steps to reproduce

Referencing an autoloaded constant in an environment configuration (e.g. config/environments/development.rb) results in a NameError ("uninitialized constant"). This worked correctly in Rails 6.0.3.4.

An example repo demonstrating the problem is available here: https://github.com/samstickland/rails-6.1-constant-resolution-issue . Running "rails c" is not possible in this repo with Rails 6.1. The first commit in the repo is the result of "rails new" and the second demonstrates the issue

Expected behavior

The constant should be resolved

Actual behavior

The constant is not resolved.

System configuration

Rails version: 6.1.0

Ruby version: 2.7.2

@abhaynikam
Copy link
Contributor

@samstickland I think Rails 6.1 doesn't eager load lib/* in zeitwerk mode. By any chance, were you using :classic loader in Rails 6.0.3.4 version?

In the lib directory, zeitwerk would only autoload: lib/assets/* and lib/tasks/**/*.rake. Trying running following to find which paths are autoloaded for your application:

bin/rails runner 'puts ActiveSupport::Dependencies.autoload_paths'

@samstickland
Copy link
Author

It's in app/lib, not lib, which is present in the autoloading paths:

$ bin/rails runner 'puts ActiveSupport::Dependencies.autoload_paths'
Running via Spring preloader in process 49020
/Users/samuelstickland/development/hubbado/constant-issue/app/channels
/Users/samuelstickland/development/hubbado/constant-issue/app/controllers
/Users/samuelstickland/development/hubbado/constant-issue/app/controllers/concerns
/Users/samuelstickland/development/hubbado/constant-issue/app/helpers
/Users/samuelstickland/development/hubbado/constant-issue/app/jobs
/Users/samuelstickland/development/hubbado/constant-issue/app/lib
/Users/samuelstickland/development/hubbado/constant-issue/app/mailers
/Users/samuelstickland/development/hubbado/constant-issue/app/models
/Users/samuelstickland/development/hubbado/constant-issue/app/models/concerns
/Users/samuelstickland/.gem/ruby/2.7.2/gems/actiontext-6.1.0/app/helpers
/Users/samuelstickland/.gem/ruby/2.7.2/gems/actiontext-6.1.0/app/models
/Users/samuelstickland/.gem/ruby/2.7.2/gems/actionmailbox-6.1.0/app/controllers
/Users/samuelstickland/.gem/ruby/2.7.2/gems/actionmailbox-6.1.0/app/jobs
/Users/samuelstickland/.gem/ruby/2.7.2/gems/actionmailbox-6.1.0/app/models
/Users/samuelstickland/.gem/ruby/2.7.2/gems/activestorage-6.1.0/app/controllers
/Users/samuelstickland/.gem/ruby/2.7.2/gems/activestorage-6.1.0/app/controllers/concerns
/Users/samuelstickland/.gem/ruby/2.7.2/gems/activestorage-6.1.0/app/jobs
/Users/samuelstickland/.gem/ruby/2.7.2/gems/activestorage-6.1.0/app/models
/Users/samuelstickland/development/hubbado/constant-issue/test/mailers/previews

Additionally if I remove the line from config/environments/development.rb then I can start the rails console and at the console that constant is available, as expected.

@composerinteralia
Copy link
Member

Thanks for the issue, and for the example repo. I believe this was an intentional breaking change for Rails 6.1. This would have worked in Rails 6.0, but it would have output a deprecation warning. I double checked using your example repo back on Rails 6.0.3.4 and I can see this in log/development.log:

DEPRECATION WARNING: Initialization autoloaded the constants MyNamespace and MyNamespace::MyClass.

Being able to do this is deprecated. Autoloading during initialization is going
to be an error condition in future versions of Rails.

Reloading does not reboot the application, and therefore code executed during
initialization does not run again. So, if you reload MyNamespace, for example,
the expected changes won't be reflected in that stale Module object.

These autoloaded constants have been unloaded.

Please, check the "Autoloading and Reloading Constants" guide for solutions.
 (called from <main> at rails-6.1-constant-resolution-issue/config/environment.rb:5)

In general I would recommend addressing all deprecation warnings before upgrading to the next version.

@composerinteralia
Copy link
Member

Actually, I still see that deprecation warning in the code, so I am probably missing something here.

@samstickland
Copy link
Author

@composerinteralia Oh interesting.. The deprecation warning made it into the log, but I would had expected to had seen it printed before the console startup too. As it was I had entirely missed it.

@composerinteralia
Copy link
Member

composerinteralia commented Dec 22, 2020

Looks like 43863bf changed this behavior, so I think I was wrong about it being an intentional breaking change. But it would have eventually been an intentional breaking change 🤣.

@glinesbdev
Copy link

I'm also experiencing this issue inside of config/environments/production.rb. I can see that the bin/rails runner 'puts ActiveSupport::Dependencies.autoload_paths' command includes the folder that I want to load but I'm getting an NameError: uninitialized constant error. The path of my class I want to use is in app/services. Is there a working workaround for this issue?

@samstickland
Copy link
Author

@glinesbdev In my case I was able to move the objects referenced out of the autoloading paths (we keep autoloading off on ./lib for just these sort of use cases)

If that's not possible you might be able to move required setup to an initializer instead?

@glinesbdev
Copy link

glinesbdev commented Dec 23, 2020

The problem is that we use the code in that path in our application as well so it needs to be auto loaded. The other problem is that I need to use that code to add production env config. I'll have to research / test setting those settings in initializers instead of environments.

@rafaelfranca
Copy link
Member

cc @fxn

@fxn
Copy link
Member

fxn commented Dec 29, 2020

Yes. The reason for this is explained in the warning message.

MyNamespace::MyClass is reloadable because it's in app/lib. Whatever you do in config/environments/development.rb with MyNamespace::MyClass is not going to be reflected if you edit its implementation, because those initialization files do not run again.

Options:

  1. If you do not need this class to be reloadable, please move it to lib and issue a require call in config/environments/*.rb. If you change its code, a server restart is needed.
  2. If you need it to be reloadable, wrap the code in a to_prepare block, or upgrade to Zeitwerk 2.4.2 and set an on_load callback on Rails.autoloaders.main.

@fxn
Copy link
Member

fxn commented Dec 29, 2020

@glinesbdev Please do not move that to the initializers, it is still the same, and it will err in the future. The err turn arrived before to config/environments because atoloading here is way less common in comparison.

@rafaelfranca
Copy link
Member

@fxn is the autoload disable in 6.1 already or only will be in 6.2? The issue here is that people are getting NameError instead of the deprecation. If it is expected to raise in 6.1 we can close this issue.

@fxn
Copy link
Member

fxn commented Dec 29, 2020

The warning shipped with 6.0.0.

In 6.1 you cannot autoload in config/environments/*.rb, that is the first change. You still can autoload in config/initializers/*.rb to ease the transition, but you should not, and it will err in Rails 7 (at least, that is the plan).

We can close this one.

@fxn fxn closed this as completed Dec 29, 2020
@rromanchuk
Copy link

I actually didn't understand wrap the code in a to_prepare block documentation until just now. I think because i didn't read the setup example above and i nope'd when i saw examples with global assignments $PAYMENT_GATEWAY and environment branching.

In my case different environments use different formatting, but also instantiated and referenced in other contexts outside environment booting.

# staging.rb
config.log_formatter = SpecialJsonFormatter.new # app/lib/special_json_formatter.rb

You might see a lot of duplicates of this tickets rolling in (ex, see suggested autoloading below), so it might help modify the title a bit to help search. I only got here after realizing it was unique to environment file usage, and double checking release notes. I just assumed stale/corrupted bootsnap cache, or documented zeitwerk change, but bin/rails zeitwerk:check was passing etc.

Sidekiq - Troubleshooting autoloading

@fxn
Copy link
Member

fxn commented Dec 30, 2020

@rromanchuk Ah! In that case the proper solution is to move your code to lib and issue a require call.

When the application boots, config is initialized. But the actual affected component does not operate with config at runtime in general, and if it does, that is private implementation. Normally, config is just a proxy that transports user configuration into internal component state. This is schematically what happens:

Let's imagine railtie foo provides Foo::Service, which has a class attribute endpoint. As a Ruby programmer developing foo you have

class Foo::Service
  cattr_accessor :endpoint
end

OK. In general, the internal way to represent things is not exposed, the public interface to it goes via config, an indirection, so that railtie has an initializer that does this:

initializer "init.foo" do
  Foo::Service.endpoint = config.foo.endpoint
end

so to speak.

That is tied to the boot process, initializers run only once. If you change config.foo.endpoint on reload via to_prepare, that does not run the initializer again. The endpoint value should not be reloadable, because it is used in a way that makes reloading pointless. See what I mean?

So, better require from lib, so that it is clear that SpecialJsonFormatter is a class whose modification needs a server restart.

@fxn
Copy link
Member

fxn commented Dec 30, 2020

@rromanchuk Let me add that I explained this to help understand the reason why this is the best option. However, I do not assume you had to know this, I believe this is not well documented. Maybe we need a clear contract of what can be done and what cannot be done at each stage of the boot process.

@jrochkind
Copy link
Contributor

I've run into this exact issue, helpful to find it it in github, great! Still a bit confused about what's going on.

Let's say I have something in app/*/ that I want to access in an environment configuration file.

OK, it won't autoload.... do I have any other way to use it? I think it's not allowed to somehow "manually" load things that are set to be auto-loaded...

So Rails 6.1 just comes with a restriction that you can't reference any files controlled by auto-loading in environment configuration files? This is challenging for me. I do a lot of this, and it's hard for me to avoid doing it and stay DRY.

I hadn't noticed the deprecation in rails 6.0, but went and looked at my logs and confirmed it was there.

You still can autoload in config/initializers/*.rb to ease the transition, but you should not, and it will err in Rails 7 (at least, that is the plan).

OK, so don't do that as a workaround. So... there is no workaround? I'm just not allowed to reference any of my app's classes in initialization/configuration code?

One common case I am seeing in doing this is setting configuration for a gem SomeGem::Config.something = in an initializer, when the SomeGem::Config class is set by that gem to be auto-loaded. So... this is actually a bug in SomeGem, it can not have SomeGem::Config be in an auto-loaded directory and also have apps use it in initializers, do I have this right?

This is... challenging. I'm rather confused.

jrochkind added a commit to sciencehistory/kithe that referenced this issue Jan 11, 2021
This makes it easier to use in working and non-deprecated way in Rails initialization code, which is one of it's main use cases.

Changes to Rails auto-loading/zeitwerk make it so that you should not be referencing auto-loaded code in Rails configuration/initialization phases.

rails/rails#40904

As we now explicitly require kithe/config_base in the kithe/engine file, this change should be transparent and backwards-compatible.
@jrochkind
Copy link
Contributor

The Rails Guide referenced in the deprecation notice is pretty good, recommended. I'm starting to wrap my head around it.

https://guides.rubyonrails.org/autoloading_and_reloading_constants.html#reloading-and-stale-objects

@jrochkind
Copy link
Contributor

OK, so I want to reference some custom logic in config/initializers.

It can't be auto-loaded, so I put my custom class in local app ./lib.

I'm having trouble figuring out how to reference that in eg ./config/application.rb. lib does not seem to be on the load path yet at the top of that file, so I can't just require 'my_thing'. It gets worse if I want to reference it in say ERB in a database.yml of course.

If anyone has any thoughts or hints for patterns here, it would be appreciated. Makes sense not to reference auto-loaded things in config/initializer; but it makes sense that we should be able to reference custom local classes somehow, and i'm having trouble with it.

@fxn
Copy link
Member

fxn commented Jan 12, 2021

@jrochkind lib is added to $LOAD_PATH as part of the application initialization.

The part of the boot process that is relevant for this happens when Rails::Application is subclassed. That means that you have it available inside the body of your application class defined in config/application.rb, and from that point anywhere else afterwards (initializers, the file where it is used, etc.).

@jrochkind
Copy link
Contributor

jrochkind commented Jan 12, 2021

@fxn Thanks! Weirdly, I could not get this working in the "naive" approach, I'm not sure if that is expected or not. But I did figure out a way that worked for me.

I have an app we'll call MyApp. It has a class MyApp::Helper at ./lib/my_app/helper, which I want to use in ./config/environments/test.rb and ./config/environments/production.rb, as well as in some files in ./config/initializers/*

I tried putting require "my_app/helper" at the top of ./config/application.rb, but this resulted in an error on app boot LoadError: cannot load such file -- my_app/helper

Maybe this is actually expected, at the point ./config/application.rb itself is loaded, things aren't set up sufficiently?

It appears to work if I added require "my_app/helper" to the top of both ./config/environments/test.rb and ./config/environments/production.rb. As well as, obviously, any other environments I might want to use, production, or if I've defined any custom ones.

But since I'm also using MyApp::Helper in initializers, which aren't tied to any particular environment, I really wanted to put the require in a single global place, instead of having to remember to include it in every environments/*.rb I might have.

Putting it in a config.before_configuration block in config/application.rb also seemed to do the trick.

module MyApp
  class Application < Rails::Application

    config.before_configuration do
      require 'my_app/helper'
    end
  end
end

Sorry if this is distracting or if I'm still confused about something. But it took me a day or so to get through this and figure out what would work, so thought I would leave this for others. Also curious for any feedback. I think advice and patterns for moving custom classes needed in boot to non-auto-loaded locations will be a thing others find themselves needing.

@fxn
Copy link
Member

fxn commented Jan 12, 2021

@jrochkind before the line with the class keyword, the boot process is mostly loading dependencies. When Rails::Application has been subclassed, you have lib. That means you can simply write

class MyApp::Application < Rails::Application
  # lib is in $LOAD_PATH now.
  require "my_app/helper"
  ...
end

Alternatively, you can also use require_relative, in which case you do not depend on $LOAD_PATH, and can be put at the top:

require_relative "../lib/my_app/helper"

but it is, perhaps, uglier.

@jrochkind
Copy link
Contributor

Thanks @fxn! Confirmed that worked, the require inside Application class body.

I don't know why I didn't try that first, I thought I did, but clearly not! It is a little bit more straightforward than the config.before_configuration rigamarole.

Hopefully this will be findable and help others trying to move from using auto-loaded classes in configuration/initialization process. The little gotchas made it take more time than I thought it would.

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

8 participants