Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Make rescue_from behave like rescue when dealing with subclasses. Clo…

…ses #10079 [fxn]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@8081 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
commit 788ece4799da9727dcc0f249c456041b01f62c98 1 parent aa1313d
@NZKoz NZKoz authored
View
57 actionpack/lib/action_controller/rescue.rb
@@ -41,8 +41,8 @@ def self.included(base) #: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 @@ def process_with_exception(request, response, exception)
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 @@ def rescue_from(*klasses, &block)
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 @@ def response_code_for_rescue(exception)
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
View
159 actionpack/test/controller/rescue_test.rb
@@ -5,35 +5,62 @@
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 @@ def not_implemented
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 @@ def test_rescue_handler
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 @@ def with_rails_root(path = nil)
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
Please sign in to comment.
Something went wrong with that request. Please try again.