Permalink
Browse files

Enable Exception Fallback/Failsafe Behavior

Right now we have a default error handling app `ActionDispatch::PublicExceptions` that renders the static 404 & 500 pages. If you don't like it you can override this behavior by specifying an exceptions app in the config of your application using `config.exceptions_app`. Right now if you do so, and you have an exception in your exceptions app, your users will not see your 404 or 500 pages, but instead a FAILSAFE_RESPONSE that simply renders some text.

This change gives the ability to have MULTIPLE fallback apps. So anyone can write exception handling apps, and if those apps fails, it will continue to try to use the public app. This design also allows the developer to specify additional apps if they choose.

There are two key things in this PR, first that this fallback behavior to the public exceptions app is automatic, and second that by default you can specify multiple apps.

This is an alternative implementation of an existing PR #7804 made at the behest of @jeremy. I believe this is a cleaner implementation.

ATP railties and actionpack cc/ @rafael @rafaelfranca @spastorino @steveklabnik
  • Loading branch information...
1 parent 0a1c611 commit 5c5b023e62737365e5b958b1a9ed51206c7860aa @schneems committed Dec 1, 2012
@@ -0,0 +1,39 @@
+require 'action_dispatch/http/request'
+require 'action_dispatch/middleware/exception_wrapper'
+
+module ActionDispatch
+ # This class is used by ShowException middleware to guarantee
+ # a response is returned by the app. This class can take multiple
+ # exception applications calling each in turn until one returns
+ # successfully. If no apps return successfully, a FAILSAFE_RESPONSE
+ # is delivered
+ class ExceptionFailsafe
+ FAILSAFE_RESPONSE = [500, { 'Content-Type' => 'text/plain' },
+ ["500 Internal Server Error\n" <<
+ "If you are the administrator of this website, then please read this web " <<
+ "application's log file and/or the web server's log file to find out what " <<
+ "went wrong."]]
+
+ def initialize(*exception_apps)
+ @exception_apps = exception_apps.flatten.compact.uniq
+ end
+
+ def call(env)
+ try_exception_apps(env, @exception_apps.dup)
+ end
+
+ private
+
+ # Takes an array of exception apps, pops one off the front
+ # and attempts to render the exception. Continues until
+ # all apps have errored, then returns the FAILSAFE_RESPONSE
+ def try_exception_apps(env, exception_apps)
+ return FAILSAFE_RESPONSE if exception_apps.blank?
+ exception_app = exception_apps.shift
+ return exception_app.call(env)
+ rescue Exception => failsafe_error
+ $stderr.puts "Error during failsafe response: #{failsafe_error}\n #{failsafe_error.backtrace * "\n "}"
+ try_exception_apps(env, exception_apps)
+ end
+ end
+end
@@ -1,39 +1,30 @@
require 'action_dispatch/http/request'
require 'action_dispatch/middleware/exception_wrapper'
+require 'action_dispatch/middleware/exception_failsafe'
module ActionDispatch
# This middleware rescues any exception returned by the application
- # and calls an exceptions app that will wrap it in a format for the end user.
+ # and calls ActionDispatch::ExceptionFailsafe app that will wrap it
+ # in a format for the end user.
#
- # The exceptions app should be passed as parameter on initialization
+ # Multiple exceptions apps can be passed as parameters on initialization
# of ShowExceptions. Every time there is an exception, ShowExceptions will
# store the exception in env["action_dispatch.exception"], rewrite the
# PATH_INFO to the exception status code and call the rack app.
#
# If the application returns a "X-Cascade" pass response, this middleware
# will send an empty response as result with the correct status code.
- # If any exception happens inside the exceptions app, this middleware
- # catches the exceptions and returns a FAILSAFE_RESPONSE.
class ShowExceptions
- FAILSAFE_RESPONSE = [500, { 'Content-Type' => 'text/plain' },
- ["500 Internal Server Error\n" <<
- "If you are the administrator of this website, then please read this web " <<
- "application's log file and/or the web server's log file to find out what " <<
- "went wrong."]]
-
def initialize(app, exceptions_app)
@app = app
@exceptions_app = exceptions_app
end
def call(env)
- begin
- response = @app.call(env)
- rescue Exception => exception
- raise exception if env['action_dispatch.show_exceptions'] == false
- end
-
- response || render_exception(env, exception)
+ return @app.call(env)
+ rescue Exception => exception
+ raise exception if env['action_dispatch.show_exceptions'] == false
+ render_exception(env, exception)
end
private
@@ -45,9 +36,6 @@ def render_exception(env, exception)
env["PATH_INFO"] = "/#{status}"
response = @exceptions_app.call(env)
response[1]['X-Cascade'] == 'pass' ? pass_response(status) : response
- rescue Exception => failsafe_error
- $stderr.puts "Error during failsafe response: #{failsafe_error}\n #{failsafe_error.backtrace * "\n "}"
- FAILSAFE_RESPONSE
end
def pass_response(status)
@@ -98,7 +98,7 @@ class ShowFailsafeExceptionsTest < ActionDispatch::IntegrationTest
def test_render_failsafe_exception
@app = ShowExceptionsOverridenController.action(:boom)
@exceptions_app = @app.instance_variable_get(:@exceptions_app)
- @app.instance_variable_set(:@exceptions_app, nil)
+ @app.instance_variable_set(:@exceptions_app, ActionDispatch::ExceptionFailsafe.new([]))
$stderr = StringIO.new
get '/', {}, 'HTTP_ACCEPT' => 'text/json'
@@ -352,7 +352,7 @@ def default_middleware_stack #:nodoc:
middleware.use ::Rack::MethodOverride
middleware.use ::ActionDispatch::RequestId
middleware.use ::Rails::Rack::Logger, config.log_tags # must come after Rack::MethodOverride to properly log overridden methods
- middleware.use ::ActionDispatch::ShowExceptions, config.exceptions_app || ActionDispatch::PublicExceptions.new(Rails.public_path)
+ middleware.use ::ActionDispatch::ShowExceptions, ActionDispatch::ExceptionFailsafe.new(config.exceptions_app, ActionDispatch::PublicExceptions.new(Rails.public_path))
middleware.use ::ActionDispatch::DebugExceptions, app
middleware.use ::ActionDispatch::RemoteIp, config.action_dispatch.ip_spoofing_check, config.action_dispatch.trusted_proxies
@@ -60,6 +60,41 @@ def index
assert_equal "YOU FAILED BRO", last_response.body
end
+ test "falls back on public app if custom exception app raises error" do
+ add_to_config <<-RUBY
+ config.exceptions_app = lambda do |env|
+ raise "hover boards don't work on water"
+ end
+ RUBY
+
+ app.config.action_dispatch.show_exceptions = true
+
+ get "/foo"
+ assert_equal 404, last_response.status
+ assert_match "The page you were looking for doesn't exist.", last_response.body
+ end
+
+ test "accepts multiple apps for fallback" do
+ add_to_config <<-RUBY
+ exception_apps =
+ config.exceptions_app = [
+ lambda do |env|
+ raise "oppa gangnam style"
+ end,
+ lambda do |env|
+ [404, { "Content-Type" => "text/plain" }, ["YOU FAILED BRO"]]
+ end
+ ]
+ RUBY
+
+ app.config.action_dispatch.show_exceptions = true
+
+ get "/foo"
+ assert_equal 404, last_response.status
+ assert_equal "YOU FAILED BRO", last_response.body
+ end
+
+
test "unspecified route when action_dispatch.show_exceptions is not set raises an exception" do
app.config.action_dispatch.show_exceptions = false

0 comments on commit 5c5b023

Please sign in to comment.