diff options
author | Sean Griffin <sean@seantheprogrammer.com> | 2016-03-11 10:14:50 -0700 |
---|---|---|
committer | Sean Griffin <sean@seantheprogrammer.com> | 2016-03-11 10:53:01 -0700 |
commit | b76b817629bf1504e3109c9872b250513690561d (patch) | |
tree | 8f9f28e60a6b7f46020b6a4bc4dde39742f916ca | |
parent | 0ff6eb34892514b811077ca7dd39340c96836f75 (diff) | |
download | rails-b76b817629bf1504e3109c9872b250513690561d.tar.gz rails-b76b817629bf1504e3109c9872b250513690561d.tar.bz2 rails-b76b817629bf1504e3109c9872b250513690561d.zip |
Use the most highest priority exception handler when cause is set
There was some subtle breakage caused by #18774, when we removed
`#original_exception` in favor of `#cause`. However, `#cause` is
automatically set by Ruby when raising an exception from a rescue block.
With this change, we will use whichever handler has the highest priority
(whichever call to `rescue_from` came last). In cases where the outer
has lower precidence than the cause, but the outer is what should be
handled, cause will need to be explicitly unset.
Fixes #23925
-rw-r--r-- | actionpack/lib/action_controller/metal/rescue.rb | 8 | ||||
-rw-r--r-- | actionpack/test/controller/rescue_test.rb | 33 | ||||
-rw-r--r-- | activesupport/lib/active_support/rescuable.rb | 10 |
3 files changed, 49 insertions, 2 deletions
diff --git a/actionpack/lib/action_controller/metal/rescue.rb b/actionpack/lib/action_controller/metal/rescue.rb index 81b9a7b9ed..0621a7368c 100644 --- a/actionpack/lib/action_controller/metal/rescue.rb +++ b/actionpack/lib/action_controller/metal/rescue.rb @@ -7,8 +7,12 @@ module ActionController #:nodoc: include ActiveSupport::Rescuable def rescue_with_handler(exception) - if exception.cause && handler_for_rescue(exception.cause) - exception = exception.cause + if exception.cause + handler_index = index_of_handler_for_rescue(exception) || Float::INFINITY + cause_handler_index = index_of_handler_for_rescue(exception.cause) + if cause_handler_index && cause_handler_index <= handler_index + exception = exception.cause + end end super(exception) end diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb index f42bef883f..ed78f859ce 100644 --- a/actionpack/test/controller/rescue_test.rb +++ b/actionpack/test/controller/rescue_test.rb @@ -147,6 +147,24 @@ class RescueController < ActionController::Base end end + def exception_with_more_specific_handler_for_wrapper + raise RecordInvalid + rescue + raise NotAuthorized + end + + def exception_with_more_specific_handler_for_cause + raise NotAuthorized + rescue + raise RecordInvalid + end + + def exception_with_no_handler_for_wrapper + raise RecordInvalid + rescue + raise RangeError + end + protected def deny_access head :forbidden @@ -301,6 +319,21 @@ class RescueControllerTest < ActionController::TestCase get :resource_unavailable_raise_as_string assert_equal "RescueController::ResourceUnavailableToRescueAsString", @response.body end + + test 'rescue when wrapper has more specific handler than cause' do + get :exception_with_more_specific_handler_for_wrapper + assert_response :unprocessable_entity + end + + test 'rescue when cause has more specific handler than wrapper' do + get :exception_with_more_specific_handler_for_cause + assert_response :unprocessable_entity + end + + test 'rescue when cause has handler, but wrapper doesnt' do + get :exception_with_no_handler_for_wrapper + assert_response :unprocessable_entity + end end class RescueTest < ActionDispatch::IntegrationTest diff --git a/activesupport/lib/active_support/rescuable.rb b/activesupport/lib/active_support/rescuable.rb index fcf5553061..73bc52b56f 100644 --- a/activesupport/lib/active_support/rescuable.rb +++ b/activesupport/lib/active_support/rescuable.rb @@ -115,5 +115,15 @@ module ActiveSupport end end end + + def index_of_handler_for_rescue(exception) + handlers = self.class.rescue_handlers.reverse_each.with_index + _, index = handlers.detect do |(klass_name, _), _| + klass = self.class.const_get(klass_name) rescue nil + klass ||= (klass_name.constantize rescue nil) + klass === exception if klass + end + index + end end end |