Merge should_capture into before_send

lib/govuk_app_config/govuk_error/configuration.rb

@@ -12,12 +12,14 @@ def initialize(_raven_configuration)
       @data_sync = GovukDataSync.new(ENV["GOVUK_DATA_SYNC_PERIOD"])
       self.active_sentry_environments = []
       self.data_sync_excluded_exceptions = []
-      self.should_capture = ignore_exceptions_based_on_env_and_data_sync
+      self.before_send = lambda do |error_or_event, hint|
+        (ignore_exceptions_based_on_env_and_data_sync.call(error_or_event, hint) && increment_govuk_statsd_counters.call(error_or_event, hint))
+      end
     end
 
-    def should_capture=(closure)
-      combined = lambda do |error_or_event|
-        (ignore_exceptions_based_on_env_and_data_sync.call(error_or_event) && closure.call(error_or_event))
+    def before_send=(closure)
+      combined = lambda do |error_or_event, hint|
+        (ignore_exceptions_based_on_env_and_data_sync.call(error_or_event, hint) && increment_govuk_statsd_counters.call(error_or_event, hint) && closure.call(error_or_event, hint))
       end
 
       super(combined)
@@ -26,18 +28,18 @@ def should_capture=(closure)
   protected
 
     def ignore_exceptions_based_on_env_and_data_sync
-      lambda do |error_or_event|
-        ignore_exceptions_if_not_in_active_sentry_env.call(error_or_event) &&
-          ignore_excluded_exceptions_in_data_sync.call(error_or_event)
+      lambda do |error_or_event, hint|
+        error_or_event if ignore_exceptions_if_not_in_active_sentry_env.call(error_or_event, hint) &&
+          ignore_excluded_exceptions_in_data_sync.call(error_or_event, hint)
       end
     end
 
     def ignore_exceptions_if_not_in_active_sentry_env
-      ->(_error_or_event) { active_sentry_environments.include?(sentry_environment) }
+      ->(_error_or_event, _hint) { active_sentry_environments.include?(sentry_environment) }
     end
 
     def ignore_excluded_exceptions_in_data_sync
-      lambda { |error_or_event|
+      lambda { |error_or_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 = Raven::Utils::ExceptionCauseChain.exception_to_array(error_or_event)
@@ -51,5 +53,13 @@ def ignore_excluded_exceptions_in_data_sync
         !(data_sync.in_progress? && data_sync_ignored_error)
       }
     end
+
+    def increment_govuk_statsd_counters
+      lambda { |error_or_event, _hint|
+        GovukStatsd.increment("errors_occurred")
+        GovukStatsd.increment("error_types.#{error_or_event.class.name.demodulize.underscore}")
+        error_or_event
+      }
+    end
   end
 end

lib/govuk_app_config/govuk_error/configure.rb

@@ -1,10 +1,4 @@
 GovukError.configure do |config|
-  config.before_send = proc { |e|
-    GovukStatsd.increment("errors_occurred")
-    GovukStatsd.increment("error_types.#{e.class.name.demodulize.underscore}")
-    e
-  }
-
   config.silence_ready = !Rails.env.production? if defined?(Rails)
 
   # These are the environments (described by the `SENTRY_CURRENT_ENV`

spec/govuk_error/configuration_spec.rb

@@ -11,20 +11,20 @@
     end
   end
 
-  describe ".should_capture" do
+  describe ".before_send" do
     let(:configuration) { GovukError::Configuration.new(Raven.configuration) }
 
     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.should_capture.call(StandardError.new)).to eq(false)
+        expect(configuration.before_send.call(StandardError.new)).to be_falsy
       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.should_capture.call(StandardError.new)).to eq(true)
+        expect(configuration.before_send.call(StandardError.new)).to be_truthy
       end
     end
 
@@ -39,19 +39,19 @@
       end
 
       it "should capture errors by default" do
-        expect(configuration.should_capture.call(StandardError.new)).to eq(true)
+        expect(configuration.before_send.call(StandardError.new)).to be_truthy
       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.should_capture.call(StandardError.new)).to eq(false)
+        expect(configuration.before_send.call(StandardError.new)).to be_falsy
       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.should_capture.call(StandardError.new)).to eq(false)
+        expect(configuration.before_send.call(StandardError.new)).to be_falsy
       end
 
       it "should ignore errors whose underlying cause is an exception in data_sync_excluded_exceptions" do
@@ -71,15 +71,15 @@
         end
 
         expect(chained_exception).to be_an_instance_of(SomeOtherError)
-        expect(configuration.should_capture.call(chained_exception)).to eq(false)
+        expect(configuration.before_send.call(chained_exception)).to be_falsy
       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.should_capture.call(SomeInheritedClass.new)).to eq(false)
+        expect(configuration.before_send.call(SomeInheritedClass.new)).to be_falsy
       end
     end
 
@@ -96,22 +96,22 @@
       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.should_capture.call(StandardError.new)).to eq(true)
+        expect(configuration.before_send.call(StandardError.new)).to be_truthy
       end
     end
   end
 
-  describe ".should_capture=" do
-    it "Allows apps to add their own `should_capture` callback, that is evaluated alongside the default. If both return `true`, then we should capture, but if either returns `false`, then we shouldn't." do
+  describe ".before_send=" do
+    it "Allows apps to add their own `before_send` callback, that is evaluated alongside the default. If both return `true`, then we should capture, but if either returns `false`, then we shouldn't." do
       ClimateControl.modify SENTRY_CURRENT_ENV: "production" do
         raven_configurator = GovukError::Configuration.new(Raven.configuration)
         raven_configurator.active_sentry_environments << "production"
-        raven_configurator.should_capture = lambda do |error_or_event|
+        raven_configurator.before_send = lambda do |error_or_event, _hint|
           error_or_event == "do capture"
         end
 
-        expect(raven_configurator.should_capture.call("do capture")).to eq(true)
-        expect(raven_configurator.should_capture.call("don't capture")).to eq(false)
+        expect(raven_configurator.before_send.call("do capture")).to be_truthy
+        expect(raven_configurator.before_send.call("don't capture", {})).to be_falsy
       end
     end
   end