From 4adb5b8088af55e029714b13d3e246483aace446 Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Mon, 15 Dec 2025 13:12:19 -0800 Subject: [PATCH 1/8] Add $feature_flag_error property to track flag evaluation failures Track errors in feature flag evaluation by adding a `$feature_flag_error` property to the `$feature_flag_called` event. --- posthog/client.py | 36 +++- posthog/test/test_feature_flag_result.py | 214 +++++++++++++++++++++++ 2 files changed, 246 insertions(+), 4 deletions(-) diff --git a/posthog/client.py b/posthog/client.py index 426bb24c..ade5523f 100644 --- a/posthog/client.py +++ b/posthog/client.py @@ -36,12 +36,14 @@ from posthog.request import ( DEFAULT_HOST, APIError, + QuotaLimitError, batch_post, determine_server_host, flags, get, remote_config, ) +import requests.exceptions from posthog.contexts import ( _get_current_context, get_context_distinct_id, @@ -1539,6 +1541,7 @@ def _get_feature_flag_result( flag_details = None request_id = None evaluated_at = None + feature_flag_error: Optional[str] = None flag_value = self._locally_evaluate_flag( key, distinct_id, groups, person_properties, group_properties @@ -1563,7 +1566,7 @@ def _get_feature_flag_result( ) elif not only_evaluate_locally: try: - flag_details, request_id, evaluated_at = ( + flag_details, request_id, evaluated_at, errors_while_computing = ( self._get_feature_flag_details_from_server( key, distinct_id, @@ -1573,6 +1576,11 @@ def _get_feature_flag_result( disable_geoip, ) ) + if errors_while_computing: + feature_flag_error = "errors_while_computing_flags" + elif flag_details is None: + feature_flag_error = "flag_missing" + flag_result = FeatureFlagResult.from_flag_details( flag_details, override_match_value ) @@ -1586,9 +1594,23 @@ def _get_feature_flag_result( self.log.debug( f"Successfully computed flag remotely: #{key} -> #{flag_result}" ) + except QuotaLimitError as e: + self.log.exception(f"[FEATURE FLAGS] Quota limit exceeded: {e}") + feature_flag_error = "quota_limited" + except requests.exceptions.Timeout as e: + self.log.exception(f"[FEATURE FLAGS] Request timed out: {e}") + feature_flag_error = "timeout" + except requests.exceptions.ConnectionError as e: + self.log.exception(f"[FEATURE FLAGS] Connection error: {e}") + feature_flag_error = "connection_error" + except APIError as e: + self.log.exception(f"[FEATURE FLAGS] API error: {e}") + feature_flag_error = f"api_error_{e.status}" except Exception as e: self.log.exception(f"[FEATURE FLAGS] Unable to get flag remotely: {e}") + feature_flag_error = "unknown_error" + if feature_flag_error: # Fallback to cached value if remote evaluation fails if self.flag_cache: stale_result = self.flag_cache.get_stale_cached_flag( @@ -1612,6 +1634,7 @@ def _get_feature_flag_result( request_id, evaluated_at, flag_details, + feature_flag_error, ) return flag_result @@ -1814,9 +1837,10 @@ def _get_feature_flag_details_from_server( person_properties: dict[str, str], group_properties: dict[str, str], disable_geoip: Optional[bool], - ) -> tuple[Optional[FeatureFlag], Optional[str], Optional[int]]: + ) -> tuple[Optional[FeatureFlag], Optional[str], Optional[int], bool]: """ - Calls /flags and returns the flag details, request id, and evaluated at timestamp + Calls /flags and returns the flag details, request id, evaluated at timestamp, + and whether there were errors while computing flags. """ resp_data = self.get_flags_decision( distinct_id, @@ -1828,9 +1852,10 @@ def _get_feature_flag_details_from_server( ) request_id = resp_data.get("requestId") evaluated_at = resp_data.get("evaluatedAt") + errors_while_computing = resp_data.get("errorsWhileComputingFlags", False) flags = resp_data.get("flags") flag_details = flags.get(key) if flags else None - return flag_details, request_id, evaluated_at + return flag_details, request_id, evaluated_at, errors_while_computing def _capture_feature_flag_called( self, @@ -1844,6 +1869,7 @@ def _capture_feature_flag_called( request_id: Optional[str], evaluated_at: Optional[int], flag_details: Optional[FeatureFlag], + feature_flag_error: Optional[str] = None, ): feature_flag_reported_key = ( f"{key}_{'::null::' if response is None else str(response)}" @@ -1878,6 +1904,8 @@ def _capture_feature_flag_called( ) if flag_details.metadata.id: properties["$feature_flag_id"] = flag_details.metadata.id + if feature_flag_error: + properties["$feature_flag_error"] = feature_flag_error self.capture( "$feature_flag_called", diff --git a/posthog/test/test_feature_flag_result.py b/posthog/test/test_feature_flag_result.py index d1e9b464..2652b6e3 100644 --- a/posthog/test/test_feature_flag_result.py +++ b/posthog/test/test_feature_flag_result.py @@ -438,6 +438,220 @@ def test_get_feature_flag_result_unknown_flag(self, patch_capture, patch_flags): "$feature_flag_response": None, "locally_evaluated": False, "$feature/no-person-flag": None, + "$feature_flag_error": "flag_missing", + }, + groups={}, + disable_geoip=None, + ) + + @mock.patch("posthog.client.flags") + @mock.patch.object(Client, "capture") + def test_get_feature_flag_result_with_errors_while_computing_flags( + self, patch_capture, patch_flags + ): + """Test that errors_while_computing_flags is included in the $feature_flag_called event. + + When the server returns errorsWhileComputingFlags=true, it indicates that there + was an error computing one or more flags. We include this in the event so users + can identify and debug flag evaluation issues. + """ + patch_flags.return_value = { + "flags": { + "my-flag": { + "key": "my-flag", + "enabled": True, + "variant": None, + "reason": {"description": "Matched condition set 1"}, + "metadata": {"id": 1, "version": 1, "payload": None}, + }, + }, + "requestId": "test-request-id-789", + "errorsWhileComputingFlags": True, + } + + flag_result = self.client.get_feature_flag_result("my-flag", "some-distinct-id") + + self.assertEqual(flag_result.enabled, True) + patch_capture.assert_called_with( + "$feature_flag_called", + distinct_id="some-distinct-id", + properties={ + "$feature_flag": "my-flag", + "$feature_flag_response": True, + "locally_evaluated": False, + "$feature/my-flag": True, + "$feature_flag_request_id": "test-request-id-789", + "$feature_flag_reason": "Matched condition set 1", + "$feature_flag_id": 1, + "$feature_flag_version": 1, + "$feature_flag_error": "errors_while_computing_flags", + }, + groups={}, + disable_geoip=None, + ) + + @mock.patch("posthog.client.flags") + @mock.patch.object(Client, "capture") + def test_get_feature_flag_result_flag_not_in_response( + self, patch_capture, patch_flags + ): + """Test that when a flag is not in the API response, we capture flag_missing error. + + This happens when a flag doesn't exist or the user doesn't match any conditions. + """ + patch_flags.return_value = { + "flags": { + "other-flag": { + "key": "other-flag", + "enabled": True, + "variant": None, + "reason": {"description": "Matched condition set 1"}, + "metadata": {"id": 1, "version": 1, "payload": None}, + }, + }, + "requestId": "test-request-id-456", + } + + flag_result = self.client.get_feature_flag_result( + "missing-flag", "some-distinct-id" + ) + + self.assertIsNone(flag_result) + patch_capture.assert_called_with( + "$feature_flag_called", + distinct_id="some-distinct-id", + properties={ + "$feature_flag": "missing-flag", + "$feature_flag_response": None, + "locally_evaluated": False, + "$feature/missing-flag": None, + "$feature_flag_request_id": "test-request-id-456", + "$feature_flag_error": "flag_missing", + }, + groups={}, + disable_geoip=None, + ) + + @mock.patch("posthog.client.flags") + @mock.patch.object(Client, "capture") + def test_get_feature_flag_result_unknown_error(self, patch_capture, patch_flags): + """Test that unexpected exceptions are captured as unknown_error.""" + patch_flags.side_effect = Exception("Unexpected error") + + flag_result = self.client.get_feature_flag_result("my-flag", "some-distinct-id") + + self.assertIsNone(flag_result) + patch_capture.assert_called_with( + "$feature_flag_called", + distinct_id="some-distinct-id", + properties={ + "$feature_flag": "my-flag", + "$feature_flag_response": None, + "locally_evaluated": False, + "$feature/my-flag": None, + "$feature_flag_error": "unknown_error", + }, + groups={}, + disable_geoip=None, + ) + + @mock.patch("posthog.client.flags") + @mock.patch.object(Client, "capture") + def test_get_feature_flag_result_timeout_error(self, patch_capture, patch_flags): + """Test that timeout errors are captured specifically.""" + import requests.exceptions + + patch_flags.side_effect = requests.exceptions.Timeout("Request timed out") + + flag_result = self.client.get_feature_flag_result("my-flag", "some-distinct-id") + + self.assertIsNone(flag_result) + patch_capture.assert_called_with( + "$feature_flag_called", + distinct_id="some-distinct-id", + properties={ + "$feature_flag": "my-flag", + "$feature_flag_response": None, + "locally_evaluated": False, + "$feature/my-flag": None, + "$feature_flag_error": "timeout", + }, + groups={}, + disable_geoip=None, + ) + + @mock.patch("posthog.client.flags") + @mock.patch.object(Client, "capture") + def test_get_feature_flag_result_connection_error(self, patch_capture, patch_flags): + """Test that connection errors are captured specifically.""" + import requests.exceptions + + patch_flags.side_effect = requests.exceptions.ConnectionError( + "Connection refused" + ) + + flag_result = self.client.get_feature_flag_result("my-flag", "some-distinct-id") + + self.assertIsNone(flag_result) + patch_capture.assert_called_with( + "$feature_flag_called", + distinct_id="some-distinct-id", + properties={ + "$feature_flag": "my-flag", + "$feature_flag_response": None, + "locally_evaluated": False, + "$feature/my-flag": None, + "$feature_flag_error": "connection_error", + }, + groups={}, + disable_geoip=None, + ) + + @mock.patch("posthog.client.flags") + @mock.patch.object(Client, "capture") + def test_get_feature_flag_result_api_error(self, patch_capture, patch_flags): + """Test that API errors include the status code.""" + from posthog.request import APIError + + patch_flags.side_effect = APIError(500, "Internal server error") + + flag_result = self.client.get_feature_flag_result("my-flag", "some-distinct-id") + + self.assertIsNone(flag_result) + patch_capture.assert_called_with( + "$feature_flag_called", + distinct_id="some-distinct-id", + properties={ + "$feature_flag": "my-flag", + "$feature_flag_response": None, + "locally_evaluated": False, + "$feature/my-flag": None, + "$feature_flag_error": "api_error_500", + }, + groups={}, + disable_geoip=None, + ) + + @mock.patch("posthog.client.flags") + @mock.patch.object(Client, "capture") + def test_get_feature_flag_result_quota_limited(self, patch_capture, patch_flags): + """Test that quota limit errors are captured specifically.""" + from posthog.request import QuotaLimitError + + patch_flags.side_effect = QuotaLimitError(429, "Rate limit exceeded") + + flag_result = self.client.get_feature_flag_result("my-flag", "some-distinct-id") + + self.assertIsNone(flag_result) + patch_capture.assert_called_with( + "$feature_flag_called", + distinct_id="some-distinct-id", + properties={ + "$feature_flag": "my-flag", + "$feature_flag_response": None, + "locally_evaluated": False, + "$feature/my-flag": None, + "$feature_flag_error": "quota_limited", }, groups={}, disable_geoip=None, From b4d67bf6dce276151dead60eabef83c3b04231fc Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Mon, 15 Dec 2025 14:23:34 -0800 Subject: [PATCH 2/8] Refactor requests exception imports through request.py Export RequestsTimeout and RequestsConnectionError from posthog/request.py to keep all requests library imports in one place and avoid mypy issues. --- posthog/client.py | 7 ++++--- posthog/request.py | 6 ++++++ posthog/test/test_feature_flag_result.py | 10 ++++------ 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/posthog/client.py b/posthog/client.py index ade5523f..d9b29ae1 100644 --- a/posthog/client.py +++ b/posthog/client.py @@ -37,13 +37,14 @@ DEFAULT_HOST, APIError, QuotaLimitError, + RequestsConnectionError, + RequestsTimeout, batch_post, determine_server_host, flags, get, remote_config, ) -import requests.exceptions from posthog.contexts import ( _get_current_context, get_context_distinct_id, @@ -1597,10 +1598,10 @@ def _get_feature_flag_result( except QuotaLimitError as e: self.log.exception(f"[FEATURE FLAGS] Quota limit exceeded: {e}") feature_flag_error = "quota_limited" - except requests.exceptions.Timeout as e: + except RequestsTimeout as e: self.log.exception(f"[FEATURE FLAGS] Request timed out: {e}") feature_flag_error = "timeout" - except requests.exceptions.ConnectionError as e: + except RequestsConnectionError as e: self.log.exception(f"[FEATURE FLAGS] Connection error: {e}") feature_flag_error = "connection_error" except APIError as e: diff --git a/posthog/request.py b/posthog/request.py index 7199f3a8..138d621d 100644 --- a/posthog/request.py +++ b/posthog/request.py @@ -312,6 +312,12 @@ class QuotaLimitError(APIError): pass +# Re-export requests exceptions for use in client.py +# This keeps all requests library imports centralized in this module +RequestsTimeout = requests.exceptions.Timeout +RequestsConnectionError = requests.exceptions.ConnectionError + + class DatetimeSerializer(json.JSONEncoder): def default(self, obj: Any): if isinstance(obj, (date, datetime)): diff --git a/posthog/test/test_feature_flag_result.py b/posthog/test/test_feature_flag_result.py index 2652b6e3..4d60ba2e 100644 --- a/posthog/test/test_feature_flag_result.py +++ b/posthog/test/test_feature_flag_result.py @@ -559,9 +559,9 @@ def test_get_feature_flag_result_unknown_error(self, patch_capture, patch_flags) @mock.patch.object(Client, "capture") def test_get_feature_flag_result_timeout_error(self, patch_capture, patch_flags): """Test that timeout errors are captured specifically.""" - import requests.exceptions + from posthog.request import RequestsTimeout - patch_flags.side_effect = requests.exceptions.Timeout("Request timed out") + patch_flags.side_effect = RequestsTimeout("Request timed out") flag_result = self.client.get_feature_flag_result("my-flag", "some-distinct-id") @@ -584,11 +584,9 @@ def test_get_feature_flag_result_timeout_error(self, patch_capture, patch_flags) @mock.patch.object(Client, "capture") def test_get_feature_flag_result_connection_error(self, patch_capture, patch_flags): """Test that connection errors are captured specifically.""" - import requests.exceptions + from posthog.request import RequestsConnectionError - patch_flags.side_effect = requests.exceptions.ConnectionError( - "Connection refused" - ) + patch_flags.side_effect = RequestsConnectionError("Connection refused") flag_result = self.client.get_feature_flag_result("my-flag", "some-distinct-id") From 244f26e3004a73fe5813d3a366bbee26c8e7ccd3 Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Mon, 15 Dec 2025 14:59:10 -0800 Subject: [PATCH 3/8] Address PR review feedback - Fix fallback logic to only trigger on actual exceptions, not when errors_while_computing or flag_missing is set from a successful API response - Change log.exception() to log.warning() for expected operational errors (quota limits, timeouts, connection errors, API errors) to reduce log noise - Keep log.exception() only for truly unexpected errors (unknown_error) - Extract stale cache fallback into _get_stale_flag_fallback() helper method --- posthog/client.py | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/posthog/client.py b/posthog/client.py index d9b29ae1..84ccbede 100644 --- a/posthog/client.py +++ b/posthog/client.py @@ -1509,6 +1509,19 @@ def feature_enabled( return None return bool(response) + def _get_stale_flag_fallback( + self, distinct_id: ID_TYPES, key: str + ) -> Optional[FeatureFlagResult]: + """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: + self.log.info( + f"[FEATURE FLAGS] Using stale cached value for flag {key}" + ) + return stale_result + return None + def _get_feature_flag_result( self, key: str, @@ -1596,32 +1609,25 @@ def _get_feature_flag_result( f"Successfully computed flag remotely: #{key} -> #{flag_result}" ) except QuotaLimitError as e: - self.log.exception(f"[FEATURE FLAGS] Quota limit exceeded: {e}") + self.log.warning(f"[FEATURE FLAGS] Quota limit exceeded: {e}") feature_flag_error = "quota_limited" + flag_result = self._get_stale_flag_fallback(distinct_id, key) except RequestsTimeout as e: - self.log.exception(f"[FEATURE FLAGS] Request timed out: {e}") + self.log.warning(f"[FEATURE FLAGS] Request timed out: {e}") feature_flag_error = "timeout" + flag_result = self._get_stale_flag_fallback(distinct_id, key) except RequestsConnectionError as e: - self.log.exception(f"[FEATURE FLAGS] Connection error: {e}") + self.log.warning(f"[FEATURE FLAGS] Connection error: {e}") feature_flag_error = "connection_error" + flag_result = self._get_stale_flag_fallback(distinct_id, key) except APIError as e: - self.log.exception(f"[FEATURE FLAGS] API error: {e}") + self.log.warning(f"[FEATURE FLAGS] API error: {e}") feature_flag_error = f"api_error_{e.status}" + flag_result = self._get_stale_flag_fallback(distinct_id, key) except Exception as e: self.log.exception(f"[FEATURE FLAGS] Unable to get flag remotely: {e}") feature_flag_error = "unknown_error" - - if feature_flag_error: - # Fallback to cached value if remote evaluation fails - if self.flag_cache: - stale_result = self.flag_cache.get_stale_cached_flag( - distinct_id, key - ) - if stale_result: - self.log.info( - f"[FEATURE FLAGS] Using stale cached value for flag {key}" - ) - flag_result = stale_result + flag_result = self._get_stale_flag_fallback(distinct_id, key) if send_feature_flag_events: self._capture_feature_flag_called( From d9f380dbc771b2fae7f7e9e92223d1b1bf2ea7b8 Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Mon, 15 Dec 2025 15:55:09 -0800 Subject: [PATCH 4/8] Add tests for stale cache fallback and error absence - Add TestFeatureFlagErrorWithStaleCacheFallback test class with 4 tests: - test_timeout_error_returns_stale_cached_value - test_connection_error_returns_stale_cached_value - test_api_error_returns_stale_cached_value - test_error_without_cache_returns_none - Add negative assertions to verify $feature_flag_error is absent on success: - test_get_feature_flag_result_boolean_local_evaluation - test_get_feature_flag_result_variant_local_evaluation - test_get_feature_flag_result_boolean_decide - test_get_feature_flag_result_variant_decide --- posthog/test/test_feature_flag_result.py | 187 +++++++++++++++++++++++ 1 file changed, 187 insertions(+) diff --git a/posthog/test/test_feature_flag_result.py b/posthog/test/test_feature_flag_result.py index 4d60ba2e..eab87c07 100644 --- a/posthog/test/test_feature_flag_result.py +++ b/posthog/test/test_feature_flag_result.py @@ -241,6 +241,9 @@ def test_get_feature_flag_result_boolean_local_evaluation(self, patch_capture): groups={}, disable_geoip=None, ) + # Verify error property is NOT present on successful evaluation + captured_properties = patch_capture.call_args[1]["properties"] + self.assertNotIn("$feature_flag_error", captured_properties) @mock.patch.object(Client, "capture") def test_get_feature_flag_result_variant_local_evaluation(self, patch_capture): @@ -295,6 +298,9 @@ def test_get_feature_flag_result_variant_local_evaluation(self, patch_capture): groups={}, disable_geoip=None, ) + # Verify error property is NOT present on successful evaluation + captured_properties = patch_capture.call_args[1]["properties"] + self.assertNotIn("$feature_flag_error", captured_properties) another_flag_result = self.client.get_feature_flag_result( "person-flag", "another-distinct-id", person_properties={"region": "USA"} @@ -360,6 +366,9 @@ def test_get_feature_flag_result_boolean_decide(self, patch_capture, patch_flags groups={}, disable_geoip=None, ) + # Verify error property is NOT present on successful evaluation + captured_properties = patch_capture.call_args[1]["properties"] + self.assertNotIn("$feature_flag_error", captured_properties) @mock.patch("posthog.client.flags") @mock.patch.object(Client, "capture") @@ -403,6 +412,9 @@ def test_get_feature_flag_result_variant_decide(self, patch_capture, patch_flags groups={}, disable_geoip=None, ) + # Verify error property is NOT present on successful evaluation + captured_properties = patch_capture.call_args[1]["properties"] + self.assertNotIn("$feature_flag_error", captured_properties) @mock.patch("posthog.client.flags") @mock.patch.object(Client, "capture") @@ -654,3 +666,178 @@ def test_get_feature_flag_result_quota_limited(self, patch_capture, patch_flags) groups={}, disable_geoip=None, ) + + +class TestFeatureFlagErrorWithStaleCacheFallback(unittest.TestCase): + """Tests for stale cache fallback behavior when flag evaluation fails. + + When the PostHog API is unavailable (timeout, connection error, etc.), the SDK + falls back to stale cached flag values if available. These tests verify that: + 1. The stale cached value is returned when an error occurs + 2. The $feature_flag_error property is still set (for debugging) + 3. The response reflects the cached value, not None + """ + + def set_fail(self, e, batch): + """Mark the failure handler""" + print("FAIL", e, batch) # noqa: T201 + self.failed = True + + def setUp(self): + self.failed = False + # Create client with memory-based flag cache enabled + self.client = Client( + FAKE_TEST_API_KEY, + on_error=self.set_fail, + flag_fallback_cache_url="memory://local/?ttl=300&size=10000", + ) + + def _populate_stale_cache(self, distinct_id, flag_key, flag_result): + """Pre-populate the flag cache with a value that will be used for stale fallback.""" + self.client.flag_cache.set_cached_flag( + 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_timeout_error_returns_stale_cached_value(self, patch_capture, patch_flags): + """Test that timeout errors return stale cached value when available.""" + from posthog.request import RequestsTimeout + + # Pre-populate cache with a flag result + cached_result = FeatureFlagResult.from_value_and_payload( + "my-flag", "cached-variant", '{"from": "cache"}' + ) + self._populate_stale_cache("some-distinct-id", "my-flag", cached_result) + + # Simulate timeout error + patch_flags.side_effect = RequestsTimeout("Request timed out") + + flag_result = self.client.get_feature_flag_result("my-flag", "some-distinct-id") + + # Should return the stale cached value + self.assertIsNotNone(flag_result) + self.assertEqual(flag_result.variant, "cached-variant") + self.assertEqual(flag_result.payload, {"from": "cache"}) + + # Error should still be tracked for debugging + patch_capture.assert_called_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": "cache"}, + "$feature_flag_error": "timeout", + }, + groups={}, + disable_geoip=None, + ) + + @mock.patch("posthog.client.flags") + @mock.patch.object(Client, "capture") + def test_connection_error_returns_stale_cached_value( + self, patch_capture, patch_flags + ): + """Test that connection errors return stale cached value when available.""" + from posthog.request import RequestsConnectionError + + # Pre-populate cache with a boolean flag result + cached_result = FeatureFlagResult.from_value_and_payload("my-flag", True, None) + self._populate_stale_cache("some-distinct-id", "my-flag", cached_result) + + # Simulate connection error + patch_flags.side_effect = RequestsConnectionError("Connection refused") + + flag_result = self.client.get_feature_flag_result("my-flag", "some-distinct-id") + + # Should return the stale cached value + self.assertIsNotNone(flag_result) + self.assertEqual(flag_result.enabled, True) + self.assertIsNone(flag_result.variant) + + # Error should still be tracked + patch_capture.assert_called_with( + "$feature_flag_called", + distinct_id="some-distinct-id", + properties={ + "$feature_flag": "my-flag", + "$feature_flag_response": True, + "locally_evaluated": False, + "$feature/my-flag": True, + "$feature_flag_error": "connection_error", + }, + groups={}, + disable_geoip=None, + ) + + @mock.patch("posthog.client.flags") + @mock.patch.object(Client, "capture") + def test_api_error_returns_stale_cached_value(self, patch_capture, patch_flags): + """Test that API errors return stale cached value when available.""" + from posthog.request import APIError + + # Pre-populate cache + cached_result = FeatureFlagResult.from_value_and_payload( + "my-flag", "control", None + ) + self._populate_stale_cache("some-distinct-id", "my-flag", cached_result) + + # Simulate API error + patch_flags.side_effect = APIError(503, "Service unavailable") + + flag_result = self.client.get_feature_flag_result("my-flag", "some-distinct-id") + + # Should return the stale cached value + self.assertIsNotNone(flag_result) + self.assertEqual(flag_result.variant, "control") + + # Error should still be tracked with status code + patch_capture.assert_called_with( + "$feature_flag_called", + distinct_id="some-distinct-id", + properties={ + "$feature_flag": "my-flag", + "$feature_flag_response": "control", + "locally_evaluated": False, + "$feature/my-flag": "control", + "$feature_flag_error": "api_error_503", + }, + groups={}, + disable_geoip=None, + ) + + @mock.patch("posthog.client.flags") + @mock.patch.object(Client, "capture") + def test_error_without_cache_returns_none(self, patch_capture, patch_flags): + """Test that errors return None when no stale cache is available.""" + from posthog.request import RequestsTimeout + + # Do NOT populate cache - no fallback available + + patch_flags.side_effect = RequestsTimeout("Request timed out") + + flag_result = self.client.get_feature_flag_result("my-flag", "some-distinct-id") + + # Should return None since no cache available + self.assertIsNone(flag_result) + + # Error should still be tracked + patch_capture.assert_called_with( + "$feature_flag_called", + distinct_id="some-distinct-id", + properties={ + "$feature_flag": "my-flag", + "$feature_flag_response": None, + "locally_evaluated": False, + "$feature/my-flag": None, + "$feature_flag_error": "timeout", + }, + groups={}, + disable_geoip=None, + ) From a474f9c29f4bb8390a373b07dddfab848826ea3e Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Mon, 15 Dec 2025 15:58:06 -0800 Subject: [PATCH 5/8] Report combined errors when both errors_while_computing and flag_missing When the server returns errorsWhileComputingFlags=true AND the requested flag is not in the response, report both conditions as a comma-separated string: "errors_while_computing_flags,flag_missing" This provides better debugging context when both conditions occur. --- posthog/client.py | 9 ++++-- posthog/test/test_feature_flag_result.py | 36 ++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/posthog/client.py b/posthog/client.py index 84ccbede..f1ea3e18 100644 --- a/posthog/client.py +++ b/posthog/client.py @@ -1590,10 +1590,13 @@ def _get_feature_flag_result( disable_geoip, ) ) + errors = [] if errors_while_computing: - feature_flag_error = "errors_while_computing_flags" - elif flag_details is None: - feature_flag_error = "flag_missing" + errors.append("errors_while_computing_flags") + if flag_details is None: + errors.append("flag_missing") + if errors: + feature_flag_error = ",".join(errors) flag_result = FeatureFlagResult.from_flag_details( flag_details, override_match_value diff --git a/posthog/test/test_feature_flag_result.py b/posthog/test/test_feature_flag_result.py index eab87c07..550f6a71 100644 --- a/posthog/test/test_feature_flag_result.py +++ b/posthog/test/test_feature_flag_result.py @@ -544,6 +544,42 @@ def test_get_feature_flag_result_flag_not_in_response( disable_geoip=None, ) + @mock.patch("posthog.client.flags") + @mock.patch.object(Client, "capture") + def test_get_feature_flag_result_errors_computing_and_flag_missing( + self, patch_capture, patch_flags + ): + """Test that both errors are reported when errorsWhileComputingFlags=true AND flag is missing. + + This can happen when the server encounters errors computing flags AND the requested + flag is not in the response. Both conditions should be reported for debugging. + """ + patch_flags.return_value = { + "flags": {}, # Flag is missing + "requestId": "test-request-id-999", + "errorsWhileComputingFlags": True, # But errors also occurred + } + + flag_result = self.client.get_feature_flag_result( + "missing-flag", "some-distinct-id" + ) + + self.assertIsNone(flag_result) + patch_capture.assert_called_with( + "$feature_flag_called", + distinct_id="some-distinct-id", + properties={ + "$feature_flag": "missing-flag", + "$feature_flag_response": None, + "locally_evaluated": False, + "$feature/missing-flag": None, + "$feature_flag_request_id": "test-request-id-999", + "$feature_flag_error": "errors_while_computing_flags,flag_missing", + }, + groups={}, + disable_geoip=None, + ) + @mock.patch("posthog.client.flags") @mock.patch.object(Client, "capture") def test_get_feature_flag_result_unknown_error(self, patch_capture, patch_flags): From 7e8973486756f347e39a5eb9948a9023efd3db73 Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Mon, 15 Dec 2025 16:08:38 -0800 Subject: [PATCH 6/8] Add FeatureFlagError constants class for error type values - Add FeatureFlagError class to types.py with constants: - ERRORS_WHILE_COMPUTING, FLAG_MISSING, QUOTA_LIMITED - TIMEOUT, CONNECTION_ERROR, UNKNOWN_ERROR - api_error(status) static method for dynamic error strings - Update client.py to use FeatureFlagError constants instead of magic strings - Update all tests to use constants for maintainability This improves maintainability by: - Single source of truth for error values - IDE autocomplete and typo detection - Documentation of analytics-stable values --- posthog/client.py | 15 ++++----- posthog/test/test_feature_flag_result.py | 34 ++++++++++++--------- posthog/types.py | 39 ++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 21 deletions(-) diff --git a/posthog/client.py b/posthog/client.py index f1ea3e18..ce503822 100644 --- a/posthog/client.py +++ b/posthog/client.py @@ -56,6 +56,7 @@ ) from posthog.types import ( FeatureFlag, + FeatureFlagError, FeatureFlagResult, FlagMetadata, FlagsAndPayloads, @@ -1592,9 +1593,9 @@ def _get_feature_flag_result( ) errors = [] if errors_while_computing: - errors.append("errors_while_computing_flags") + errors.append(FeatureFlagError.ERRORS_WHILE_COMPUTING) if flag_details is None: - errors.append("flag_missing") + errors.append(FeatureFlagError.FLAG_MISSING) if errors: feature_flag_error = ",".join(errors) @@ -1613,23 +1614,23 @@ def _get_feature_flag_result( ) except QuotaLimitError as e: self.log.warning(f"[FEATURE FLAGS] Quota limit exceeded: {e}") - feature_flag_error = "quota_limited" + feature_flag_error = FeatureFlagError.QUOTA_LIMITED flag_result = self._get_stale_flag_fallback(distinct_id, key) except RequestsTimeout as e: self.log.warning(f"[FEATURE FLAGS] Request timed out: {e}") - feature_flag_error = "timeout" + feature_flag_error = FeatureFlagError.TIMEOUT flag_result = self._get_stale_flag_fallback(distinct_id, key) except RequestsConnectionError as e: self.log.warning(f"[FEATURE FLAGS] Connection error: {e}") - feature_flag_error = "connection_error" + feature_flag_error = FeatureFlagError.CONNECTION_ERROR flag_result = self._get_stale_flag_fallback(distinct_id, key) except APIError as e: self.log.warning(f"[FEATURE FLAGS] API error: {e}") - feature_flag_error = f"api_error_{e.status}" + feature_flag_error = FeatureFlagError.api_error(e.status) flag_result = self._get_stale_flag_fallback(distinct_id, key) except Exception as e: self.log.exception(f"[FEATURE FLAGS] Unable to get flag remotely: {e}") - feature_flag_error = "unknown_error" + feature_flag_error = FeatureFlagError.UNKNOWN_ERROR flag_result = self._get_stale_flag_fallback(distinct_id, key) if send_feature_flag_events: diff --git a/posthog/test/test_feature_flag_result.py b/posthog/test/test_feature_flag_result.py index 550f6a71..352b0fcb 100644 --- a/posthog/test/test_feature_flag_result.py +++ b/posthog/test/test_feature_flag_result.py @@ -4,7 +4,13 @@ from posthog.client import Client from posthog.test.test_utils import FAKE_TEST_API_KEY -from posthog.types import FeatureFlag, FeatureFlagResult, FlagMetadata, FlagReason +from posthog.types import ( + FeatureFlag, + FeatureFlagError, + FeatureFlagResult, + FlagMetadata, + FlagReason, +) class TestFeatureFlagResult(unittest.TestCase): @@ -450,7 +456,7 @@ def test_get_feature_flag_result_unknown_flag(self, patch_capture, patch_flags): "$feature_flag_response": None, "locally_evaluated": False, "$feature/no-person-flag": None, - "$feature_flag_error": "flag_missing", + "$feature_flag_error": FeatureFlagError.FLAG_MISSING, }, groups={}, disable_geoip=None, @@ -496,7 +502,7 @@ def test_get_feature_flag_result_with_errors_while_computing_flags( "$feature_flag_reason": "Matched condition set 1", "$feature_flag_id": 1, "$feature_flag_version": 1, - "$feature_flag_error": "errors_while_computing_flags", + "$feature_flag_error": FeatureFlagError.ERRORS_WHILE_COMPUTING, }, groups={}, disable_geoip=None, @@ -538,7 +544,7 @@ def test_get_feature_flag_result_flag_not_in_response( "locally_evaluated": False, "$feature/missing-flag": None, "$feature_flag_request_id": "test-request-id-456", - "$feature_flag_error": "flag_missing", + "$feature_flag_error": FeatureFlagError.FLAG_MISSING, }, groups={}, disable_geoip=None, @@ -574,7 +580,7 @@ def test_get_feature_flag_result_errors_computing_and_flag_missing( "locally_evaluated": False, "$feature/missing-flag": None, "$feature_flag_request_id": "test-request-id-999", - "$feature_flag_error": "errors_while_computing_flags,flag_missing", + "$feature_flag_error": f"{FeatureFlagError.ERRORS_WHILE_COMPUTING},{FeatureFlagError.FLAG_MISSING}", }, groups={}, disable_geoip=None, @@ -597,7 +603,7 @@ def test_get_feature_flag_result_unknown_error(self, patch_capture, patch_flags) "$feature_flag_response": None, "locally_evaluated": False, "$feature/my-flag": None, - "$feature_flag_error": "unknown_error", + "$feature_flag_error": FeatureFlagError.UNKNOWN_ERROR, }, groups={}, disable_geoip=None, @@ -622,7 +628,7 @@ def test_get_feature_flag_result_timeout_error(self, patch_capture, patch_flags) "$feature_flag_response": None, "locally_evaluated": False, "$feature/my-flag": None, - "$feature_flag_error": "timeout", + "$feature_flag_error": FeatureFlagError.TIMEOUT, }, groups={}, disable_geoip=None, @@ -647,7 +653,7 @@ def test_get_feature_flag_result_connection_error(self, patch_capture, patch_fla "$feature_flag_response": None, "locally_evaluated": False, "$feature/my-flag": None, - "$feature_flag_error": "connection_error", + "$feature_flag_error": FeatureFlagError.CONNECTION_ERROR, }, groups={}, disable_geoip=None, @@ -672,7 +678,7 @@ def test_get_feature_flag_result_api_error(self, patch_capture, patch_flags): "$feature_flag_response": None, "locally_evaluated": False, "$feature/my-flag": None, - "$feature_flag_error": "api_error_500", + "$feature_flag_error": FeatureFlagError.api_error(500), }, groups={}, disable_geoip=None, @@ -697,7 +703,7 @@ def test_get_feature_flag_result_quota_limited(self, patch_capture, patch_flags) "$feature_flag_response": None, "locally_evaluated": False, "$feature/my-flag": None, - "$feature_flag_error": "quota_limited", + "$feature_flag_error": FeatureFlagError.QUOTA_LIMITED, }, groups={}, disable_geoip=None, @@ -769,7 +775,7 @@ def test_timeout_error_returns_stale_cached_value(self, patch_capture, patch_fla "locally_evaluated": False, "$feature/my-flag": "cached-variant", "$feature_flag_payload": {"from": "cache"}, - "$feature_flag_error": "timeout", + "$feature_flag_error": FeatureFlagError.TIMEOUT, }, groups={}, disable_geoip=None, @@ -806,7 +812,7 @@ def test_connection_error_returns_stale_cached_value( "$feature_flag_response": True, "locally_evaluated": False, "$feature/my-flag": True, - "$feature_flag_error": "connection_error", + "$feature_flag_error": FeatureFlagError.CONNECTION_ERROR, }, groups={}, disable_geoip=None, @@ -842,7 +848,7 @@ def test_api_error_returns_stale_cached_value(self, patch_capture, patch_flags): "$feature_flag_response": "control", "locally_evaluated": False, "$feature/my-flag": "control", - "$feature_flag_error": "api_error_503", + "$feature_flag_error": FeatureFlagError.api_error(503), }, groups={}, disable_geoip=None, @@ -872,7 +878,7 @@ def test_error_without_cache_returns_none(self, patch_capture, patch_flags): "$feature_flag_response": None, "locally_evaluated": False, "$feature/my-flag": None, - "$feature_flag_error": "timeout", + "$feature_flag_error": FeatureFlagError.TIMEOUT, }, groups={}, disable_geoip=None, diff --git a/posthog/types.py b/posthog/types.py index f706eacc..780fd78c 100644 --- a/posthog/types.py +++ b/posthog/types.py @@ -307,3 +307,42 @@ def to_payloads(response: FlagsResponse) -> Optional[dict[str, str]]: and value.enabled and value.metadata.payload is not None } + + +class FeatureFlagError: + """Error type constants for the $feature_flag_error property. + + These values are sent in analytics events to track flag evaluation failures. + They should not be changed without considering impact on existing dashboards + and queries that filter on these values. + + Error values: + ERRORS_WHILE_COMPUTING: Server returned errorsWhileComputingFlags=true + FLAG_MISSING: Requested flag not in API response + QUOTA_LIMITED: Rate/quota limit exceeded + TIMEOUT: Request timed out + CONNECTION_ERROR: Network connectivity issue + UNKNOWN_ERROR: Unexpected exceptions + + For API errors with status codes, use the api_error() method which returns + a string like "api_error_500". + """ + + ERRORS_WHILE_COMPUTING = "errors_while_computing_flags" + FLAG_MISSING = "flag_missing" + QUOTA_LIMITED = "quota_limited" + TIMEOUT = "timeout" + CONNECTION_ERROR = "connection_error" + UNKNOWN_ERROR = "unknown_error" + + @staticmethod + def api_error(status: int) -> str: + """Generate API error string with status code. + + Args: + status: HTTP status code from the API error + + Returns: + Error string like "api_error_500" + """ + return f"api_error_{status}" From 677b0ae46ccc14676eac0f916e1bccc492b8afdd Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Mon, 15 Dec 2025 16:20:23 -0800 Subject: [PATCH 7/8] Remove print statements from test failure handlers --- posthog/test/test_feature_flag_result.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/posthog/test/test_feature_flag_result.py b/posthog/test/test_feature_flag_result.py index 352b0fcb..7de428c6 100644 --- a/posthog/test/test_feature_flag_result.py +++ b/posthog/test/test_feature_flag_result.py @@ -195,7 +195,6 @@ def tearDownClass(cls): def set_fail(self, e, batch): """Mark the failure handler""" - print("FAIL", e, batch) # noqa: T201 self.failed = True def setUp(self): @@ -722,7 +721,6 @@ class TestFeatureFlagErrorWithStaleCacheFallback(unittest.TestCase): def set_fail(self, e, batch): """Mark the failure handler""" - print("FAIL", e, batch) # noqa: T201 self.failed = True def setUp(self): From 0665e19502ed169344a76656c4026720b2f2bc44 Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Mon, 15 Dec 2025 16:21:56 -0800 Subject: [PATCH 8/8] Fix mypy type error in FeatureFlagError.api_error method Accept Union[int, str] to match APIError.status type. --- posthog/types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/posthog/types.py b/posthog/types.py index 780fd78c..03104278 100644 --- a/posthog/types.py +++ b/posthog/types.py @@ -336,7 +336,7 @@ class FeatureFlagError: UNKNOWN_ERROR = "unknown_error" @staticmethod - def api_error(status: int) -> str: + def api_error(status: Union[int, str]) -> str: """Generate API error string with status code. Args: