Permalink
Browse files

Introduce ActionController::Base.rescue_from to declare exception-han…

…dling 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
  • Loading branch information...
1 parent c619003 commit a6f49d9b786bfcc049a99f843de8e4d838de1179 @jeremy jeremy committed Sep 23, 2007
Showing with 91 additions and 12 deletions.
  1. +2 −0 actionpack/CHANGELOG
  2. +51 −2 actionpack/lib/action_controller/rescue.rb
  3. +38 −10 actionpack/test/controller/rescue_test.rb
View
@@ -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]
@@ -41,6 +41,9 @@ 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.extend(ClassMethods)
base.class_eval do
alias_method_chain :perform_action, :rescue
@@ -51,6 +54,33 @@ module ClassMethods #: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 @@ def rescue_action(exception)
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 @@ def log_error(exception) #:doc:
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 @@ def rescue_action_in_public(exception) #:doc:
# 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 @@ def rescue_action_locally(exception)
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 @@ def response_code_for_rescue(exception)
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)
@@ -3,6 +3,15 @@
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 @@ def method_not_allowed
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 @@ def setup
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 @@ def test_rescue_action_in_public_otherwise
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 @@ def test_rescue_action_in_public_without_error_file
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 @@ def test_rescue_unknown_action_in_public_without_error_file
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 @@ def test_rescue_missing_template_in_public
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 @@ def test_rescue_action_locally
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 @@ def test_local_request_when_remote_addr_isnt_locahost
end
end
-
def test_rescue_responses
responses = ActionController::Base.rescue_responses
@@ -182,7 +201,6 @@ def test_rescue_templates
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 @@ def test_method_not_allowed
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 =

0 comments on commit a6f49d9

Please sign in to comment.