Query the original exception rather than Sentry::Event object

In the `before_send` callbacks, the first parameter is of type
`Sentry::Event`, and the second parameter (`hint`) is a hash which
contains the original exception at key `exception:`.

Our datasync-ignorable-exception logic relies on querying the
exception chain to see if any of the exceptions in the chain are
of an ignorable exception type. This logic doesn't work when we
view the `Sentry::Event` object (as we have been doing) - we must
instead look at the `hint[:exception]` object.

The consequence of this bug was that our logic to ignore certain
exceptions that occur during the datasync, no longer worked, and
we've therefore seen many thousands of Sentry errors since #196
was merged.

However, it wasn't as simple as just checking the `hint` parameter.
The `hint` parameter was missing, due to the way we had constructed
our tests. Sentry automatically provides the `hint` parameter, but
only if the event has been sent through all of the layers of
Sentry's architecture, which took a bit of trial and error to get
right. I took inspiration from [hub_spec.rb] and [sentry_spec.rb]
from the Sentry gem repository.

[hub_spec.rb]: https://github.com/getsentry/sentry-ruby/blob/e4b6d66612d9ce601596c23a5dd0d035c559d980/sentry-ruby/spec/sentry/hub_spec.rb
[sentry_spec.rb]: https://github.com/getsentry/sentry-ruby/blob/ace176cfceaefc91e29364a1c4180a67337e8ab0/sentry-ruby/spec/sentry_spec.rb

This has also exposed how our GovukStatsd incrementing was not
working, as it would log all errors as `error_types.sentry_event`
rather than their original exception types. This is fixed too.

lib/govuk_app_config/govuk_error/configuration.rb

@@ -27,38 +27,39 @@ def before_send=(closure)
   protected
 
     def ignore_exceptions_if_not_in_active_sentry_env
-      ->(error_or_event, _hint) { error_or_event if active_sentry_environments.include?(sentry_environment) }
+      ->(event, _hint) { event if active_sentry_environments.include?(sentry_environment) }
     end
 
     def ignore_excluded_exceptions_in_data_sync
-      lambda { |error_or_event, _hint|
+      lambda { |event, hint|
         data_sync_ignored_error = data_sync_excluded_exceptions.any? do |exception_to_ignore|
           exception_to_ignore = Object.const_get(exception_to_ignore) unless exception_to_ignore.is_a?(Module)
-          exception_chain = Sentry::Utils::ExceptionCauseChain.exception_to_array(error_or_event)
+          "exception_to_ignore"
+          exception_chain = Sentry::Utils::ExceptionCauseChain.exception_to_array(hint[:exception])
           exception_chain.any? { |exception| exception.is_a?(exception_to_ignore) }
         rescue NameError
           # the exception type represented by the exception_to_ignore string
           # doesn't even exist in this environment, so won't be found in the chain
           false
         end
 
-        error_or_event unless data_sync.in_progress? && data_sync_ignored_error
+        event unless data_sync.in_progress? && data_sync_ignored_error
       }
     end
 
     def increment_govuk_statsd_counters
-      lambda { |error_or_event, _hint|
+      lambda { |event, hint|
         GovukStatsd.increment("errors_occurred")
-        GovukStatsd.increment("error_types.#{error_or_event.class.name.demodulize.underscore}")
-        error_or_event
+        GovukStatsd.increment("error_types.#{hint[:exception].class.name.demodulize.underscore}")
+        event
       }
     end
 
     def run_before_send_callbacks
-      lambda do |error_or_event, hint|
-        result = error_or_event
+      lambda do |event, hint|
+        result = event
         @before_send_callbacks.each do |callback|
-          result = callback.call(error_or_event, hint)
+          result = callback.call(event, hint)
           break if result.nil?
         end
         result

spec/govuk_error/configuration_spec.rb

@@ -27,14 +27,16 @@
     it "ignores errors if they happen in an environment we don't care about" do
       ClimateControl.modify SENTRY_CURRENT_ENV: "some-temporary-environment" do
         configuration.active_sentry_environments << "production"
-        expect(configuration.before_send.call(StandardError.new)).to be_nil
+        sentry_client = send_event_to_sentry(StandardError.new, configuration)
+        expect(sentry_client.transport.events).to be_empty
       end
     end
 
     it "captures errors if they happen in an environment we care about" do
       ClimateControl.modify SENTRY_CURRENT_ENV: "production" do
         configuration.active_sentry_environments << "production"
-        expect(configuration.before_send.call(StandardError.new)).to be_truthy
+        sentry_client = send_event_to_sentry(StandardError.new, configuration)
+        expect(sentry_client.transport.events.count).to eq(1)
       end
     end
 
@@ -49,19 +51,22 @@
       end
 
       it "should capture errors by default" do
-        expect(configuration.before_send.call(StandardError.new)).to be_truthy
+        sentry_client = send_event_to_sentry(StandardError.new, configuration)
+        expect(sentry_client.transport.events.count).to eq(1)
       end
 
       it "should ignore errors that have been added as a string to data_sync_excluded_exceptions" do
         configuration.data_sync_excluded_exceptions << "StandardError"
 
-        expect(configuration.before_send.call(StandardError.new)).to be_nil
+        sentry_client = send_event_to_sentry(StandardError.new, configuration)
+        expect(sentry_client.transport.events).to be_empty
       end
 
       it "should ignore errors that have been added as a class to data_sync_excluded_exceptions" do
         configuration.data_sync_excluded_exceptions << StandardError
 
-        expect(configuration.before_send.call(StandardError.new)).to be_nil
+        sentry_client = send_event_to_sentry(StandardError.new, configuration)
+        expect(sentry_client.transport.events).to be_empty
       end
 
       it "should ignore errors whose underlying cause is an exception in data_sync_excluded_exceptions" do
@@ -81,15 +86,17 @@
         end
 
         expect(chained_exception).to be_an_instance_of(SomeOtherError)
-        expect(configuration.before_send.call(chained_exception)).to be_nil
+        sentry_client = send_event_to_sentry(chained_exception, configuration)
+        expect(sentry_client.transport.events).to be_empty
       end
 
       it "should ignore errors that are subclasses of an exception in data_sync_excluded_exceptions" do
         stub_const("SomeClass", Class.new(StandardError))
         stub_const("SomeInheritedClass", Class.new(SomeClass))
 
         configuration.data_sync_excluded_exceptions << "SomeClass"
-        expect(configuration.before_send.call(SomeInheritedClass.new)).to be_nil
+        sentry_client = send_event_to_sentry(SomeInheritedClass.new, configuration)
+        expect(sentry_client.transport.events).to be_empty
       end
     end
 
@@ -106,7 +113,8 @@
       it "should capture errors even if they are in the list of data_sync_excluded_exceptions" do
         configuration.data_sync_excluded_exceptions << "StandardError"
 
-        expect(configuration.before_send.call(StandardError.new)).to be_truthy
+        sentry_client = send_event_to_sentry(StandardError.new, configuration)
+        expect(sentry_client.transport.events.count).to eq(1)
       end
     end
 
@@ -116,7 +124,7 @@
           configuration.active_sentry_environments << "production"
           expect(GovukStatsd).to receive(:increment).exactly(1).times.with("errors_occurred")
           expect(GovukStatsd).to receive(:increment).exactly(1).times.with("error_types.standard_error")
-          configuration.before_send.call(StandardError.new)
+          send_event_to_sentry(StandardError.new, configuration)
         end
       end
     end
@@ -134,7 +142,7 @@
             error_or_event
           end
 
-          configuration.before_send.call(StandardError.new)
+          send_event_to_sentry(StandardError.new, configuration)
         end
       end
     end
@@ -157,7 +165,7 @@
             error_or_event
           end
 
-          configuration.before_send.call(StandardError.new)
+          send_event_to_sentry(StandardError.new, configuration)
         end
       end
     end
@@ -168,12 +176,15 @@
       ClimateControl.modify SENTRY_CURRENT_ENV: "production" do
         sentry_configurator = GovukError::Configuration.new(Sentry::Configuration.new)
         sentry_configurator.active_sentry_environments << "production"
-        sentry_configurator.before_send = lambda do |error_or_event, _hint|
-          error_or_event if error_or_event == "do capture"
+        stub_const("CustomError", Class.new(StandardError))
+        sentry_configurator.before_send = lambda do |event, hint|
+          event if hint[:exception].is_a?(CustomError)
         end
 
-        expect(sentry_configurator.before_send.call("do capture")).to be_truthy
-        expect(sentry_configurator.before_send.call("don't capture", {})).to be_nil
+        sentry_client = send_event_to_sentry(CustomError.new, sentry_configurator)
+        expect(sentry_client.transport.events.count).to eq(1)
+        sentry_client = send_event_to_sentry(StandardError.new, sentry_configurator)
+        expect(sentry_client.transport.events).to be_empty
       end
     end
 
@@ -188,8 +199,19 @@
         expect(GovukStatsd).not_to receive(:increment).with("errors_occurred")
         expect(GovukStatsd).not_to receive(:increment).with("error_types.standard_error")
 
-        sentry_configurator.before_send.call(StandardError.new)
+        send_event_to_sentry(StandardError.new, sentry_configurator)
       end
     end
   end
+
+  def send_event_to_sentry(event, configuration)
+    # prevent the sending happening in a separate worker, which would cause async results
+    configuration.background_worker_threads = 0
+    # force configuration to allow sending
+    allow(configuration).to receive(:sending_allowed?).and_return(true)
+    sentry_client = Sentry::Client.new(configuration)
+    sentry_hub = Sentry::Hub.new(sentry_client, Sentry::Scope.new)
+    sentry_hub.send(:capture_exception, event)
+    sentry_client
+  end
 end