Merge pull request rails#53964 from byroot/missing-controller-500

Return a 500 on MissingController errors

Author: byroot

actionpack/CHANGELOG.md

@@ -166,6 +166,19 @@
 
     *Jeremy Green*
 
+*   A route pointing to a non-existing controller now returns a 500 instead of a 404.
+
+    A controller not existing isn't a routing error that should result
+    in a 404, but a programming error that should result in a 500 and
+    be reported.
+
+    Until recently, this was hard to untangle because of the support
+    for dynamic `:controller` segment in routes, but since this is
+    deprecated and will be removed in Rails 8.1, we can now easily
+    not consider missing controllers as routing errors.
+
+    *Jean Boussier*
+
 *   Add `check_collisions` option to `ActionDispatch::Session::CacheStore`.
 
     Newly generated session ids use 128 bits of randomness, which is more than

actionpack/lib/action_dispatch/routing/route_set.rb

@@ -59,8 +59,6 @@ def serve(req)
         private
           def controller(req)
             req.controller_class
-          rescue NameError => e
-            raise ActionController::RoutingError, e.message, e.backtrace
           end
 
           def dispatch(controller, action, req, res)

actionpack/test/controller/routing_test.rb

@@ -1283,10 +1283,12 @@ def test_route_constraints_with_options_method_condition_is_valid
 
   def test_route_error_with_missing_controller
     set.draw do
-      get    "/people" => "missing#index"
+      get    "/people" => "does_not_exists#index"
     end
 
-    assert_raises(ActionController::RoutingError) { request_path_params "/people" }
+    assert_raises(ActionDispatch::MissingController, match: "uninitialized constant DoesNotExistsController") do
+      request_path_params "/people"
+    end
   end
 
   def test_recognize_with_encoded_id_and_regex

actionpack/test/dispatch/routing_test.rb

@@ -4998,49 +4998,6 @@ def test_positional_args_with_format_false
   end
 end
 
-class TestErrorsInController < ActionDispatch::IntegrationTest
-  class ::PostsController < ActionController::Base
-    def foo
-      nil.i_do_not_exist
-    end
-
-    def bar
-      NonExistingClass.new
-    end
-  end
-
-  Routes = ActionDispatch::Routing::RouteSet.new
-  Routes.draw do
-    ActionDispatch.deprecator.silence do
-      get "/:controller(/:action)"
-    end
-  end
-
-  APP = build_app Routes
-
-  def app
-    APP
-  end
-
-  def test_legit_no_method_errors_are_not_caught
-    get "/posts/foo"
-    assert_equal 500, response.status
-  end
-
-  def test_legit_name_errors_are_not_caught
-    get "/posts/bar"
-    assert_equal 500, response.status
-  end
-
-  def test_legit_routing_not_found_responses
-    get "/posts/baz"
-    assert_equal 404, response.status
-
-    get "/i_do_not_exist"
-    assert_equal 404, response.status
-  end
-end
-
 class TestPartialDynamicPathSegments < ActionDispatch::IntegrationTest
   Routes = ActionDispatch::Routing::RouteSet.new
   Routes.draw do