Skip to content

Commit

Permalink
ActionDispatch::Executor: report errors handled by ShowExceptions
Browse files Browse the repository at this point in the history
Fix: rails#51002

In the default middleware stack, the `ShowExceptions` middleware is
lower than `ActionDispatch::Execturor` and will handle most exceptions
causing `Executor` not to witness any.

Instead we need to rely on `action_dispatch.exception` being added
into the request env.
  • Loading branch information
byroot committed Feb 12, 2024
1 parent 9940dc8 commit 3d29555
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 6 deletions.
3 changes: 3 additions & 0 deletions actionpack/lib/action_dispatch/middleware/executor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ 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")
end
returned = response << ::Rack::BodyProxy.new(response.pop) { state.complete! }
rescue => error
@executor.error_reporter.report(error, handled: false, source: "application.action_dispatch")
Expand Down
15 changes: 9 additions & 6 deletions actionpack/lib/action_dispatch/middleware/show_exceptions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,15 @@ def call(env)
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
fallback_to_html_format_if_invalid_mime_type(request)
request.path_info = "/#{status}"
request.request_method = "GET"
response = @exceptions_app.call(request.env)

sub_request = request.dup
sub_request.set_header "action_dispatch.original_path", request.path_info
sub_request.set_header "action_dispatch.original_request_method", request.raw_request_method

fallback_to_html_format_if_invalid_mime_type(sub_request)
sub_request.path_info = "/#{status}"
sub_request.request_method = "GET"
response = @exceptions_app.call(sub_request.env)
response[1][Constants::X_CASCADE] == "pass" ? pass_response(status) : response
rescue Exception => failsafe_error
$stderr.puts "Error during failsafe response: #{failsafe_error}\n #{failsafe_error.backtrace * "\n "}"
Expand Down
27 changes: 27 additions & 0 deletions actionpack/test/dispatch/executor_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,33 @@ def test_body_abandoned
assert_equal requests_count - 1, completed
end

def test_error_reporting
raised_error = nil
error_report = assert_error_reported do
raised_error = assert_raises TypeError do
call_and_return_body { 1 + "1" }
end
end
assert_same raised_error, error_report.error
end

def test_error_reporting_with_show_exception
middleware = Rack::Lint.new(
ActionDispatch::Executor.new(
ActionDispatch::ShowExceptions.new(
Rack::Lint.new(->(_env) { 1 + "1" }),
->(_env) { [500, {}, ["Oops"]] },
),
executor,
)
)

env = Rack::MockRequest.env_for("", {})
error_report = assert_error_reported do
middleware.call(env)
end
end

private
def call_and_return_body(&block)
app = block || proc { [200, {}, []] }
Expand Down

0 comments on commit 3d29555

Please sign in to comment.