Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert rails_report_rescued_exceptions = false #140

Merged
merged 3 commits into from
Mar 17, 2020

Conversation

kevindew
Copy link
Member

This reverts #138

It turns out that this setting was more aggressive than we anticipated
and was actually preventing the reporting of any exception that Rails
handled. We had thought that this only prevented the reporting of
exceptions specially configured as Rails rescue responses.

It transpires that this stops the reporting of any exceptions that are
handled by ActionDispatch::ShowExceptions 1 which is effectively all
Rails exceptions as per raven-ruby 2.

It turns out that this setting was more aggressive than we anticipated
and was actually preventing the reporting of any exception that Rails
handled. We had thought that this only prevented the reporting of
exceptions specially configured as Rails rescue responses.

It transpires that this stops the reporting of any exceptions that are
handled by ActionDispatch::ShowExceptions [1] which is effectively all
Rails exceptions as per raven-ruby [2].

[1]: https://github.com/rails/rails/blob/38ec8db3f7d3c79936cdaa0a136b26ee1c977f7d/actionpack/lib/action_dispatch/middleware/show_exceptions.rb
[2]: https://github.com/getsentry/raven-ruby/blob/d9d41784c3b5888295bf4b013416f132045b22d4/lib/raven/integrations/rails.rb#L51-L52
This adds in all the Rails default exceptions as is listed in:
https://github.com/rails/rails/blob/38ec8db3f7d3c79936cdaa0a136b26ee1c977f7d/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb
and removes a legacy exception that was removed in Rails 4.

I did consider doing something like:

```
if defined?(ActionDispatch)
  config.excluded_exceptions += ActionDispatch::ExceptionWrapper.rescue_responses.keys
end
```

but I thought it was a bit opaque, particularly because the
rescue_responses attribute is edited during Rails initialisation, so
could be hard to understand why this had a particular setting.
@kevindew kevindew merged commit e3874f5 into master Mar 17, 2020
@kevindew kevindew deleted the revert-exception-rescue branch March 17, 2020 10:58
kevindew added a commit that referenced this pull request Mar 17, 2020
 * Revert using Sentry option of rails_report_rescued_exceptions (#140)
@kevindew kevindew mentioned this pull request Mar 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants