Log missing key warning after Rails initialization completes (#444)

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,9 @@ class Railtie < Rails::Railtie
 
     config.before_initialize do
       # Configure bugsnag rails defaults
-      Bugsnag.configure do |config|
+      # Skipping API key validation as the key may be set later in an
+      # initializer. If not, the key will be validated in after_initialize.
+      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,66 @@ def debug(name, &block)
       end
     end
 
+    context "using configure" do
+      before 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)
+      end
+
+      context "API key is not specified" do
+        it "skips logging a warning if validate_api_key is false" do
+          Bugsnag.configure(false)
+          expect(@logger.logs.size).to eq(0)
+        end
+
+        it "logs a warning by default" do
+          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 "logs a warning if validate_api_key is true" do
+          Bugsnag.configure(true)
+          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
+      end
+
+      context "API key is set" do
+        it "skips logging a warning when configuring with an API key" do
+          Bugsnag.configure do |config|
+            config.api_key = 'd57a2472bd130ac0ab0f52715bbdc600'
+          end
+          expect(@logger.logs.size).to eq(0)
+        end
+
+        it "logs a warning if the configured API key is invalid" do
+          Bugsnag.configure do |config|
+            config.api_key = 'WARNING: not a real key'
+          end
+          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
+      end
+    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-invalid-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-invalid-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-invalid-initializer-config/config/initializers/bugsnag.rb

@@ -0,0 +1,3 @@
+Bugsnag.configure do |config|
+  config.api_key = '1'
+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_invalid_key.rb

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

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,138 @@
+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'
+    is_jruby = defined?(RUBY_ENGINE) && RUBY_ENGINE == 'jruby'
+    incompatible = (RUBY_VERSION < '2.0.0') || is_jruby
+
+    before do
+      skip "Incompatible with Ruby <2.0 and JRuby" if incompatible
+      @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
+        skip "Incompatible with Ruby <2.0 and JRuby" if incompatible
+        @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
+        skip "Incompatible with Ruby <2.0 and JRuby" if incompatible
+        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
+        skip "Incompatible with Ruby <2.0 and JRuby" if incompatible
+        output = run_app('rails-no-config')
+        expect(output).to include(key_warning)
+      end
+    end
+
+    context 'sets an invalid API key using the BUGSNAG_API_KEY env var' do
+      it 'logs a warning' do
+        skip "Incompatible with Ruby <2.0 and JRuby" if incompatible
+        output = run_app('rails-invalid-initializer-config')
+        expect(output).to include(key_warning)
+      end
+    end
+
+    context 'sets an invalid API key using the BUGSNAG_API_KEY env var' do
+      it 'logs a warning' do
+        skip "Incompatible with Ruby <2.0 and JRuby" if incompatible
+        @env['BUGSNAG_API_KEY'] = 'not a real key'
+        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 'sets an invalid API key using Bugsnag.configure' do
+      it 'logs a warning' do
+        output = run_app('configure_invalid_key')
+        expect(output).to match(key_warning)
+      end
+    end
+
+    context 'sets an invalid API key using the BUGSNAG_API_KEY env var' do
+      it 'logs a warning' do
+        @env['BUGSNAG_API_KEY'] = 'bad key bad key whatcha gonna do'
+        output = run_app('no_config')
+        expect(output).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