Merge pull request rails#51107 from Shopify/report-handled-action-dispatch-errors-differently

Do not report rendered errors except 500

Author: rafaelfranca

actionpack/lib/action_dispatch/middleware/executor.rb

@@ -14,9 +14,12 @@ def call(env)
       state = @executor.run!(reset: true)
       begin
         response = @app.call(env)
-        if rendered_error = env["action_dispatch.exception"]
-          @executor.error_reporter.report(rendered_error, handled: false, source: "application.action_dispatch")
+
+        if env["action_dispatch.report_exception"]
+          error = env["action_dispatch.exception"]
+          @executor.error_reporter.report(error, handled: false, source: "application.action_dispatch")
         end
+
         returned = response << ::Rack::BodyProxy.new(response.pop) { state.complete! }
       rescue => error
         @executor.error_reporter.report(error, handled: false, source: "application.action_dispatch")

actionpack/lib/action_dispatch/middleware/show_exceptions.rb

@@ -35,6 +35,7 @@ def call(env)
       backtrace_cleaner = request.get_header("action_dispatch.backtrace_cleaner")
       wrapper = ExceptionWrapper.new(backtrace_cleaner, exception)
       request.set_header "action_dispatch.exception", wrapper.unwrapped_exception
+      request.set_header "action_dispatch.report_exception", !wrapper.rescue_response?
 
       if wrapper.show?(request)
         render_exception(request.dup, wrapper)

actionpack/test/dispatch/executor_test.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require "abstract_unit"
+require "active_support/core_ext/object/with"
 
 class ExecutorTest < ActiveSupport::TestCase
   class MyBody < Array
@@ -147,6 +148,28 @@ def test_error_reporting_with_show_exception
     assert_instance_of TypeError, error_report.error
   end
 
+  class BusinessAsUsual < StandardError; end
+
+  def test_handled_error_is_not_reported
+    middleware = Rack::Lint.new(
+      ActionDispatch::Executor.new(
+        ActionDispatch::ShowExceptions.new(
+          Rack::Lint.new(->(_env) { raise BusinessAsUsual }),
+          ->(env) { [418, {}, ["I'm a teapot"]] },
+        ),
+        executor,
+      )
+    )
+
+    env = Rack::MockRequest.env_for("", {})
+    ActionDispatch::ExceptionWrapper.with(rescue_responses: { BusinessAsUsual.name => 418 }) do
+      assert_no_error_reported do
+        response = middleware.call(env)
+        assert_equal 418, response[0]
+      end
+    end
+  end
+
   private
     def call_and_return_body(&block)
       app = block || proc { [200, {}, []] }