fix(rails): Log missing key warning after Rails initialization completes

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

lib/bugsnag.rb

@@ -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
 
     ##
@@ -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
 

lib/bugsnag/integrations/railtie.rb

@@ -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

spec/configuration_spec.rb

@@ -115,6 +115,32 @@ def debug(name, &block)
       end
     end
 
+    it "logs a warning when configuring without an API key" do
+      Bugsnag.configuration.api_key = nil
+      Bugsnag.instance_variable_set("@key_warning", nil)
+      ENV['BUGSNAG_API_KEY'] = nil
+      expect(@logger.logs.size).to eq(0)
+      Bugsnag.configure
+      expect(@logger.logs.size).to eq(1)
+      log = @logger.logs.first
+      expect(log).to eq({
+        :level => "warning",
+        :name => "[Bugsnag]",
+        :message => "No valid API key has been set, notifications will not be sent"
+      })
+    end
+
+    it "skips logging a warning when configuring with an API key" do
+      Bugsnag.configuration.api_key = nil
+      Bugsnag.instance_variable_set("@key_warning", nil)
+      ENV['BUGSNAG_API_KEY'] = nil
+      expect(@logger.logs.size).to eq(0)
+      Bugsnag.configure do |config|
+        config.api_key = 'd57a2472bd130ac0ab0f52715bbdc600'
+      end
+      expect(@logger.logs.size).to eq(0)
+    end
+
     it "should log info messages to the set logger" do
       expect(@logger.logs.size).to eq(0)
       Bugsnag.configuration.info("Info message")

spec/fixtures/apps/rails-initializer-config/Gemfile

@@ -0,0 +1,4 @@
+source 'https://rubygems.org'
+
+gem 'railties', '4.2.10', require: %w(action_controller rails)
+gem 'bugsnag', path: '../../../..'

spec/fixtures/apps/rails-initializer-config/config.ru

@@ -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!
+}

spec/fixtures/apps/rails-initializer-config/config/initializers/bugsnag.rb

@@ -0,0 +1,3 @@
+Bugsnag.configure do |config|
+  config.api_key = 'c34a2472bd240ac0ab0f52715bbdc05d'
+end

spec/fixtures/apps/rails-no-config/Gemfile

@@ -0,0 +1,4 @@
+source 'https://rubygems.org'
+
+gem 'railties', '4.2.10', require: %w(action_controller rails)
+gem 'bugsnag', path: '../../../..'

spec/fixtures/apps/rails-no-config/config.ru

@@ -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!
+}

spec/fixtures/apps/scripts/Gemfile

@@ -0,0 +1,3 @@
+source 'https://rubygems.org'
+
+gem 'bugsnag', path: '../../../..'

spec/fixtures/apps/scripts/configure_key.rb

@@ -0,0 +1,5 @@
+require 'bugsnag'
+
+Bugsnag.configure do |config|
+  config.api_key = 'f25a2472bd230ac0ab0fa2715bbdc654'
+end

spec/fixtures/apps/scripts/no_config.rb

@@ -0,0 +1,3 @@
+require 'bugsnag'
+
+Bugsnag.configure

spec/integrations/logger_spec.rb

@@ -0,0 +1,104 @@
+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'
+      is_jruby = defined?(RUBY_ENGINE) && RUBY_ENGINE == 'jruby'
+      incompatible = is_jruby or RUBY_VERSION < '2.0.0'
+      skip "Incompatible with Ruby <2.0 and JRuby" if incompatible
+    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