Distinguish missing controller exceptions from unrelated NameError

Fix: https://github.com/rails/rails/issues/37650

The classic autoloader used to totally unregister any constant that
failed midway. Which mean `"SomeConst".constantize` was idempotent.

However Zeitwerk rely on normal `Kernel#require` behavior, which mean
that if an exception is raised during a class/module definition,
it will be left incompletely defined. For instance:

```ruby
class FooController
  ::DoesNotExist

  def index
  end
end
```

Will leave `FooController` defined, but without its `index` method.

Because of this, when silencing a NameError, it's important
to make sure the missing constant is really the one we were trying
to load.
This commit is contained in:
Jean Boussier 2019-11-29 13:30:18 +01:00
parent 5c4afb6b52
commit 54878cd44b
5 changed files with 16 additions and 5 deletions

View File

@ -74,7 +74,7 @@ PATH
i18n (>= 1.6, < 2)
minitest (~> 5.1)
tzinfo (~> 1.1)
zeitwerk (~> 2.2)
zeitwerk (~> 2.2, >= 2.2.2)
rails (6.1.0.alpha)
actioncable (= 6.1.0.alpha)
actionmailbox (= 6.1.0.alpha)
@ -529,7 +529,7 @@ GEM
websocket-extensions (0.1.4)
xpath (3.2.0)
nokogiri (~> 1.8)
zeitwerk (2.2.0)
zeitwerk (2.2.2)
PLATFORMS
java

View File

@ -40,6 +40,9 @@ module ActionDispatch
class IllegalStateError < StandardError
end
class MissingController < NameError
end
eager_autoload do
autoload_under "http" do
autoload :ContentSecurityPolicy

View File

@ -98,7 +98,7 @@ module ActionDispatch
def binary_params_for?(controller, action)
controller_class_for(controller).binary_params_for?(action)
rescue NameError
rescue MissingController
false
end

View File

@ -86,7 +86,15 @@ module ActionDispatch
if name
controller_param = name.underscore
const_name = controller_param.camelize << "Controller"
ActiveSupport::Dependencies.constantize(const_name)
begin
ActiveSupport::Dependencies.constantize(const_name)
rescue NameError => error
if error.missing_name == const_name || const_name.start_with?("#{error.missing_name}::")
raise MissingController.new(error.message, error.name)
else
raise
end
end
else
PASS_NOT_FOUND
end

View File

@ -37,5 +37,5 @@ Gem::Specification.new do |s|
s.add_dependency "tzinfo", "~> 1.1"
s.add_dependency "minitest", "~> 5.1"
s.add_dependency "concurrent-ruby", "~> 1.0", ">= 1.0.2"
s.add_dependency "zeitwerk", "~> 2.2"
s.add_dependency "zeitwerk", "~> 2.2", ">= 2.2.2"
end