From 535f1514c672fcf8ac7e06082c2201f33bb9bb1b Mon Sep 17 00:00:00 2001 From: Anders Asheim Hennum Date: Wed, 4 Nov 2026 09:57:50 +0100 Subject: [PATCH 1/2] fix(flags): return no payload for a failed flag record A record the server marks failed reads as disabled, so it now yields no payload, even when it still carries metadata.payload. get_feature_flag_payload(), get_feature_flag_result().payload and a snapshot's get_flag_payload() return None for it, and its $feature_flag_called events leave out $feature_flag_payload. get_all_flags_and_payloads() and get_feature_payloads() already left it out, because they keep only enabled flags. This holds for /flags?v=4 records and for records from older servers. The 7.82.1 comparison's failed flag now carries a payload, and the fixture is re-recorded with posthog 7.82.1. Every other recorded result is unchanged. The test names the payload difference for the getters and for the server-failed cache history. --- .../changesets/no-payload-for-failed-flag.md | 5 ++ .../test/fixtures/released_flags_7_82_1.json | 26 ++++++++-- posthog/test/released_flags_scenarios.py | 3 +- posthog/test/test_flags_released_behaviour.py | 31 ++++++++++- posthog/test/test_flags_v4_response.py | 52 +++++++++++++++++++ posthog/types.py | 18 ++++--- 6 files changed, 123 insertions(+), 12 deletions(-) create mode 100644 .sampo/changesets/no-payload-for-failed-flag.md diff --git a/.sampo/changesets/no-payload-for-failed-flag.md b/.sampo/changesets/no-payload-for-failed-flag.md new file mode 100644 index 000000000..69ef828f7 --- /dev/null +++ b/.sampo/changesets/no-payload-for-failed-flag.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: patch +--- + +A flag record that the server marks `failed` no longer returns a payload, even when the record still carries `metadata.payload`. A failed flag reads as disabled, so `get_feature_flag_payload()`, `get_feature_flag_result().payload` and a snapshot's `get_flag_payload()` now return `None` for it, and its `$feature_flag_called` events leave out `$feature_flag_payload`. `get_all_flags_and_payloads()` and `get_feature_payloads()` already left it out. This applies to responses from both `/flags?v=4` and older servers. diff --git a/posthog/test/fixtures/released_flags_7_82_1.json b/posthog/test/fixtures/released_flags_7_82_1.json index 627f5336e..2856b2f18 100644 --- a/posthog/test/fixtures/released_flags_7_82_1.json +++ b/posthog/test/fixtures/released_flags_7_82_1.json @@ -990,6 +990,9 @@ "$feature_flag_evaluated_at": 1800000000000, "$feature_flag_has_experiment": false, "$feature_flag_id": 700, + "$feature_flag_payload": { + "columns": 2 + }, "$feature_flag_reason": "Matched condition set 1", "$feature_flag_request_id": "60000000-0000-4000-8000-000000000000", "$feature_flag_response": false, @@ -1005,6 +1008,9 @@ "$feature_flag_evaluated_at": 1800000000000, "$feature_flag_has_experiment": false, "$feature_flag_id": 700, + "$feature_flag_payload": { + "columns": 2 + }, "$feature_flag_reason": "Matched condition set 1", "$feature_flag_request_id": "60000000-0000-4000-8000-000000000000", "$feature_flag_response": false, @@ -1020,6 +1026,9 @@ "$feature_flag_evaluated_at": 1800000000000, "$feature_flag_has_experiment": false, "$feature_flag_id": 700, + "$feature_flag_payload": { + "columns": 2 + }, "$feature_flag_reason": "Matched condition set 1", "$feature_flag_request_id": "60000000-0000-4000-8000-000000000000", "$feature_flag_response": false, @@ -1232,6 +1241,9 @@ "$feature_flag_evaluated_at": 1800000000000, "$feature_flag_has_experiment": false, "$feature_flag_id": 700, + "$feature_flag_payload": { + "columns": 2 + }, "$feature_flag_reason": "Matched condition set 1", "$feature_flag_request_id": "60000000-0000-4000-8000-000000000000", "$feature_flag_response": false, @@ -1366,7 +1378,9 @@ "get_feature_flag_payload:bool-off": null, "get_feature_flag_payload:bool-on": null, "get_feature_flag_payload:missing": null, - "get_feature_flag_payload:server-failed": null, + "get_feature_flag_payload:server-failed": { + "columns": 2 + }, "get_feature_flag_payload:sparse": null, "get_feature_flag_payload:v1-bool-payload": { "cta": "Try it", @@ -1402,7 +1416,9 @@ "get_feature_flag_result:server-failed": [ false, null, - null, + { + "columns": 2 + }, "Matched condition set 1" ], "get_feature_flag_result:sparse": [ @@ -1559,7 +1575,7 @@ "description": null, "has_experiment": false, "id": 700, - "payload": null, + "payload": "{\"columns\": 2}", "version": 3 }, "reason": { @@ -1743,7 +1759,9 @@ "snapshot.get_flag_payload:bool-off": null, "snapshot.get_flag_payload:bool-on": null, "snapshot.get_flag_payload:missing": null, - "snapshot.get_flag_payload:server-failed": null, + "snapshot.get_flag_payload:server-failed": { + "columns": 2 + }, "snapshot.get_flag_payload:sparse": null, "snapshot.get_flag_payload:v1-bool-payload": { "cta": "Try it", diff --git a/posthog/test/released_flags_scenarios.py b/posthog/test/released_flags_scenarios.py index b3cacdac1..a1fccb10c 100644 --- a/posthog/test/released_flags_scenarios.py +++ b/posthog/test/released_flags_scenarios.py @@ -76,7 +76,8 @@ def _flag(key: str, value: Any, **fields: Any) -> Dict[str, Any]: _flag("v2-number", 12.5, config_version=2), _flag("v2-object", {"layout": "compact", "n": [1, True]}, config_version=2), _flag("sparse", "compact", metadata={}, has_experiment=None, reason=None), - _flag("server-failed", "compact", failed=True), + # A failed record that still carries a payload. + _flag("server-failed", "compact", failed=True, payload='{"columns": 2}'), ] diff --git a/posthog/test/test_flags_released_behaviour.py b/posthog/test/test_flags_released_behaviour.py index 7ad8022f8..3141dd891 100644 --- a/posthog/test/test_flags_released_behaviour.py +++ b/posthog/test/test_flags_released_behaviour.py @@ -38,6 +38,26 @@ def test_every_legacy_getter_and_event_matches_7_82_1(results): assert (sparse["id"], sparse["version"]) == (0, 0) sparse["id"] = sparse["version"] = None + # A failed record reads as disabled, so it has no payload, even when the record + # carries one. 7.82.1 returned that payload from the payload reads and sent it on + # the flag's $feature_flag_called events. + failed = expected["get_flags_decision"]["flags"]["server-failed"]["metadata"] + assert failed["payload"] == '{"columns": 2}' + failed["payload"] = None + for read in ["get_feature_flag_payload", "snapshot.get_flag_payload"]: + assert expected[f"{read}:server-failed"] == {"columns": 2} + expected[f"{read}:server-failed"] = None + assert expected["get_feature_flag_result:server-failed"][2] == {"columns": 2} + expected["get_feature_flag_result:server-failed"][2] = None + failed_events = [ + properties + for _, properties in expected["events"] + if properties.get("$feature_flag") == "server-failed" + ] + assert failed_events + for properties in failed_events: + assert properties.pop("$feature_flag_payload") == {"columns": 2} + assert results["getters"] == expected @@ -71,5 +91,14 @@ def test_fallback_cache_histories_match_7_82_1(results, name): # An unreadable record now fails only itself, but a single-flag read of it # still reports what 7.82.1 reported when the whole response failed: an # unknown_error, no request details, and the cached value. - expected = RELEASED["cache histories"][name] + expected = copy.deepcopy(RELEASED["cache histories"][name]) + if name == "good, server failed, timeout": + # A failed record has no payload, even when the record carries one, so + # neither its read nor the cached result read after the timeout returns + # one, and their events leave it out. 7.82.1 returned it from both. + for result in expected["results"][1:]: + assert result[2] == {"theme": "dark"} + result[2] = None + for _, properties in expected["events"][1:]: + assert properties.pop("$feature_flag_payload") == {"theme": "dark"} assert results["cache histories"][name] == expected diff --git a/posthog/test/test_flags_v4_response.py b/posthog/test/test_flags_v4_response.py index 662011dce..31e3a4179 100644 --- a/posthog/test/test_flags_v4_response.py +++ b/posthog/test/test_flags_v4_response.py @@ -746,6 +746,58 @@ def test_a_config_version_1_payload_is_kept(): assert flags["string-flag"].metadata.payload == '{"columns": 2}' +def _failed_with_payload(shape: str) -> Dict[str, Any]: + """A failed record that still carries a payload, in the given response shape.""" + if shape == "v4": + # A producer-valid failed config version 1 record with a payload. + [case] = [c for c in READERS["cases"] if c["id"] == "readers.v1_failed"] + response = _build(READERS, case) + record = response["flags"]["garden-layout"] + assert record["failed"] is True + assert record["metadata"]["payload"] == '{"columns":2}' + return response + record = _older_record(enabled=False, variant=None, failed=True) + record["metadata"]["payload"] = '{"columns":2}' + return {"flags": {"garden-layout": record}, "requestId": "r1"} + + +@pytest.mark.parametrize("shape", ["v4", "older"]) +def test_a_failed_record_has_no_payload_on_any_read(shape, client): + # A failed flag reads as disabled, so a payload on it would surprise the caller. + response = _failed_with_payload(shape) + [flag] = normalize_flags_response(copy.deepcopy(response))["flags"].values() + assert flag._failed is True + assert flag.metadata.payload is None + + with mock.patch("posthog.client.flags", return_value=copy.deepcopy(response)): + result = client.get_feature_flag_result("garden-layout", DISTINCT_ID) + assert result is not None and result.payload is None + assert client.get_feature_flag_payload("garden-layout", DISTINCT_ID) is None + assert ( + client.get_feature_flag_payload( + "garden-layout", "user-2", send_feature_flag_events=True + ) + is None + ) + payloads = client.get_all_flags_and_payloads(DISTINCT_ID)["featureFlagPayloads"] + assert "garden-layout" not in (payloads or {}) + assert "garden-layout" not in (client.get_feature_payloads(DISTINCT_ID) or {}) + snapshot = client.evaluate_flags(DISTINCT_ID) + assert snapshot.get_flag_payload("garden-layout") is None + + +async def test_a_failed_record_has_no_payload_on_the_async_client(): + with mock.patch( + "posthog.async_client._async_flags", + new=mock.AsyncMock(return_value=_failed_with_payload("v4")), + ): + client = AsyncPosthog("project-key", send=False) + snapshot = await client.evaluate_flags(DISTINCT_ID) + await client.shutdown() + + assert snapshot.get_flag_payload("garden-layout") is None + + def test_feature_flag_value_defaults_to_the_legacy_value(): assert FeatureFlag.from_value_and_payload("a", "control", None)._value == "control" assert FeatureFlag.from_value_and_payload("a", True, None)._value is True diff --git a/posthog/types.py b/posthog/types.py index 85938a939..370044f84 100644 --- a/posthog/types.py +++ b/posthog/types.py @@ -389,13 +389,15 @@ def _parse_typed_record(cls: Type[_F], key: str, resp: Any) -> _F: if raw_metadata is None: metadata = LegacyFlagMetadata(payload=encoded_payload) else: + # A number or object value is its own payload; a payload the record also + # carries is ignored. A failed record reads as disabled, so it has no + # payload, even when the record carries one. + payload = encoded_payload + if payload is None and not failed: + payload = raw_metadata.get("payload") metadata = FlagMetadata( id=_read_int(raw_metadata.get("id"), "metadata.id"), - # A number or object value is its own payload; a payload the record - # also carries is ignored. - payload=encoded_payload - if encoded_payload is not None - else raw_metadata.get("payload"), + payload=payload, version=_read_int(raw_metadata.get("version"), "metadata.version"), description=_read_typed( raw_metadata.get("description", ""), "metadata.description", str @@ -469,6 +471,10 @@ def _parse_older_record(cls: Type[_F], key: str, resp: Dict[str, Any]) -> _F: except _MalformedRecord as error: return _malformed_record(cls, key, str(error)) + failed = resp.get("failed") is True + if failed and raw_metadata: + # A failed record has no payload, even when the record carries one. + raw_metadata = {**raw_metadata, "payload": None} flag = cls( key=key, # An older server always sends ``enabled``; a record without it keeps @@ -478,7 +484,7 @@ def _parse_older_record(cls: Type[_F], key: str, resp: Dict[str, Any]) -> _F: reason=FlagReason.from_json(raw_reason), metadata=FlagMetadata.from_json(raw_metadata), ) - return _set_private_fields(flag, _failed=resp.get("failed") is True) + return _set_private_fields(flag, _failed=failed) def _read_record_key(resp: Dict[str, Any]) -> None: From 2fb783e35f90eb7262f07841f24f8015a34121af Mon Sep 17 00:00:00 2001 From: Anders Asheim Hennum Date: Thu, 5 Nov 2026 18:48:12 +0100 Subject: [PATCH 2/2] test(flags): a disabled record that is not failed keeps its payload Co-Authored-By: Claude Opus 5.5 --- posthog/test/test_flags_v4_response.py | 51 ++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/posthog/test/test_flags_v4_response.py b/posthog/test/test_flags_v4_response.py index 31e3a4179..30e22a7fb 100644 --- a/posthog/test/test_flags_v4_response.py +++ b/posthog/test/test_flags_v4_response.py @@ -786,6 +786,57 @@ def test_a_failed_record_has_no_payload_on_any_read(shape, client): assert snapshot.get_flag_payload("garden-layout") is None +def _disabled_with_payload(shape: str, failed: Any) -> Dict[str, Any]: + """A disabled record that is not failed and carries a payload. + + ``failed`` is the value of the record's ``failed`` field, or ``_ABSENT`` to + leave the field out. + """ + if shape == "v4": + [case] = [c for c in READERS["cases"] if c["id"] == "readers.v1_at_v4"] + response = _build(READERS, case) + record = response["flags"]["garden-layout"] + record["value"] = False + record["reason"] = {"code": "no_condition_match", "condition_index": None} + del record["metadata"]["variant_key"] + assert record["metadata"]["payload"] == '{"columns":2}' + else: + record = _older_record(enabled=False, variant=None) + record["metadata"]["payload"] = '{"columns":2}' + response = {"flags": {"garden-layout": record}, "requestId": "r1"} + if failed is _ABSENT: + record.pop("failed", None) + else: + record["failed"] = failed + return response + + +_ABSENT = object() + + +# A null ``failed`` is a null optional field, which both parsers ignore, so it is a +# valid control in both shapes. +@pytest.mark.parametrize( + "failed", [False, None, _ABSENT], ids=["false", "null", "absent"] +) +@pytest.mark.parametrize("shape", ["v4", "older"]) +def test_a_disabled_record_that_is_not_failed_keeps_its_payload(shape, failed, client): + # The control for the test above: only a failed record loses its payload. + response = _disabled_with_payload(shape, failed) + [flag] = normalize_flags_response(copy.deepcopy(response))["flags"].values() + assert flag._failed is False + assert flag.metadata.payload == '{"columns":2}' + + with mock.patch("posthog.client.flags", return_value=copy.deepcopy(response)): + result = client.get_feature_flag_result("garden-layout", DISTINCT_ID) + assert result is not None and result.payload == {"columns": 2} + assert client.get_feature_flag_payload("garden-layout", DISTINCT_ID) == { + "columns": 2 + } + snapshot = client.evaluate_flags(DISTINCT_ID) + assert snapshot.get_flag_payload("garden-layout") == {"columns": 2} + + async def test_a_failed_record_has_no_payload_on_the_async_client(): with mock.patch( "posthog.async_client._async_flags",