Extract ActionController rescue templates into Rescue and ShowExceptions middleware.

This commit breaks all exception catching plugins like ExceptionNotifier. These plugins should be rewritten as middleware instead overriding Controller#rescue_action_in_public.

13 files changed, 61 insertions, 328 deletions

diff --git a/actionpack/lib/action_controller/base/base.rb b/actionpack/lib/action_controller/base/base.rb
index 60567454b1..7bbde519cc 100644
--- a/actionpack/lib/action_controller/base/base.rb
+++ b/actionpack/lib/action_controller/base/base.rb
@@ -30,10 +30,6 @@ module ActionController #:nodoc:
     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:
@@ -510,9 +506,8 @@ module ActionController #:nodoc:
 
     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
 
diff --git a/actionpack/lib/action_controller/base/rescue.rb b/actionpack/lib/action_controller/base/rescue.rb
new file mode 100644
index 0000000000..2717a06a37
--- /dev/null
+++ b/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
diff --git a/actionpack/lib/action_controller/dispatch/dispatcher.rb b/actionpack/lib/action_controller/dispatch/dispatcher.rb
index 25844bf2a2..c0eb359547 100644
--- a/actionpack/lib/action_controller/dispatch/dispatcher.rb
+++ b/actionpack/lib/action_controller/dispatch/dispatcher.rb
@@ -75,18 +75,10 @@ module ActionController
     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
diff --git a/actionpack/lib/action_controller/dispatch/middlewares.rb b/actionpack/lib/action_controller/dispatch/middlewares.rb
index b5adbae746..31a7b00d28 100644
--- a/actionpack/lib/action_controller/dispatch/middlewares.rb
+++ b/actionpack/lib/action_controller/dispatch/middlewares.rb
@@ -3,6 +3,11 @@ use "Rack::Lock", :if => lambda {
 }
 
 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 }
diff --git a/actionpack/lib/action_controller/dispatch/rescue.rb b/actionpack/lib/action_controller/dispatch/rescue.rb
deleted file mode 100644
index 2f3b40c231..0000000000
--- a/actionpack/lib/action_controller/dispatch/rescue.rb
+++ /dev/null
@@ -1,185 +0,0 @@
-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
-    LOCALHOST = '127.0.0.1'.freeze
-
-    DEFAULT_RESCUE_RESPONSE = :internal_server_error
-    DEFAULT_RESCUE_RESPONSES = {
-      'ActionController::RoutingError'             => :not_found,
-      'ActionController::UnknownAction'            => :not_found,
-      'ActiveRecord::RecordNotFound'               => :not_found,
-      'ActiveRecord::StaleObjectError'             => :conflict,
-      'ActiveRecord::RecordInvalid'                => :unprocessable_entity,
-      'ActiveRecord::RecordNotSaved'               => :unprocessable_entity,
-      'ActionController::MethodNotAllowed'         => :method_not_allowed,
-      'ActionController::NotImplemented'           => :not_implemented,
-      'ActionController::InvalidAuthenticityToken' => :unprocessable_entity
-    }
-
-    DEFAULT_RESCUE_TEMPLATE = 'diagnostics'
-    DEFAULT_RESCUE_TEMPLATES = {
-      'ActionView::MissingTemplate'       => 'missing_template',
-      'ActionController::RoutingError'    => 'routing_error',
-      'ActionController::UnknownAction'   => 'unknown_action',
-      'ActionView::TemplateError'         => 'template_error'
-    }
-
-    RESCUES_TEMPLATE_PATH = ActionView::Template::FileSystemPath.new(
-      File.join(File.dirname(__FILE__), "templates"))
-
-    def self.included(base) #:nodoc:
-      base.cattr_accessor :rescue_responses
-      base.rescue_responses = Hash.new(DEFAULT_RESCUE_RESPONSE)
-      base.rescue_responses.update DEFAULT_RESCUE_RESPONSES
-
-      base.cattr_accessor :rescue_templates
-      base.rescue_templates = Hash.new(DEFAULT_RESCUE_TEMPLATE)
-      base.rescue_templates.update DEFAULT_RESCUE_TEMPLATES
-
-      base.extend(ClassMethods)
-      base.send :include, ActiveSupport::Rescuable
-
-      base.class_eval do
-        alias_method_chain :perform_action, :rescue
-      end
-    end
-
-    module ClassMethods
-      def call_with_exception(env, exception) #:nodoc:
-        request = env["action_controller.rescue.request"] ||= ActionDispatch::Request.new(env)
-        response = env["action_controller.rescue.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) ||
-          rescue_action_without_handler(exception)
-      end
-
-      # Overwrite to implement custom logging of errors. By default
-      # logs as fatal.
-      def log_error(exception) #:doc:
-        ActiveSupport::Deprecation.silence do
-          if ActionView::TemplateError === exception
-            logger.fatal(exception.to_s)
-          else
-            logger.fatal(
-              "\n#{exception.class} (#{exception.message}):\n  " +
-              clean_backtrace(exception).join("\n  ") + "\n\n"
-            )
-          end
-        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.
-      def rescue_action_in_public(exception) #:doc:
-        render_optional_error_file response_code_for_rescue(exception)
-      end
-
-      # Attempts to render a static error page based on the
-      # <tt>status_code</tt> thrown, or just return headers if no such file
-      # exists. At first, it will try to render a localized static page.
-      # For example, if a 500 error is being handled Rails and locale is :da,
-      # it will first attempt to render the file at <tt>public/500.da.html</tt>
-      # then attempt to render <tt>public/500.html</tt>. If none of them exist,
-      # the body of the response will be left empty.
-      def render_optional_error_file(status_code)
-        status = interpret_status(status_code)
-        locale_path = "#{Rails.public_path}/#{status[0,3]}.#{I18n.locale}.html" if I18n.locale
-        path = "#{Rails.public_path}/#{status[0,3]}.html"
-
-        if locale_path && File.exist?(locale_path)
-          render :file => locale_path, :status => status, :content_type => Mime::HTML
-        elsif File.exist?(path)
-          render :file => path, :status => status, :content_type => Mime::HTML
-        else
-          head status
-        end
-      end
-
-      # True if the request came from localhost, 127.0.0.1. Override this
-      # method if you wish to redefine the meaning of a local request to
-      # include remote IP addresses or other criteria.
-      def local_request? #:doc:
-        request.remote_addr == LOCALHOST && request.remote_ip == LOCALHOST
-      end
-
-      # Render detailed diagnostics for unhandled exceptions rescued from
-      # a controller action.
-      def rescue_action_locally(exception)
-        @template.instance_variable_set("@exception", exception)
-        @template.instance_variable_set("@rescues_path", RESCUES_TEMPLATE_PATH)
-        @template.instance_variable_set("@contents",
-          @template._render_template(template_path_for_local_rescue(exception)))
-
-        response.content_type = Mime::HTML
-        response.status = interpret_status(response_code_for_rescue(exception))
-        
-        content = @template._render_template(rescues_path("layout"))
-        render_for_text(content)
-      end
-
-      def rescue_action_without_handler(exception)
-        log_error(exception) if logger
-        erase_results if performed?
-
-        # Let the exception alter the response if it wants.
-        # For example, MethodNotAllowed sets the Allow header.
-        if exception.respond_to?(:handle_response!)
-          exception.handle_response!(response)
-        end
-
-        if consider_all_requests_local || local_request?
-          rescue_action_locally(exception)
-        else
-          rescue_action_in_public(exception)
-        end
-      end
-
-    private
-      def perform_action_with_rescue #:nodoc:
-        perform_action_without_rescue
-      rescue Exception => exception
-        rescue_action(exception)
-      end
-
-      def rescues_path(template_name)
-        RESCUES_TEMPLATE_PATH.find_by_parts("rescues/#{template_name}.erb")
-      end
-
-      def template_path_for_local_rescue(exception)
-        rescues_path(rescue_templates[exception.class.name])
-      end
-
-      def response_code_for_rescue(exception)
-        rescue_responses[exception.class.name]
-      end
-
-      def clean_backtrace(exception)
-        defined?(Rails) && Rails.respond_to?(:backtrace_cleaner) ?
-          Rails.backtrace_cleaner.clean(exception.backtrace) :
-          exception.backtrace
-      end
-  end
-end
diff --git a/actionpack/lib/action_controller/dispatch/templates/rescues/_request_and_response.erb b/actionpack/lib/action_controller/dispatch/templates/rescues/_request_and_response.erb
deleted file mode 100644
index 64b34650b1..0000000000
--- a/actionpack/lib/action_controller/dispatch/templates/rescues/_request_and_response.erb
+++ /dev/null
@@ -1,24 +0,0 @@
-<% unless @exception.blamed_files.blank? %>
-  <% if (hide = @exception.blamed_files.length > 8) %>
-    <a href="#" onclick="document.getElementById('blame_trace').style.display='block'; return false;">Show blamed files</a>
-  <% end %>
-  <pre id="blame_trace" <%='style="display:none"' if hide %>><code><%=h @exception.describe_blame %></code></pre>
-<% end %>
-
-<%
-  clean_params = request.parameters.clone
-  clean_params.delete("action")
-  clean_params.delete("controller")
-
-  request_dump = clean_params.empty? ? 'None' : clean_params.inspect.gsub(',', ",\n")
-%>
-
-<h2 style="margin-top: 30px">Request</h2>
-<p><b>Parameters</b>: <pre><%=h request_dump %></pre></p>
-
-<p><a href="#" onclick="document.getElementById('session_dump').style.display='block'; return false;">Show session dump</a></p>
-<div id="session_dump" style="display:none"><%= debug(request.session.instance_variable_get("@data")) %></div>
-
-
-<h2 style="margin-top: 30px">Response</h2>
-<p><b>Headers</b>: <pre><%=h response ? response.headers.inspect.gsub(',', ",\n") : 'None' %></pre></p>
diff --git a/actionpack/lib/action_controller/dispatch/templates/rescues/_trace.erb b/actionpack/lib/action_controller/dispatch/templates/rescues/_trace.erb
deleted file mode 100644
index bb2d8375bd..0000000000
--- a/actionpack/lib/action_controller/dispatch/templates/rescues/_trace.erb
+++ /dev/null
@@ -1,26 +0,0 @@
-<%
-  traces = [
-    ["Application Trace", @exception.application_backtrace],
-    ["Framework Trace", @exception.framework_backtrace],
-    ["Full Trace", @exception.clean_backtrace]
-  ]
-  names = traces.collect {|name, trace| name}
-%>
-
-<p><code>RAILS_ROOT: <%= defined?(RAILS_ROOT) ? RAILS_ROOT : "unset" %></code></p>
-
-<div id="traces">
-  <% names.each do |name| %>
-    <%
-      show = "document.getElementById('#{name.gsub /\s/, '-'}').style.display='block';"
-      hide = (names - [name]).collect {|hide_name| "document.getElementById('#{hide_name.gsub /\s/, '-'}').style.display='none';"}
-    %>
-    <a href="#" onclick="<%= hide %><%= show %>; return false;"><%= name %></a> <%= '|' unless names.last == name %>
-  <% end %>
-
-  <% traces.each do |name, trace| %>
-    <div id="<%= name.gsub /\s/, '-' %>" style="display: <%= name == "Application Trace" ? 'block' : 'none' %>;">
-      <pre><code><%= trace.join "\n" %></code></pre>
-    </div>
-  <% end %>
-</div>
diff --git a/actionpack/lib/action_controller/dispatch/templates/rescues/diagnostics.erb b/actionpack/lib/action_controller/dispatch/templates/rescues/diagnostics.erb
deleted file mode 100644
index e5c647c826..0000000000
--- a/actionpack/lib/action_controller/dispatch/templates/rescues/diagnostics.erb
+++ /dev/null
@@ -1,10 +0,0 @@
-<h1>
-  <%=h @exception.class.to_s %>
-  <% if request.parameters['controller'] %>
-    in <%=h request.parameters['controller'].humanize %>Controller<% if request.parameters['action'] %>#<%=h request.parameters['action'] %><% end %>
-  <% end %>
-</h1>
-<pre><%=h @exception.clean_message %></pre>
-
-<%= @template._render_template(@rescues_path.find_by_parts("rescues/_trace.erb")) %>
-<%= @template._render_template(@rescues_path.find_by_parts("rescues/_request_and_response.erb")) %>
\ No newline at end of file
diff --git a/actionpack/lib/action_controller/dispatch/templates/rescues/layout.erb b/actionpack/lib/action_controller/dispatch/templates/rescues/layout.erb
deleted file mode 100644
index 4a04742e40..0000000000
--- a/actionpack/lib/action_controller/dispatch/templates/rescues/layout.erb
+++ /dev/null
@@ -1,29 +0,0 @@
-<html xmlns="http://www.w3.org/1999/xhtml">
-<head>
-  <title>Action Controller: Exception caught</title>
-  <style>
-    body { background-color: #fff; color: #333; }
-
-    body, p, ol, ul, td {
-      font-family: verdana, arial, helvetica, sans-serif;
-      font-size:   13px;
-      line-height: 18px;
-    }
-
-    pre {
-      background-color: #eee;
-      padding: 10px;
-      font-size: 11px;
-    }
-
-    a { color: #000; }
-    a:visited { color: #666; }
-    a:hover { color: #fff; background-color:#000; }
-  </style>
-</head>
-<body>
-
-<%= @contents %>
-
-</body>
-</html>
\ No newline at end of file
diff --git a/actionpack/lib/action_controller/dispatch/templates/rescues/missing_template.erb b/actionpack/lib/action_controller/dispatch/templates/rescues/missing_template.erb
deleted file mode 100644
index dbfdf76947..0000000000
--- a/actionpack/lib/action_controller/dispatch/templates/rescues/missing_template.erb
+++ /dev/null
@@ -1,2 +0,0 @@
-<h1>Template is missing</h1>
-<p><%=h @exception.message %></p>
diff --git a/actionpack/lib/action_controller/dispatch/templates/rescues/routing_error.erb b/actionpack/lib/action_controller/dispatch/templates/rescues/routing_error.erb
deleted file mode 100644
index ccfa858cce..0000000000
--- a/actionpack/lib/action_controller/dispatch/templates/rescues/routing_error.erb
+++ /dev/null
@@ -1,10 +0,0 @@
-<h1>Routing Error</h1>
-<p><pre><%=h @exception.message %></pre></p>
-<% unless @exception.failures.empty? %><p>
-  <h2>Failure reasons:</h2>
-  <ol>
-  <% @exception.failures.each do |route, reason| %>
-    <li><code><%=h route.inspect.gsub('\\', '') %></code> failed because <%=h reason.downcase %></li>
-  <% end %>
-  </ol>
-</p><% end %>
diff --git a/actionpack/lib/action_controller/dispatch/templates/rescues/template_error.erb b/actionpack/lib/action_controller/dispatch/templates/rescues/template_error.erb
deleted file mode 100644
index 2e34e03bd5..0000000000
--- a/actionpack/lib/action_controller/dispatch/templates/rescues/template_error.erb
+++ /dev/null
@@ -1,21 +0,0 @@
-<h1>
-  <%=h @exception.original_exception.class.to_s %> in
-  <%=h request.parameters["controller"].capitalize if request.parameters["controller"]%>#<%=h request.parameters["action"] %>
-</h1>
-
-<p>
-  Showing <i><%=h @exception.file_name %></i> where line <b>#<%=h @exception.line_number %></b> raised:
-  <pre><code><%=h @exception.message %></code></pre>
-</p>
-
-<p>Extracted source (around line <b>#<%=h @exception.line_number %></b>):
-<pre><code><%=h @exception.source_extract %></code></pre></p>
-
-<p><%=h @exception.sub_template_message %></p>
-
-<% @real_exception = @exception
-   @exception = @exception.original_exception || @exception %>
-<%= render :file => @rescues_path["rescues/_trace.erb"] %>
-<% @exception = @real_exception %>
-
-<%= render :file => @rescues_path["rescues/_request_and_response.erb"] %>
diff --git a/actionpack/lib/action_controller/dispatch/templates/rescues/unknown_action.erb b/actionpack/lib/action_controller/dispatch/templates/rescues/unknown_action.erb
deleted file mode 100644
index 683379da10..0000000000
--- a/actionpack/lib/action_controller/dispatch/templates/rescues/unknown_action.erb
+++ /dev/null
@@ -1,2 +0,0 @@
-<h1>Unknown action</h1>
-<p><%=h @exception.message %></p>