Merge Failsafe middleware into ShowExceptions

8118fca9 · Joshua Peek · c3319504 · 8118fca9 · 8118fca9 · c3319504
9 changed file
--- a/actionpack/lib/action_controller/dispatch/middlewares.rb
+++ b/actionpack/lib/action_controller/dispatch/middlewares.rb
@@ -2,7 +2,6 @@
  !ActionController::Base.allow_concurrency
 }
-use "ActionDispatch::Failsafe"
 use "ActionDispatch::ShowExceptions", lambda { ActionController::Base.consider_all_requests_local }
 use "ActionDispatch::Rescue", lambda {
  controller = (::ApplicationController rescue ActionController::Base)

--- a/actionpack/lib/action_dispatch.rb
+++ b/actionpack/lib/action_dispatch.rb
@@ -38,7 +38,6 @@ module ActionDispatch
  autoload :Response, 'action_dispatch/http/response'
  autoload :StatusCodes, 'action_dispatch/http/status_codes'
-  autoload :Failsafe, 'action_dispatch/middleware/failsafe'
  autoload :ParamsParser, 'action_dispatch/middleware/params_parser'
  autoload :Rescue, 'action_dispatch/middleware/rescue'
  autoload :ShowExceptions, 'action_dispatch/middleware/show_exceptions'

--- a/actionpack/lib/action_dispatch/middleware/failsafe.rb
+++ b/actionpack/lib/action_dispatch/middleware/failsafe.rb
-module ActionDispatch
-  class Failsafe
-    cattr_accessor :error_file_path
-    self.error_file_path = Rails.public_path if defined?(Rails.public_path)
-    def initialize(app)
-      @app = app
-    end
-    def call(env)
-      @app.call(env)
-    rescue Exception => exception
-      if env["rails.raise_exceptions"]
-        raise
-      else
-        failsafe_response(exception)
-      end
-    end
-    private
-      def failsafe_response(exception)
-        log_failsafe_exception(exception)
-        [500, {'Content-Type' => 'text/html'}, failsafe_response_body]
-      rescue Exception => failsafe_error # Logger or IO errors
-        $stderr.puts "Error during failsafe response: #{failsafe_error}"
-      end
-      def failsafe_response_body
-        error_path = "#{self.class.error_file_path}/500.html"
-        if File.exist?(error_path)
-          [File.read(error_path)]
-        else
-          ["<html><body><h1>500 Internal Server Error</h1></body></html>"]
-        end
-      end
-      def log_failsafe_exception(exception)
-        message = "/!\\ FAILSAFE /!\\  #{Time.now}\n  Status: 500 Internal Server Error\n"
-        message << "  #{exception}\n    #{exception.backtrace.join("\n    ")}" if exception
-        failsafe_logger.fatal(message)
-      end
-      def failsafe_logger
-        if defined?(Rails) && Rails.logger
-          Rails.logger
-        else
-          Logger.new($stderr)
-        end
-      end
-  end
-end
--- a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb
+++ b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb
@@ -4,8 +4,11 @@ class ShowExceptions
    LOCALHOST = '127.0.0.1'.freeze
-    DEFAULT_RESCUE_RESPONSE = :internal_server_error
+    RESCUES_TEMPLATE_PATH = File.join(File.dirname(__FILE__), 'templates')
-    DEFAULT_RESCUE_RESPONSES = {
+    cattr_accessor :rescue_responses
+    @@rescue_responses = Hash.new(:internal_server_error)
+    @@rescue_responses.update({
      'ActionController::RoutingError'             => :not_found,
      'ActionController::UnknownAction'            => :not_found,
      'ActiveRecord::RecordNotFound'               => :not_found,
@@ -15,25 +18,19 @@ class ShowExceptions
      'ActionController::MethodNotAllowed'         => :method_not_allowed,
      'ActionController::NotImplemented'           => :not_implemented,
      'ActionController::InvalidAuthenticityToken' => :unprocessable_entity
-    }
+    })
-    DEFAULT_RESCUE_TEMPLATE = 'diagnostics'
+    cattr_accessor :rescue_templates
-    DEFAULT_RESCUE_TEMPLATES = {
+    @@rescue_templates = Hash.new('diagnostics')
+    @@rescue_templates.update({
      'ActionView::MissingTemplate'       => 'missing_template',
      'ActionController::RoutingError'    => 'routing_error',
      'ActionController::UnknownAction'   => 'unknown_action',
      'ActionView::TemplateError'         => 'template_error'
-    }
+    })
-    RESCUES_TEMPLATE_PATH = File.join(File.dirname(__FILE__), 'templates')
-    cattr_accessor :rescue_responses
-    @@rescue_responses = Hash.new(DEFAULT_RESCUE_RESPONSE)
-    @@rescue_responses.update DEFAULT_RESCUE_RESPONSES
-    cattr_accessor :rescue_templates
+    FAILSAFE_RESPONSE = [500, {'Content-Type' => 'text/html'},
-    @@rescue_templates = Hash.new(DEFAULT_RESCUE_TEMPLATE)
+      ['<html><body><h1>500 Internal Server Error</h1></body></html>']]
-    @@rescue_templates.update DEFAULT_RESCUE_TEMPLATES
    def initialize(app, consider_all_requests_local = false)
      @app = app
@@ -43,34 +40,35 @@ def initialize(app, consider_all_requests_local = false)
    def call(env)
      @app.call(env)
    rescue Exception => exception
-      raise exception if env['rack.test']
+      raise exception if env['action_dispatch.show_exceptions'] == false
+      render_exception(env, exception)
+    end
-      log_error(exception) if logger
+    private
+      def render_exception(env, exception)
+        log_error(exception)
-      request = Request.new(env)
+        request = Request.new(env)
-      if @consider_all_requests_local || local_request?(request)
+        if @consider_all_requests_local || local_request?(request)
-        rescue_action_locally(request, exception)
+          rescue_action_locally(request, exception)
-      else
+        else
-        rescue_action_in_public(exception)
+          rescue_action_in_public(exception)
+        end
+      rescue Exception => failsafe_error
+        $stderr.puts "Error during failsafe response: #{failsafe_error}"
+        FAILSAFE_RESPONSE
      end
-    end
-    private
      # Render detailed diagnostics for unhandled exceptions rescued from
      # a controller action.
      def rescue_action_locally(request, exception)
        template = ActionView::Base.new([RESCUES_TEMPLATE_PATH],
-          :template => template,
          :request => request,
          :exception => exception
        )
        file = "rescues/#{@@rescue_templates[exception.class.name]}.erb"
        body = template.render(:file => file, :layout => 'rescues/layout.erb')
+        render(status_code(exception), body)
-        headers = {'Content-Type' => 'text/html', 'Content-Length' => body.length.to_s}
-        status = status_code(exception)
-        [status, headers, body]
      end
      # Attempts to render a static error page based on the
@@ -86,11 +84,11 @@ def rescue_action_in_public(exception)
        path = "#{public_path}/#{status}.html"
        if locale_path && File.exist?(locale_path)
-          render_public_file(status, locale_path)
+          render(status, File.read(locale_path))
        elsif File.exist?(path)
-          render_public_file(status, path)
+          render(status, File.read(path))
        else
-          [status, {'Content-Type' => 'text/html', 'Content-Length' => '0'}, []]
+          render(status, '')
        end
      end
@@ -99,24 +97,21 @@ def local_request?(request)
        request.remote_addr == LOCALHOST && request.remote_ip == LOCALHOST
      end
-      def render_public_file(status, path)
-        body = File.read(path)
-        [status, {'Content-Type' => 'text/html', 'Content-Length' => body.length.to_s}, body]
-      end
      def status_code(exception)
        interpret_status(@@rescue_responses[exception.class.name]).to_i
      end
+      def render(status, body)
+        [status, {'Content-Type' => 'text/html', 'Content-Length' => body.length.to_s}, body]
+      end
      def public_path
-        if defined?(Rails)
+        defined?(Rails.public_path) ? Rails.public_path : 'public_path'
-          Rails.public_path
-        else
-          "public"
-        end
      end
-      def log_error(exception) #:doc:
+      def log_error(exception)
+        return unless logger
        ActiveSupport::Deprecation.silence do
          if ActionView::TemplateError === exception
            logger.fatal(exception.to_s)
@@ -136,9 +131,7 @@ def clean_backtrace(exception)
      end
      def logger
-        if defined?(Rails.logger)
+        defined?(Rails.logger) ? Rails.logger : Logger.new($stderr)
-          Rails.logger
-        end
      end
  end
 end
--- a/actionpack/test/controller/rescue_test.rb
+++ b/actionpack/test/controller/rescue_test.rb
 require 'abstract_unit'
+module ActionDispatch
+  class ShowExceptions
+    private
+      def public_path
+        "#{FIXTURE_LOAD_PATH}/public"
+      end
+      # Silence logger
+      def logger
+        nil
+      end
+  end
+end
 class RescueController < ActionController::Base
  class NotAuthorized < StandardError
  end

--- a/actionpack/test/dispatch/show_exceptions_test.rb
+++ b/actionpack/test/dispatch/show_exceptions_test.rb
@@ -6,6 +6,11 @@ class ShowExceptions
      def public_path
        "#{FIXTURE_LOAD_PATH}/public"
      end
+      # Silence logger
+      def logger
+        nil
+      end
  end
 end

--- a/actionpack/test/new_base/render_action_test.rb
+++ b/actionpack/test/new_base/render_action_test.rb
@@ -92,7 +92,7 @@ class TestLayoutTrue < SimpleRouteCase
    test "raises an exception when requesting a layout and none exist" do
      assert_raise(ArgumentError, /no default layout for RenderAction::BasicController in/) do 
-        get "/render_action/basic/hello_world_with_layout", {}, "rails.raise_exceptions" => true
+        get "/render_action/basic/hello_world_with_layout", {}, "action_dispatch.show_exceptions" => false
      end
    end
  end
@@ -118,7 +118,7 @@ class TestCustomLayout < SimpleRouteCase
    test "raises an exception when requesting a layout that does not exist" do
      assert_raise(ActionView::MissingTemplate) do
-        get "/render_action/basic/hello_world_with_custom_layout", {}, "rails.raise_exceptions" => true
+        get "/render_action/basic/hello_world_with_custom_layout", {}, "action_dispatch.show_exceptions" => false
      end
    end
  end

--- a/actionpack/test/new_base/render_test.rb
+++ b/actionpack/test/new_base/render_test.rb
@@ -48,7 +48,7 @@ class TestBasic < SimpleRouteCase
    test "raises an exception" do
      assert_raises(AbstractController::DoubleRenderError) do
-        get "/render/double_render", {}, "rails.raise_exceptions" => true
+        get "/render/double_render", {}, "action_dispatch.show_exceptions" => false
      end
    end
  end
@@ -58,13 +58,13 @@ class TestOnlyRenderPublicActions < SimpleRouteCase
    test "raises an exception when a method of Object is called" do
      assert_raises(AbstractController::ActionNotFound) do
-        get "/render/blank_render/clone", {}, "rails.raise_exceptions" => true
+        get "/render/blank_render/clone", {}, "action_dispatch.show_exceptions" => false
      end
    end
    test "raises an exception when a private method is called" do
      assert_raises(AbstractController::ActionNotFound) do
-        get "/render/blank_render/secretz", {}, "rails.raise_exceptions" => true
+        get "/render/blank_render/secretz", {}, "action_dispatch.show_exceptions" => false
      end
    end
  end

--- a/railties/lib/initializer.rb
+++ b/railties/lib/initializer.rb
@@ -462,7 +462,7 @@ def initialize_cache
        if RAILS_CACHE.respond_to?(:middleware)
          # Insert middleware to setup and teardown local cache for each request
-          configuration.middleware.insert_after(:"ActionDispatch::Failsafe", RAILS_CACHE.middleware)
+          configuration.middleware.insert_after(:"Rack::Lock", RAILS_CACHE.middleware)
        end
      end
    end