Introduce ActionController::Base.rescue_from to declare exception-handling methods. Cleaner style than the case-heavy rescue_action_in_public. Closes #9449.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@7597 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

3 files changed, 91 insertions, 12 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index 713e25396e..8d25ff47f3 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Introduce ActionController::Base.rescue_from to declare exception-handling methods. Cleaner style than the case-heavy rescue_action_in_public.  #9449 [norbert]
+
 * Rename some RequestForgeryProtection methods.  The class method is now #protect_from_forgery, and the default parameter is now 'authenticity_token'.  [Rick]
 
 * Merge csrf_killer plugin into rails.  Adds RequestForgeryProtection model that verifies session-specific _tokens for non-GET requests.  [Rick]
diff --git a/actionpack/lib/action_controller/rescue.rb b/actionpack/lib/action_controller/rescue.rb
index c1d2152acb..a34356f172 100644
--- a/actionpack/lib/action_controller/rescue.rb
+++ b/actionpack/lib/action_controller/rescue.rb
@@ -41,6 +41,9 @@ 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.extend(ClassMethods)
       base.class_eval do
         alias_method_chain :perform_action, :rescue
@@ -51,6 +54,33 @@ module ActionController #:nodoc:
       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.
+      #
+      # class ApplicationController < ActionController::Base
+      #   rescue_from User::NotAuthorized, :with => :deny_access # self defined exception
+      #   rescue_from ActiveRecord::RecordInvalid, :with => :show_errors
+      #
+      #   protected
+      #     def deny_access
+      #       ...
+      #     end
+      #
+      #     def show_errors(exception)
+      #       exception.record.new_record? ? ...
+      #     end
+      # end
+      def rescue_from(*klasses)
+        options = klasses.extract_options!
+        unless options.has_key?(:with) # allow nil
+          raise ArgumentError, "Need a handler. Supply an options hash that has a :with key as the last argument."
+        end
+
+        klasses.each do |klass|
+          rescue_handlers[klass.name] = options[:with]
+        end
+      end
     end
 
     protected
@@ -59,6 +89,8 @@ module ActionController #:nodoc:
         log_error(exception) if logger
         erase_results if performed?
 
+        return if rescue_action_with_handler(exception)
+
         # Let the exception alter the response if it wants.
         # For example, MethodNotAllowed sets the Allow header.
         if exception.respond_to?(:handle_response!)
@@ -87,7 +119,6 @@ module ActionController #:nodoc:
         end
       end
 
-
       # Overwrite to implement public exception handling (for requests answering false to <tt>local_request?</tt>).  By
       # default will call render_optional_error_file.  Override this method to provide more user friendly error messages.s
       def rescue_action_in_public(exception) #:doc:
@@ -97,7 +128,7 @@ module ActionController #:nodoc:
       # Attempts to render a static error page based on the <tt>status_code</tt> thrown,
       # or just return headers if no such file exists. For example, if a 500 error is 
       # being handled Rails will first attempt to render the file at <tt>public/500.html</tt>. 
-      # If the file doesn't exist, the body of the response will be left empty
+      # If the file doesn't exist, the body of the response will be left empty.
       def render_optional_error_file(status_code)
         status = interpret_status(status_code)
         path = "#{RAILS_ROOT}/public/#{status[0,3]}.html"
@@ -129,6 +160,18 @@ module ActionController #:nodoc:
         render_for_file(rescues_path("layout"), response_code_for_rescue(exception))
       end
 
+      # Tries to rescue the exception by looking up and calling a registered handler.
+      def rescue_action_with_handler(exception)
+        if handler = handler_for_rescue(exception)
+          if handler.arity != 0
+            handler.call(exception)
+          else
+            handler.call
+          end
+          true # don't rely on the return value of the handler
+        end
+      end
+
     private
       def perform_action_with_rescue #:nodoc:
         perform_action_without_rescue
@@ -148,6 +191,12 @@ module ActionController #:nodoc:
         rescue_responses[exception.class.name]
       end
 
+      def handler_for_rescue(exception)
+        if handler = rescue_handlers[exception.class.name]
+          method(handler)
+        end
+      end
+
       def clean_backtrace(exception)
         if backtrace = exception.backtrace
           if defined?(RAILS_ROOT)
diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb
index 6756e226c3..f0d0526a36 100644
--- a/actionpack/test/controller/rescue_test.rb
+++ b/actionpack/test/controller/rescue_test.rb
@@ -3,6 +3,15 @@ require File.dirname(__FILE__) + '/../abstract_unit'
 uses_mocha 'rescue' do
 
 class RescueController < ActionController::Base
+  class NotAuthorized < StandardError
+  end
+
+  class RecordInvalid < StandardError
+  end
+
+  rescue_from NotAuthorized, :with => :deny_access
+  rescue_from RecordInvalid, :with => :show_errors
+
   def raises
     render :text => 'already rendered'
     raise "don't panic!"
@@ -15,10 +24,27 @@ class RescueController < ActionController::Base
   def not_implemented
     raise ActionController::NotImplemented.new(:get, :put)
   end
+
+  def not_authorized
+    raise NotAuthorized
+  end
+
+  def record_invalid
+    raise RecordInvalid
+  end
   
-  def missing_template; end
-end
+  def missing_template
+  end
+
+  protected
+    def deny_access
+      head :forbidden
+    end
 
+    def show_errors(exception)
+      head :unprocessable_entity
+    end
+end
 
 class RescueTest < Test::Unit::TestCase
   FIXTURE_PUBLIC = "#{File.dirname(__FILE__)}/../fixtures".freeze
@@ -38,7 +64,6 @@ class RescueTest < Test::Unit::TestCase
     end
   end
 
-
   def test_rescue_action_locally_if_all_requests_local
     @controller.expects(:local_request?).never
     @controller.expects(:rescue_action_locally).with(@exception)
@@ -69,7 +94,6 @@ class RescueTest < Test::Unit::TestCase
     end
   end
 
-
   def test_rescue_action_in_public_with_error_file
     with_rails_root FIXTURE_PUBLIC do
       with_all_requests_local false do
@@ -93,7 +117,6 @@ class RescueTest < Test::Unit::TestCase
     assert_equal ' ', @response.body
   end
 
-
   def test_rescue_unknown_action_in_public_with_error_file
     with_rails_root FIXTURE_PUBLIC do
       with_all_requests_local false do
@@ -117,7 +140,6 @@ class RescueTest < Test::Unit::TestCase
     assert_equal ' ', @response.body
   end
 
-
   def test_rescue_missing_template_in_public
     with_rails_root FIXTURE_PUBLIC do
       with_all_requests_local true do
@@ -129,7 +151,6 @@ class RescueTest < Test::Unit::TestCase
     assert @response.body.include?('missing_template'), "Response should include the template name."
   end
 
-
   def test_rescue_action_locally
     get :raises
     assert_response :internal_server_error
@@ -138,7 +159,6 @@ class RescueTest < Test::Unit::TestCase
     assert @response.body.include?("don't panic"), "Response should include exception message."
   end
 
-
   def test_local_request_when_remote_addr_is_localhost
     @controller.expects(:request).returns(@request).at_least_once
     with_remote_addr '127.0.0.1' do
@@ -153,7 +173,6 @@ class RescueTest < Test::Unit::TestCase
     end
   end
 
-
   def test_rescue_responses
     responses = ActionController::Base.rescue_responses
 
@@ -182,7 +201,6 @@ class RescueTest < Test::Unit::TestCase
     assert_equal 'template_error',    templates[ActionView::TemplateError.name]
   end
 
-
   def test_clean_backtrace
     with_rails_root nil do
       # No action if RAILS_ROOT isn't set.
@@ -217,6 +235,16 @@ class RescueTest < Test::Unit::TestCase
     assert_equal "GET, HEAD, PUT", @response.headers['Allow']
   end
 
+  def test_rescue_handler
+    get :not_authorized
+    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
+
   protected
     def with_all_requests_local(local = true)
       old_local, ActionController::Base.consider_all_requests_local =