mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Handle throwing in controller action in log subscriber
When throw was used in a controller action, and there is matching catch around the request in a Rack middleware, then :exception won't be present in the event payload. This is because ActiveSupport::Notifications::Instrumenter.instrument sets :exception in a rescue handler, but rescue is never called in a throw/catch scenario: catch(:halt) do begin throw :halt rescue Exception => e puts "rescue" # never reached ensure puts "ensure" end end Missing :exception was actually handled prior to Rails 6.1.0, but an optimization updated the code to assume this was present. So this can be considered a regression fix.
This commit is contained in:
parent
dde814e906
commit
53adf53bc5
3 changed files with 17 additions and 1 deletions
|
@ -1,5 +1,9 @@
|
||||||
## Unreleased
|
## Unreleased
|
||||||
|
|
||||||
|
* Fix error in `ActionController::LogSubscriber` that would happen when throwing inside a controller action.
|
||||||
|
|
||||||
|
*Janko Marohnić*
|
||||||
|
|
||||||
* Change the request method to a `GET` when passing failed requests down to `config.exceptions_app`.
|
* Change the request method to a `GET` when passing failed requests down to `config.exceptions_app`.
|
||||||
|
|
||||||
*Alex Robbin*
|
*Alex Robbin*
|
||||||
|
|
|
@ -23,7 +23,7 @@ module ActionController
|
||||||
additions = ActionController::Base.log_process_action(payload)
|
additions = ActionController::Base.log_process_action(payload)
|
||||||
status = payload[:status]
|
status = payload[:status]
|
||||||
|
|
||||||
if status.nil? && (exception_class_name = payload[:exception].first)
|
if status.nil? && (exception_class_name = payload[:exception]&.first)
|
||||||
status = ActionDispatch::ExceptionWrapper.status_code_for_exception(exception_class_name)
|
status = ActionDispatch::ExceptionWrapper.status_code_for_exception(exception_class_name)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -64,6 +64,10 @@ module Another
|
||||||
render inline: "<%= cache_unless(true, 'foo') { 'bar' } %>"
|
render inline: "<%= cache_unless(true, 'foo') { 'bar' } %>"
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def with_throw
|
||||||
|
throw :halt
|
||||||
|
end
|
||||||
|
|
||||||
def with_exception
|
def with_exception
|
||||||
raise Exception
|
raise Exception
|
||||||
end
|
end
|
||||||
|
@ -190,6 +194,14 @@ class ACLogSubscriberTest < ActionController::TestCase
|
||||||
assert_match(/Completed 200 OK in \d+ms/, logs[1])
|
assert_match(/Completed 200 OK in \d+ms/, logs[1])
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def test_process_action_with_throw
|
||||||
|
catch(:halt) do
|
||||||
|
get :with_throw
|
||||||
|
wait
|
||||||
|
end
|
||||||
|
assert_match(/Completed in \d+ms/, logs[1])
|
||||||
|
end
|
||||||
|
|
||||||
def test_append_info_to_payload_is_called_even_with_exception
|
def test_append_info_to_payload_is_called_even_with_exception
|
||||||
begin
|
begin
|
||||||
get :with_exception
|
get :with_exception
|
||||||
|
|
Loading…
Reference in a new issue