Only send notices if configured with an api_key #268

Merged
merged 1 commit into from May 2, 2014

Projects

None yet

2 participants

@alext
Contributor
alext commented Jan 27, 2014

Currently if the gem is included, but not configured, it will intercept exceptions and attempt to send them to api.airbrake.io without an API key.

This changes the behaviour of send_notice to only send a notice if airbrake is configured. This defines configured mean it has a non-blank API key.

This is desirable because we want to be able to add airbrake to our application, and have the initializer added to the app at deploy time in some deploy targets (but not others).

@alext alext added a commit to alphagov/signon that referenced this pull request Feb 4, 2014
@alext alext Switch to using released airbrake gem
It looks like airbrake/airbrake#268 isn't going
ot be merged anytime soon, so I found another way to disable
notifications until the config is overwritten (on deploy).
54c2b7e
@alext alext referenced this pull request in alphagov/signon Feb 4, 2014
Merged

Switch to using released airbrake gem #193

@alif alif commented on an outdated diff May 1, 2014
lib/airbrake.rb
@@ -147,7 +147,7 @@ def build_lookup_hash_for(exception, options = {})
private
def send_notice(notice)
- if configuration.public?
+ if configuration.configured? and configuration.public?
@alif
alif May 1, 2014 Contributor

It's preferable to use && for logic instead of and

@alif alif commented on an outdated diff May 1, 2014
lib/airbrake/configuration.rb
@@ -256,6 +256,10 @@ def merge(hash)
to_hash.merge(hash)
end
+ def configured?
+ ! (api_key.nil? or api_key.empty?)
@alif
alif May 1, 2014 Contributor

This is a bit pedantic but it would be better to check that !api_key.nil? && !api_key.empty?. Reads a little nicer.

@alif
Contributor
alif commented May 1, 2014

@alext not sure if you were still interested in getting this merged, but I had a couple minor notes.

@alext @alext alext Only send notices if configured with api_key
Currently if the gem is included, but not configured, it will intercept
exceptions and attempt to send them to api.airbrake.io without an
api_key.

This changes the behaviour of send_notice to only send a notice if
airbrake is configured.  This defines configured mean it has a non-blank
api_key.
ce0d0f1
@alext
Contributor
alext commented May 1, 2014

Thanks for the comments @alif, I've updated the PR to make the changes you suggested.

It would be great if this could be merged. At the moment we're having to work around this by adding "production" to config.development_environments in a placeholder config file.

@alif alif merged commit e7bf49d into airbrake:master May 2, 2014
@tadast tadast added a commit to alphagov/smart-answers that referenced this pull request May 6, 2015
@tadast tadast Update airbrake
As per airbrake/airbrake#268 it allows
to remove the hack to avoid airbrake gem attempting to send error
notices to errbit when not configured.
ef67f3c
@tadast tadast added a commit to alphagov/smart-answers that referenced this pull request May 6, 2015
@tadast tadast Update airbrake
As per airbrake/airbrake#268 it allows
to remove the hack to avoid airbrake gem attempting to send error
notices to errbit when not configured.
a427fb0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment