Skip to content

Commit

Permalink
Extract ActionController rescue templates into Rescue and ShowExcepti…
Browse files Browse the repository at this point in the history
…ons middleware.

This commit breaks all exception catching plugins like ExceptionNotifier. These plugins should be rewritten as middleware instead overriding Controller#rescue_action_in_public.
  • Loading branch information
josh committed May 3, 2009
1 parent 1c6fcbf commit 11af089
Show file tree
Hide file tree
Showing 28 changed files with 403 additions and 556 deletions.
2 changes: 1 addition & 1 deletion actionpack/lib/action_controller.rb
Expand Up @@ -60,7 +60,7 @@ def self.load_all!
autoload :Redirector, 'action_controller/base/redirect'
autoload :Renderer, 'action_controller/base/render'
autoload :RequestForgeryProtection, 'action_controller/base/request_forgery_protection'
autoload :Rescue, 'action_controller/dispatch/rescue'
autoload :Rescue, 'action_controller/base/rescue'
autoload :Resources, 'action_controller/routing/resources'
autoload :Responder, 'action_controller/base/responder'
autoload :Routing, 'action_controller/routing'
Expand Down
9 changes: 2 additions & 7 deletions actionpack/lib/action_controller/base/base.rb
Expand Up @@ -30,10 +30,6 @@ def initialize(*allowed_methods)
def allowed_methods_header
allowed_methods.map { |method_symbol| method_symbol.to_s.upcase } * ', '
end

def handle_response!(response)
response.headers['Allow'] ||= allowed_methods_header
end
end

class NotImplemented < MethodNotAllowed #:nodoc:
Expand Down Expand Up @@ -510,9 +506,8 @@ def exempt_from_layout(*types)

public
def call(env)
# HACK: For global rescue to have access to the original request and response
request = env["action_dispatch.rescue.request"] ||= ActionDispatch::Request.new(env)
response = env["action_dispatch.rescue.response"] ||= ActionDispatch::Response.new
request = ActionDispatch::Request.new(env)
response = ActionDispatch::Response.new
process(request, response).to_a
end

Expand Down
50 changes: 50 additions & 0 deletions actionpack/lib/action_controller/base/rescue.rb
@@ -0,0 +1,50 @@
module ActionController #:nodoc:
# Actions that fail to perform as expected throw exceptions. These
# exceptions can either be rescued for the public view (with a nice
# user-friendly explanation) or for the developers view (with tons of
# debugging information). The developers view is already implemented by
# the Action Controller, but the public view should be tailored to your
# specific application.
#
# The default behavior for public exceptions is to render a static html
# file with the name of the error code thrown. If no such file exists, an
# empty response is sent with the correct status code.
#
# You can override what constitutes a local request by overriding the
# <tt>local_request?</tt> method in your own controller. Custom rescue
# behavior is achieved by overriding the <tt>rescue_action_in_public</tt>
# and <tt>rescue_action_locally</tt> methods.
module Rescue
def self.included(base) #:nodoc:
base.send :include, ActiveSupport::Rescuable
base.extend(ClassMethods)

base.class_eval do
alias_method_chain :perform_action, :rescue
end
end

module ClassMethods
def rescue_action(env)
exception = env.delete('action_dispatch.rescue.exception')
request = ActionDispatch::Request.new(env)
response = ActionDispatch::Response.new
new.process(request, response, :rescue_action, exception).to_a
end
end

protected
# Exception handler called when the performance of an action raises
# an exception.
def rescue_action(exception)
rescue_with_handler(exception) || raise(exception)
end

private
def perform_action_with_rescue
perform_action_without_rescue
rescue Exception => exception
rescue_action(exception)
end
end
end
16 changes: 4 additions & 12 deletions actionpack/lib/action_controller/dispatch/dispatcher.rb
Expand Up @@ -75,18 +75,10 @@ def call(env)
end

def _call(env)
begin
run_callbacks :before_dispatch
Routing::Routes.call(env)
rescue Exception => exception
if controller ||= (::ApplicationController rescue Base)
controller.call_with_exception(env, exception)
else
raise exception
end
ensure
run_callbacks :after_dispatch, :enumerator => :reverse_each
end
run_callbacks :before_dispatch
Routing::Routes.call(env)
ensure
run_callbacks :after_dispatch, :enumerator => :reverse_each
end

def flush_logger
Expand Down
5 changes: 5 additions & 0 deletions actionpack/lib/action_controller/dispatch/middlewares.rb
Expand Up @@ -3,6 +3,11 @@
}

use "ActionDispatch::Failsafe"
use "ActionDispatch::ShowExceptions", lambda { ActionController::Base.consider_all_requests_local }
use "ActionDispatch::Rescue", lambda {
controller = (::ApplicationController rescue ActionController::Base)
controller.method(:rescue_action)
}

use lambda { ActionController::Base.session_store },
lambda { ActionController::Base.session_options }
Expand Down
185 changes: 0 additions & 185 deletions actionpack/lib/action_controller/dispatch/rescue.rb

This file was deleted.

This file was deleted.

2 changes: 2 additions & 0 deletions actionpack/lib/action_dispatch.rb
Expand Up @@ -48,6 +48,8 @@ module ActionDispatch
autoload :Failsafe, 'action_dispatch/middleware/failsafe'
autoload :ParamsParser, 'action_dispatch/middleware/params_parser'
autoload :Reloader, 'action_dispatch/middleware/reloader'
autoload :Rescue, 'action_dispatch/middleware/rescue'
autoload :ShowExceptions, 'action_dispatch/middleware/show_exceptions'
autoload :MiddlewareStack, 'action_dispatch/middleware/stack'

autoload :Assertions, 'action_dispatch/testing/assertions'
Expand Down
14 changes: 14 additions & 0 deletions actionpack/lib/action_dispatch/middleware/rescue.rb
@@ -0,0 +1,14 @@
module ActionDispatch
class Rescue
def initialize(app, rescuer)
@app, @rescuer = app, rescuer
end

def call(env)
@app.call(env)
rescue Exception => exception
env['action_dispatch.rescue.exception'] = exception
@rescuer.call(env)
end
end
end

1 comment on commit 11af089

@hsribei
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So now #local_request? isn't as easily overridable. Is there a better place where this could be defined so it's explicitly exposed for overriding? This is what I'm using now: http://zapt.in/Ln1.

Please sign in to comment.