fix: Return None for malformed feature flag payloads

.sampo/changesets/steadfast-baroness-vellamo.md

@@ -0,0 +1,5 @@
+---
+pypi/posthog: patch
+---
+
+Return None for malformed JSON feature flag payloads instead of returning the raw string or raising a JSONDecodeError.

posthog/client.py

@@ -1,6 +1,5 @@
 import atexit
 import inspect
-import json
 import logging
 import os
 import sys
@@ -93,7 +92,8 @@
     remote_config,
     reset_sessions,
 )
-from posthog.types import (
+from .types import (
+    _parse_flag_payload,
     FeatureFlag,
     FeatureFlagError,
     FeatureFlagResult,
@@ -326,20 +326,6 @@ def _parse_has_experiment(value: Any) -> Optional[bool]:
     return value if isinstance(value, bool) else None
 
 
-def _parse_flag_payload(raw_payload: Any) -> Optional[Any]:
-    """Flag payloads are stored as JSON strings, both in the ``/flags`` response
-    metadata and in the local-evaluation flag definitions, so decode them before
-    handing them to callers. A string that isn't valid JSON is passed through as-is."""
-    if isinstance(raw_payload, str):
-        if not raw_payload:
-            return None
-        try:
-            return json.loads(raw_payload)
-        except (json.JSONDecodeError, TypeError):
-            return raw_payload
-    return raw_payload
-
-
 def _metadata_has_experiment(metadata: Any) -> Optional[bool]:
     """Server-reported experiment linkage from flag metadata; ``None`` when absent
     (e.g. ``LegacyFlagMetadata``, which doesn't carry the field)."""

posthog/test/test_evaluate_flags.py

@@ -404,10 +404,10 @@ def test_local_payloads_are_parsed(self, patch_flags):
         self.assertEqual(patch_flags.call_count, 0)
 
     @mock.patch("posthog.client.flags")
-    def test_non_json_local_payload_is_passed_through(self, patch_flags):
+    def test_non_json_local_payload_returns_none(self, patch_flags):
         flags = self.client.evaluate_flags("user-1")
 
-        self.assertEqual(flags.get_flag_payload("plain-payload"), "not json")
+        self.assertIsNone(flags.get_flag_payload("plain-payload"))
         self.assertEqual(patch_flags.call_count, 0)
 
     @mock.patch("posthog.client.flags")

posthog/test/test_flag_payload_parsing.py

@@ -0,0 +1,69 @@
+from unittest.mock import patch
+
+import pytest
+
+from posthog.client import Client
+
+
+@pytest.mark.parametrize("local", [True, False])
+@pytest.mark.parametrize("legacy", [True, False])
+@pytest.mark.parametrize(
+    "raw, expected",
+    [
+        ('{"broken":', None),
+        ("not json", None),
+        ("   ", None),
+        ("", None),
+        ("[1, 2]", [1, 2]),
+        ('{"ok": true}', {"ok": True}),
+        ('"text"', "text"),
+        ("false", False),
+        ("0", 0),
+        ("null", None),
+        ({"decoded": True}, {"decoded": True}),
+        (None, None),
+    ],
+)
+def test_payload_parsing(local, legacy, raw, expected):
+    client = Client("test-key", send=False)
+    if local:
+        client.feature_flags = [
+            {
+                "id": 1,
+                "key": "test-flag",
+                "active": True,
+                "filters": {
+                    "groups": [{"properties": [], "rollout_percentage": 100}],
+                    "payloads": {"true": raw},
+                },
+            }
+        ]
+    response = {
+        "flags": {
+            "test-flag": {
+                "key": "test-flag",
+                "enabled": True,
+                "variant": None,
+                "reason": {"code": "condition_match", "description": "Matched"},
+                "metadata": {"id": 1, "version": 1, "payload": raw},
+            }
+        }
+    }
+    try:
+        with (
+            patch.object(client, "load_feature_flags"),
+            patch("posthog.client.flags", return_value=response) as request,
+        ):
+            if legacy:
+                with pytest.warns(DeprecationWarning):
+                    result = client.get_feature_flag_payload(
+                        "test-flag", "user", only_evaluate_locally=local
+                    )
+            else:
+                result = client.evaluate_flags(
+                    "user", only_evaluate_locally=local
+                ).get_flag_payload("test-flag")
+            assert result == expected
+            assert request.call_count == (0 if local else 1)
+    finally:
+        client.shutdown()

posthog/types.py

@@ -4,6 +4,16 @@
 
 FlagValue = Union[bool, str]
 
+
+def _parse_flag_payload(raw_payload: Any) -> Optional[Any]:
+    if isinstance(raw_payload, str):
+        try:
+            return json.loads(raw_payload)
+        except json.JSONDecodeError:
+            return None
+    return raw_payload
+
+
 # Type alias for the before_send callback function
 # Takes an event dictionary and returns the modified event or None to drop it
 BeforeSendCallback = Callable[[dict[str, Any]], Optional[dict[str, Any]]]
@@ -231,9 +241,7 @@ def from_value_and_payload(
             key=key,
             enabled=enabled,
             variant=variant,
-            payload=json.loads(payload)
-            if isinstance(payload, str) and payload
-            else payload,
+            payload=_parse_flag_payload(payload),
             reason=None,
         )
 
@@ -271,12 +279,7 @@ def from_flag_details(
             key=details.key,
             enabled=enabled,
             variant=variant,
-            payload=(
-                json.loads(details.metadata.payload)
-                if isinstance(details.metadata.payload, str)
-                and details.metadata.payload
-                else details.metadata.payload
-            ),
+            payload=_parse_flag_payload(details.metadata.payload),
             reason=details.reason.description if details.reason else None,
         )