From 36306b252f61953566a355d5f40f754bfbfd61e9 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Fri, 8 Oct 2021 09:34:36 +0100 Subject: [PATCH 1/3] Refactor conditional Extract to local explanatory variable for line length and readability. Improve readability by eliminating double negation. --- config/environments/production.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/config/environments/production.rb b/config/environments/production.rb index 0f684b3b75..589e5de902 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -124,7 +124,11 @@ exception_notifier_prefix << "[#{ AlaveteliConfiguration.domain }] " end - if !AlaveteliConfiguration.exception_notifications_from.blank? && !AlaveteliConfiguration.exception_notifications_to.blank? + notify_exceptions = + AlaveteliConfiguration.exception_notifications_from.present? && + AlaveteliConfiguration.exception_notifications_to.present? + + if notify_exceptions middleware.use ExceptionNotification::Rack, :ignore_exceptions => ['ActionController::BadRequest'] + ExceptionNotifier.ignored_exceptions, :email => { From 25599b66ee48bdb760f698cf78c0a019abd09851 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Fri, 8 Oct 2021 09:38:44 +0100 Subject: [PATCH 2/3] Refactor ExceptionNotification config * Extract `ignore_exceptions` local variable for line length * Fix Hash style --- config/environments/production.rb | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/config/environments/production.rb b/config/environments/production.rb index 589e5de902..86ad6c7479 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -129,12 +129,16 @@ AlaveteliConfiguration.exception_notifications_to.present? if notify_exceptions + ignored_exceptions = %w( + ActionController::BadRequest + ) + ExceptionNotifier.ignored_exceptions + middleware.use ExceptionNotification::Rack, - :ignore_exceptions => ['ActionController::BadRequest'] + ExceptionNotifier.ignored_exceptions, - :email => { - :email_prefix => exception_notifier_prefix, - :sender_address => AlaveteliConfiguration.exception_notifications_from, - :exception_recipients => AlaveteliConfiguration.exception_notifications_to + ignore_exceptions: ignored_exceptions, + email: { + email_prefix: exception_notifier_prefix, + sender_address: AlaveteliConfiguration.exception_notifications_from, + exception_recipients: AlaveteliConfiguration.exception_notifications_to } end From 1b6eb8fe5e8137c6291dd740354ff59a67beed57 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Fri, 8 Oct 2021 09:46:46 +0100 Subject: [PATCH 3/3] Ignore ActionDispatch::Http::MimeNegotiation::InvalidType This expcetion was added between Rails 5.2 and 6.0 [1] in #35604 [2] and #40353. Under Rails 6, in development a 406 HTTP status is returned: > curl -I -H "Content-Type: *" http://localhost:3000 HTTP/1.1 406 Not Acceptable In production, we also get a 406: > curl -I -H "Content-Type: *" https://www.whatdotheyknow.com/cy/request/merthy_road_whitchurch_cardiff HTTP/2 406 This appears to be the correct behaviour and in the PRs above the Rails core team suggest ignoring these exceptions [4]. > You're right that it's not something you can fix which is why we > suggest ignoring it for exception monitoring services like Appsignal, > Sentry and Bugsnag but it's still logged so you can go back and > analyse things if necessary. Fixes https://github.com/mysociety/alaveteli/issues/6553. Hat tip to @gbp for the investigation and explanation included in this commit message. [1] https://github.com/rails/rails/blob/1b5658b562b29f8aab7e80cbab8adbd2ddea5447/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb#L15 [2] https://github.com/rails/rails/pull/35604 [3] https://github.com/rails/rails/pull/40353 [4] https://github.com/rails/rails/pull/35604#issuecomment-835274505 --- config/environments/production.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/config/environments/production.rb b/config/environments/production.rb index 86ad6c7479..afa31643d6 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -131,6 +131,7 @@ if notify_exceptions ignored_exceptions = %w( ActionController::BadRequest + ActionDispatch::Http::MimeNegotiation::InvalidType ) + ExceptionNotifier.ignored_exceptions middleware.use ExceptionNotification::Rack,