diff --git a/.sampo/changesets/flag-definitions-rejected-key-once.md b/.sampo/changesets/flag-definitions-rejected-key-once.md new file mode 100644 index 000000000..a4c2cfb3a --- /dev/null +++ b/.sampo/changesets/flag-definitions-rejected-key-once.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: patch +--- + +A client whose secret key can't read flag definitions, such as a key without the Feature flags: Read scope, now logs one warning instead of an error on every poll. The warning names the missing scope and how to turn off local evaluation. It logs again after definitions load and the key is rejected later. diff --git a/posthog/client.py b/posthog/client.py index ce5eb3352..6fb2b80a8 100644 --- a/posthog/client.py +++ b/posthog/client.py @@ -952,6 +952,7 @@ def __init__( ) self._flags_etag: Optional[str] = None self._flag_definition_fetch_generation = 0 + self._reported_rejected_secret_key = False self._flag_definition_published_generation = 0 self._flag_definition_cache_generation = 0 self._flag_definition_publication_lock = threading.RLock() @@ -3698,6 +3699,7 @@ def _fetch_feature_flags_from_api(self): self._flags_etag = response.etag self._flag_definition_published_generation = fetch_generation self._flag_definition_cache_generation = fetch_generation + self._reported_rejected_secret_key = False self.log.debug( "[FEATURE FLAGS] Flags not modified (304), using cached data" ) @@ -3713,6 +3715,7 @@ def _fetch_feature_flags_from_api(self): old_fingerprint = self._update_flag_state( response.data, _fingerprint=fingerprint ) + self._reported_rejected_secret_key = False if self._flag_definition_cache_provider: cache_data_to_store = { @@ -3757,12 +3760,7 @@ def _fetch_feature_flags_from_api(self): if fetch_generation <= self._flag_definition_published_generation: self.log.debug("[FEATURE FLAGS] Ignoring stale API error response") elif e.status == 401: - detail = ( - f"Error loading feature flags: {e.message}. " - "Please verify both your project_api_key and secret_key. " - "More information: https://posthog.com/docs/api/overview" - ) - self.log.error("[FEATURE FLAGS] %s", detail) + detail = self._report_rejected_secret_key_once(e) self.feature_flags = [] self.group_type_mapping = {} self.cohorts = {} @@ -3810,6 +3808,8 @@ def _fetch_feature_flags_from_api(self): status=402, message="PostHog feature flags quota limited", ) + elif e.status == 403: + self._report_rejected_secret_key_once(e) else: self.log.error(f"[FEATURE FLAGS] Error loading feature flags: {e}") except Exception as e: @@ -3826,6 +3826,22 @@ def _fetch_feature_flags_from_api(self): self._last_feature_flag_poll = datetime.now(tz=timezone.utc) + def _report_rejected_secret_key_once(self, error: APIError) -> str: + detail = ( + f"Your secret key can't load feature flag definitions for local evaluation " + f"({error.status}: {error.message}). The key is invalid or lacks the " + "Feature flags: Read scope. Add the scope to the key, or pass " + "enable_local_evaluation=False if you don't evaluate flags locally. " + "This warning won't repeat until definitions load. " + "More information: https://posthog.com/docs/feature-flags/local-evaluation" + ) + if self._reported_rejected_secret_key: + self.log.debug("[FEATURE FLAGS] %s", detail) + else: + self._reported_rejected_secret_key = True + self.log.warning("[FEATURE FLAGS] %s", detail) + return detail + def load_feature_flags(self): """ Load feature flags for local evaluation. diff --git a/posthog/test/test_client.py b/posthog/test/test_client.py index 2a1923028..5ccc5f962 100644 --- a/posthog/test/test_client.py +++ b/posthog/test/test_client.py @@ -1103,7 +1103,7 @@ def test_load_feature_flags_unauthorized(self, patch_get): patch_get.side_effect = APIError(401, "Unauthorized") client = Client(FAKE_TEST_API_KEY, secret_key="test") - with self.assertLogs("posthog", level="ERROR") as logs: + with self.assertLogs("posthog", level="WARNING") as logs: client._load_feature_flags() self.assertEqual(client.feature_flags, []) @@ -1111,8 +1111,7 @@ def test_load_feature_flags_unauthorized(self, patch_get): self.assertEqual(client.group_type_mapping, {}) self.assertEqual(client.cohorts, {}) self.assertIn("Unauthorized", logs.output[0]) - self.assertIn("project_api_key", logs.output[0]) - self.assertIn("secret_key", logs.output[0]) + self.assertIn("Feature flags: Read", logs.output[0]) @mock.patch("posthog.client.flags") def test_dont_override_capture_with_local_flags(self, patch_flags): diff --git a/posthog/test/test_feature_flags.py b/posthog/test/test_feature_flags.py index ff1d56593..08a506c76 100644 --- a/posthog/test/test_feature_flags.py +++ b/posthog/test/test_feature_flags.py @@ -3911,17 +3911,53 @@ def test_load_feature_flags_refetches_after_state_reset(self, status, patch_get) self.assertEqual(client.feature_flags[0]["key"], "new-flag") self.assertEqual(client._flags_etag, '"new-etag"') + @parameterized.expand([(401,), (403,)]) + @mock.patch("posthog.client.get") + def test_rejected_secret_key_warns_once_across_polls(self, status, patch_get): + patch_get.side_effect = APIError(status, "rejected") + client = Client(FAKE_TEST_API_KEY, secret_key="test", send=False) + + with self.assertLogs("posthog", level="WARNING") as logs: + for _ in range(5): + client._load_feature_flags() + + self.assertEqual(patch_get.call_count, 5) + self.assertEqual(len(logs.records), 1) + self.assertEqual(logs.records[0].levelname, "WARNING") + self.assertIn("Feature flags: Read", logs.output[0]) + self.assertIn("enable_local_evaluation=False", logs.output[0]) + + @mock.patch("posthog.client.get") + def test_rejected_secret_key_warns_again_after_definitions_load(self, patch_get): + loaded = GetResponse( + data={"flags": [], "group_type_mapping": {}, "cohorts": {}}, + etag=None, + ) + patch_get.side_effect = [ + APIError(401, "rejected"), + APIError(401, "rejected"), + loaded, + APIError(401, "rejected"), + APIError(401, "rejected"), + ] + client = Client(FAKE_TEST_API_KEY, secret_key="test", send=False) + + with self.assertLogs("posthog", level="WARNING") as logs: + for _ in range(5): + client._load_feature_flags() + + self.assertEqual(len(logs.records), 2) + @mock.patch("posthog.client.Poller") @mock.patch("posthog.client.get") def test_load_feature_flags_wrong_key(self, patch_get, _patch_poll): patch_get.side_effect = APIError(401, "Unauthorized") client = Client(FAKE_TEST_API_KEY, secret_key=FAKE_TEST_API_KEY) - with self.assertLogs("posthog", level="ERROR") as logs: + with self.assertLogs("posthog", level="WARNING") as logs: client.load_feature_flags() self.assertIn("Unauthorized", logs.output[0]) - self.assertIn("project_api_key", logs.output[0]) - self.assertIn("secret_key", logs.output[0]) + self.assertIn("Feature flags: Read", logs.output[0]) client.debug = True with self.assertRaisesRegex(APIError, "Unauthorized"): client.load_feature_flags()