diff options
-rw-r--r-- | actionpack/lib/action_controller/rescue.rb | 57 | ||||
-rw-r--r-- | actionpack/test/controller/rescue_test.rb | 159 |
2 files changed, 210 insertions, 6 deletions
diff --git a/actionpack/lib/action_controller/rescue.rb b/actionpack/lib/action_controller/rescue.rb index fa847a8274..7f82e38f54 100644 --- a/actionpack/lib/action_controller/rescue.rb +++ b/actionpack/lib/action_controller/rescue.rb @@ -41,8 +41,8 @@ module ActionController #:nodoc: base.rescue_templates = Hash.new(DEFAULT_RESCUE_TEMPLATE) base.rescue_templates.update DEFAULT_RESCUE_TEMPLATES - base.class_inheritable_hash :rescue_handlers - base.rescue_handlers = {} + base.class_inheritable_array :rescue_handlers + base.rescue_handlers = [] base.extend(ClassMethods) base.class_eval do @@ -55,13 +55,27 @@ module ActionController #:nodoc: new.process(request, response, :rescue_action, exception) end - # Rescue exceptions raised in controller actions by passing at least one exception class and a :with option that contains the name of the method to be called to respond to the exception. - # Handler methods that take one argument will be called with the exception, so that the exception can be inspected when dealing with it. + # Rescue exceptions raised in controller actions. + # + # <tt>rescue_from</tt> receives a series of exception classes or class + # names, and a trailing :with option with the name of a method or a Proc + # object to be called to handle them. Alternatively a block can be given. + # + # Handlers that take one argument will be called with the exception, so + # that the exception can be inspected when dealing with it. + # + # Handlers are inherited. They are searched from right to left, from + # bottom to top, and up the hierarchy. The handler of the first class for + # which exception.is_a?(klass) holds true is the one invoked, if any. # # class ApplicationController < ActionController::Base # rescue_from User::NotAuthorized, :with => :deny_access # self defined exception # rescue_from ActiveRecord::RecordInvalid, :with => :show_errors # + # rescue_from 'MyAppError::Base' do |exception| + # render :xml => exception, :status => 500 + # end + # # protected # def deny_access # ... @@ -78,7 +92,17 @@ module ActionController #:nodoc: end klasses.each do |klass| - rescue_handlers[klass.name] = options[:with] + key = if klass.is_a?(Class) && klass <= Exception + klass.name + elsif klass.is_a?(String) + klass + else + raise(ArgumentError, "#{klass} is neither an Exception nor a String") + end + + # Order is important, we put the pair at the end. When dealing with an + # exception we will follow the documented order going from right to left. + rescue_handlers << [key, options[:with]] end end end @@ -192,7 +216,28 @@ module ActionController #:nodoc: end def handler_for_rescue(exception) - case handler = rescue_handlers[exception.class.name] + # We go from right to left because pairs are pushed onto rescue_handlers + # as rescue_from declarations are found. + _, handler = *rescue_handlers.reverse.detect do |klass_name, handler| + # The purpose of allowing strings in rescue_from is to support the + # declaration of handler associations for exception classes whose + # definition is yet unknown. + # + # Since this loop needs the constants it would be inconsistent to + # assume they should exist at this point. An early raised exception + # could trigger some other handler and the array could include + # precisely a string whose corresponding constant has not yet been + # seen. This is why we are tolerant to unkown constants. + # + # Note that this tolerance only matters if the exception was given as + # a string, otherwise a NameError will be raised by the interpreter + # itself when rescue_from CONSTANT is executed. + klass = self.class.const_get(klass_name) rescue nil + klass ||= klass_name.constantize rescue nil + exception.is_a?(klass) if klass + end + + case handler when Symbol method(handler) when Proc diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb index cf4dcac029..a63fd06005 100644 --- a/actionpack/test/controller/rescue_test.rb +++ b/actionpack/test/controller/rescue_test.rb @@ -5,35 +5,62 @@ uses_mocha 'rescue' do class RescueController < ActionController::Base class NotAuthorized < StandardError end + class NotAuthorizedToRescueAsString < StandardError + end class RecordInvalid < StandardError end + class RecordInvalidToRescueAsString < StandardError + end class NotAllowed < StandardError end + class NotAllowedToRescueAsString < StandardError + end class InvalidRequest < StandardError end + class InvalidRequestToRescueAsString < StandardError + end class BadGateway < StandardError end + class BadGatewayToRescueAsString < StandardError + end class ResourceUnavailable < StandardError end + class ResourceUnavailableToRescueAsString < StandardError + end + + # We use a fully-qualified name in some strings, and a relative constant + # name in some other to test correct handling of both cases. rescue_from NotAuthorized, :with => :deny_access + rescue_from 'RescueController::NotAuthorizedToRescueAsString', :with => :deny_access + rescue_from RecordInvalid, :with => :show_errors + rescue_from 'RescueController::RecordInvalidToRescueAsString', :with => :show_errors rescue_from NotAllowed, :with => proc { head :forbidden } + rescue_from 'RescueController::NotAllowedToRescueAsString', :with => proc { head :forbidden } + rescue_from InvalidRequest, :with => proc { |exception| render :text => exception.message } + rescue_from 'InvalidRequestToRescueAsString', :with => proc { |exception| render :text => exception.message } rescue_from BadGateway do head :status => 502 end + rescue_from 'BadGatewayToRescueAsString' do + head :status => 502 + end rescue_from ResourceUnavailable do |exception| render :text => exception.message end + rescue_from 'ResourceUnavailableToRescueAsString' do |exception| + render :text => exception.message + end def raises render :text => 'already rendered' @@ -51,26 +78,44 @@ class RescueController < ActionController::Base def not_authorized raise NotAuthorized end + def not_authorized_raise_as_string + raise NotAuthorizedToRescueAsString + end def not_allowed raise NotAllowed end + def not_allowed_raise_as_string + raise NotAllowedToRescueAsString + end def invalid_request raise InvalidRequest end + def invalid_request_raise_as_string + raise InvalidRequestToRescueAsString + end def record_invalid raise RecordInvalid end + def record_invalid_raise_as_string + raise RecordInvalidToRescueAsString + end def bad_gateway raise BadGateway end + def bad_gateway_raise_as_string + raise BadGatewayToRescueAsString + end def resource_unavailable raise ResourceUnavailable end + def resource_unavailable_raise_as_string + raise ResourceUnavailableToRescueAsString + end def missing_template end @@ -278,32 +323,58 @@ class RescueTest < Test::Unit::TestCase get :not_authorized assert_response :forbidden end + def test_rescue_handler_string + get :not_authorized_raise_as_string + assert_response :forbidden + end def test_rescue_handler_with_argument @controller.expects(:show_errors).once.with { |e| e.is_a?(Exception) } get :record_invalid end + def test_rescue_handler_with_argument_as_string + @controller.expects(:show_errors).once.with { |e| e.is_a?(Exception) } + get :record_invalid_raise_as_string + end def test_proc_rescue_handler get :not_allowed assert_response :forbidden end + def test_proc_rescue_handler_as_string + get :not_allowed_raise_as_string + assert_response :forbidden + end def test_proc_rescue_handle_with_argument get :invalid_request assert_equal "RescueController::InvalidRequest", @response.body end + def test_proc_rescue_handle_with_argument_as_string + get :invalid_request_raise_as_string + assert_equal "RescueController::InvalidRequestToRescueAsString", @response.body + end def test_block_rescue_handler get :bad_gateway assert_response 502 end + def test_block_rescue_handler_as_string + get :bad_gateway_raise_as_string + assert_response 502 + end def test_block_rescue_handler_with_argument get :resource_unavailable assert_equal "RescueController::ResourceUnavailable", @response.body end + def test_block_rescue_handler_with_argument_as_string + get :resource_unavailable_raise_as_string + assert_equal "RescueController::ResourceUnavailableToRescueAsString", @response.body + end + + protected def with_all_requests_local(local = true) old_local, ActionController::Base.consider_all_requests_local = @@ -339,4 +410,92 @@ class RescueTest < Test::Unit::TestCase end end +class ExceptionInheritanceRescueController < ActionController::Base + + class ParentException < StandardError + end + + class ChildException < ParentException + end + + class GrandchildException < ChildException + end + + rescue_from ChildException, :with => lambda { head :ok } + rescue_from ParentException, :with => lambda { head :created } + rescue_from GrandchildException, :with => lambda { head :no_content } + + def raise_parent_exception + raise ParentException + end + + def raise_child_exception + raise ChildException + end + + def raise_grandchild_exception + raise GrandchildException + end +end + +class ExceptionInheritanceRescueTest < Test::Unit::TestCase + + def setup + @controller = ExceptionInheritanceRescueController.new + @request = ActionController::TestRequest.new + @response = ActionController::TestResponse.new + end + + def test_bottom_first + get :raise_grandchild_exception + assert_response :no_content + end + + def test_inheritance_works + get :raise_child_exception + assert_response :created + end +end + +class ControllerInheritanceRescueController < ExceptionInheritanceRescueController + class FirstExceptionInChildController < StandardError + end + + class SecondExceptionInChildController < StandardError + end + + rescue_from FirstExceptionInChildController, 'SecondExceptionInChildController', :with => lambda { head :gone } + + def raise_first_exception_in_child_controller + raise FirstExceptionInChildController + end + + def raise_second_exception_in_child_controller + raise SecondExceptionInChildController + end +end + +class ControllerInheritanceRescueControllerTest < Test::Unit::TestCase + + def setup + @controller = ControllerInheritanceRescueController.new + @request = ActionController::TestRequest.new + @response = ActionController::TestResponse.new + end + + def test_first_exception_in_child_controller + get :raise_first_exception_in_child_controller + assert_response :gone + end + + def test_second_exception_in_child_controller + get :raise_second_exception_in_child_controller + assert_response :gone + end + + def test_exception_in_parent_controller + get :raise_parent_exception + assert_response :created + end +end end # uses_mocha |