Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .sampo/changesets/flag-definitions-rejected-key-once.md
Original file line number Diff line number Diff line change
@@ -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.
28 changes: 22 additions & 6 deletions posthog/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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"
)
Expand All @@ -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 = {
Expand Down Expand Up @@ -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 = {}
Expand Down Expand Up @@ -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:
Expand All @@ -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.
Expand Down
5 changes: 2 additions & 3 deletions posthog/test/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -1103,16 +1103,15 @@ 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, [])
self.assertEqual(client.feature_flags_by_key, {})
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):
Expand Down
42 changes: 39 additions & 3 deletions posthog/test/test_feature_flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Loading