diff --git a/lib/govuk_app_config/govuk_error/configuration.rb b/lib/govuk_app_config/govuk_error/configuration.rb index ec9967bb..8278ebf1 100644 --- a/lib/govuk_app_config/govuk_error/configuration.rb +++ b/lib/govuk_app_config/govuk_error/configuration.rb @@ -27,14 +27,15 @@ 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 @@ -42,23 +43,23 @@ def ignore_excluded_exceptions_in_data_sync 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 diff --git a/spec/govuk_error/configuration_spec.rb b/spec/govuk_error/configuration_spec.rb index 6f130ea9..5fe86774 100644 --- a/spec/govuk_error/configuration_spec.rb +++ b/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,7 +86,8 @@ 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 @@ -89,7 +95,8 @@ 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