Better exception handling in the SDK (#27)

Author: aliabbasrizvi

optimizely/event_builder.py

@@ -2,6 +2,7 @@
 from abc import abstractmethod
 from abc import abstractproperty
 
+from . import exceptions
 from . import project_config
 from . import version
 from .helpers import enums
@@ -416,4 +417,4 @@ def get_event_builder(config, bucketer):
   if config_version == project_config.V2_CONFIG_VERSION:
     return EventBuilderV2(config, bucketer)
 
-  raise Exception(enums.Errors.UNSUPPORTED_CONFIG_VERSION)
+  raise exceptions.InvalidInputException(enums.Errors.UNSUPPORTED_DATAFILE_VERSION)

optimizely/exceptions.py

@@ -23,6 +23,11 @@ class InvalidGroupException(Exception):
   pass
 
 
+class InvalidInputException(Exception):
+  """ Raised when provided datafile, event dispatcher, logger or error handler is invalid. """
+  pass
+
+
 class InvalidVariationException(Exception):
   """ Raised when provided variation is invalid. """
   pass

optimizely/helpers/enums.py

@@ -22,6 +22,7 @@ class Errors(object):
   INVALID_AUDIENCE_ERROR = 'Provided audience is not in datafile.'
   INVALID_EXPERIMENT_KEY_ERROR = 'Provided experiment is not in datafile.'
   INVALID_EVENT_KEY_ERROR = 'Provided event is not in datafile.'
+  INVALID_DATAFILE = 'Datafile has invalid format. Failing "{}".'
   INVALID_GROUP_ID_ERROR = 'Provided group is not in datafile.'
   INVALID_VARIATION_ERROR = 'Provided variation is not in datafile.'
-  UNSUPPORTED_CONFIG_VERSION = 'Datafile provided has unsupported version.'
+  UNSUPPORTED_DATAFILE_VERSION = 'Provided datafile has unsupported version.'

optimizely/helpers/validator.py

@@ -17,10 +17,10 @@ def is_datafile_valid(datafile):
 
   try:
     datafile_json = json.loads(datafile)
+    datafile_version = datafile_json.get('version')
   except:
     return False
 
-  datafile_version = datafile_json.get('version')
   json_schema = None
 
   if datafile_version == project_config.V1_CONFIG_VERSION:

optimizely/optimizely.py

@@ -1,3 +1,6 @@
+import logging
+import sys
+
 from . import bucketer
 from . import event_builder
 from . import exceptions
@@ -27,14 +30,33 @@ def __init__(self, datafile, event_dispatcher=None, logger=None, error_handler=N
                             By default JSON schema validation will be performed.
     """
 
+    self.is_valid = True
     self.event_dispatcher = event_dispatcher or default_event_dispatcher
     self.logger = logger or noop_logger
     self.error_handler = error_handler or noop_error_handler
-    self._validate_inputs(datafile, skip_json_validation)
 
-    self.config = project_config.ProjectConfig(datafile, self.logger, self.error_handler)
+    try:
+      self._validate_inputs(datafile, skip_json_validation)
+    except exceptions.InvalidInputException as error:
+      self.is_valid = False
+      logging.error(str(error))
+      return
+
+    try:
+      self.config = project_config.ProjectConfig(datafile, self.logger, self.error_handler)
+    except:
+      self.is_valid = False
+      self.config = None
+      logging.error(enums.Errors.INVALID_INPUT_ERROR.format('datafile'))
+      return
+
     self.bucketer = bucketer.Bucketer(self.config)
-    self.event_builder = event_builder.get_event_builder(self.config, self.bucketer)
+
+    try:
+      self.event_builder = event_builder.get_event_builder(self.config, self.bucketer)
+    except:
+      self.is_valid = False
+      logging.error(enums.Errors.UNSUPPORTED_DATAFILE_VERSION)
 
   def _validate_inputs(self, datafile, skip_json_validation):
     """ Helper method to validate all input parameters.
@@ -48,16 +70,16 @@ def _validate_inputs(self, datafile, skip_json_validation):
     """
 
     if not skip_json_validation and not validator.is_datafile_valid(datafile):
-      raise Exception(enums.Errors.INVALID_INPUT_ERROR.format('datafile'))
+     raise exceptions.InvalidInputException(enums.Errors.INVALID_INPUT_ERROR.format('datafile'))
 
     if not validator.is_event_dispatcher_valid(self.event_dispatcher):
-      raise Exception(enums.Errors.INVALID_INPUT_ERROR.format('event_dispatcher'))
+     raise exceptions.InvalidInputException(enums.Errors.INVALID_INPUT_ERROR.format('event_dispatcher'))
 
     if not validator.is_logger_valid(self.logger):
-      raise Exception(enums.Errors.INVALID_INPUT_ERROR.format('logger'))
+     raise exceptions.InvalidInputException(enums.Errors.INVALID_INPUT_ERROR.format('logger'))
 
     if not validator.is_error_handler_valid(self.error_handler):
-      raise Exception(enums.Errors.INVALID_INPUT_ERROR.format('error_handler'))
+     raise exceptions.InvalidInputException(enums.Errors.INVALID_INPUT_ERROR.format('error_handler'))
 
   def _validate_preconditions(self, experiment, user_id, attributes):
     """ Helper method to validate all pre-conditions before we go ahead to bucket user.
@@ -105,6 +127,10 @@ def activate(self, experiment_key, user_id, attributes=None):
       None if user is not in experiment or if experiment is not Running.
     """
 
+    if not self.is_valid:
+      logging.error(enums.Errors.INVALID_DATAFILE.format('activate'))
+      return None
+
     experiment = self.config.get_experiment_from_key(experiment_key)
     if not experiment:
       self.logger.log(enums.LogLevels.INFO, 'Not activating user "%s".' % user_id)
@@ -126,7 +152,11 @@ def activate(self, experiment_key, user_id, attributes=None):
     self.logger.log(enums.LogLevels.DEBUG,
                     'Dispatching impression event to URL %s with params %s.' % (impression_event.url,
                                                                                 impression_event.params))
-    self.event_dispatcher.dispatch_event(impression_event)
+    try:
+      self.event_dispatcher.dispatch_event(impression_event)
+    except:
+      error = sys.exc_info()[1]
+      self.logger.log(enums.LogLevels.ERROR, 'Unable to dispatch impression event. Error: %s' % str(error))
 
     return variation.key
 
@@ -140,13 +170,17 @@ def track(self, event_key, user_id,  attributes=None, event_value=None):
       event_value: Value associated with the event. Can be used to represent revenue in cents.
     """
 
+    if not self.is_valid:
+      logging.error(enums.Errors.INVALID_DATAFILE.format('track'))
+      return
+
     if attributes and not validator.are_attributes_valid(attributes):
       self.logger.log(enums.LogLevels.ERROR, 'Provided attributes are in an invalid format.')
       self.error_handler.handle_error(exceptions.InvalidAttributeException(enums.Errors.INVALID_ATTRIBUTE_FORMAT))
       return
 
     event = self.config.get_event(event_key)
-    if not event.experimentIds:
+    if not event:
       self.logger.log(enums.LogLevels.INFO, 'Not tracking user "%s" for event "%s".' % (user_id, event_key))
       return
 
@@ -167,7 +201,14 @@ def track(self, event_key, user_id,  attributes=None, event_value=None):
       self.logger.log(enums.LogLevels.DEBUG,
                       'Dispatching conversion event to URL %s with params %s.' % (conversion_event.url,
                                                                                   conversion_event.params))
-      self.event_dispatcher.dispatch_event(conversion_event)
+      try:
+        self.event_dispatcher.dispatch_event(conversion_event)
+      except:
+        error = sys.exc_info()[1]
+        self.logger.log(enums.LogLevels.ERROR, 'Unable to dispatch conversion event. Error: %s' % str(error))
+
+    else:
+      self.logger.log(enums.LogLevels.INFO, 'There are no valid experiments for event "%s" to track.' % event_key)
 
   def get_variation(self, experiment_key, user_id, attributes=None):
     """ Gets variation where user will be bucketed.
@@ -182,6 +223,10 @@ def get_variation(self, experiment_key, user_id, attributes=None):
       None if user is not in experiment or if experiment is not Running.
     """
 
+    if not self.is_valid:
+      logging.error(enums.Errors.INVALID_DATAFILE.format('get_variation'))
+      return None
+
     experiment = self.config.get_experiment_from_key(experiment_key)
     if not experiment:
       return None
@@ -194,4 +239,3 @@ def get_variation(self, experiment_key, user_id, attributes=None):
       return variation.key
 
     return None
-

optimizely/project_config.py

@@ -37,14 +37,14 @@ def __init__(self, datafile, logger, error_handler):
     self.audiences = config.get('audiences', [])
 
     # Utility maps for quick lookup
-    self.group_id_map = self._generate_key_map_entity(self.groups, 'id', entities.Group)
-    self.experiment_key_map = self._generate_key_map_entity(self.experiments, 'key', entities.Experiment)
-    self.event_key_map = self._generate_key_map_entity(self.events, 'key', entities.Event)
-    self.attribute_key_map = self._generate_key_map_entity(self.attributes, 'key', entities.Attribute)
-    self.audience_id_map = self._generate_key_map_entity(self.audiences, 'id', entities.Audience)
+    self.group_id_map = self._generate_key_map(self.groups, 'id', entities.Group)
+    self.experiment_key_map = self._generate_key_map(self.experiments, 'key', entities.Experiment)
+    self.event_key_map = self._generate_key_map(self.events, 'key', entities.Event)
+    self.attribute_key_map = self._generate_key_map(self.attributes, 'key', entities.Attribute)
+    self.audience_id_map = self._generate_key_map(self.audiences, 'id', entities.Audience)
     self.audience_id_map = self._deserialize_audience(self.audience_id_map)
     for group in self.group_id_map.values():
-      experiments_in_group_key_map = self._generate_key_map_entity(group.experiments, 'key', entities.Experiment)
+      experiments_in_group_key_map = self._generate_key_map(group.experiments, 'key', entities.Experiment)
       for experiment in experiments_in_group_key_map.values():
         experiment.__dict__.update({
           'groupId': group.id,
@@ -57,33 +57,15 @@ def __init__(self, datafile, logger, error_handler):
     self.variation_id_map = {}
     for experiment in self.experiment_key_map.values():
       self.experiment_id_map[experiment.id] = experiment
-      self.variation_key_map[experiment.key] = self._generate_key_map_entity(
+      self.variation_key_map[experiment.key] = self._generate_key_map(
         experiment.variations, 'key', entities.Variation
       )
       self.variation_id_map[experiment.key] = {}
       for variation in self.variation_key_map.get(experiment.key).values():
         self.variation_id_map[experiment.key][variation.id] = variation
 
   @staticmethod
-  def _generate_key_map(list, key):
-    """ Helper method to generate map from key to dict in list of dicts.
-
-    Args:
-      list: List consisting of dict.
-      key: Key in each dict which will be key in the map.
-
-    Returns:
-      Map mapping key to dict.
-    """
-
-    key_map = {}
-    for obj in list:
-      key_map[obj[key]] = obj
-
-    return key_map
-
-  @staticmethod
-  def _generate_key_map_entity(list, key, entity_class):
+  def _generate_key_map(list, key, entity_class):
     """ Helper method to generate map from key to entity object for given list of dicts.
 
     Args:

tests/test_event_builder.py

@@ -2,7 +2,9 @@
 import unittest
 
 from optimizely import event_builder
+from optimizely import exceptions
 from optimizely import version
+from optimizely.helpers import enums
 from . import base
 
 
@@ -183,7 +185,7 @@ def test_get_event_builder__invalid_datafile_version(self):
     """ Test that get_event_builder raises exception for unsupported datafile version. """
 
     with mock.patch('optimizely.project_config.ProjectConfig.get_version', return_value='unsupported_version'):
-      self.assertRaisesRegexp(Exception, 'Datafile provided has unsupported version.',
+      self.assertRaisesRegexp(exceptions.InvalidInputException, enums.Errors.UNSUPPORTED_DATAFILE_VERSION,
                               event_builder.get_event_builder, self.optimizely.config, self.optimizely.bucketer)
 
   def test_create_impression_event(self):