Add tests for ActiveSupport::Rescuable. Use ActiveSupport::Rescuable in ActionController::Base.

1 files changed, 54 insertions, 20 deletions

diff --git a/activesupport/lib/active_support/rescuable.rb b/activesupport/lib/active_support/rescuable.rb
index d1a4366636..f2bc12e832 100644
--- a/activesupport/lib/active_support/rescuable.rb
+++ b/activesupport/lib/active_support/rescuable.rb
@@ -1,28 +1,49 @@
 module ActiveSupport
+  # Rescuable module adds support for easier exception handling.
   module Rescuable
     def self.included(base) # :nodoc:
-      base.class_inheritable_array :rescue_handlers
+      base.class_inheritable_accessor :rescue_handlers
       base.rescue_handlers = []
+
       base.extend(ClassMethods)
     end
 
     module ClassMethods
-      def enable_rescue_for(*methods)
-        methods.each do |method|
-          class_eval <<-EOS
-            def #{method}_with_rescue(*args, &block)
-              #{method}_without_rescue(*args, &block)
-            rescue Exception => exception
-              rescue_with_handler(exception)
-            end
-
-            alias_method_chain :#{method}, :rescue
-          EOS
-        end
-      end
-
+      # Rescue exceptions raised in controller actions.
+      #
+      # <tt>rescue_from</tt> receives a series of exception classes or class
+      # names, and a trailing <tt>:with</tt> 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 <tt>exception.is_a?(klass)</tt> 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
+      #         ...
+      #       end
+      #
+      #       def show_errors(exception)
+      #         exception.record.new_record? ? ...
+      #       end
+      #   end
       def rescue_from(*klasses, &block)
         options = klasses.extract_options!
+
         unless options.has_key?(:with)
           if block_given?
             options[:with] = block
@@ -46,18 +67,31 @@ module ActiveSupport
       end
     end
 
+    # Tries to rescue the exception by looking up and calling a registered handler.
     def rescue_with_handler(exception)
       if handler = handler_for_rescue(exception)
         handler.arity != 0 ? handler.call(exception) : handler.call
-      else
-        raise exception
+        true # don't rely on the return value of the handler
       end
     end
 
     def handler_for_rescue(exception)
-      # use reverse so what is added last is found first
-      _, handler = *rescue_handlers.reverse.detect do |klass_name, handler|
-        # allow strings to support constants that are not defined yet
+      # We go from right to left because pairs are pushed onto rescue_handlers
+      # as rescue_from declarations are found.
+      _, handler = Array(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 unknown 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