Skip to content

Commit

Permalink
Use middleware instead of method override to handle rescued exceptions
Browse files Browse the repository at this point in the history
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 #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.
  • Loading branch information
st0012 committed Dec 25, 2020
1 parent db3dc57 commit 0974ef8
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 15 deletions.
2 changes: 1 addition & 1 deletion sentry-rails/lib/sentry/rails/capture_exceptions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 4 additions & 14 deletions sentry-rails/lib/sentry/rails/railtie.rb
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
18 changes: 18 additions & 0 deletions sentry-rails/lib/sentry/rails/rescued_exception_interceptor.rb
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 0974ef8

Please sign in to comment.