Merge pull request #87 from airbrake/80-config-warning

config: don't raise when current environment is ignored

CHANGELOG.md

@@ -3,6 +3,11 @@ Airbrake Ruby Changelog
 
 ### master
 
+* Stopped raising error when the notifier lacks either project ID or project key
+  and also told to ignore current environment. As the result, empty string for
+  `project_key` is also validated now (forbidden)
+  ([#87](https://github.com/airbrake/airbrake-ruby/pull/87))
+
 ### [v1.3.2][v1.3.2] (May 27, 2016)
 
 * Fixed bug when the library raises unwanted exception, when current environment

lib/airbrake-ruby/config.rb

@@ -125,6 +125,22 @@ def merge(config_hash)
       self
     end
 
+    ##
+    # @return [Boolean] true if the config meets the requirements, false
+    #   otherwise
+    def valid?
+      return true if ignored_environment?
+      return false unless project_id.is_a?(Integer)
+      project_key.is_a?(String) && !project_key.empty?
+    end
+
+    ##
+    # @return [Boolean] true if the config ignores current environment, false
+    #   otherwise
+    def ignored_environment?
+      ignore_environments.include?(environment)
+    end
+
     private
 
     def set_option(option, value)

lib/airbrake-ruby/notifier.rb

@@ -30,7 +30,7 @@ class Notifier
     def initialize(user_config)
       @config = (user_config.is_a?(Config) ? user_config : Config.new(user_config))
 
-      unless [@config.project_id, @config.project_key].all?
+      unless @config.valid?
         raise Airbrake::Error, 'both :project_id and :project_key are required'
       end
 
@@ -143,9 +143,7 @@ def convert_to_exception(ex)
     end
 
     def send_notice(exception, params, sender = default_sender)
-      if @config.ignore_environments.any?
-        return if @config.ignore_environments.include?(@config.environment)
-      end
+      return if @config.ignored_environment?
 
       notice = build_notice(exception, params)
       @filter_chain.refine(notice)

spec/config_spec.rb

@@ -76,4 +76,75 @@
       end
     end
   end
+
+  describe "#valid?" do
+    context "when project_id is nil" do
+      it "returns false" do
+        config.project_id = nil
+        config.project_key = '123'
+
+        expect(config).not_to be_valid
+      end
+    end
+
+    context "when project_key is nil" do
+      it "returns false" do
+        config.project_id = 123
+        config.project_key = nil
+
+        expect(config).not_to be_valid
+      end
+    end
+
+    context "when the current environment is ignored" do
+      context "and when the notifier misconfigures configure project_key & project_id" do
+        it "returns true" do
+          config.project_id = Object.new
+          config.project_key = Object.new
+          config.environment = :bingo
+          config.ignore_environments = [:bingo]
+
+          expect(config).to be_valid
+        end
+      end
+
+      context "and when the notifier configures project_key & project_id" do
+        it "returns true" do
+          config.project_id = 123
+          config.project_key = '321'
+          config.environment = :bingo
+          config.ignore_environments = [:bingo]
+
+          expect(config).to be_valid
+        end
+      end
+    end
+
+    context "when the project_id value is not an Integer" do
+      it "returns false" do
+        config.project_id = '123'
+        config.project_key = '321'
+
+        expect(config).not_to be_valid
+      end
+    end
+
+    context "when the project_key value is not a String" do
+      it "returns false" do
+        config.project_id = 123
+        config.project_key = 321
+
+        expect(config).not_to be_valid
+      end
+    end
+
+    context "when the project_key value is an empty String" do
+      it "returns false" do
+        config.project_id = 123
+        config.project_key = ''
+
+        expect(config).not_to be_valid
+      end
+    end
+  end
 end