Exceptions not sent to Sentry from development environment (better_errors) #738

shrirambalakrishnan · 2017-08-12T04:50:51Z

In development environment, only when exception is captured explicitly using Raven.capture, the exception is getting sent to Sentry and shown in the list of exceptions.

But if config.consider_all_requests_local = false in development.rb, the exceptions are sent.

Related issue - #202

Gem versions:
rails-4.2.7.1
sentry-raven-2.6.2

The text was updated successfully, but these errors were encountered:

nateberkopec · 2017-08-14T15:14:41Z

I'm going to need some additional reproduction steps. Using a blank "rails new" application in development (so config.consider_all_requests_local = true), I can successfully raise and capture exceptions w/Sentry, e.g.:

class HelloController < ApplicationController
  def exception
    raise "Hi!"
  end
end

nateberkopec · 2017-08-14T15:52:24Z

You can try modifying this script to reproduce your issue.

irobayna · 2017-08-21T20:19:11Z

@nateberkopec Same here with an Ruby on Rails API, I do see upon startup

INFO -- sentry: ** [Raven] Raven 2.6.3 ready to catch errors

specs:

Rails 5.1.3
Puma 3.9.1
Ruby 2.4.1-p111

BTW, if I send a command to capture message that works:

Raven.capture_message("Something went very wrong")

Your example above does not...

nateberkopec · 2017-10-09T20:41:54Z

Please feel free to reopen with reproduction steps.

d1ceward · 2018-04-25T11:57:16Z

I have same problem and for me it's caused by better_errors gem.

Simply add better_errors gem to your script to reproduce.

punitcse · 2018-07-06T10:27:04Z

I also have the same issue using
rails 5.2
sentry-raven-2.7.4

kerwin-personalyze · 2018-07-06T16:55:43Z

Hi, I also encountered the same issue.
I don't have better_errors gem in my project.

What I did was to declare this in my controller

      def index
        1/0
      end

and have the following set in my sentry.rb

Raven.configure do |config|
  config.dsn = *******
  config.sanitize_fields = Rails.application.config.filter_parameters.map(&:to_s)
  config.environments = %w[development]
  config.excluded_exceptions = []
end

If I invoke Raven.send_capture(), then it works.

brutuscat · 2018-10-30T12:59:01Z

Hello, I stumbled against this "conflict" with the BetterErrors middleware too. IMHO, given that the BetterErrors middleware is inserted after the DebugExceptions and exceptions for some reason never reach DebugExceptions (is as if the bubbling goes the other way in the stack?), maybe we should consider either monkey patching the BetterErrors::Middleware#protected_app_call or move the Rack middleware to the end instead that at the beginning.

silent-e · 2018-12-12T00:27:03Z

I'm seeing this issue as well and I can reproduce the weirdness 100% of the time in my development env. With better_errors installed Raven does not capture the error. Without better_errors enabled, the error is captured.

As better_errors is more important to me in the development environment I'm just going to disable Raven in dev.

(e)

alphabt · 2019-05-24T12:40:28Z

+1. Would be nice to have this be compatible with better_errors.

cjlarose · 2020-02-28T20:45:30Z

Raven inserts its middleware, Raven::Rack, in the first (outermost) position. This is desirable because it catches any exceptions that bubble up, especially those that happen in the middleware stack itself.

BetterErrors inserts its middleware, BetterErrors::Middleware, just after DebugExceptions, if it is defined. In the PR that put it there, it's mentioned that this is nice since BetterErrors will render a page for exceptions that originate in all middlewares that are after DebugExceptions. It must be after DebugExceptions itself, however, since DebugExceptions would otherwise swallow the exception before BetterErrors could access it.

In order to prevent DebugExceptions from swallowing the exception, Raven instead patches ActionDispatch::DebugExceptions and ActionDispatch::ShowExceptions in that way that makes it so that errors are reported to Sentry first before rendering the error page.

So, one way to add compatibility for BetterErrors to Raven is to patch BetterErrors::Middleware in a similar way that Raven currently does to ActionDispatch::DebugExcecptions and ActionDispatch::ShowExceptions.

In a Rails initializer,

# https://github.com/getsentry/raven-ruby/issues/738#issuecomment-592720365
if defined?(::BetterErrors::Middleware)
  module BetterErrorsRavenCompatibility
    def show_error_page(env, exception = nil)
      begin
        Raven::Rack.capture_exception exception, env if exception
      rescue StandardError # rubocop:disable Lint/HandleExceptions
      end

      super
    end
  end

  Rails.application.config.after_initialize do
    if Raven.configuration.rails_report_rescued_exceptions
      BetterErrors::Middleware.prepend BetterErrorsRavenCompatibility
    end
  end
end

This works, but since it's basically a monkey-patch to BetterErrors, it relies on private API that might change between versions.

It's not clear what the best long-term fix is, though. It doesn't really make sense for BetterErrors to automatically detect Raven and to automatically report the error. Similarly, I could understand if maintainers of Raven didn't want to add custom code for maintaining BetterErrors support.

The right fix might just be to add documentation for Raven somewhere that includes a patch like this.

nateberkopec · 2020-02-28T20:56:12Z

@cjlarose I don't maintain this anymore (someone else at Sentry does) but just wanted to leave a comment: you leave some of the amazingly clear and detailed explanations of bugs on OSS repos that I have ever seen.

sedubois · 2020-12-23T13:49:49Z

We encounter the same issue in our app with better_errors (for now we use the mentioned workaround, thanks!).

@st0012 what is the intended solution considering the "wontfix" label added on Sep 17?

st0012 · 2020-12-23T14:17:00Z

this is not really a sentry SDK's issue imo (nor better_error's though).

this kind of issue can happen when any Ruby gem's middleware tries to rescue errors before Sentry's middleware (which is not hard to do as the Sentry's middleware will be placed at the very front). and from a SDK's point of view, we can't possibly detect and resolve each of those cases from the SDK itself. so imo the best solution should be to have a standalone gem, like sentry-better_error for injecting the above patch.

but given that this is not a bug in SDK itself + we still have higher priority tasks, we don't plan to add the additional support at the moment. this is why it's marked as wontfix. but it'll also be kept open for people to look up the solution easier.

sedubois · 2020-12-23T14:26:43Z

OK. For info here is our initializer after updating from sentry-raven to sentry-ruby 4.1.1:

# https://github.com/getsentry/sentry-ruby/issues/738#issuecomment-750323266
if defined?(::BetterErrors::Middleware)
  module BetterErrorsSentryCompatibility
    def show_error_page(env, exception = nil)
      begin
        Sentry::Rack.capture_exception exception, env if exception
      rescue StandardError # rubocop:disable Lint/HandleExceptions
      end

      super
    end
  end

  Rails.application.config.after_initialize do
    if Sentry.configuration.rails.report_rescued_exceptions
      BetterErrors::Middleware.prepend BetterErrorsSentryCompatibility
    end
  end
end

st0012 · 2020-12-23T14:29:01Z

@sedubois thanks for posting an updated version 👍

There are a few drawbacks on the method override approach: 1. It's hard to properly cleanup the patched method after tests. This makes setting app 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 aren'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, but it should be able to solve the same problem and cause less bugs.

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.

st0012 · 2020-12-25T06:59:12Z

Good news: I came up with another way to address these kinds of issue in #1168. The PR should fix this issue and will be included in version 4.1.2 of sentry-rails.

However, I won't backport this fix to the old sentry-raven SDK as this is not a critical issue. So I highly recommend you to migrate to the new SDK by following this guideline.

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.

#1168) * Allow configuring test app dynamically as well * Group test cases base on prod/dev setup * 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 #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. * Remove DebugExceptionsCatcher

cooperka · 2020-12-31T00:22:40Z

With the latest sentry-rails v4.1.2, errors weren't being sent in development when I explicitly specified config.environment = "development".

This is the default (it uses RAILS_ENV) so I removed the config.environment line and it fixed the problem for me. Strange but happy it's working now.

nateberkopec added the needs-repro label Aug 14, 2017

nateberkopec mentioned this issue Aug 14, 2017

error tracking with rails is not working #735

Closed

nateberkopec closed this as completed Oct 9, 2017

nateberkopec reopened this Apr 26, 2018

nateberkopec changed the title ~~Exceptions not sent to Sentry from development environment~~ Exceptions not sent to Sentry from development environment (better_errors Aug 10, 2018

nateberkopec changed the title ~~Exceptions not sent to Sentry from development environment (better_errors~~ Exceptions not sent to Sentry from development environment (better_errors) Aug 10, 2018

nateberkopec changed the title ~~Exceptions not sent to Sentry from development environment (better_errors)~~ Exceptions not sent to Sentry from development environment (better_errors) Aug 10, 2018

nateberkopec mentioned this issue Aug 10, 2018

conflict with better_errors #853

Closed

st0012 added this to Needs triage in 3.x via automation Aug 14, 2020

st0012 added this to the 3.1.0 milestone Aug 14, 2020

st0012 moved this from Needs triage to Medium priority in 3.x Aug 14, 2020

st0012 added Type: Bug and removed needs-repro labels Aug 14, 2020

st0012 moved this from Medium priority to High priority in 3.x Sep 3, 2020

st0012 added the wontfix label Sep 17, 2020

st0012 removed this from High priority in 3.x Sep 17, 2020

st0012 removed this from the 3.1.0 milestone Sep 17, 2020

st0012 mentioned this issue Dec 25, 2020

Use middleware instead of method override to handle rescued exceptions #1168

Merged

st0012 added this to the 4.1.2 milestone Dec 25, 2020

st0012 added this to To do in 4.x via automation Dec 25, 2020

st0012 self-assigned this Dec 25, 2020

st0012 removed the wontfix label Dec 25, 2020

st0012 closed this as completed in #1168 Dec 25, 2020

4.x automation moved this from To do to Done Dec 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Exceptions not sent to Sentry from development environment (better_errors) #738

Exceptions not sent to Sentry from development environment (better_errors) #738

shrirambalakrishnan commented Aug 12, 2017

nateberkopec commented Aug 14, 2017

nateberkopec commented Aug 14, 2017

irobayna commented Aug 21, 2017

nateberkopec commented Oct 9, 2017

d1ceward commented Apr 25, 2018

punitcse commented Jul 6, 2018

kerwin-personalyze commented Jul 6, 2018

brutuscat commented Oct 30, 2018

silent-e commented Dec 12, 2018

alphabt commented May 24, 2019

cjlarose commented Feb 28, 2020 •

edited

Loading

nateberkopec commented Feb 28, 2020

sedubois commented Dec 23, 2020 •

edited

Loading

st0012 commented Dec 23, 2020 •

edited

Loading

sedubois commented Dec 23, 2020 •

edited

Loading

st0012 commented Dec 23, 2020

st0012 commented Dec 25, 2020

cooperka commented Dec 31, 2020

Exceptions not sent to Sentry from development environment (better_errors) #738

Exceptions not sent to Sentry from development environment (better_errors) #738

Comments

shrirambalakrishnan commented Aug 12, 2017

nateberkopec commented Aug 14, 2017

nateberkopec commented Aug 14, 2017

irobayna commented Aug 21, 2017

nateberkopec commented Oct 9, 2017

d1ceward commented Apr 25, 2018

punitcse commented Jul 6, 2018

kerwin-personalyze commented Jul 6, 2018

brutuscat commented Oct 30, 2018

silent-e commented Dec 12, 2018

alphabt commented May 24, 2019

cjlarose commented Feb 28, 2020 • edited Loading

nateberkopec commented Feb 28, 2020

sedubois commented Dec 23, 2020 • edited Loading

st0012 commented Dec 23, 2020 • edited Loading

sedubois commented Dec 23, 2020 • edited Loading

st0012 commented Dec 23, 2020

st0012 commented Dec 25, 2020

cooperka commented Dec 31, 2020

cjlarose commented Feb 28, 2020 •

edited

Loading

sedubois commented Dec 23, 2020 •

edited

Loading

st0012 commented Dec 23, 2020 •

edited

Loading

sedubois commented Dec 23, 2020 •

edited

Loading