fix(rails): Log missing key warning after Rails initialization completes #444
Conversation
lib/bugsnag/integrations/railtie.rb
Outdated
| @@ -69,6 +69,7 @@ class Railtie < Rails::Railtie | |||
| end | |||
| end | |||
| end | |||
| Bugsnag.configure(true) # Validate API key is set | |||
There was a problem hiding this comment.
You dont even need this because the previous configure will also validate?
There was a problem hiding this comment.
There isn't another configure call if the generator was never run.
There was a problem hiding this comment.
Oh you mean the immediately preceding line? Yes, that would be more logical.
kattrali
force-pushed
the
kattrali/fix-rails-init-warning
branch
2 times, most recently
from
06aad0d to
a168884
Compare
kattrali
force-pushed
the
kattrali/fix-rails-init-warning
branch
6 times, most recently
from
cddf5f7 to
6d8f71f
Compare
When not initializing using the BUGSNAG_API_KEY environment variable, the first time the Railtie integration is loaded, no API key is available. At the bottom of the configure method, a warning is emitted stating that no API key has been set, though this may not be true after config/initializers/bugsnag.rb runs. This is the behavior being observed in #391, causing concern that bugsnag is not working correctly when in fact the log message is premature. This change allows for Bugsnag.configure to be called without validating the key, allowing the Railtie integration to add an additional check once Rails initialization is complete. Fixes #391
kattrali
force-pushed
the
kattrali/fix-rails-init-warning
branch
from
6d8f71f to
8389d2b
Compare
| @@ -37,14 +37,10 @@ class << self | |||
| # Configure the Bugsnag notifier application-wide settings. | |||
| # | |||
| # Yields a configuration object to use to set application settings. | |||
| def configure | |||
| def configure(validate_api_key=true) | |||
There was a problem hiding this comment.
Can this be a named parameter as calling configure(false|true) is not obvious what it will do
There was a problem hiding this comment.
Supporting Ruby 1.9.3 makes this difficult as 2.0.0 introduced named parameters. It could potentially be something for the next major release.
lib/bugsnag/integrations/railtie.rb
Outdated
| @@ -36,7 +36,7 @@ class Railtie < Rails::Railtie | |||
|
|
|||
| config.before_initialize do | |||
| # Configure bugsnag rails defaults | |||
| Bugsnag.configure do |config| | |||
| Bugsnag.configure(false) do |config| | |||
There was a problem hiding this comment.
a comment might be good to explain why we are not validating the key here
spec/configuration_spec.rb
Outdated
| end | ||
| expect(@logger.logs.size).to eq(0) | ||
| end | ||
|
|
There was a problem hiding this comment.
is it worth adding similar test cases for when we pass in the new method parameter in these scenarios?
| expect(output).to include(key_warning) | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
a test in here where the env variable is provided but is not a valid api key and configure is not called might be a good addition
kattrali
force-pushed
the
kattrali/fix-rails-init-warning
branch
from
3ae6f7a to
e7533e6
Compare
kattrali
force-pushed
the
kattrali/fix-rails-init-warning
branch
from
e7533e6 to
50c0525
Compare
When not initializing using the
BUGSNAG_API_KEYenvironment variable,the first time the Railtie integration is loaded, no API key is
available. At the bottom of the configure method, a warning is emitted
stating that no API key has been set, though this may not be true after
config/initializers/bugsnag.rbruns. This is the behavior being observedin #391, causing concern that bugsnag is not working correctly when in
fact the log message is premature.
This change allows for
Bugsnag.configureto be called without validatingthe key, allowing the Railtie integration to add an additional check
once Rails initialization is complete.
Fixes #391