Do not report rendered errors except 500

In `4067c9565a5da78a72e375a2d959000147f02c34` `ActionDispatch::Executor`
started to report all errors, even the ones that were "handled" by the application.
This leads to errors like `ActionController::RoutingError` polluting error trackers
while not being actionable since they do not represent an exceptional situation.

This commit changes the behavior to only report errors that are not
considered "handled" based on the `ActionDispatch::ExceptionWrapper.rescue_responses` list.
This commit is contained in:
Nikita Vasilevsky 2024-02-16 17:34:26 +00:00 committed by Rafael Mendonça França
parent c676398d5e
commit a8d1d927e8
No known key found for this signature in database
GPG Key ID: FC23B6D0F1EEE948
3 changed files with 29 additions and 2 deletions

View File

@ -14,9 +14,12 @@ module ActionDispatch
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")

View File

@ -35,6 +35,7 @@ module ActionDispatch
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)

View File

@ -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 @@ class ExecutorTest < ActiveSupport::TestCase
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, {}, []] }