Skip to content
This repository

Enable Exception Fallback/Failsafe Behavior #8425

Open
wants to merge 1 commit into from

4 participants

Richard Schneeman Steve Richert Steve Klabnik Łukasz Strzałkowski
Richard Schneeman
Collaborator

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 fail, 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 error rendering 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

Richard Schneeman 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
5c5b023
Steve Richert

"Exception in your exceptions app" == inception app

Steve Klabnik
Collaborator

Looks fine to me.

Richard Schneeman
Collaborator

fun story, i just got assigned this issue from codetriage: So, ping.

Łukasz Strzałkowski
Collaborator

@schneems What's the status here? Can #7804 be closed? I guess if any of them will be merged, it'll be this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Dec 05, 2012
Richard Schneeman 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
5c5b023
This page is out of date. Refresh to see the latest.
39  actionpack/lib/action_dispatch/middleware/exception_failsafe.rb
... ...
@@ -0,0 +1,39 @@
  1
+require 'action_dispatch/http/request'
  2
+require 'action_dispatch/middleware/exception_wrapper'
  3
+
  4
+module ActionDispatch
  5
+  # This class is used by ShowException middleware to guarantee
  6
+  # a response is returned by the app. This class can take multiple
  7
+  # exception applications calling each in turn until one returns
  8
+  # successfully. If no apps return successfully, a FAILSAFE_RESPONSE
  9
+  # is delivered
  10
+  class ExceptionFailsafe
  11
+    FAILSAFE_RESPONSE = [500, { 'Content-Type' => 'text/plain' },
  12
+      ["500 Internal Server Error\n" <<
  13
+       "If you are the administrator of this website, then please read this web " <<
  14
+       "application's log file and/or the web server's log file to find out what " <<
  15
+       "went wrong."]]
  16
+
  17
+    def initialize(*exception_apps)
  18
+      @exception_apps = exception_apps.flatten.compact.uniq
  19
+    end
  20
+
  21
+    def call(env)
  22
+      try_exception_apps(env, @exception_apps.dup)
  23
+    end
  24
+
  25
+    private
  26
+
  27
+    # Takes an array of exception apps, pops one off the front
  28
+    # and attempts to render the exception. Continues until
  29
+    # all apps have errored, then returns the FAILSAFE_RESPONSE
  30
+    def try_exception_apps(env, exception_apps)
  31
+      return FAILSAFE_RESPONSE if exception_apps.blank?
  32
+      exception_app = exception_apps.shift
  33
+      return exception_app.call(env)
  34
+    rescue Exception => failsafe_error
  35
+      $stderr.puts "Error during failsafe response: #{failsafe_error}\n  #{failsafe_error.backtrace * "\n  "}"
  36
+      try_exception_apps(env, exception_apps)
  37
+    end
  38
+  end
  39
+end
28  actionpack/lib/action_dispatch/middleware/show_exceptions.rb
... ...
@@ -1,39 +1,30 @@
1 1
 require 'action_dispatch/http/request'
2 2
 require 'action_dispatch/middleware/exception_wrapper'
  3
+require 'action_dispatch/middleware/exception_failsafe'
3 4
 
4 5
 module ActionDispatch
5 6
   # This middleware rescues any exception returned by the application
6  
-  # and calls an exceptions app that will wrap it in a format for the end user.
  7
+  # and calls ActionDispatch::ExceptionFailsafe app that will wrap it
  8
+  # in a format for the end user.
7 9
   #
8  
-  # The exceptions app should be passed as parameter on initialization
  10
+  # Multiple exceptions apps can be passed as parameters on initialization
9 11
   # of ShowExceptions. Every time there is an exception, ShowExceptions will
10 12
   # store the exception in env["action_dispatch.exception"], rewrite the
11 13
   # PATH_INFO to the exception status code and call the rack app.
12 14
   #
13 15
   # If the application returns a "X-Cascade" pass response, this middleware
14 16
   # will send an empty response as result with the correct status code.
15  
-  # If any exception happens inside the exceptions app, this middleware
16  
-  # catches the exceptions and returns a FAILSAFE_RESPONSE.
17 17
   class ShowExceptions
18  
-    FAILSAFE_RESPONSE = [500, { 'Content-Type' => 'text/plain' },
19  
-      ["500 Internal Server Error\n" <<
20  
-       "If you are the administrator of this website, then please read this web " <<
21  
-       "application's log file and/or the web server's log file to find out what " <<
22  
-       "went wrong."]]
23  
-
24 18
     def initialize(app, exceptions_app)
25 19
       @app = app
26 20
       @exceptions_app = exceptions_app
27 21
     end
28 22
 
29 23
     def call(env)
30  
-      begin
31  
-        response = @app.call(env)
32  
-      rescue Exception => exception
33  
-        raise exception if env['action_dispatch.show_exceptions'] == false
34  
-      end
35  
-
36  
-      response || render_exception(env, exception)
  24
+      return @app.call(env)
  25
+    rescue Exception => exception
  26
+      raise exception if env['action_dispatch.show_exceptions'] == false
  27
+      render_exception(env, exception)
37 28
     end
38 29
 
39 30
     private
@@ -45,9 +36,6 @@ def render_exception(env, exception)
45 36
       env["PATH_INFO"] = "/#{status}"
46 37
       response = @exceptions_app.call(env)
47 38
       response[1]['X-Cascade'] == 'pass' ? pass_response(status) : response
48  
-    rescue Exception => failsafe_error
49  
-      $stderr.puts "Error during failsafe response: #{failsafe_error}\n  #{failsafe_error.backtrace * "\n  "}"
50  
-      FAILSAFE_RESPONSE
51 39
     end
52 40
 
53 41
     def pass_response(status)
2  actionpack/test/controller/show_exceptions_test.rb
@@ -98,7 +98,7 @@ class ShowFailsafeExceptionsTest < ActionDispatch::IntegrationTest
98 98
     def test_render_failsafe_exception
99 99
       @app = ShowExceptionsOverridenController.action(:boom)
100 100
       @exceptions_app = @app.instance_variable_get(:@exceptions_app)
101  
-      @app.instance_variable_set(:@exceptions_app, nil)
  101
+      @app.instance_variable_set(:@exceptions_app, ActionDispatch::ExceptionFailsafe.new([]))
102 102
       $stderr = StringIO.new
103 103
 
104 104
       get '/', {}, 'HTTP_ACCEPT' => 'text/json'
2  railties/lib/rails/application.rb
@@ -352,7 +352,7 @@ def default_middleware_stack #:nodoc:
352 352
         middleware.use ::Rack::MethodOverride
353 353
         middleware.use ::ActionDispatch::RequestId
354 354
         middleware.use ::Rails::Rack::Logger, config.log_tags # must come after Rack::MethodOverride to properly log overridden methods
355  
-        middleware.use ::ActionDispatch::ShowExceptions, config.exceptions_app || ActionDispatch::PublicExceptions.new(Rails.public_path)
  355
+        middleware.use ::ActionDispatch::ShowExceptions, ActionDispatch::ExceptionFailsafe.new(config.exceptions_app, ActionDispatch::PublicExceptions.new(Rails.public_path))
356 356
         middleware.use ::ActionDispatch::DebugExceptions, app
357 357
         middleware.use ::ActionDispatch::RemoteIp, config.action_dispatch.ip_spoofing_check, config.action_dispatch.trusted_proxies
358 358
 
35  railties/test/application/middleware/exceptions_test.rb
@@ -60,6 +60,41 @@ def index
60 60
       assert_equal "YOU FAILED BRO", last_response.body
61 61
     end
62 62
 
  63
+    test "falls back on public app if custom exception app raises error" do
  64
+      add_to_config <<-RUBY
  65
+        config.exceptions_app = lambda do |env|
  66
+          raise "hover boards don't work on water"
  67
+        end
  68
+      RUBY
  69
+
  70
+      app.config.action_dispatch.show_exceptions = true
  71
+
  72
+      get "/foo"
  73
+      assert_equal 404, last_response.status
  74
+      assert_match "The page you were looking for doesn't exist.", last_response.body
  75
+    end
  76
+
  77
+    test "accepts multiple apps for fallback" do
  78
+      add_to_config <<-RUBY
  79
+      exception_apps =
  80
+        config.exceptions_app = [
  81
+          lambda do |env|
  82
+            raise "oppa gangnam style"
  83
+          end,
  84
+          lambda do |env|
  85
+            [404, { "Content-Type" => "text/plain" }, ["YOU FAILED BRO"]]
  86
+          end
  87
+      ]
  88
+      RUBY
  89
+
  90
+      app.config.action_dispatch.show_exceptions = true
  91
+
  92
+      get "/foo"
  93
+      assert_equal 404, last_response.status
  94
+      assert_equal "YOU FAILED BRO", last_response.body
  95
+    end
  96
+
  97
+
63 98
     test "unspecified route when action_dispatch.show_exceptions is not set raises an exception" do
64 99
       app.config.action_dispatch.show_exceptions = false
65 100
 
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.