Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release 4.0.0.pre.2 #202

Merged
merged 4 commits into from
Jun 4, 2021
Merged

Release 4.0.0.pre.2 #202

merged 4 commits into from
Jun 4, 2021

Conversation

ChrisBAshton
Copy link
Contributor

@ChrisBAshton ChrisBAshton commented Jun 4, 2021

Attempting to pull govuk_app_config v4.0.0.pre.1 into a project led to a failure, because our default Sentry configuration is
currently invalid. This is now fixed up, and tests added to prevent that happening again.

Attempting to pull govuk_app_config v4.0.0.pre.1 into a project
led to a failure, because our default Sentry configuration is
currently invalid. This will be fixed up in subsequent commits,
but to begin with, we should have a test that exercises our default
config and ensures that it doesn't raise any errors.
In sentry-ruby, this raises an error:

```
NoMethodError: undefined method `transport_failure_callback=' for #<GovukError::Configuration:0x000055f6d3cd1368>
```

`transport_failure_callback` was removed and, in the migration
guide, advised to be swapped with `before_send`:
https://docs.sentry.io/platforms/ruby/migration/

However, there is no documentation around _how_ to replace this
with `before_send`, which is already rather a complicated method
as we've overloaded it with conditional-exception-ignoring logic.

I fear that the most straightforward option is to simply remove it.
It will mean making some tweaks to our Grafana dashboards in
govuk-puppet:
https://github.com/alphagov/govuk-puppet/search?q=error_reports_failed
We were seeing errors:

```
  1) GovukError.configure should contain only valid Sentry config
     Failure/Error: config.silence_ready = !Rails.env.production? if defined?(Rails)

     NoMethodError:
       undefined method `silence_ready=' for #<GovukError::Configuration:0x000055c592cd9e18>
      ./lib/govuk_app_config/govuk_error/configure.rb:2:in `block in <top (required)>'
      ./lib/govuk_app_config/govuk_error.rb:21:in `configure'
      ./lib/govuk_app_config/govuk_error/configure.rb:1:in `<top (required)>'
      /var/lib/jenkins/bundles/ruby/2.6.0/gems/activesupport-6.1.3.2/lib/active_support/dependencies.rb:332:in `require'
      /var/lib/jenkins/bundles/ruby/2.6.0/gems/activesupport-6.1.3.2/lib/active_support/dependencies.rb:332:in `block in require'
      /var/lib/jenkins/bundles/ruby/2.6.0/gems/activesupport-6.1.3.2/lib/active_support/dependencies.rb:299:in `load_dependency'
      /var/lib/jenkins/bundles/ruby/2.6.0/gems/activesupport-6.1.3.2/lib/active_support/dependencies.rb:332:in `require'
      ./spec/govuk_error/configure_spec.rb:7:in `block (2 levels) in <top (required)>'
      /var/lib/jenkins/bundles/ruby/2.6.0/gems/webmock-3.13.0/lib/webmock/rspec.rb:37:in `block (2 levels) in <top (required)>'
```

It doesn't appear to be in the migration guide, and is still
listed as an option in https://docs.sentry.io/clients/ruby/configuration/options/,
but the functionality is trivial, so no real harm in removing.
@ChrisBAshton ChrisBAshton marked this pull request as ready for review June 4, 2021 13:47
Copy link
Contributor

@thomasleese thomasleese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice test!

@ChrisBAshton ChrisBAshton merged commit 54cbccc into main Jun 4, 2021
@ChrisBAshton ChrisBAshton deleted the test-sentry-config branch June 4, 2021 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants