From 0974ef8d4b9cf0674797c1737fafa9faa690d840 Mon Sep 17 00:00:00 2001
From: st0012 <stan001212@gmail.com>
Date: Fri, 25 Dec 2020 10:42:06 +0800
Subject: [PATCH] Use middleware instead of method override to handle rescued
 exceptions

There are a few drawbacks to the method override approach:
1. It's hard to properly clean up the patched method after tests. This
   makes setting apps for different setups very hard.
2. We need to target specific middlewares to make it work. If there's
   another exception-rescuing middleware in the middle that isn't patched,
   the exception would still not be reported.
   See https://github.com/getsentry/sentry-ruby/issues/738

So this commit inserts a new middleware for capturing rescued
exceptions. It works like this:

- The new middleware should be the last middleware of the app.
- When `config.rails.report_rescued_exceptions = true` there's an exception raised
  1. the exception will be stored in the request env under the `"sentry.rescued_exception"` key.
  2. the CaptureExceptions middleware will detect that env and report
     the exception.
- When `config.rails.report_rescued_exceptions = false` there's an exception raised
  - It doesn't store the exception in request env, so if the exception
    is captured by other middlewares, it won't be reported.

Although the approach is very different than the old one, it should
be able to solve the same problem and cause fewer bugs.
---
 .../lib/sentry/rails/capture_exceptions.rb     |  2 +-
 sentry-rails/lib/sentry/rails/railtie.rb       | 18 ++++--------------
 .../rails/rescued_exception_interceptor.rb     | 18 ++++++++++++++++++
 3 files changed, 23 insertions(+), 15 deletions(-)
 create mode 100644 sentry-rails/lib/sentry/rails/rescued_exception_interceptor.rb

diff --git a/sentry-rails/lib/sentry/rails/capture_exceptions.rb b/sentry-rails/lib/sentry/rails/capture_exceptions.rb
index 2f8f339b7..149c8d394 100644
--- a/sentry-rails/lib/sentry/rails/capture_exceptions.rb
+++ b/sentry-rails/lib/sentry/rails/capture_exceptions.rb
@@ -2,7 +2,7 @@ module Sentry
   module Rails
     class CaptureExceptions < Sentry::Rack::CaptureExceptions
       def collect_exception(env)
-        super || env["action_dispatch.exception"]
+        super || env["action_dispatch.exception"] || env["sentry.rescued_exception"]
       end
 
       def transaction_op
diff --git a/sentry-rails/lib/sentry/rails/railtie.rb b/sentry-rails/lib/sentry/rails/railtie.rb
index 25add782f..0413a9f8d 100644
--- a/sentry-rails/lib/sentry/rails/railtie.rb
+++ b/sentry-rails/lib/sentry/rails/railtie.rb
@@ -1,5 +1,6 @@
 require "rails"
 require "sentry/rails/capture_exceptions"
+require "sentry/rails/rescued_exception_interceptor"
 require "sentry/rails/backtrace_cleaner"
 require "sentry/rails/controller_methods"
 require "sentry/rails/controller_transaction"
@@ -10,7 +11,10 @@ module Sentry
   class Railtie < ::Rails::Railtie
     # middlewares can't be injected after initialize
     initializer "sentry.use_rack_middleware" do |app|
+      # need to be placed at first to capture as many errors as possible
       app.config.middleware.insert 0, Sentry::Rails::CaptureExceptions
+      # need to be placed at last to smuggle app exceptions via env
+      app.config.middleware.use(Sentry::Rails::RescuedExceptionInterceptor)
     end
 
     config.after_initialize do
@@ -19,7 +23,6 @@ class Railtie < ::Rails::Railtie
       configure_sentry_logger
       extend_controller_methods
       extend_active_job
-      override_exceptions_handling
       override_streaming_reporter
       setup_backtrace_cleanup_callback
       inject_breadcrumbs_logger
@@ -57,19 +60,6 @@ def setup_backtrace_cleanup_callback
       end
     end
 
-    def override_exceptions_handling
-      if Sentry.configuration.rails.report_rescued_exceptions
-        require 'sentry/rails/overrides/debug_exceptions_catcher'
-        if defined?(::ActionDispatch::DebugExceptions)
-          exceptions_class = ::ActionDispatch::DebugExceptions
-        elsif defined?(::ActionDispatch::ShowExceptions)
-          exceptions_class = ::ActionDispatch::ShowExceptions
-        end
-
-        exceptions_class.send(:prepend, Sentry::Rails::Overrides::DebugExceptionsCatcher)
-      end
-    end
-
     def override_streaming_reporter
       ActiveSupport.on_load :action_view do
         ActionView::StreamingTemplateRenderer::Body.send(:prepend, Sentry::Rails::Overrides::StreamingReporter)
diff --git a/sentry-rails/lib/sentry/rails/rescued_exception_interceptor.rb b/sentry-rails/lib/sentry/rails/rescued_exception_interceptor.rb
new file mode 100644
index 000000000..3beaff3ef
--- /dev/null
+++ b/sentry-rails/lib/sentry/rails/rescued_exception_interceptor.rb
@@ -0,0 +1,18 @@
+module Sentry
+  module Rails
+    class RescuedExceptionInterceptor
+      def initialize(app)
+        @app = app
+      end
+
+      def call(env)
+        begin
+          @app.call(env)
+        rescue => e
+          env["sentry.rescued_exception"] = e if Sentry.configuration.rails.report_rescued_exceptions
+          raise e
+        end
+      end
+    end
+  end
+end