Skip to content

Commit

Permalink
feat: include object id/key in invalid object errors (#301)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
  • Loading branch information
stoneman and Mat001 authored Oct 10, 2023
1 parent 85512a2 commit 9b77a5b
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 45 deletions.
80 changes: 48 additions & 32 deletions lib/optimizely/config/datafile_project_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

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

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

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

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

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

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

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

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

Expand All @@ -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)
Expand All @@ -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

Expand All @@ -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
Expand Down
64 changes: 54 additions & 10 deletions lib/optimizely/exceptions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

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

Expand Down
6 changes: 3 additions & 3 deletions spec/config/datafile_project_config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down

0 comments on commit 9b77a5b

Please sign in to comment.