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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rails 4.2 Regression: before_configuration hook is run too late #19880

Closed
laserlemon opened this issue Apr 23, 2015 · 13 comments

Comments

@laserlemon
Copy link
Contributor

commented Apr 23, 2015

The problem 馃槩

According to the latest Rails guides as of this writing:

before_configuration: This is run as soon as the application constant inherits from Rails::Application. The config calls are evaluated before this happens.

Part of the issue here may be poor wording of this documentation. The meaning of the last sentence isn't entirely clear. My focus for this issue is the first sentence:

This is run as soon as the application constant inherits from Rails::Application.

This was true until recently. In the latest release, before_configuration hooks are not run immediately after the application constant definition/inheritance, but should be.

Rails 4锔忊儯馃憠1锔忊儯

Here's a slimmed down config/application.rb under Rails 4.1.10 to demonstrate:

require "rails"

class EarlyRailtie < Rails::Railtie
  config.before_configuration do
    puts "before_configuration hook"
  end
end

module Example
  class Application < Rails::Application
    puts "before config call"
    config.foo = "bar"
    puts "after config call"
  end
end

When I run bundle exec rails console, I get the following output:

before_configuration hook
before config call
after config call

This is the expected behavior and what's described by the Rails Guides. 馃憤

Rails 4锔忊儯馃憠2锔忊儯

With the same application.rb as above, Rails 4.2.1 produces a different result:

before config call
before_configuration hook
after config call

Rails 4.2 is running before_configuration hooks later than should. 馃憥

It looks like the config method is delegated to instance, which is overridden in Rails::Application to run the load hooks. This lazy evaluation is the cause of the before_configuration hook's delay.

Might it work to initialize the Rails::Application instance immediately upon constant inheritance?

So what? :trollface:

The before_configuration hook is meant to be the earliest hook run during the Rails initialization process where one can count on Rails already being initialized with Rails.root, Rails.env, etc.

Figaro is a Rails configuration library so before_configuration is the logical choice as Figaro makes use of both Rails.root and Rails.env. With the release of Rails 4.2, there have been reports of Figaro not running early enough during Rails initialization, which led me to discover this regression.

Any library that depends on the ability to run as early as possible during Rails initialization could suffer from this behavior, particularly configuration libraries.

Please let me know if I can provide any more information.

Thank you! 馃憦 鉂わ笍 馃弳 馃拹

@rafaelfranca

This comment has been minimized.

Copy link
Member

commented Apr 23, 2015

oh man. I hate these railties problem, thank you so much for finding it.

I'll take a look why it changed and try to fix it but if anyone to do before than me please go ahead.

@rafaelfranca rafaelfranca self-assigned this Apr 23, 2015
@laserlemon

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2015

Thank you for the prompt attention!

@rhuppert

This comment has been minimized.

Copy link

commented Apr 23, 2015

I've been struggling for the past several days with parsing issues with the interpretation of Rails.application.secrets and .bashrc in 4.2.1 and ruby 2.2.2. Perhaps there is a connection?

@pixeltrix

This comment has been minimized.

Copy link
Member

commented Apr 23, 2015

I think it was probably d25fe31 that caused this because it explicitly defers instantiating the application instance so that methods can be overridden.

@kugaevsky

This comment has been minimized.

Copy link

commented May 26, 2015

@laserlemon is there any workaround there? All my application's configuration is based on figaro (thank you for awesome gem) and it's really hard to maintain different yml-files for almost each gem.

@laserlemon

This comment has been minimized.

Copy link
Contributor Author

commented Jun 2, 2015

In Figaro's case, you can call Figaro.load manually immediately after the Rails application class definition, until this regression is patched.

@repinel

This comment has been minimized.

Copy link
Member

commented Aug 7, 2015

As @pixeltrix suspected, I bisected and confirmed it was caused by d25fe31.

@rafaelfranca Do you see a way of running the hooks when inherited without losing the lazy loading feature?

@rails-bot rails-bot added the stale label Dec 23, 2015
@rails-bot

This comment has been minimized.

Copy link

commented Dec 23, 2015

This issue has been automatically marked as stale because it has not been commented on for at least
three months.

The resources of the Rails team are limited, and so we are asking for your help.

If you can still reproduce this error on the 4-2-stable, 4-1-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

@laserlemon

This comment has been minimized.

Copy link
Contributor Author

commented Dec 23, 2015

Oh no! 馃槺 I'll try to look into whether this is still an issue on 4-2-stable and 4-1-stable and report back. Thank you!

@laserlemon

This comment has been minimized.

Copy link
Contributor Author

commented Dec 28, 2015

As originally reported, I can confirm that this is still an issue on the 4-2-stable branch, while the 4-1-stable branch still behaves as expected. In addition, master (0d2675f) is showing the broken behavior. I believe this should be reopened as it's still an issue. Thank you!

@eileencodes

This comment has been minimized.

Copy link
Member

commented Dec 28, 2015

@laserlemon the issue isn't closed, it's just marked as stale. It will be closed in the next round if still stale. I'll add it to pinned so it doesn't end up closed.

Thanks for following up :)

@eileencodes eileencodes added pinned and removed stale labels Dec 28, 2015
@AmitPatel-BoTreeConsulting

This comment has been minimized.

Copy link

commented Mar 1, 2016

I am not sure but I am facing the issue with config gem in Rails 4.2.5 running on Phusion Passenger in production, which could be related to this.

http://stackoverflow.com/questions/35698111/settings-returns-nil-for-all-configurations-which-reads-from-environment-variabl

@maclover7

This comment has been minimized.

Copy link
Member

commented May 4, 2016

@rafaelfranca @tenderlove Do we want to try to work out a solution for this, in time for Rails 5.0? This is regression from 4.1 --> 4.2 馃槵

@maclover7 maclover7 added the regression label May 4, 2016
y-yagi added a commit to y-yagi/rails that referenced this issue Aug 19, 2016
鈥nherits from Rails::Application

Until Rails 4.1, `before_configuration` run as soon as the application constant
inherits from `Rails::Application`.
However, in d25fe31, it has been modified to
run at instantiation process.

This modify to `before_configuration` is run at same timing as to Rails 4.1.

Fixes rails#19880
aliou added a commit to aliou/rails-api that referenced this issue Feb 21, 2017
Rails 4.2.x introduced a regression where `before_configuration` hooks
were no longer run as soon as the Rails Application inherits from
`Rails::Application`. Rails 4.2.8 fixed this regression. However it
seems like it broke applications using calling methods from the
Application class in the `before_configuration` hook as they do not seem
to be defined. This commit is moves the generator setup in the hook
itself.

Rails issue about the regression:
rails/rails#19880

Rails commit introducing the regression:
rails/rails@d25fe31

Rails PR fixing the regression:
rails/rails#26216
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
10 participants
You can鈥檛 perform that action at this time.