Handled Sinatra exceptions still captured

[baelter]

Exceptions handled by Sinatra error blocks. eg.

error MyError do
  halt 400, "yadayada"
end

are still captured by the Sentry Rack middleware.

I think there should at least be a way to disable this.

Looking at the code I think we need to add something to Sinatra so the Sentry middleware can determine if the error is handled or not. As of now env['sintra.error'] is still set for handled errors. Still opening an issue here first to see what your thoughts are on this.

[baelter]

Perhaps the middleware should be configurable on which http status codes to capture Sinatra exceptions on?

[st0012]

Can you use config.excluded_exceptions to exclude the exceptions you don't want to see?
Mapping status code with exception seems to be a Sinatra feature (e.g. not supported by Rack) if I understand it correctly? In that case, I don't think we should bake that into sentry-ruby.

Or, do you think there are other Sinatra related features you want to see and it'd be a good idea to have sentry-sinatra?

[baelter]

We don't want to have to update excluded_exceptions every time we add a new handled exception, or refactor everything to use a shared base class.
Maybe application specific how you want to handle this, but if anyone ends up here with the same question, this is what we did to solve it.

Add a custom rack middleware that does

env.delete('sinatra.error')
env.delete('rack.exception')

On "handled" exceptions that should not go to Sentry. E.g. unless status.between?(499, 600)

[st0012]

@baelter Sorry for the long waiting because I also needed to learn Sinatra first to come up with a solution. I think you should be able to skip handled exceptions with before_send, like:

  config.before_send = lambda do |event, hint|
    handled_errors = Sinatra::Application.errors.keys.grep(Class) # skip status codes
    should_skip = false

    handled_errors.each do |error|
      if hint[:exception].is_a?(error)
        should_skip = true
      end
    end

    event unless should_skip
  end

It'll automatically skip the exceptions that are already registered with the error method.
However, this snippet won't work if you have async configured. In that case, you'll need to get the exception info differently. But async is not recommended anymore (see #1522) so I'd advise you to drop it if you still use it.

[baelter]

Thanks a lot, will consider that!

[st0012]

@baelter Glad to help. I'm closing this for now 🙂