feat(flags): read the /flags?v=4 response (#1031)

Author: andehen

.github/workflows/sdk-compliance.yml

@@ -33,7 +33,7 @@ jobs:
     with:
       adapter-dockerfile: "sdk_compliance_adapter/Dockerfile"
       adapter-context: "."
-      test-harness-version: "1.1.1"
+      test-harness-version: "1.23.0"
       continue-on-error: false
       report-name: "sdk-compliance-report-v0"
 
@@ -43,6 +43,6 @@ jobs:
     with:
       adapter-dockerfile: "sdk_compliance_adapter/Dockerfile.v1"
       adapter-context: "."
-      test-harness-version: "1.1.1"
+      test-harness-version: "1.23.0"
       continue-on-error: false
       report-name: "sdk-compliance-report-v1"

.sampo/changesets/flags-v4-response.md

@@ -0,0 +1,5 @@
+---
+pypi/posthog: minor
+---
+
+Request `/flags?v=4` and read each flag's typed value, reason and metadata. They stay internal for now: this release adds no public methods, fields or types for them, and `repr`, equality and `dataclasses.asdict` of a flag show only its existing fields. Servers that still send the older response keep working, and a flag record with a field of the wrong JSON type fails only that flag. For valid flag records, existing getters, callbacks and `$feature_flag_called` properties return the same values as before: a number or object value reads as `True`, with the value as its payload. Two things can differ: the human-readable reason text of a config version 2 flag is the one the server sends at v4, and a record with a known field of the wrong JSON type (for example a string `reason`) now reads as a failed flag instead of being read anyway or failing the whole response. The SDK logs once per client when a server sends a response without typed values. `FlagMetadata.id` and `FlagMetadata.version` are `None` when a record has none, instead of `0`, so their type is now `Optional[int]`.

posthog/_async_request.py

@@ -149,7 +149,7 @@ async def async_flags(
                     _serialize_flags_body, api_key, request_data
                 )
                 response = await http_client.post(
-                    "/flags/?v=2", content=data, headers=headers, timeout=timeout
+                    "/flags/?v=4", content=data, headers=headers, timeout=timeout
                 )
                 return await asyncio.to_thread(_process_flags_response, response)
             except httpx_module.TransportError:

posthog/async_client.py

@@ -40,8 +40,6 @@
     MAX_DICT_SIZE as _MAX_DICT_SIZE,
     _MINIMAL_FLAG_CALLED_EVENT_PROPERTIES,
     Client as _SyncClient,
-    _metadata_has_experiment,
-    _parse_flag_payload,
     add_context_tags as _add_context_tags,
     get_identity_state as _get_identity_state,
     stringify_id as _stringify_id,
@@ -74,11 +72,17 @@
 from .feature_flag_evaluations import (
     FeatureFlagEvaluations,
     _EvaluatedFlagRecord,
+    _RemoteFlagRecord,
     _FeatureFlagEvaluationsHost,
 )
 from .release_id import _resolve_release_id
 from .request import QuotaLimitError, determine_server_host, normalize_host
-from .types import FlagMetadata, FlagValue, normalize_flags_response
+from .types import (
+    FlagValue,
+    _has_untyped_flag_records,
+    _UNTYPED_FLAGS_RESPONSE_MESSAGE,
+    normalize_flags_response,
+)
 from .utils import SizeLimitedDict, _normalize_timestamp, clean, system_context
 from .version import VERSION
 
@@ -231,6 +235,8 @@ def __init__(
             self.log.setLevel(logging.DEBUG)
         else:
             self.log.setLevel(logging.WARNING)
+        # Whether this client has logged that /flags sent records without typed values.
+        self._logged_untyped_flags_response = False
 
         if not self.api_key:
             self.log.error(
@@ -927,6 +933,11 @@ async def _get_flags_decision(
             client=self._get_http_client(),
             **request_data,
         )
+        if not self._logged_untyped_flags_response and _has_untyped_flag_records(
+            response
+        ):
+            self._logged_untyped_flags_response = True
+            self.log.info(_UNTYPED_FLAGS_RESPONSE_MESSAGE)
         return normalize_flags_response(response)
 
     async def evaluate_flags(
@@ -1004,24 +1015,7 @@ async def evaluate_flags(
             for key, detail in response.get("flags", {}).items():
                 if requested_keys is not None and key not in requested_keys:
                     continue
-                metadata = detail.metadata
-                records[key] = _EvaluatedFlagRecord(
-                    key=key,
-                    enabled=detail.enabled,
-                    variant=detail.variant,
-                    payload=_parse_flag_payload(metadata.payload),
-                    id=metadata.id if isinstance(metadata, FlagMetadata) else None,
-                    version=(
-                        metadata.version if isinstance(metadata, FlagMetadata) else None
-                    ),
-                    reason=(
-                        detail.reason.description
-                        if detail.reason and detail.reason.description
-                        else None
-                    ),
-                    locally_evaluated=False,
-                    has_experiment=_metadata_has_experiment(metadata),
-                )
+                records[key] = _RemoteFlagRecord(detail)
         except QuotaLimitError:
             self.log.warning(
                 "[FEATURE FLAGS] Feature flag evaluation was quota limited"

posthog/client.py

@@ -77,7 +77,9 @@
 )
 from posthog.feature_flag_evaluations import (
     FeatureFlagEvaluations,
+    _LocalFlagRecord,
     _EvaluatedFlagRecord,
+    _RemoteFlagRecord,
     _FeatureFlagEvaluationsHost,
 )
 from posthog.feature_flags import (
@@ -123,6 +125,8 @@
     FlagValue,
     SendFeatureFlagsOptions,
     UnresolvedFlagReason,
+    _has_untyped_flag_records,
+    _UNTYPED_FLAGS_RESPONSE_MESSAGE,
     normalize_flags_response,
     to_flags_and_payloads,
     to_payloads,
@@ -1016,6 +1020,8 @@ def __init__(
         # the /flags v2 response and the local-evaluation payload. False until the
         # server reports it, so full events are the fail-safe.
         self._minimal_flag_called_events: bool = False
+        # Whether this client has logged that /flags sent records without typed values.
+        self._logged_untyped_flags_response = False
 
         self.capture_trace_context = capture_trace_context
         self.capture_exception_code_variables = capture_exception_code_variables
@@ -1716,6 +1722,11 @@ def _get_flags_decision(
             **request_data,
         )
 
+        if not self._logged_untyped_flags_response and _has_untyped_flag_records(
+            resp_data
+        ):
+            self._logged_untyped_flags_response = True
+            self.log.info(_UNTYPED_FLAGS_RESPONSE_MESSAGE)
         response = normalize_flags_response(resp_data)
         # Server-controlled gate for minimal $feature_flag_called events. Only the
         # v2 response shape carries it; absent (legacy shape, older server, team
@@ -4172,9 +4183,23 @@ def _get_feature_flag_result(
                     flag_details, override_match_value
                 )
 
+                if (
+                    flag_details is not None
+                    and flag_details._malformed_field is not None
+                ):
+                    # The record could not be read, so it reads as disabled. It must
+                    # not replace the last good cached value, which it falls back to
+                    # like a failed request, and it reports what a failed request
+                    # reports: an unknown error and no request details.
+                    feature_flag_error = FeatureFlagError.UNKNOWN_ERROR
+                    request_id = None
+                    evaluated_at = None
+                    flag_result = (
+                        self._get_stale_flag_fallback(distinct_id, key) or flag_result
+                    )
                 # The request-start generation is an invalidation boundary, not
                 # a claim about the server's definitions. Refresh rejects late writes.
-                if self.flag_cache and flag_result:
+                elif self.flag_cache and flag_result:
                     self.flag_cache._set_cached_remote_flag(
                         distinct_id, key, flag_result, local_definition_version
                     )
@@ -4991,7 +5016,7 @@ def evaluate_flags(
 
         for key, value in local_flags.items():
             flag_def = feature_flags_by_key.get(key) or {}
-            records[key] = _EvaluatedFlagRecord(
+            records[key] = _LocalFlagRecord(
                 key=key,
                 enabled=value is not False,
                 variant=value if isinstance(value, str) else None,
@@ -5034,34 +5059,7 @@ def evaluate_flags(
                         continue
                     if key in locally_evaluated_keys:
                         continue
-                    payload = _parse_flag_payload(
-                        detail.metadata.payload
-                        if isinstance(detail.metadata, FlagMetadata)
-                        else getattr(detail.metadata, "payload", None)
-                    )
-                    records[key] = _EvaluatedFlagRecord(
-                        key=key,
-                        enabled=detail.enabled,
-                        variant=detail.variant,
-                        payload=payload,
-                        id=(
-                            detail.metadata.id
-                            if isinstance(detail.metadata, FlagMetadata)
-                            else None
-                        ),
-                        version=(
-                            detail.metadata.version
-                            if isinstance(detail.metadata, FlagMetadata)
-                            else None
-                        ),
-                        reason=(
-                            detail.reason.description
-                            if detail.reason and detail.reason.description
-                            else None
-                        ),
-                        locally_evaluated=False,
-                        has_experiment=_metadata_has_experiment(detail.metadata),
-                    )
+                    records[key] = _RemoteFlagRecord(detail)
             except QuotaLimitError as e:
                 self.log.warning(f"[FEATURE FLAGS] Quota limit exceeded: {e}")
                 quota_limited = True

posthog/feature_flag_evaluations.py

@@ -9,12 +9,14 @@
 from types import MappingProxyType
 from typing import Any, Callable, Dict, List, Mapping, Optional, Set, Union
 
-from posthog.types import FlagValue, UnresolvedFlagReason
+from .types import FeatureFlag as _FeatureFlag
+from .types import FlagMetadata as _FlagMetadata
+from .types import FlagValue, UnresolvedFlagReason, _parse_flag_payload
 
 
 @dataclass(frozen=True)
-class _EvaluatedFlagRecord:
-    """Internal per-flag record stored by a FeatureFlagEvaluations instance."""
+class _LocalFlagRecord:
+    """Internal per-flag record of a locally evaluated flag."""
 
     key: str
     enabled: bool
@@ -29,6 +31,70 @@ class _EvaluatedFlagRecord:
     has_experiment: Optional[bool] = None
 
 
+_UNDECODED: Any = object()
+
+
+class _RemoteFlagRecord:
+    """Internal per-flag record of a remotely evaluated flag.
+
+    The parsed ``/flags`` record is the only copy of the flag: the record fields are
+    read from it, and the payload is decoded on its first read and then kept.
+    """
+
+    __slots__ = ("details", "_payload")
+
+    locally_evaluated = False
+
+    def __init__(self, details: _FeatureFlag) -> None:
+        # The parsed /flags record, with its typed value, reason and metadata.
+        self.details = details
+        self._payload: Any = _UNDECODED
+
+    @property
+    def key(self) -> str:
+        return self.details.key
+
+    @property
+    def enabled(self) -> bool:
+        return self.details.enabled
+
+    @property
+    def variant(self) -> Optional[str]:
+        return self.details.variant
+
+    @property
+    def payload(self) -> Optional[Any]:
+        if self._payload is _UNDECODED:
+            self._payload = _parse_flag_payload(
+                getattr(self.details.metadata, "payload", None)
+            )
+        return self._payload
+
+    @property
+    def id(self) -> Optional[int]:
+        metadata = self.details.metadata
+        return metadata.id if isinstance(metadata, _FlagMetadata) else None
+
+    @property
+    def version(self) -> Optional[int]:
+        metadata = self.details.metadata
+        return metadata.version if isinstance(metadata, _FlagMetadata) else None
+
+    @property
+    def reason(self) -> Optional[str]:
+        reason = self.details.reason
+        return reason.description if reason and reason.description else None
+
+    @property
+    def has_experiment(self) -> Optional[bool]:
+        metadata = self.details.metadata
+        return metadata.has_experiment if isinstance(metadata, _FlagMetadata) else None
+
+
+# The per-flag record stored by a FeatureFlagEvaluations instance.
+_EvaluatedFlagRecord = Union[_LocalFlagRecord, _RemoteFlagRecord]
+
+
 @dataclass
 class _FeatureFlagEvaluationsHost:
     """Callbacks the evaluations object uses to talk back to the client.

posthog/request.py

@@ -344,7 +344,7 @@ def flags(
             res = post(
                 api_key,
                 host,
-                "/flags/?v=2",
+                "/flags/?v=4",
                 gzip,
                 timeout,
                 session=_get_flags_session(),

posthog/test/fixtures/feature_flag_rules_v2/SOURCE.json

@@ -0,0 +1,26 @@
+{
+  "description": "Copied unchanged from the released feature flag rules v2 contract in PostHog/posthog-sdk-test-harness. Do not edit these files; copy a newer release instead.",
+  "repository": "https://github.com/PostHog/posthog-sdk-test-harness",
+  "release": "1.22.0",
+  "revision": "17d861f8e230fe943934d70e3eebf5cff6bf3aa4",
+  "contract_directory": "contracts/feature_flag_rules_v2",
+  "contract_version": "7.0.0",
+  "files": {
+    "readers.json": {
+      "source_path": "fixtures/wire/readers.json",
+      "sha256": "fd8edd89f827b732c4659ce7246eab534b0106b96a465a4a2d98cfa0a17d7827"
+    },
+    "responses.json": {
+      "source_path": "fixtures/wire/responses.json",
+      "sha256": "39aa62e340d66903e2fe86da226e622979c7dbc2efbf2b32afd040248a707c61"
+    },
+    "legacy_projection.json": {
+      "source_path": "corpus/legacy_projection.json",
+      "sha256": "536c8386eefa75721d113bd8718ddeb7fb1aad3b4f2cce1fbcf516a346a16e26"
+    },
+    "v2_value_evaluation.json": {
+      "source_path": "corpus/v2_value_evaluation.json",
+      "sha256": "aaddd05e47fc5a4ae07ea42ada5a659798f4b5fc8d0c0827f6a68e4daf68c128"
+    }
+  }
+}