Skip to content

Commit

Permalink
Fix before_send lambdas not being called
Browse files Browse the repository at this point in the history
In #196, we changed `should_capture` to `before_send` in preparation
for [the removal of the `should_capture` method](https://github.com/getsentry/sentry-ruby/blob/master/MIGRATION.md#removed)
in sentry-ruby.

Because we already had some `before_send` logic, and it wasn't
idempotent (i.e. it must only be evaluated once, as it increments
counters and we want to avoid double-counting), we had to
re-architect how the default behaviour is merged with any custom
behaviour the downstream developer may set.

Since #196 was merged, we've seen errors occuring in Sentry, which
should be being ignored via the datasync logic. This suggests the
lambdas are not being evaluated.

I believe the removal of `before_send=` in configure.rb in
https://github.com/alphagov/govuk_app_config/pull/196/files#diff-5617fa12f2b7ce8393099a22ed6ae89a20e286273e17190f2bf9ec48ef5b46a3L2
has meant some of our setup code would not be reached, unless the
downstream app calls` before_send=` (and none of them do so far).

I also think the override of the `before_send =` on `super` may
be causing problems.

I've reverted to the previous architecture, whilst keeping the new
method names, to use an invocation of `super` that we know was
once working.

As a caveat, we've had to pass a harmless lambda in configure.rb,
to ensure the necessary setup code is called.
  • Loading branch information
ChrisBAshton committed May 21, 2021
1 parent df6a3cb commit 2616f57
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 2 deletions.
2 changes: 1 addition & 1 deletion lib/govuk_app_config/govuk_error/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ def initialize(_raven_configuration)
ignore_excluded_exceptions_in_data_sync,
increment_govuk_statsd_counters,
]
super.before_send = run_before_send_callbacks
end

def before_send=(closure)
@before_send_callbacks.insert(-2, closure)
super(run_before_send_callbacks)
end

protected
Expand Down
4 changes: 4 additions & 0 deletions lib/govuk_app_config/govuk_error/configure.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@
"GdsApi::ContentStore::ItemNotFound",
]

config.before_send = lambda { |error_or_event, _hint|
error_or_event
}

config.transport_failure_callback = proc {
GovukStatsd.increment("error_reports_failed")
}
Expand Down
8 changes: 7 additions & 1 deletion spec/govuk_error/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,13 @@
end

describe ".before_send" do
let(:configuration) { GovukError::Configuration.new(Raven.configuration) }
let(:configuration) do
configuration = GovukError::Configuration.new(Raven.configuration)
configuration.before_send = lambda { |error_or_event, _hint|
error_or_event
}
configuration
end

it "ignores errors if they happen in an environment we don't care about" do
ClimateControl.modify SENTRY_CURRENT_ENV: "some-temporary-environment" do
Expand Down

0 comments on commit 2616f57

Please sign in to comment.