diff --git a/.sampo/changesets/noop-flag-api-errors.md b/.sampo/changesets/noop-flag-api-errors.md new file mode 100644 index 00000000..ec6f73b4 --- /dev/null +++ b/.sampo/changesets/noop-flag-api-errors.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: patch +--- + +Return empty flag defaults from Client flag helpers when the flags API fails. diff --git a/posthog/client.py b/posthog/client.py index 5a3bd85a..d2cccc26 100644 --- a/posthog/client.py +++ b/posthog/client.py @@ -611,6 +611,30 @@ def get_flags_decision( Category: Feature flags """ + try: + return self._get_flags_decision( + distinct_id, + groups, + person_properties, + group_properties, + disable_geoip, + flag_keys_to_evaluate, + device_id=device_id, + ) + except Exception as err: + self.log.exception("Unable to get feature flags: %s", err) + return normalize_flags_response({}) + + def _get_flags_decision( + self, + distinct_id: Optional[ID_TYPES] = None, + groups: Optional[dict] = None, + person_properties=None, + group_properties=None, + disable_geoip=None, + flag_keys_to_evaluate: Optional[list[str]] = None, + device_id: Optional[str] = None, + ) -> FlagsResponse: if self.disabled: return normalize_flags_response({}) @@ -2175,7 +2199,7 @@ def _get_feature_flag_details_from_server( 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( + resp_data = self._get_flags_decision( distinct_id, groups, person_properties, @@ -2447,7 +2471,7 @@ def get_all_flags_and_payloads( if fallback_to_flags and not only_evaluate_locally: try: - decide_response = self.get_flags_decision( + decide_response = self._get_flags_decision( distinct_id, groups=groups, person_properties=person_properties, @@ -2584,11 +2608,11 @@ def evaluate_flags( locally_evaluated_keys.add(key) # Fall back to remote evaluation for any flags the poller couldn't resolve locally. - # Use ``get_flags_decision`` directly so the resulting records carry id/version/reason + # Use the flags decision path directly so the resulting records carry id/version/reason # and fired ``$feature_flag_called`` events match what ``get_feature_flag()`` emits. if fallback_to_server and not only_evaluate_locally: try: - response = self.get_flags_decision( + response = self._get_flags_decision( distinct_id, groups=groups, person_properties=person_properties, diff --git a/posthog/test/test_client.py b/posthog/test/test_client.py index 5275eb6c..4fac43c8 100644 --- a/posthog/test/test_client.py +++ b/posthog/test/test_client.py @@ -106,6 +106,38 @@ def test_disabled_client_does_not_get_flags_decision(self, patch_flags): ) patch_flags.assert_not_called() + @mock.patch("posthog.client.flags") + def test_client_flag_helpers_return_defaults_on_api_error(self, patch_flags): + patch_flags.side_effect = APIError(401, "Unauthorized") + client = Client(FAKE_TEST_API_KEY, send=False) + + test_cases = [ + ( + "get_flags_decision", + lambda: client.get_flags_decision("distinct_id")["flags"], + {}, + ), + ( + "get_feature_variants", + lambda: client.get_feature_variants("distinct_id"), + {}, + ), + ( + "get_feature_payloads", + lambda: client.get_feature_payloads("distinct_id"), + {}, + ), + ( + "get_feature_flags_and_payloads", + lambda: client.get_feature_flags_and_payloads("distinct_id"), + {"featureFlags": {}, "featureFlagPayloads": {}}, + ), + ] + + for method_name, call_helper, expected in test_cases: + with self.subTest(method=method_name): + self.assertEqual(call_helper(), expected) + def test_empty_flush(self): self.client.flush()