fix: treat failed feature flag records as unevaluated

.sampo/changesets/fix-failed-flag-records.md

@@ -0,0 +1,5 @@
+---
+pypi/posthog: patch
+---
+
+Treat failed feature flag records as unevaluated, report evaluation failure diagnostics, and preserve successful local values and payloads when remote fallback fails.

posthog/client.py

@@ -4646,6 +4646,8 @@ def _get_feature_flag_result(
                     errors.append(FeatureFlagError.ERRORS_WHILE_COMPUTING)
                 if flag_details is None:
                     errors.append(FeatureFlagError.FLAG_MISSING)
+                elif flag_details._failed:
+                    errors.append("evaluation_failed")
                 if errors:
                     feature_flag_error = ",".join(errors)
 
@@ -4677,10 +4679,11 @@ def _get_feature_flag_result(
                 self.log.debug(
                     f"Successfully computed flag remotely: #{key} -> #{flag_result}"
                 )
-                # A payload lookup for a caller's match_value reports that value, not
-                # the record's. A failure above falls back to a stale cached value.
-                value_from_details = (
-                    flag_details is not None and override_match_value is None
+                # A successful payload override reports the caller's value. A
+                # failed record still supplies diagnostics, never the override.
+                value_from_details = flag_details is not None and (
+                    override_match_value is None
+                    or (flag_details._failed and flag_details._malformed_field is None)
                 )
             except QuotaLimitError as e:
                 self.log.warning(f"[FEATURE FLAGS] Quota limit exceeded: {e}")
@@ -5321,7 +5324,32 @@ def get_all_flags_and_payloads(
                     flag_keys_to_evaluate=flag_keys_to_evaluate,
                     device_id=device_id,
                 )
-                response = to_flags_and_payloads(decide_response)
+                remote_response = to_flags_and_payloads(decide_response)
+                # Only explicit failed records preserve same-context local pairs;
+                # successful remote records retain the existing precedence.
+                failed_keys = {
+                    key
+                    for key, detail in decide_response.get("flags", {}).items()
+                    if detail._failed and detail._malformed_field is None
+                }
+                response = {
+                    "featureFlags": {
+                        **{
+                            k: v
+                            for k, v in (response["featureFlags"] or {}).items()
+                            if k in failed_keys
+                        },
+                        **(remote_response["featureFlags"] or {}),
+                    },
+                    "featureFlagPayloads": {
+                        **{
+                            k: v
+                            for k, v in (response["featureFlagPayloads"] or {}).items()
+                            if k in failed_keys
+                        },
+                        **(remote_response["featureFlagPayloads"] or {}),
+                    },
+                }
             except Exception as e:
                 self.log.exception(
                     f"[FEATURE FLAGS] Unable to get feature flags and payloads: {e}"
@@ -5535,7 +5563,14 @@ def evaluate_flags(
             quota_limited=quota_limited,
             minimal_flag_called_events=minimal_flag_called_events,
             unresolved_flags={
-                key: reason for key, reason in unresolved.items() if key not in records
+                key: reason
+                for key, reason in unresolved.items()
+                if key not in records
+                or (
+                    isinstance(record := records[key], _RemoteFlagRecord)
+                    and record.details._failed
+                    and record.details._malformed_field is None
+                )
             },
         )
 

posthog/feature_flag_evaluations.py

@@ -152,7 +152,14 @@ def __init__(
         """Internal — instances are created by the SDK via ``Client.evaluate_flags()``."""
         self._host = host
         self._distinct_id = distinct_id
-        self._flags = flags
+        self._failed_flags = {
+            key: flag
+            for key, flag in flags.items()
+            if isinstance(flag, _RemoteFlagRecord)
+            and flag.details._failed
+            and flag.details._malformed_field is None
+        }
+        self._flags = {k: v for k, v in flags.items() if k not in self._failed_flags}
         self._groups: Dict[str, Union[str, int]] = dict(groups or {})
         self._disable_geoip = disable_geoip
         self._request_id = request_id
@@ -337,8 +344,9 @@ def _record_access(self, key: str, send_feature_flag_events: bool) -> None:
         if not self._distinct_id:
             return
 
-        flag = self._flags.get(key)
-        if flag is None:
+        failed = key in self._failed_flags
+        flag = self._flags.get(key) or self._failed_flags.get(key)
+        if flag is None or failed:
             response: Optional[FlagValue] = None
         elif not flag.enabled:
             response = False
@@ -376,7 +384,9 @@ def _record_access(self, key: str, send_feature_flag_events: bool) -> None:
             errors.append("errors_while_computing_flags")
         if self._quota_limited:
             errors.append("quota_limited")
-        if flag is None:
+        if failed:
+            errors.append("evaluation_failed")
+        elif flag is None:
             errors.append(
                 "local_evaluation_inconclusive"
                 if key in self._unresolved_flags

posthog/test/test_evaluate_flags.py

@@ -885,19 +885,60 @@ def test_dependency_resolved_locally_picks_its_condition(self, patch_flags):
         self.assertEqual(flags.get_flag("target"), "a")
         patch_flags.assert_not_called()
 
+    @parameterized.expand([(True,), (False,)])
     @mock.patch("posthog.client.flags")
-    def test_flag_resolved_by_remote_fallback_is_not_unresolved(self, patch_flags):
+    def test_flag_resolved_by_remote_fallback_is_not_unresolved(
+        self, enabled, patch_flags
+    ):
         patch_flags.return_value = {
-            "flags": {"checkout": {"enabled": True, "variant": None}}
+            "flags": {"checkout": {"enabled": enabled, "variant": None}}
         }
 
         flags = self.client.evaluate_flags("user-123")
 
-        self.assertIs(flags.get_flag("checkout"), True)
+        self.assertIs(flags.get_flag("checkout"), enabled)
         self.assertEqual(sorted(flags.keys), ["beta-ui", "checkout"])
         self.assertEqual(dict(flags.unresolved_flags), {})
         patch_flags.assert_called_once()
 
+    @parameterized.expand([(None,), (False,), (True,)])
+    @mock.patch("posthog.client.flags")
+    @mock.patch.object(Client, "capture")
+    def test_failed_remote_fallback_preserves_unresolved_reason(
+        self, error_bit, patch_capture, patch_flags
+    ):
+        response = {
+            "flags": {
+                key: {
+                    "enabled": False,
+                    "failed": True,
+                    "metadata": {"payload": '"placeholder"'},
+                }
+                for key in ("checkout", "remote-only")
+            }
+        }
+        if error_bit is not None:
+            response["errorsWhileComputingFlags"] = error_bit
+        patch_flags.return_value = response
+
+        flags = self.client.evaluate_flags("user-123")
+
+        self.assertEqual(
+            dict(flags.unresolved_flags),
+            {"checkout": UnresolvedFlagReason.EXPERIENCE_CONTINUITY},
+        )
+        self.assertEqual(flags.keys, ["beta-ui"])
+        self.assertIsNone(flags.get_flag("checkout", send_feature_flag_events=False))
+        self.assertTrue(
+            flags.is_enabled("checkout", True, send_feature_flag_events=False)
+        )
+        self.assertIsNone(flags.get_flag_payload("checkout"))
+        self.assertNotIn("$feature/checkout", flags._get_event_properties())
+        self.assertEqual(dict(flags.only(["checkout"]).unresolved_flags), {})
+        self.assertEqual(dict(flags.only_accessed().unresolved_flags), {})
+        patch_capture.assert_not_called()
+        patch_flags.assert_called_once()
+
     @parameterized.expand(
         [
             (

posthog/test/test_experiment_exposure.py

@@ -517,18 +517,28 @@ def _attribution_response(source: str) -> Dict[str, Any]:
     "source, expected", ATTRIBUTION_CASES, ids=[c[0] for c in ATTRIBUTION_CASES]
 )
 def test_flag_called_attribution_follows_the_record(source, expected, path):
-    events = _read_events(path, _attribution_response(source), key="garden-layout")
+    response = _attribution_response(source)
+    failed = response["flags"]["garden-layout"].get("failed") is True
+    events = _read_events(path, response, key="garden-layout")
 
     [called] = _named(events, FLAG_CALLED, "garden-layout")
     assert _attribution(called.properties) == expected
-    FLAG_CALLED_SCHEMA.validate(_projection(FLAG_CALLED_SCHEMA, called.properties))
+    if failed:
+        # The vendored schema predates the no-result sentinel for failed records.
+        assert called.properties["$feature_flag_response"] is None
+        assert set(called.properties["$feature_flag_error"].split(",")) == {
+            "evaluation_failed",
+            "errors_while_computing_flags",
+        }
+    else:
+        FLAG_CALLED_SCHEMA.validate(_projection(FLAG_CALLED_SCHEMA, called.properties))
     exposures = _named(events, EXPOSURE, "garden-layout")
     assert len(exposures) == (1 if "$feature_flag_variant_key" in expected else 0)
 
 
 # A record whose reason names no rule, or that failed, reports no rule even when it
-# carries one. Neither sends an exposure. The last element says whether the record is
-# a valid server outcome, which the contract's event schema then accepts.
+# carries one. Neither sends an exposure. The last element says whether a successful
+# record is a valid server outcome, which the contract's event schema then accepts.
 STRAY_RULE_CASES = [
     ("no_rule_match", {}, True),
     ("missing_group_key", {}, True),
@@ -569,7 +579,10 @@ def test_rule_fields_are_sent_only_for_reasons_that_name_a_rule(
         "$feature_flag_reason_code": code,
     }
     assert _named(events, EXPOSURE) == []
-    if valid_outcome:
+    if fields.get("failed") is True:
+        assert called.properties["$feature_flag_response"] is None
+        assert called.properties["$feature_flag_error"] == "evaluation_failed"
+    elif valid_outcome:
         FLAG_CALLED_SCHEMA.validate(_projection(FLAG_CALLED_SCHEMA, called.properties))