update: improve logging and error handling in bucketer and decision service

Author: FarhanAnjum-opti

optimizely/bucketer.py

@@ -124,7 +124,7 @@ def bucket(
             variation = project_config.get_variation_from_id_by_experiment_id(experiment.id, variation_id)
             return variation, decide_reasons
 
-        elif not decide_reasons:
+        else:
             message = 'Bucketed into an empty traffic range. Returning nil.'
             project_config.logger.info(message)
             decide_reasons.append(message)

optimizely/decision/optimizely_decision.py

@@ -66,5 +66,5 @@ def new_error_decision(cls, key: str, user: OptimizelyUserContext, reasons: list
             rule_key=None,
             flag_key=key,
             user_context=user,
-            reasons=[reasons[-1]] if reasons else []
+            reasons=reasons if reasons else []
         )

optimizely/decision_service.py

@@ -187,12 +187,13 @@ def _get_decision_for_cmab_experiment(
             error_message = Errors.CMAB_FETCH_FAILED_DETAILED.format(
                 experiment.key
             )
+            decide_reasons.append(error_message)
             if self.logger:
                 self.logger.error(f'{error_message} {str(e)}')
             return {
                 "error": True,
                 "result": None,
-                "reasons": [error_message],
+                "reasons": decide_reasons,
             }
 
     def set_forced_variation(

tests/test_bucketing.py

@@ -337,7 +337,12 @@ def test_bucket__experiment_in_group(self):
                 variation
             )
         mock_config_logging.debug.assert_called_once_with('Assigned bucket 8400 to user with bucketing ID "test_user".')
-        mock_config_logging.info.assert_called_once_with('User "test_user" is in no experiment.')
+        mock_config_logging.info.assert_has_calls(
+            [
+                mock.call('User "test_user" is in no experiment.'),
+                mock.call('Bucketed into an empty traffic range. Returning nil.')
+            ]
+        )
 
         # In group, no matching experiment
         with mock.patch(
@@ -378,8 +383,11 @@ def test_bucket__experiment_in_group(self):
                 variation
             )
         mock_config_logging.debug.assert_called_once_with('Assigned bucket 42 to user with bucketing ID "test_user".')
-        mock_config_logging.info.assert_called_once_with(
-            'User "test_user" is not in experiment "group_exp_2" of group 19228.'
+        mock_config_logging.info.assert_has_calls(
+            [
+                mock.call('User "test_user" is not in experiment "group_exp_2" of group 19228.'),
+                mock.call('Bucketed into an empty traffic range. Returning nil.')
+            ]
         )
 
         # In group no matching variation

tests/test_optimizely.py

@@ -5743,7 +5743,7 @@ def test_decide_returns_error_decision_when_decision_service_fails(self):
         ):
             # Call decide
             decision = user_context.decide('test_feature_in_experiment')
-            print(decision.__dict__)
+
             # Verify the decision contains the error information
             self.assertFalse(decision.enabled)
             self.assertIsNone(decision.variation_key)