Skip to content

Commit 26e8cdc

Browse files
authored
Merge pull request rails#54455 from Shopify/report_all_errors
Allow execution wrapper to handle all exceptions
2 parents 57d59a6 + 9ea2930 commit 26e8cdc

File tree

5 files changed

+15
-6
lines changed

5 files changed

+15
-6
lines changed

actionpack/lib/action_dispatch/middleware/executor.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ def call(env)
2121
end
2222

2323
returned = response << ::Rack::BodyProxy.new(response.pop) { state.complete! }
24-
rescue => error
24+
rescue Exception => error
2525
request = ActionDispatch::Request.new env
2626
backtrace_cleaner = request.get_header("action_dispatch.backtrace_cleaner")
2727
wrapper = ExceptionWrapper.new(backtrace_cleaner, error)

actionpack/test/dispatch/executor_test.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,9 @@ def test_body_abandoned
122122

123123
def test_error_reporting
124124
raised_error = nil
125-
error_report = assert_error_reported do
126-
raised_error = assert_raises TypeError do
127-
call_and_return_body { 1 + "1" }
125+
error_report = assert_error_reported(Exception) do
126+
raised_error = assert_raises Exception do
127+
call_and_return_body { raise Exception }
128128
end
129129
end
130130
assert_same raised_error, error_report.error

activesupport/CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,12 @@
1+
* Allow execution wrapping to handle all exceptions
2+
3+
If a more serious error like `SystemStackError` or `NoMemoryError` happens,
4+
the error reporter should be able to report these kinds of exceptions.
5+
Some are not recoverable, but many are, so we should at least try to
6+
report them.
7+
8+
*Gannon McGibbon*
9+
110
* `ActiveSupport::Testing::Parallelization.before_fork_hook` allows declaration of callbacks that
211
are invoked immediately before forking test workers.
312

activesupport/lib/active_support/execution_wrapper.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ def self.wrap(source: "application.active_support")
8989
instance = run!
9090
begin
9191
yield
92-
rescue => error
92+
rescue Exception => error
9393
error_reporter&.report(error, handled: false, source: source)
9494
raise
9595
ensure

activesupport/test/executor_test.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
require_relative "abstract_unit"
44

55
class ExecutorTest < ActiveSupport::TestCase
6-
class DummyError < RuntimeError
6+
class DummyError < Exception
77
end
88

99
class ErrorSubscriber

0 commit comments

Comments
 (0)