From afe96a4a90afea4e7549af75e77d8ac93470b4fc Mon Sep 17 00:00:00 2001 From: Kevin Dew Date: Tue, 17 Mar 2020 09:12:23 +0000 Subject: [PATCH 1/3] Revert rails_report_rescued_exceptions = false 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 --- lib/govuk_app_config/configure.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/govuk_app_config/configure.rb b/lib/govuk_app_config/configure.rb index 5b48f19a..57d3fd98 100644 --- a/lib/govuk_app_config/configure.rb +++ b/lib/govuk_app_config/configure.rb @@ -8,9 +8,16 @@ config.silence_ready = !Rails.env.production? if defined?(Rails) config.excluded_exceptions = [ + 'AbstractController::ActionNotFound', + 'ActionController::BadRequest', + 'ActionController::InvalidAuthenticityToken', + 'ActionController::ParameterMissing', + 'ActionController::RoutingError', 'ActionController::UnknownAction', + 'ActionController::UnknownHttpMethod', 'ActionDispatch::RemoteIp::IpSpoofAttackError', 'ActiveJob::DeserializationError', + 'ActiveRecord::RecordNotFound', 'CGI::Session::CookieStore::TamperedWithCookie', 'GdsApi::HTTPIntermittentServerError', 'GdsApi::TimedOutException', @@ -26,8 +33,4 @@ config.transport_failure_callback = Proc.new { GovukStatsd.increment("error_reports_failed") } - - # This stops exceptions rescued by rails from appearing in Sentry. - # See https://www.rubydoc.info/gems/sentry-raven/1.2.2/Raven%2FConfiguration:rails_report_rescued_exceptions - config.rails_report_rescued_exceptions = false end From 2ebf6f0819f8a583122f5cf389c7044228a2d9a6 Mon Sep 17 00:00:00 2001 From: Kevin Dew Date: Tue, 17 Mar 2020 09:47:32 +0000 Subject: [PATCH 2/3] Add all default Rails rescue response exceptions to ignore list 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. --- lib/govuk_app_config/configure.rb | 38 +++++++++++++++++++------------ 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/lib/govuk_app_config/configure.rb b/lib/govuk_app_config/configure.rb index 57d3fd98..261691b8 100644 --- a/lib/govuk_app_config/configure.rb +++ b/lib/govuk_app_config/configure.rb @@ -8,21 +8,29 @@ config.silence_ready = !Rails.env.production? if defined?(Rails) config.excluded_exceptions = [ - 'AbstractController::ActionNotFound', - 'ActionController::BadRequest', - 'ActionController::InvalidAuthenticityToken', - 'ActionController::ParameterMissing', - 'ActionController::RoutingError', - 'ActionController::UnknownAction', - 'ActionController::UnknownHttpMethod', - 'ActionDispatch::RemoteIp::IpSpoofAttackError', - 'ActiveJob::DeserializationError', - 'ActiveRecord::RecordNotFound', - 'CGI::Session::CookieStore::TamperedWithCookie', - 'GdsApi::HTTPIntermittentServerError', - 'GdsApi::TimedOutException', - 'Mongoid::Errors::DocumentNotFound', - 'Sinatra::NotFound', + # default Rails rescue responses + "ActionController::RoutingError", + "AbstractController::ActionNotFound", + "ActionController::MethodNotAllowed", + "ActionController::UnknownHttpMethod", + "ActionController::NotImplemented", + "ActionController::UnknownFormat", + "Mime::Type::InvalidMimeType", + "ActionController::MissingExactTemplate", + "ActionController::InvalidAuthenticityToken", + "ActionController::InvalidCrossOriginRequest", + "ActionDispatch::Http::Parameters::ParseError", + "ActionController::BadRequest", + "ActionController::ParameterMissing", + "Rack::QueryParser::ParameterTypeError", + "Rack::QueryParser::InvalidParameterError", + # additional items + "ActiveJob::DeserializationError", + "CGI::Session::CookieStore::TamperedWithCookie", + "GdsApi::HTTPIntermittentServerError", + "GdsApi::TimedOutException", + "Mongoid::Errors::DocumentNotFound", + "Sinatra::NotFound", ] # This will exclude exceptions that are triggered by one of the ignored From e486f67796427c600116d4208bbc320f0886c311 Mon Sep 17 00:00:00 2001 From: Kevin Dew Date: Tue, 17 Mar 2020 09:56:38 +0000 Subject: [PATCH 3/3] Update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0342d8ae..c912988c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# Unreleased + +* Revert using Sentry option of rails_report_rescued_exceptions (https://github.com/alphagov/govuk_app_config/pull/140) + # 2.1.0 * Stop exceptions rescued by rails from appearing in Sentry (https://github.com/alphagov/govuk_app_config/pull/138)