Merge pull request rails#45867 from jdufresne/show-rescuable-exceptions

Make the test environment show rescuable exceptions in responses

Author: rafaelfranca

actionmailbox/test/dummy/config/environments/test.rb

@@ -29,7 +29,7 @@
   config.cache_store = :null_store
 
   # Raise exceptions instead of rendering exception templates.
-  config.action_dispatch.show_exceptions = false
+  config.action_dispatch.show_exceptions = :rescuable
 
   # Disable request forgery protection in test environment.
   config.action_controller.allow_forgery_protection = false

actionpack/CHANGELOG.md

@@ -1,3 +1,11 @@
+*   Change `action_dispatch.show_exceptions` to one of `:all`, `:rescuable`, or
+    `:none`. `:all` and `:none` behave the same as the previous `true` and
+    `false` respectively. The new `:rescuable` option will only show exceptions
+    that can be rescued (e.g. `ActiveRecord::RecordNotFound`). `:rescuable` is
+    now the default for the test environment.
+
+    *Jon Dufresne*
+
 *   `config.action_dispatch.cookies_serializer` now accepts `:message_pack` and
     `:message_pack_allow_marshal` as serializers. These serializers require the
     [`msgpack` gem](https://rubygems.org/gems/msgpack) (>= 1.7.0).

actionpack/lib/action_dispatch/http/request.rb

@@ -191,13 +191,6 @@ def http_auth_salt
       get_header "action_dispatch.http_auth_salt"
     end
 
-    def show_exceptions? # :nodoc:
-      # We're treating `nil` as "unset", and we want the default setting to be
-      # `true`. This logic should be extracted to `env_config` and calculated
-      # once.
-      !(get_header("action_dispatch.show_exceptions") == false)
-    end
-
     # Returns a symbol form of the #request_method.
     def request_method_symbol
       HTTP_METHOD_LOOKUP[request_method]

actionpack/lib/action_dispatch/middleware/debug_exceptions.rb

@@ -40,7 +40,7 @@ def call(env)
       wrapper = ExceptionWrapper.new(backtrace_cleaner, exception)
 
       invoke_interceptors(request, exception, wrapper)
-      raise exception unless request.show_exceptions?
+      raise exception unless wrapper.show?(request)
       render_exception(request, exception, wrapper)
     end
 

actionpack/lib/action_dispatch/middleware/exception_wrapper.rb

@@ -174,6 +174,29 @@ def self.status_code_for_exception(class_name)
       Rack::Utils.status_code(@@rescue_responses[class_name])
     end
 
+    def show?(request)
+      # We're treating `nil` as "unset", and we want the default setting to be
+      # `:all`. This logic should be extracted to `env_config` and calculated
+      # once.
+      config = request.get_header("action_dispatch.show_exceptions")
+
+      # Include true and false for backwards compatibility.
+      case config
+      when :none
+        false
+      when :rescuable
+        rescue_response?
+      when true
+        ActionDispatch.deprecator.warn("Setting action_dispatch.show_exceptions to true is deprecated. Set to :all instead.")
+        true
+      when false
+        ActionDispatch.deprecator.warn("Setting action_dispatch.show_exceptions to false is deprecated. Set to :none instead.")
+        false
+      else
+        true
+      end
+    end
+
     def rescue_response?
       @@rescue_responses.key?(exception.class.name)
     end

actionpack/lib/action_dispatch/middleware/show_exceptions.rb

@@ -31,18 +31,18 @@ def call(env)
       @app.call(env)
     rescue Exception => exception
       request = ActionDispatch::Request.new env
-      if request.show_exceptions?
-        render_exception(request, exception)
+      backtrace_cleaner = request.get_header("action_dispatch.backtrace_cleaner")
+      wrapper = ExceptionWrapper.new(backtrace_cleaner, exception)
+      if wrapper.show?(request)
+        render_exception(request, wrapper)
       else
         raise exception
       end
     end
 
     private
-      def render_exception(request, exception)
-        backtrace_cleaner = request.get_header "action_dispatch.backtrace_cleaner"
-        wrapper = ExceptionWrapper.new(backtrace_cleaner, exception)
-        status  = wrapper.status_code
+      def render_exception(request, wrapper)
+        status = wrapper.status_code
         request.set_header "action_dispatch.exception", wrapper.unwrapped_exception
         request.set_header "action_dispatch.original_path", request.path_info
         request.set_header "action_dispatch.original_request_method", request.raw_request_method

actionpack/lib/action_dispatch/railtie.rb

@@ -9,7 +9,7 @@ class Railtie < Rails::Railtie # :nodoc:
     config.action_dispatch = ActiveSupport::OrderedOptions.new
     config.action_dispatch.x_sendfile_header = nil
     config.action_dispatch.ip_spoofing_check = true
-    config.action_dispatch.show_exceptions = true
+    config.action_dispatch.show_exceptions = :all
     config.action_dispatch.tld_length = 1
     config.action_dispatch.ignore_accept_header = false
     config.action_dispatch.rescue_templates = {}

actionpack/test/controller/new_base/render_action_test.rb

@@ -89,7 +89,7 @@ def setup
 
     test "rendering with layout => true" do
       assert_raise(ArgumentError) do
-        get "/render_action/basic/hello_world_with_layout", headers: { "action_dispatch.show_exceptions" => false }
+        get "/render_action/basic/hello_world_with_layout", headers: { "action_dispatch.show_exceptions" => :none }
       end
     end
 
@@ -109,7 +109,7 @@ def setup
 
     test "rendering with layout => 'greetings'" do
       assert_raise(ActionView::MissingTemplate) do
-        get "/render_action/basic/hello_world_with_custom_layout", headers: { "action_dispatch.show_exceptions" => false }
+        get "/render_action/basic/hello_world_with_custom_layout", headers: { "action_dispatch.show_exceptions" => :none }
       end
     end
   end

actionpack/test/controller/new_base/render_test.rb

@@ -79,7 +79,7 @@ class RenderTest < Rack::TestCase
         end
 
         assert_raises(AbstractController::DoubleRenderError) do
-          get "/render/double_render", headers: { "action_dispatch.show_exceptions" => false }
+          get "/render/double_render", headers: { "action_dispatch.show_exceptions" => :none }
         end
       end
     end
@@ -89,13 +89,13 @@ class TestOnlyRenderPublicActions < Rack::TestCase
     # Only public methods on actual controllers are callable actions
     test "raises an exception when a method of Object is called" do
       assert_raises(AbstractController::ActionNotFound) do
-        get "/render/blank_render/clone", headers: { "action_dispatch.show_exceptions" => false }
+        get "/render/blank_render/clone", headers: { "action_dispatch.show_exceptions" => :none }
       end
     end
 
     test "raises an exception when a private method is called" do
       assert_raises(AbstractController::ActionNotFound) do
-        get "/render/blank_render/secretz", headers: { "action_dispatch.show_exceptions" => false }
+        get "/render/blank_render/secretz", headers: { "action_dispatch.show_exceptions" => :none }
       end
     end
   end

actionpack/test/dispatch/content_security_policy_test.rb

@@ -331,7 +331,7 @@ def call(env)
       env["action_dispatch.content_security_policy"] = POLICY
       env["action_dispatch.content_security_policy_nonce_generator"] = proc { "iyhD0Yc0W+c=" }
       env["action_dispatch.content_security_policy_report_only"] = false
-      env["action_dispatch.show_exceptions"] = false
+      env["action_dispatch.show_exceptions"] = :none
 
       @app.call(env)
     end
@@ -481,7 +481,7 @@ def call(env)
       env["action_dispatch.content_security_policy"] = POLICY
       env["action_dispatch.content_security_policy_nonce_generator"] = proc { "iyhD0Yc0W+c=" }
       env["action_dispatch.content_security_policy_report_only"] = false
-      env["action_dispatch.show_exceptions"] = false
+      env["action_dispatch.show_exceptions"] = :none
 
       @app.call(env)
     end
@@ -597,7 +597,7 @@ def call(env)
       env["action_dispatch.content_security_policy"] = nil
       env["action_dispatch.content_security_policy_nonce_generator"] = nil
       env["action_dispatch.content_security_policy_report_only"] = false
-      env["action_dispatch.show_exceptions"] = false
+      env["action_dispatch.show_exceptions"] = :none
 
       @app.call(env)
     end
@@ -653,7 +653,7 @@ def call(env)
       env["action_dispatch.content_security_policy_nonce_generator"] = proc { "iyhD0Yc0W+c=" }
       env["action_dispatch.content_security_policy_report_only"] = false
       env["action_dispatch.content_security_policy_nonce_directives"] = %w(script-src)
-      env["action_dispatch.show_exceptions"] = false
+      env["action_dispatch.show_exceptions"] = :none
 
       @app.call(env)
     end
@@ -725,7 +725,7 @@ def call(env)
       env["action_dispatch.content_security_policy"] = POLICY
       env["action_dispatch.content_security_policy_nonce_generator"] = proc { "iyhD0Yc0W+c=" }
       env["action_dispatch.content_security_policy_report_only"] = false
-      env["action_dispatch.show_exceptions"] = false
+      env["action_dispatch.show_exceptions"] = :none
 
       @app.call(env)
     end