From 54878cd44b6ad3289bf10103bf8a23ae11ab5d20 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Fri, 29 Nov 2019 13:30:18 +0100 Subject: [PATCH] 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. --- Gemfile.lock | 4 ++-- actionpack/lib/action_dispatch.rb | 3 +++ actionpack/lib/action_dispatch/http/parameters.rb | 2 +- actionpack/lib/action_dispatch/http/request.rb | 10 +++++++++- activesupport/activesupport.gemspec | 2 +- 5 files changed, 16 insertions(+), 5 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 9ef915d39fbe6..50898697243d0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -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 diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 8c7f2a8eca8b7..c539fd6f82ec6 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -40,6 +40,9 @@ module ActionDispatch class IllegalStateError < StandardError end + class MissingController < NameError + end + eager_autoload do autoload_under "http" do autoload :ContentSecurityPolicy diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index 3c16817af3f61..deb499a6a0de3 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -98,7 +98,7 @@ def set_binary_encoding(params, controller, action) def binary_params_for?(controller, action) controller_class_for(controller).binary_params_for?(action) - rescue NameError + rescue MissingController false end diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index baa425c2c64ae..a5d477c76b002 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -86,7 +86,15 @@ def controller_class_for(name) 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 diff --git a/activesupport/activesupport.gemspec b/activesupport/activesupport.gemspec index e3fbcd972dbbe..ec51fbd4837dd 100644 --- a/activesupport/activesupport.gemspec +++ b/activesupport/activesupport.gemspec @@ -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