fix: Mark the EXCEPTION reason of a failed evaluation of an override flag

An evaluation that fails after reading a definition from the override
layer is still marked (OVERRIDE 1.5.4). The single-flag and all-flags
error paths of both clients now set overrideAffected on the EXCEPTION
reason from the flag's own marker, read through is_override_definition
so that the error path cannot fail on an unexpected store item.

Author: kinyoklion

ldclient/async_client.py

@@ -28,7 +28,7 @@
 )
 from ldclient.impl.client_common import secure_mode_hash as _secure_mode_hash
 from ldclient.impl.datasystem import AsyncDataSystem, DataAvailability
-from ldclient.impl.evaluator_common import error_reason
+from ldclient.impl.evaluator_common import error_reason, is_override_definition
 from ldclient.impl.events.async_event_processor import (
     DefaultAsyncEventProcessor
 )
@@ -553,6 +553,10 @@ def not_ready() -> Tuple[EvaluationDetail, Optional[FeatureFlag]]:
                 log.error("Unexpected error while evaluating feature flag \"%s\": %s" % (key, repr(e)))
                 log.debug(traceback.format_exc())
                 reason = error_reason('EXCEPTION')
+                # The evaluation read the flag definition before it failed, so a definition from
+                # the override layer marks the result.
+                if is_override_definition(flag):
+                    reason['overrideAffected'] = True
                 self._send_event(event_factory.new_default_event(flag, context, default, reason))
                 return EvaluationDetail(default, None, reason), flag
 
@@ -646,7 +650,11 @@ async def all_flags_state(self, context: Context, **kwargs) -> FeatureFlagsState
             except Exception as e:
                 log.error("Error evaluating flag \"%s\" in all_flags_state: %s" % (key, repr(e)))
                 log.debug(traceback.format_exc())
-                reason = {'kind': 'ERROR', 'errorKind': 'EXCEPTION'}
+                reason = error_reason('EXCEPTION')
+                # The evaluation read the flag definition before it failed, so a definition from
+                # the override layer marks the result.
+                if is_override_definition(flag):
+                    reason['overrideAffected'] = True
                 detail = EvaluationDetail(None, None, reason)
                 prerequisites = []
             requires_experiment_data = EventFactory.is_experiment(flag, detail.reason)

ldclient/client.py

@@ -28,6 +28,7 @@
 from ldclient.impl.datasystem import DataAvailability, DataSystem
 from ldclient.impl.datasystem.fdv2 import FDv2
 from ldclient.impl.evaluator import Evaluator, error_reason
+from ldclient.impl.evaluator_common import is_override_definition
 from ldclient.impl.events.diagnostics import (
     _DiagnosticAccumulator,
     create_diagnostic_id
@@ -539,6 +540,10 @@ def not_ready() -> Tuple[EvaluationDetail, Optional[FeatureFlag]]:
                 log.error("Unexpected error while evaluating feature flag \"%s\": %s" % (key, repr(e)))
                 log.debug(traceback.format_exc())
                 reason = error_reason('EXCEPTION')
+                # The evaluation read the flag definition before it failed, so a definition from
+                # the override layer marks the result.
+                if is_override_definition(flag):
+                    reason['overrideAffected'] = True
                 self._send_event(event_factory.new_default_event(flag, context, default, reason))
                 return EvaluationDetail(default, None, reason), flag
 
@@ -638,7 +643,11 @@ def all_flags_state(self, context: Context, **kwargs) -> FeatureFlagsState:
             except Exception as e:
                 log.error("Error evaluating flag \"%s\" in all_flags_state: %s" % (key, repr(e)))
                 log.debug(traceback.format_exc())
-                reason = {'kind': 'ERROR', 'errorKind': 'EXCEPTION'}
+                reason = error_reason('EXCEPTION')
+                # The evaluation read the flag definition before it failed, so a definition from
+                # the override layer marks the result.
+                if is_override_definition(flag):
+                    reason['overrideAffected'] = True
                 detail = EvaluationDetail(None, None, reason)
                 prerequisites = []
 

ldclient/impl/evaluator_common.py

@@ -262,6 +262,16 @@ def error_reason(error_kind: str) -> dict:
     return {'kind': 'ERROR', 'errorKind': error_kind}
 
 
+def is_override_definition(item: Any) -> bool:
+    """
+    Reports whether a flag or segment read from the store came from the override layer. The
+    data systems decode the dicts a store holds into model objects before the client reads
+    them, but an error path reads the marker this way so that an unexpected item cannot make
+    it fail too.
+    """
+    return getattr(item, 'is_override', False) is True
+
+
 def mark_override_affected(detail: EvaluationDetail) -> None:
     """
     Sets the override-affected indicator on the evaluation reason. The indicator is present

ldclient/testing/impl/test_evaluator_overrides.py

@@ -9,7 +9,7 @@
 import pytest
 
 from ldclient import Context
-from ldclient.impl.evaluator_common import EvalResult
+from ldclient.impl.evaluator_common import EvalResult, is_override_definition
 from ldclient.impl.events.types import EventFactory, EventInputEvaluation
 from ldclient.testing.builders import *
 from ldclient.testing.impl.evaluator_util import EvaluatorBuilder
@@ -314,3 +314,12 @@ def test_reason_indicator_is_absent_when_events_do_not_carry_reasons():
     flag = make_override_test_flag('feature').with_override_marker()
     result = EvaluatorBuilder().build().evaluate(flag, override_test_context, EventFactory(False, lambda: 0))
     assert_override_affected(True, result)
+
+
+def test_is_override_definition_reads_the_marker_and_never_fails():
+    flag = FlagBuilder('feature').on(False).off_variation(0).variations('off', 'on').build()
+    assert is_override_definition(flag.with_override_marker()) is True
+    assert is_override_definition(flag) is False
+    # An item without the marker attribute, such as a raw dict, reads as unmarked.
+    assert is_override_definition(flag.to_json_dict()) is False
+    assert is_override_definition(None) is False

ldclient/testing/test_async_client_overrides.py

@@ -415,3 +415,27 @@ async def test_flag_value_change_listener_sees_override_value_changes():
         assert change.new_value == 'override-value'
     finally:
         await client.close()
+
+
+async def raise_evaluation_failure(*args):
+    raise RuntimeError("evaluation failure")
+
+
+@pytest.mark.asyncio
+async def test_failed_evaluation_reason_is_marked_only_when_the_flag_came_from_the_override_layer(monkeypatch):
+    # The evaluation read the flag definition before it failed, so the failure of the override
+    # flag is marked and the failure of the ordinary flag is not, in a single evaluation and in
+    # the all-flags state.
+    source = MockOverrideSource(flags={'overridden-flag': single_value_flag('overridden-flag', True)})
+    client = await make_initialized_client({'plain-flag': single_value_flag('plain-flag', True)}, source)
+    try:
+        monkeypatch.setattr(client._evaluator, 'evaluate', raise_evaluation_failure)
+
+        assert await client.variation_detail('overridden-flag', user, 'default') == EvaluationDetail('default', None, {'kind': 'ERROR', 'errorKind': 'EXCEPTION', 'overrideAffected': True})
+        assert await client.variation_detail('plain-flag', user, 'default') == EvaluationDetail('default', None, {'kind': 'ERROR', 'errorKind': 'EXCEPTION'})
+
+        state = await client.all_flags_state(user, with_reasons=True)
+        assert state.get_flag_reason('overridden-flag') == {'kind': 'ERROR', 'errorKind': 'EXCEPTION', 'overrideAffected': True}
+        assert state.get_flag_reason('plain-flag') == {'kind': 'ERROR', 'errorKind': 'EXCEPTION'}
+    finally:
+        await client.close()

ldclient/testing/test_ldclient_overrides.py

@@ -462,3 +462,23 @@ def test_data_source_status_is_unaffected_by_overrides():
     with make_uninitialized_client(source) as client:
         assert client.is_initialized() is False
         assert client.data_source_status_provider.status.state == DataSourceState.INITIALIZING
+
+
+def raise_evaluation_failure(*args):
+    raise RuntimeError("evaluation failure")
+
+
+def test_failed_evaluation_reason_is_marked_only_when_the_flag_came_from_the_override_layer(monkeypatch):
+    # The evaluation read the flag definition before it failed, so the failure of the override
+    # flag is marked and the failure of the ordinary flag is not, in a single evaluation and in
+    # the all-flags state.
+    source = MockOverrideSource(flags={'overridden-flag': single_value_flag('overridden-flag', True)})
+    with make_initialized_client({'plain-flag': single_value_flag('plain-flag', True)}, source) as client:
+        monkeypatch.setattr(client._evaluator, 'evaluate', raise_evaluation_failure)
+
+        assert client.variation_detail('overridden-flag', user, 'default') == EvaluationDetail('default', None, {'kind': 'ERROR', 'errorKind': 'EXCEPTION', 'overrideAffected': True})
+        assert client.variation_detail('plain-flag', user, 'default') == EvaluationDetail('default', None, {'kind': 'ERROR', 'errorKind': 'EXCEPTION'})
+
+        state = client.all_flags_state(user, with_reasons=True)
+        assert state.get_flag_reason('overridden-flag') == {'kind': 'ERROR', 'errorKind': 'EXCEPTION', 'overrideAffected': True}
+        assert state.get_flag_reason('plain-flag') == {'kind': 'ERROR', 'errorKind': 'EXCEPTION'}