Skip to content

Commit

Permalink
fix(rails): Log missing key warning after Rails initialization completes
Browse files Browse the repository at this point in the history
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
  • Loading branch information
kattrali committed Apr 5, 2018
1 parent 8fb2117 commit a168884
Show file tree
Hide file tree
Showing 11 changed files with 167 additions and 7 deletions.
17 changes: 11 additions & 6 deletions lib/bugsnag.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
yield(configuration) if block_given?

@key_warning = false unless defined?(@key_warning)
if !configuration.valid_api_key? && !@key_warning
configuration.warn("No valid API key has been set, notifications will not be sent")
@key_warning = true
end
check_key_valid if validate_api_key
end

##
Expand Down Expand Up @@ -179,6 +175,15 @@ def load_integration(integration)
configuration.debug("Integration #{integration} is not currently supported")
end
end

# Check if the API key is valid and warn (once) if it is not
def check_key_valid
@key_warning = false unless defined?(@key_warning)
if !configuration.valid_api_key? && !@key_warning
configuration.warn("No valid API key has been set, notifications will not be sent")
@key_warning = true
end
end
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/bugsnag/integrations/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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|
config.logger = ::Rails.logger
config.release_stage = ENV["BUGSNAG_RELEASE_STAGE"] || ::Rails.env.to_s
config.project_root = ::Rails.root.to_s
Expand Down
4 changes: 4 additions & 0 deletions spec/fixtures/apps/rails-initializer-config/Gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
source 'https://rubygems.org'

gem 'railties', '4.2.10', require: %w(action_controller rails)
gem 'bugsnag', path: '../../../..'
16 changes: 16 additions & 0 deletions spec/fixtures/apps/rails-initializer-config/config.ru
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
Bundler.require

run InitializerConfigApp ||= Class.new(Rails::Application) {
config.secret_key_base = routes.append {
root to: proc {
[200, {"Content-Type" => "text/plain"}, ["Hello!"]]
}
}.to_s

config.cache_classes = true
config.eager_load = true
config.logger = Logger.new(STDOUT)
config.log_level = :warn

initialize!
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Bugsnag.configure do |config|
config.api_key = 'c34a2472bd240ac0ab0f52715bbdc05d'
end
4 changes: 4 additions & 0 deletions spec/fixtures/apps/rails-no-config/Gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
source 'https://rubygems.org'

gem 'railties', '4.2.10', require: %w(action_controller rails)
gem 'bugsnag', path: '../../../..'
16 changes: 16 additions & 0 deletions spec/fixtures/apps/rails-no-config/config.ru
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
Bundler.require

run NoConfigApp ||= Class.new(Rails::Application) {
config.secret_key_base = routes.append {
root to: proc {
[200, {"Content-Type" => "text/plain"}, ["Hello!"]]
}
}.to_s

config.cache_classes = true
config.eager_load = true
config.logger = Logger.new(STDOUT)
config.log_level = :warn

initialize!
}
3 changes: 3 additions & 0 deletions spec/fixtures/apps/scripts/Gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
source 'https://rubygems.org'

gem 'bugsnag', path: '../../../..'
5 changes: 5 additions & 0 deletions spec/fixtures/apps/scripts/configure_key.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
require 'bugsnag'

Bugsnag.configure do |config|
config.api_key = 'f25a2472bd230ac0ab0fa2715bbdc654'
end
3 changes: 3 additions & 0 deletions spec/fixtures/apps/scripts/no_config.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
require 'bugsnag'

Bugsnag.configure
101 changes: 101 additions & 0 deletions spec/logger_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
require 'spec_helper'

describe 'Configuration.logger' do

before do
@env = {}
end

context 'in a Rails app' do
key_warning = '[Bugsnag]: No valid API key has been set, notifications will not be sent'

before do
@env['RACK_ENV'] = 'production'
end

def run_app(name)
out_reader, out_writer = IO.pipe
Dir.chdir(File.join(File.dirname(__FILE__), "fixtures/apps/#{name}")) do
Bundler.with_clean_env do
pid = Process.spawn('bundle install',
out: out_writer.fileno,
err: out_writer.fileno)
Process.waitpid(pid, 0)
pid = Process.spawn(@env, 'bundle exec rackup config.ru',
out: out_writer.fileno,
err: out_writer.fileno)
sleep(2)
Process.kill(1, pid)
end
end
out_writer.close
output = ""
output << out_reader.gets until out_reader.eof?
output
end

context 'sets an API key using the BUGSNAG_API_KEY env var' do
it 'does not log a warning' do
@env['BUGSNAG_API_KEY'] = 'c34a2472bd240ac0ab0f52715bbdc05d'
output = run_app('rails-no-config')
expect(output).not_to include(key_warning)
end
end

context 'sets an API key using the bugsnag initializer' do
it 'does not log a warning' do
output = run_app('rails-initializer-config')
expect(output).not_to include(key_warning)
end
end

context 'skips setting an API key' do
it 'logs a warning' do
output = run_app('rails-no-config')
expect(output).to include(key_warning)
end
end
end

context 'in a script' do
key_warning = /\[Bugsnag\] .* No valid API key has been set, notifications will not be sent/

def run_app(name)
out_reader, out_writer = IO.pipe
Dir.chdir(File.join(File.dirname(__FILE__), "fixtures/apps/scripts")) do
Bundler.with_clean_env do
pid = Process.spawn(@env, "bundle exec ruby #{name}.rb",
out: out_writer.fileno,
err: out_writer.fileno)
Process.waitpid(pid, 0)
end
end
out_writer.close
output = ""
output << out_reader.gets until out_reader.eof?
output
end

context 'sets an API key using the BUGSNAG_API_KEY env var' do
it 'does not log a warning' do
@env['BUGSNAG_API_KEY'] = 'c34a2472bd240ac0ab0f52715bbdc05d'
output = run_app('no_config')
expect(output).not_to match(key_warning)
end
end

context 'sets an API key using Bugsnag.configure' do
it 'does not log a warning' do
output = run_app('configure_key')
expect(output).not_to match(key_warning)
end
end

context 'skips setting an API key' do
it 'logs a warning' do
output = run_app('no_config')
expect(output).to match(key_warning)
end
end
end
end

0 comments on commit a168884

Please sign in to comment.