From a6f49d9b786bfcc049a99f843de8e4d838de1179 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sun, 23 Sep 2007 21:56:22 +0000 Subject: 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 --- actionpack/CHANGELOG | 2 ++ actionpack/lib/action_controller/rescue.rb | 53 ++++++++++++++++++++++++++++-- actionpack/test/controller/rescue_test.rb | 48 +++++++++++++++++++++------ 3 files changed, 91 insertions(+), 12 deletions(-) (limited to 'actionpack') 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 local_request?). 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 status_code 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 public/500.html. - # 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 = -- cgit v1.2.3