feat: include object id/key in invalid object errors (#301)

* feat: include object id/key in invalid object errors

- Include object `id`/`key` in errors when objects not found in datafile
- Modifies invalid object `id`/`key` log messages to make them consistent

Include object `id`/`key` in errors when an object is not found makes them available to the user or the custom error handler.

One example of why this is useful is that the `key` of an experiment could be used within a custom error handler to fetch the details of the experiment. This would indicate whether the experiment has been paused (in which case the error could be ignored) or archived (in which case the code referencing the experiment should be removed from the application).

* feat: expose object identifiers as error properties

- Expose the object identifiers as error properties so that they are easier to use in error handlers.
- Encapsulate the error messages within the error objects to enforce consistency and to simplify initialization
- Use the messages of the error objects as log messages to enforce consistency and to simplify logging

* Update lib/optimizely/exceptions.rb typo identifier

* identifier typo

* identifier typo

* identifier typo

* identifier typo

* identifier typo

---------

Co-authored-by: Matjaz Pirnovar <Mat001@users.noreply.github.com>

Author: stoneman

lib/optimizely/config/datafile_project_config.rb

@@ -223,8 +223,9 @@ def get_experiment_from_key(experiment_key)
       experiment = @experiment_key_map[experiment_key]
       return experiment if experiment
 
-      @logger.log Logger::ERROR, "Experiment key '#{experiment_key}' is not in datafile."
-      @error_handler.handle_error InvalidExperimentError
+      invalid_experiment_error = InvalidExperimentError.new(experiment_key: experiment_key)
+      @logger.log Logger::ERROR, invalid_experiment_error.message
+      @error_handler.handle_error invalid_experiment_error
       nil
     end
 
@@ -238,8 +239,9 @@ def get_experiment_from_id(experiment_id)
       experiment = @experiment_id_map[experiment_id]
       return experiment if experiment
 
-      @logger.log Logger::ERROR, "Experiment id '#{experiment_id}' is not in datafile."
-      @error_handler.handle_error InvalidExperimentError
+      invalid_experiment_error = InvalidExperimentError.new(experiment_id: experiment_id)
+      @logger.log Logger::ERROR, invalid_experiment_error.message
+      @error_handler.handle_error invalid_experiment_error
       nil
     end
 
@@ -253,8 +255,9 @@ def get_experiment_key(experiment_id)
       experiment = @experiment_id_map[experiment_id]
       return experiment['key'] unless experiment.nil?
 
-      @logger.log Logger::ERROR, "Experiment id '#{experiment_id}' is not in datafile."
-      @error_handler.handle_error InvalidExperimentError
+      invalid_experiment_error = InvalidExperimentError.new(experiment_id: experiment_id)
+      @logger.log Logger::ERROR, invalid_experiment_error.message
+      @error_handler.handle_error invalid_experiment_error
       nil
     end
 
@@ -268,8 +271,9 @@ def get_event_from_key(event_key)
       event = @event_key_map[event_key]
       return event if event
 
-      @logger.log Logger::ERROR, "Event '#{event_key}' is not in datafile."
-      @error_handler.handle_error InvalidEventError
+      invalid_event_error = InvalidEventError.new(event_key)
+      @logger.log Logger::ERROR, invalid_event_error.message
+      @error_handler.handle_error invalid_event_error
       nil
     end
 
@@ -283,8 +287,9 @@ def get_audience_from_id(audience_id)
       audience = @audience_id_map[audience_id]
       return audience if audience
 
-      @logger.log Logger::ERROR, "Audience '#{audience_id}' is not in datafile."
-      @error_handler.handle_error InvalidAudienceError
+      invalid_audience_error = InvalidAudienceError.new(audience_id)
+      @logger.log Logger::ERROR, invalid_audience_error.message
+      @error_handler.handle_error invalid_audience_error
       nil
     end
 
@@ -308,13 +313,15 @@ def get_variation_from_id(experiment_key, variation_id)
         variation = variation_id_map[variation_id]
         return variation if variation
 
-        @logger.log Logger::ERROR, "Variation id '#{variation_id}' is not in datafile."
-        @error_handler.handle_error InvalidVariationError
+        invalid_variation_error = InvalidVariationError.new(variation_id: variation_id)
+        @logger.log Logger::ERROR, invalid_variation_error.message
+        @error_handler.handle_error invalid_variation_error
         return nil
       end
 
-      @logger.log Logger::ERROR, "Experiment key '#{experiment_key}' is not in datafile."
-      @error_handler.handle_error InvalidExperimentError
+      invalid_experiment_error = InvalidExperimentError.new(experiment_key: experiment_key)
+      @logger.log Logger::ERROR, invalid_experiment_error.message
+      @error_handler.handle_error invalid_experiment_error
       nil
     end
 
@@ -331,13 +338,15 @@ def get_variation_from_id_by_experiment_id(experiment_id, variation_id)
         variation = variation_id_map_by_experiment_id[variation_id]
         return variation if variation
 
-        @logger.log Logger::ERROR, "Variation id '#{variation_id}' is not in datafile."
-        @error_handler.handle_error InvalidVariationError
+        invalid_variation_error = InvalidVariationError.new(variation_id: variation_id)
+        @logger.log Logger::ERROR, invalid_variation_error.message
+        @error_handler.handle_error invalid_variation_error
         return nil
       end
 
-      @logger.log Logger::ERROR, "Experiment id '#{experiment_id}' is not in datafile."
-      @error_handler.handle_error InvalidExperimentError
+      invalid_experiment_error = InvalidExperimentError.new(experiment_id: experiment_id)
+      @logger.log Logger::ERROR, invalid_experiment_error.message
+      @error_handler.handle_error invalid_experiment_error
       nil
     end
 
@@ -354,13 +363,15 @@ def get_variation_id_from_key_by_experiment_id(experiment_id, variation_key)
         variation = variation_key_map[variation_key]
         return variation['id'] if variation
 
-        @logger.log Logger::ERROR, "Variation key '#{variation_key}' is not in datafile."
-        @error_handler.handle_error InvalidVariationError
+        invalid_variation_error = InvalidVariationError.new(variation_key: variation_key)
+        @logger.log Logger::ERROR, invalid_variation_error.message
+        @error_handler.handle_error invalid_variation_error
         return nil
       end
 
-      @logger.log Logger::ERROR, "Experiment id '#{experiment_id}' is not in datafile."
-      @error_handler.handle_error InvalidExperimentError
+      invalid_experiment_error = InvalidExperimentError.new(experiment_id: experiment_id)
+      @logger.log Logger::ERROR, invalid_experiment_error.message
+      @error_handler.handle_error invalid_experiment_error
       nil
     end
 
@@ -377,13 +388,15 @@ def get_variation_id_from_key(experiment_key, variation_key)
         variation = variation_key_map[variation_key]
         return variation['id'] if variation
 
-        @logger.log Logger::ERROR, "Variation key '#{variation_key}' is not in datafile."
-        @error_handler.handle_error InvalidVariationError
+        invalid_variation_error = InvalidVariationError.new(variation_key: variation_key)
+        @logger.log Logger::ERROR, invalid_variation_error.message
+        @error_handler.handle_error invalid_variation_error
         return nil
       end
 
-      @logger.log Logger::ERROR, "Experiment key '#{experiment_key}' is not in datafile."
-      @error_handler.handle_error InvalidExperimentError
+      invalid_experiment_error = InvalidExperimentError.new(experiment_key: experiment_key)
+      @logger.log Logger::ERROR, invalid_experiment_error.message
+      @error_handler.handle_error invalid_experiment_error
       nil
     end
 
@@ -397,8 +410,9 @@ def get_whitelisted_variations(experiment_id)
       experiment = @experiment_id_map[experiment_id]
       return experiment['forcedVariations'] if experiment
 
-      @logger.log Logger::ERROR, "Experiment ID '#{experiment_id}' is not in datafile."
-      @error_handler.handle_error InvalidExperimentError
+      invalid_experiment_error = InvalidExperimentError.new(experiment_id: experiment_id)
+      @logger.log Logger::ERROR, invalid_experiment_error.message
+      @error_handler.handle_error invalid_experiment_error
     end
 
     def get_attribute_id(attribute_key)
@@ -420,8 +434,9 @@ def get_attribute_id(attribute_key)
       end
       return attribute_key if has_reserved_prefix
 
-      @logger.log Logger::ERROR, "Attribute key '#{attribute_key}' is not in datafile."
-      @error_handler.handle_error InvalidAttributeError
+      invalid_attribute_error = InvalidAttributeError.new(attribute_key)
+      @logger.log Logger::ERROR, invalid_attribute_error.message
+      @error_handler.handle_error invalid_attribute_error
       nil
     end
 
@@ -439,8 +454,9 @@ def variation_id_exists?(experiment_id, variation_id)
         variation = variation_id_map[variation_id]
         return true if variation
 
-        @logger.log Logger::ERROR, "Variation ID '#{variation_id}' is not in datafile."
-        @error_handler.handle_error InvalidVariationError
+        invalid_variation_error = InvalidVariationError.new(variation_id: variation_id)
+        @logger.log Logger::ERROR, invalid_variation_error.message
+        @error_handler.handle_error invalid_variation_error
       end
 
       false

lib/optimizely/exceptions.rb

@@ -42,16 +42,28 @@ def initialize(msg = 'SDK key not provided/cannot be found in the datafile.')
   class InvalidAudienceError < Error
     # Raised when an invalid audience is provided
 
-    def initialize(msg = 'Provided audience is not in datafile.')
-      super
+    attr_reader :audience_id
+
+    def initialize(audience_id)
+      raise ArgumentError, 'audience_id must be provided' if audience_id.nil?
+
+      super("Audience id '#{audience_id}' is not in datafile.")
+
+      @audience_id = audience_id
     end
   end
 
   class InvalidAttributeError < Error
     # Raised when an invalid attribute is provided
 
-    def initialize(msg = 'Provided attribute is not in datafile.')
-      super
+    attr_reader :attribute_key
+
+    def initialize(attribute_key)
+      raise ArgumentError, 'attribute_key must be provided' if attribute_key.nil?
+
+      super("Attribute key '#{attribute_key}' is not in datafile.")
+
+      @attribute_key = attribute_key
     end
   end
 
@@ -74,24 +86,56 @@ def initialize(msg = 'Event tags provided are in an invalid format.')
   class InvalidExperimentError < Error
     # Raised when an invalid experiment key is provided
 
-    def initialize(msg = 'Provided experiment is not in datafile.')
-      super
+    attr_reader :experiment_id, :experiment_key
+
+    def initialize(experiment_id: nil, experiment_key: nil)
+      raise ArgumentError, 'Either experiment_id or experiment_key must be provided.' if experiment_id.nil? && experiment_key.nil?
+      raise ArgumentError, 'Cannot provide both experiment_id and experiment_key.' if !experiment_id.nil? && !experiment_key.nil?
+
+      if experiment_id.nil?
+        @experiment_key = experiment_key
+        identifier = "key '#{@experiment_key}'"
+      else
+        @experiment_id = experiment_id
+        identifier = "id '#{@experiment_id}'"
+      end
+
+      super("Experiment #{identifier} is not in datafile.")
     end
   end
 
   class InvalidEventError < Error
     # Raised when an invalid event key is provided
 
-    def initialize(msg = 'Provided event is not in datafile.')
-      super
+    attr_reader :event_key
+
+    def initialize(event_key)
+      raise ArgumentError, 'event_key must be provided.' if event_key.nil?
+
+      super("Event key '#{event_key}' is not in datafile.")
+
+      @event_key = event_key
     end
   end
 
   class InvalidVariationError < Error
     # Raised when an invalid variation key or ID is provided
 
-    def initialize(msg = 'Provided variation is not in datafile.')
-      super
+    attr_reader :variation_id, :variation_key
+
+    def initialize(variation_id: nil, variation_key: nil)
+      raise ArgumentError, 'Either variation_id or variation_key must be provided.' if variation_id.nil? && variation_key.nil?
+      raise ArgumentError, 'Cannot provide both variation_id and variation_key.' if !variation_id.nil? && !variation_key.nil?
+
+      if variation_id.nil?
+        identifier = "key '#{variation_key}'"
+        @variation_key = variation_key
+      else
+        identifier = "id '#{variation_id}'"
+        @variation_id = variation_id
+      end
+
+      super("Variation #{identifier} is not in datafile.")
     end
   end
 

spec/config/datafile_project_config_spec.rb

@@ -837,14 +837,14 @@
     describe 'get_event_from_key' do
       it 'should log a message when provided event key is invalid' do
         config.get_event_from_key('invalid_key')
-        expect(spy_logger).to have_received(:log).with(Logger::ERROR, "Event 'invalid_key' is not in datafile.")
+        expect(spy_logger).to have_received(:log).with(Logger::ERROR, "Event key 'invalid_key' is not in datafile.")
       end
     end
 
     describe 'get_audience_from_id' do
       it 'should log a message when provided audience ID is invalid' do
         config.get_audience_from_id('invalid_id')
-        expect(spy_logger).to have_received(:log).with(Logger::ERROR, "Audience 'invalid_id' is not in datafile.")
+        expect(spy_logger).to have_received(:log).with(Logger::ERROR, "Audience id 'invalid_id' is not in datafile.")
       end
     end
 
@@ -948,7 +948,7 @@
       it 'should log a message when there is no experiment key map for the experiment' do
         config.get_whitelisted_variations('invalid_key')
         expect(spy_logger).to have_received(:log).with(Logger::ERROR,
-                                                       "Experiment ID 'invalid_key' is not in datafile.")
+                                                       "Experiment id 'invalid_key' is not in datafile.")
       end
     end