Merge pull request #136 from launchdarkly/eb/ch73982/nil-sdk-key

fail fast for nil SDK key when appropriate

Author: eli-darkly

lib/ldclient-rb/events.rb

@@ -91,6 +91,7 @@ class StopMessage < SynchronousMessage
   # @private
   class EventProcessor
     def initialize(sdk_key, config, client = nil, diagnostic_accumulator = nil, test_properties = nil)
+      raise ArgumentError, "sdk_key must not be nil" if sdk_key.nil?  # see LDClient constructor comment on sdk_key
       @logger = config.logger
       @inbox = SizedQueue.new(config.capacity < 100 ? 100 : config.capacity)
       @flush_task = Concurrent::TimerTask.new(execution_interval: config.flush_interval) do

lib/ldclient-rb/ldclient.rb

@@ -33,6 +33,16 @@ class LDClient
     # @return [LDClient] The LaunchDarkly client instance
     #
     def initialize(sdk_key, config = Config.default, wait_for_sec = 5)
+      # Note that sdk_key is normally a required parameter, and a nil value would cause the SDK to
+      # fail in most configurations. However, there are some configurations where it would be OK
+      # (offline = true, *or* we are using LDD mode or the file data source and events are disabled
+      # so we're not connecting to any LD services) so rather than try to check for all of those
+      # up front, we will let the constructors for the data source implementations implement this
+      # fail-fast as appropriate, and just check here for the part regarding events.
+      if !config.offline? && config.send_events
+        raise ArgumentError, "sdk_key must not be nil" if sdk_key.nil?
+      end
+
       @sdk_key = sdk_key
 
       @event_factory_default = EventFactory.new(false)
@@ -352,6 +362,7 @@ def create_default_data_source(sdk_key, config, diagnostic_accumulator)
       if config.offline?
         return NullUpdateProcessor.new
       end
+      raise ArgumentError, "sdk_key must not be nil" if sdk_key.nil?  # see LDClient constructor comment on sdk_key
       requestor = Requestor.new(sdk_key, config)
       if config.stream?
         StreamProcessor.new(sdk_key, config, requestor, diagnostic_accumulator)

spec/ldclient_spec.rb

@@ -49,6 +49,46 @@ def event_processor
     client.instance_variable_get(:@event_processor)
   end
 
+  describe "constructor requirement of non-nil sdk key" do
+    it "is not enforced when offline" do
+      subject.new(nil, offline_config)
+    end
+
+    it "is not enforced if use_ldd is true and send_events is false" do
+      subject.new(nil, LaunchDarkly::Config.new({ use_ldd: true, send_events: false }))
+    end
+
+    it "is not enforced if using file data and send_events is false" do
+      source = LaunchDarkly::FileDataSource.factory({})
+      subject.new(nil, LaunchDarkly::Config.new({ data_source: source, send_events: false }))
+    end
+
+    it "is enforced in streaming mode even if send_events is false" do
+      expect {
+        subject.new(nil, LaunchDarkly::Config.new({ send_events: false }))
+      }.to raise_error(ArgumentError)
+    end
+
+    it "is enforced in polling mode even if send_events is false" do
+      expect {
+        subject.new(nil, LaunchDarkly::Config.new({ stream: false, send_events: false }))
+      }.to raise_error(ArgumentError)
+    end
+
+    it "is enforced if use_ldd is true and send_events is true" do
+      expect {
+        subject.new(nil, LaunchDarkly::Config.new({ use_ldd: true }))
+      }.to raise_error(ArgumentError)
+    end
+
+    it "is enforced if using file data and send_events is true" do
+      source = LaunchDarkly::FileDataSource.factory({})
+      expect {
+        subject.new(nil, LaunchDarkly::Config.new({ data_source: source }))
+      }.to raise_error(ArgumentError)
+    end
+  end
+
   describe '#variation' do
     feature_with_value = { key: "key", on: false, offVariation: 0, variations: ["value"], version: 100,
       trackEvents: true, debugEventsUntilDate: 1000 }