fix(flags): preserve typed Redis fallback results (#812)

* fix(flags): preserve typed Redis fallback results

* fix: keep Redis result type import private

Author: marandaneto

.sampo/changesets/pompous-duke-vainamoinen.md

@@ -0,0 +1,5 @@
+---
+pypi/posthog: patch
+---
+
+Preserve typed feature flag results in Redis fallback

posthog/client.py

@@ -2579,7 +2579,7 @@ def _get_stale_flag_fallback(
         """Returns a stale cached flag value if available, otherwise None."""
         if self.flag_cache:
             stale_result = self.flag_cache.get_stale_cached_flag(distinct_id, key)
-            if stale_result:
+            if isinstance(stale_result, FeatureFlagResult):
                 self.log.info(
                     f"[FEATURE FLAGS] Using stale cached value for flag {key}"
                 )

posthog/test/test_client.py

@@ -18,7 +18,7 @@
 from posthog.request import APIError, GetResponse
 from posthog.test.logging_helpers import capture_message_only_logs
 from posthog.test.test_utils import FAKE_TEST_API_KEY
-from posthog.types import FeatureFlag, LegacyFlagMetadata
+from posthog.types import FeatureFlag, FeatureFlagResult, LegacyFlagMetadata
 from posthog.version import VERSION
 from posthog.contexts import tag
 
@@ -198,7 +198,9 @@ def test_duplicate_client_warning_allows_intentional_multi_client_cases(
 
     def test_message_only_info_logs_include_posthog_prefix(self):
         self.client.flag_cache = mock.Mock()
-        self.client.flag_cache.get_stale_cached_flag.return_value = mock.Mock()
+        self.client.flag_cache.get_stale_cached_flag.return_value = FeatureFlagResult(
+            key="flag-key", enabled=True, variant=None, payload=None, reason=None
+        )
 
         with capture_message_only_logs(level=logging.INFO) as logs:
             self.client._get_stale_flag_fallback("distinct_id", "flag-key")

posthog/test/test_feature_flag_result.py

@@ -2,14 +2,15 @@
 from unittest import mock
 
 from posthog.client import Client
-from posthog.test.test_utils import FAKE_TEST_API_KEY
+from posthog.test.test_utils import FAKE_TEST_API_KEY, FakeRedis
 from posthog.types import (
     FeatureFlag,
     FeatureFlagError,
     FeatureFlagResult,
     FlagMetadata,
     FlagReason,
 )
+from posthog.utils import RedisFlagCache
 
 
 class TestFeatureFlagResult(unittest.TestCase):
@@ -740,6 +741,103 @@ def _populate_stale_cache(self, distinct_id, flag_key, flag_result):
             flag_definition_version=self.client.flag_definition_version,
         )
 
+    @mock.patch("posthog.client.flags")
+    @mock.patch.object(Client, "capture")
+    def test_redis_timeout_returns_stale_cached_value_and_event(
+        self, patch_capture, patch_flags
+    ):
+        """Redis stale fallback reconstructs the result used by flag-called events."""
+        from posthog.request import RequestsTimeout
+
+        self.client.flag_cache = RedisFlagCache(FakeRedis())
+        cached_result = FeatureFlagResult(
+            key="my-flag",
+            enabled=True,
+            variant="cached-variant",
+            payload={"from": "redis"},
+            reason="cached reason",
+        )
+        self._populate_stale_cache("some-distinct-id", "my-flag", cached_result)
+        patch_flags.side_effect = RequestsTimeout("Request timed out")
+
+        flag_result = self.client.get_feature_flag_result("my-flag", "some-distinct-id")
+
+        self.assertEqual(flag_result, cached_result)
+        patch_capture.assert_called_once_with(
+            "$feature_flag_called",
+            distinct_id="some-distinct-id",
+            properties={
+                "$feature_flag": "my-flag",
+                "$feature_flag_response": "cached-variant",
+                "locally_evaluated": False,
+                "$feature/my-flag": "cached-variant",
+                "$feature_flag_payload": {"from": "redis"},
+                "$feature_flag_error": FeatureFlagError.TIMEOUT,
+            },
+            groups={},
+            disable_geoip=None,
+        )
+
+    @mock.patch("posthog.client.flags")
+    @mock.patch.object(Client, "capture")
+    def test_redis_timeout_returns_stale_cached_value_without_event(
+        self, patch_capture, patch_flags
+    ):
+        """Redis stale fallback is reconstructed when event capture is disabled."""
+        from posthog.request import RequestsTimeout
+
+        self.client.flag_cache = RedisFlagCache(FakeRedis())
+        cached_result = FeatureFlagResult(
+            key="my-flag",
+            enabled=False,
+            variant=None,
+            payload={"from": "redis"},
+            reason="cached reason",
+        )
+        self._populate_stale_cache("some-distinct-id", "my-flag", cached_result)
+        patch_flags.side_effect = RequestsTimeout("Request timed out")
+
+        flag_result = self.client.get_feature_flag_result(
+            "my-flag", "some-distinct-id", send_feature_flag_events=False
+        )
+
+        self.assertEqual(flag_result, cached_result)
+        patch_capture.assert_not_called()
+
+    @mock.patch("posthog.client.flags")
+    @mock.patch.object(Client, "capture")
+    def test_legacy_redis_entry_is_safely_ignored(self, patch_capture, patch_flags):
+        """Unmarked entries from older SDKs are not returned as result dictionaries."""
+        import json
+        import time
+
+        from posthog.request import RequestsTimeout
+
+        redis = FakeRedis()
+        self.client.flag_cache = RedisFlagCache(redis)
+        cache_key = self.client.flag_cache._get_cache_key("some-distinct-id", "my-flag")
+        redis.store[cache_key] = json.dumps(
+            {
+                "flag_result": {
+                    "key": "my-flag",
+                    "enabled": True,
+                    "variant": None,
+                    "payload": {"from": "legacy"},
+                    "reason": None,
+                },
+                "flag_version": self.client.flag_definition_version,
+                "timestamp": time.time(),
+            }
+        )
+        patch_flags.side_effect = RequestsTimeout("Request timed out")
+
+        flag_result = self.client.get_feature_flag_result(
+            "my-flag", "some-distinct-id", send_feature_flag_events=False
+        )
+
+        self.assertIsNone(flag_result)
+        patch_capture.assert_not_called()
+
     @mock.patch("posthog.client.flags")
     @mock.patch.object(Client, "capture")
     def test_timeout_error_returns_stale_cached_value(self, patch_capture, patch_flags):

posthog/test/test_utils.py

@@ -766,6 +766,86 @@ def test_cache_key_and_serialization(self):
         assert self.cache._deserialize_entry("not json") is None
         assert self.cache._deserialize_entry(json.dumps({"flag_result": True})) is None
 
+    def test_feature_flag_result_round_trip(self):
+        flag_result = FeatureFlagResult(
+            key="checkout-redesign",
+            enabled=True,
+            variant="test",
+            payload={"discount": Decimal("12.5")},
+            reason="matched rollout condition",
+        )
+
+        serialized = self.cache._serialize_entry(flag_result, 3, timestamp=123)
+
+        assert json.loads(serialized) == {
+            "flag_result": {
+                "key": "checkout-redesign",
+                "enabled": True,
+                "variant": "test",
+                "payload": {"discount": 12.5},
+                "reason": "matched rollout condition",
+            },
+            "flag_version": 3,
+            "timestamp": 123,
+            "flag_result_type": "FeatureFlagResult",
+            "flag_result_schema_version": 1,
+        }
+        entry = self.cache._deserialize_entry(serialized)
+        assert entry is not None
+        assert entry.flag_result == FeatureFlagResult(
+            key="checkout-redesign",
+            enabled=True,
+            variant="test",
+            payload={"discount": 12.5},
+            reason="matched rollout condition",
+        )
+
+    @parameterized.expand(
+        [
+            ("boolean", True),
+            ("variant", "test"),
+            ("dictionary", {"enabled": True, "payload": {"plan": "pro"}}),
+        ]
+    )
+    def test_untyped_flag_result_round_trip(self, _name, flag_result):
+        serialized = self.cache._serialize_entry(flag_result, 3, timestamp=123)
+
+        assert "flag_result_type" not in json.loads(serialized)
+        entry = self.cache._deserialize_entry(serialized)
+        assert entry is not None
+        assert entry.flag_result == flag_result
+
+    def test_legacy_unmarked_feature_flag_result_remains_a_dict(self):
+        legacy_result = {
+            "key": "checkout-redesign",
+            "enabled": True,
+            "variant": None,
+            "payload": {"plan": "pro"},
+            "reason": "legacy entry",
+        }
+        serialized = json.dumps(
+            {"flag_result": legacy_result, "flag_version": 3, "timestamp": 123}
+        )
+
+        entry = self.cache._deserialize_entry(serialized)
+
+        assert entry is not None
+        assert entry.flag_result == legacy_result
+        assert isinstance(entry.flag_result, dict)
+
+    def test_unknown_feature_flag_result_schema_is_a_cache_miss(self):
+        serialized = json.dumps(
+            {
+                "flag_result": {},
+                "flag_version": 3,
+                "timestamp": 123,
+                "flag_result_type": "FeatureFlagResult",
+                "flag_result_schema_version": 2,
+            }
+        )
+
+        assert self.cache._deserialize_entry(serialized) is None
+
     def test_get_set_and_stale_cached_flags(self):
         self.cache.set_cached_flag("user123", "beta", True, 7)
 

posthog/utils.py

@@ -13,6 +13,8 @@
 import platform
 import distro  # For Linux OS detection
 
+from .types import FeatureFlagResult as _FeatureFlagResult
+
 log = logging.getLogger("posthog")
 
 
@@ -165,6 +167,8 @@ def __setitem__(self, key, value):
 CACHE_TTL = 300
 CACHE_STALE_TTL = 3600
 CACHE_KEY_PREFIX = "posthog:flags:"
+_FEATURE_FLAG_RESULT_TYPE = "FeatureFlagResult"
+_FEATURE_FLAG_RESULT_SCHEMA_VERSION = 1
 
 
 class FlagCacheEntry:
@@ -320,18 +324,29 @@ def _serialize_entry(self, flag_result, flag_definition_version, timestamp=None)
             "flag_version": flag_definition_version,
             "timestamp": timestamp,
         }
+        if isinstance(flag_result, _FeatureFlagResult):
+            # Additive metadata keeps the existing entry shape readable by older SDKs.
+            entry["flag_result_type"] = _FEATURE_FLAG_RESULT_TYPE
+            entry["flag_result_schema_version"] = _FEATURE_FLAG_RESULT_SCHEMA_VERSION
         return json.dumps(entry)
 
     def _deserialize_entry(self, data):
         try:
             entry = json.loads(data)
             flag_result = entry["flag_result"]
+            if entry.get("flag_result_type") == _FEATURE_FLAG_RESULT_TYPE:
+                if (
+                    entry.get("flag_result_schema_version")
+                    != _FEATURE_FLAG_RESULT_SCHEMA_VERSION
+                ):
+                    return None
+                flag_result = _FeatureFlagResult(**flag_result)
             return FlagCacheEntry(
                 flag_result=flag_result,
                 flag_definition_version=entry["flag_version"],
                 timestamp=entry["timestamp"],
             )
-        except (json.JSONDecodeError, KeyError, ValueError):
+        except (json.JSONDecodeError, KeyError, TypeError, ValueError):
             # If deserialization fails, treat as cache miss
             return None