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

Do not report exceptions rescued by rails #138

Merged

Conversation

AlanGabbianelli
Copy link
Contributor

@AlanGabbianelli AlanGabbianelli commented Mar 9, 2020

TL;DR: This stops exceptions rescued by rails from appearing in Sentry.

After upgrading frontend to rails 6, we started seeing this error:
Mime::Type::InvalidMimeType "charset=utf-8" is not a valid MIME type
Example here:
https://www.sentry.io/organizations/govuk/issues/1549200925/?end=2020-03-06T12%3A00%3A23&environment=staging&project=202225&query=is%3Aunresolved+url%3Ahttps%3A%2F%2Fwww-origin.staging.govuk.digital%2Freport-an-unregistered-trader-or-business&start=2020-03-05T16%3A07%3A23&utc=true

This is because, even though these errors are rescued by ActionDispatch,
they still reach Sentry. Here's the explanation from Sentry's docs:

Rails catches exceptions in the ActionDispatch::ShowExceptions or
ActionDispatch::DebugExceptions middlewares, depending on the
environment. When rails_report_rescued_exceptions is true (it is by
default), Raven will report exceptions even when they are rescued by
these middlewares.

So, in order to stop this behaviour, we initially took a double
approach:

  • set that option to be false for frontend in
    this PR to stop all
    exceptions rescued by rails (not only Mime::Type::InvalidMimeType)
    from appearing in Sentry for frontend
  • explicitly added Mime::Type::InvalidMimeType to the list of
    excluded exceptions in
    this PR to stop
    Mime::Type::InvalidMimeType from appearing in Sentry for all apps that
    depend on govuk_app_config

Setting rails_report_rescued_exceptions to false here is better
because it will apply this config to all apps that depend on
govuk_app_config, making the two PRs above redundant and saving us
from making this change in every app.

Setting config.rails_report_rescued_exceptions to false makes having
these exceptions in config.excluded_exceptions redundant.

All the exceptions present in config.action_dispatch.rescue_responses
can be removed from config.excluded_exceptions.

See
https://guides.rubyonrails.org/configuring.html#configuring-action-dispatch
for the list of exceptions in config.action_dispatch.rescue_responses.

Trello card: https://trello.com/c/Ik7ulDXQ/1768-3-upgrade-frontend-to-rails-6

TL;DR: This stops exceptions rescued by rails from appearing in Sentry.

After upgrading `frontend` to rails 6, we started seeing this error:
`Mime::Type::InvalidMimeType "charset=utf-8" is not a valid MIME type`
Example here:
https://www.sentry.io/organizations/govuk/issues/1549200925/?end=2020-03-06T12%3A00%3A23&environment=staging&project=202225&query=is%3Aunresolved+url%3Ahttps%3A%2F%2Fwww-origin.staging.govuk.digital%2Freport-an-unregistered-trader-or-business&start=2020-03-05T16%3A07%3A23&utc=true

This is because, even though these errors are rescued by ActionDispatch,
they still reach Sentry. Here's the explanation from Sentry's docs:

> Rails catches exceptions in the ActionDispatch::ShowExceptions or
> ActionDispatch::DebugExceptions middlewares, depending on the
> environment. When rails_report_rescued_exceptions is true (it is by
> default), Raven will report exceptions even when they are rescued by
> these middlewares.

So, in order to stop this behaviour, we initially took a double
approach:
- set that option to be false for `frontend` in
[this PR](alphagov/frontend#2264) to stop all
exceptions rescued by rails (not only `Mime::Type::InvalidMimeType`)
from appearing in Sentry for `frontend`
- explicitly added `Mime::Type::InvalidMimeType` to the list of
excluded exceptions in
[this PR](#137) to stop
`Mime::Type::InvalidMimeType` from appearing in Sentry for all apps that
depend on `govuk_app_config`

Setting `rails_report_rescued_exceptions` to `false` here is better
because it will apply this config to all apps that depend on
`govuk_app_config`, making the two PRs above redundant and saving us
from making this change in every app.

Trello card: https://trello.com/c/Ik7ulDXQ/1768-3-upgrade-frontend-to-rails-6
Setting `config.rails_report_rescued_exceptions` to `false` makes having
these exceptions in `config.excluded_exceptions` redundant.

All the exceptions present in `config.action_dispatch.rescue_responses`
can be removed from `config.excluded_exceptions`.

See
https://guides.rubyonrails.org/configuring.html#configuring-action-dispatch
for the list of exceptions in `config.action_dispatch.rescue_responses`.

Trello card: https://trello.com/c/Ik7ulDXQ/1768-3-upgrade-frontend-to-rails-6
@AlanGabbianelli AlanGabbianelli force-pushed the do-not-report-exceptions-rescued-by-ActionDispatch branch from 11d0191 to 8f25109 Compare March 9, 2020 12:38
@AlanGabbianelli AlanGabbianelli merged commit 18dc1ae into master Mar 9, 2020
@AlanGabbianelli AlanGabbianelli deleted the do-not-report-exceptions-rescued-by-ActionDispatch branch March 9, 2020 12:57
AlanGabbianelli pushed a commit to alphagov/frontend that referenced this pull request Mar 9, 2020
This version of the gem stops exceptions rescued by rails from reaching
Sentry. For more info on why and how, see alphagov/govuk_app_config#138

Trello card: https://trello.com/c/Ik7ulDXQ/1768-3-upgrade-frontend-to-rails-6
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