Handle uncaught exceptions (#1291)

https://eaflood.atlassian.net/browse/RUBY-2278
https://eaflood.atlassian.net/browse/RUBY-2284

app/controllers/waste_carriers_engine/application_controller.rb

@@ -2,6 +2,8 @@
 
 module WasteCarriersEngine
   class ApplicationController < ActionController::Base
+    include WasteCarriersEngine::CanAddDebugLogging
+
     # Prevent CSRF attacks by raising an exception.
     # For APIs, you may want to use :null_session instead.
     protect_from_forgery with: :exception
@@ -10,5 +12,13 @@ class ApplicationController < ActionController::Base
     layout "application"
 
     default_form_builder GOVUKDesignSystemFormBuilder::FormBuilder
+
+    rescue_from StandardError do |e|
+      Airbrake.notify e
+      Rails.logger.error "Unhandled exception: #{e}"
+      log_transient_registration_details("Uncaught system error", @transient_registration)
+      redirect_to "/bo/pages/system_error"
+    end
+
   end
 end

app/services/concerns/waste_carriers_engine/can_add_debug_logging.rb

@@ -4,39 +4,43 @@ module WasteCarriersEngine
   module CanAddDebugLogging
     extend ActiveSupport::Concern
 
-    # rubocop:disable Metrics/MethodLength, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
+    # rubocop:disable Metrics/MethodLength, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity, Metrics/AbcSize
     def log_transient_registration_details(description, transient_registration)
       return unless FeatureToggle.active?(:additional_debug_logging)
 
-      error = if transient_registration.nil?
-                StandardError.new("#{description}: transient_registration is nil")
-              else
-                StandardError.new(
-                  "#{description}: " \
-                  "type: #{transient_registration.class}, " \
-                  "reg_identifier #{transient_registration.reg_identifier}, " \
-                  "from_magic_link: #{from_magic_link(transient_registration)}, " \
-                  "workflow_state: #{transient_registration.workflow_state}, " \
-                  "workflow_history: #{transient_registration.workflow_history}, " \
-                  "tier: #{transient_registration.tier}, " \
-                  "account_email: #{transient_registration.account_email}, " \
-                  "expires_on: #{transient_registration.expires_on}, " \
-                  "renew_token: #{renew_token(transient_registration)}, " \
-                  "metaData.route: #{transient_registration.metaData.route}, " \
-                  "created_at: #{transient_registration.created_at}, " \
-                  "orders: #{transient_registration.finance_details&.orders}, " \
-                  "payments: #{transient_registration.finance_details&.payments}"
-                )
-              end
-      Airbrake.notify(error, reg_identifier: transient_registration.reg_identifier) if defined?(Airbrake)
-      Rails.logger.warn error
+      details = if transient_registration.nil?
+                  { transient_registration: nil }
+                else
+                  { type: transient_registration.class.to_s,
+                    reg_identifier: transient_registration.reg_identifier,
+                    from_magic_link: from_magic_link(transient_registration),
+                    workflow_state: transient_registration.workflow_state,
+                    workflow_history: transient_registration.workflow_history.to_s,
+                    tier: transient_registration.tier,
+                    account_email: transient_registration.account_email,
+                    expires_on: transient_registration.expires_on,
+                    renew_token: renew_token(transient_registration),
+                    "metaData.route": transient_registration.metaData.route,
+                    created_at: transient_registration.created_at,
+                    orders: transient_registration.finance_details&.orders.to_s,
+                    payments: transient_registration.finance_details&.payments.to_s }
+                end
+      Airbrake.notify(StandardError.new(description), details)
+      Rails.logger.warn "#{description}: #{details}"
 
     # Handle any exceptions which arise while logging
     rescue StandardError => e
-      Airbrake.notify(e, reg_identifier: transient_registration.reg_identifier) if defined?(Airbrake)
-      Rails.logger.warn "Error writing transient registration details to the log: #{e}"
+      # Allow for the possibility that Airbrake.notify might raise an exception
+      begin
+        Airbrake.notify(e, reg_identifier: transient_registration&.reg_identifier)
+      rescue StandardError
+        Rails.logger.warn "Message not sent to Airbrake due to exception: #{e}"
+      ensure
+        Rails.logger.warn "Error writing debugging information for transient registration " \
+                          "#{transient_registration&.reg_identifier} to the log: #{e}"
+      end
     end
-    # rubocop:enable Metrics/MethodLength, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
+    # rubocop:enable Metrics/MethodLength, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity, Metrics/AbcSize
 
     private
 

app/views/pages/system_error.html.erb

@@ -0,0 +1,8 @@
+<% content_for :title, I18n.t(".system_error_title")  %>
+
+<div class="govuk-grid-row">
+  <div class="govuk-grid-column-two-thirds">
+    <h1 class="govuk-heading-m"><%= I18n.t(".system_error_heading") %></h1>
+    <p class="govuk-body"><%= I18n.t(".system_error_text") %></p>
+  </div>
+</div>

config/locales/en.yml

@@ -44,6 +44,10 @@ en:
           other: "%{count} addresses found"
 
   # Custom error pages
+  system_error_title: System error
+  system_error_heading: A system error has occurred
+  system_error_text: We are unable to complete this request at this time. Please try again later.
+
   invalid_reg_identifier_title: Cannot find page
   invalid_reg_identifier_heading: We cannot find that page
   invalid_reg_identifier_text: If you have recently completed a registration then check your email.

spec/services/concerns/waste_carriers_engine/can_add_debug_logging_spec.rb

@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+class DebugClass
+  include WasteCarriersEngine::CanAddDebugLogging
+end
+
+module WasteCarriersEngine
+  RSpec.describe CanAddDebugLogging do
+
+    describe "#log_transient_registration_details" do
+
+      let(:transient_registration) { create(:new_registration, :has_required_data) }
+
+      subject(:log_details) { DebugClass.new.log_transient_registration_details("foo", transient_registration) }
+
+      before do
+        allow(Airbrake).to receive(:notify)
+        allow(FeatureToggle).to receive(:active?).with(:additional_debug_logging).and_return true
+      end
+
+      it "logs an error" do
+        log_details
+        expect(Airbrake).to have_received(:notify)
+      end
+
+      context "with a nil transient_registration" do
+        before { allow(transient_registration).to receive(:nil?).and_return(true) }
+
+        it "logs an error" do
+          log_details
+          expect(Airbrake).to have_received(:notify)
+        end
+      end
+
+      context "when the additional logging raises an exception" do
+        before { allow(transient_registration).to receive(:metaData).and_raise(StandardError) }
+
+        it "catches the exception and notifies Airbrake anyway" do
+          log_details
+          expect(Airbrake).to have_received(:notify)
+        end
+      end
+
+      context "when the call to Airbrake fails" do
+        before { allow(Airbrake).to receive(:notify).and_raise(StandardError) }
+
+        it "completes without raising another exception" do
+          expect { log_details }.not_to raise_error
+        end
+      end
+    end
+  end
+end

spec/services/waste_carriers_engine/registration_completion_service_spec.rb

@@ -302,46 +302,6 @@ module WasteCarriersEngine
             expect(OrderItemLog.count).to be_zero
           end
         end
-
-        context "with temporary additional debugging" do
-
-          before do
-            allow(Airbrake).to receive(:notify)
-            allow(FeatureToggle).to receive(:active?).with(:additional_debug_logging).and_return true
-            allow(FeatureToggle).to receive(:active?).with(:govpay_payments).and_return true
-          end
-
-          it "logs an error" do
-            described_class.new.log_transient_registration_details("foo", transient_registration)
-
-            expect(Airbrake).to have_received(:notify)
-          end
-
-          context "with a nil transient_registration" do
-            before { allow(transient_registration).to receive(:nil?).and_return(true) }
-
-            it "logs an error" do
-              described_class.new.log_transient_registration_details("foo", transient_registration)
-
-              expect(Airbrake).to have_received(:notify)
-            end
-          end
-
-          context "when activating the registration raises an exception" do
-            let(:registration_activation_service) { instance_double(RegistrationActivationService) }
-
-            before do
-              allow(RegistrationActivationService).to receive(:new).and_return(registration_activation_service)
-              allow(registration_activation_service).to receive(:run).and_raise(StandardError)
-            end
-
-            it "logs an error" do
-              described_class.run(transient_registration)
-
-              expect(Airbrake).to have_received(:notify).at_least(:once)
-            end
-          end
-        end
       end
     end
   end