Skip to content

Commit 687465b

Browse files
committed
Add urllib3-based retry for feature flag requests
Use urllib3's built-in Retry mechanism for feature flag POST requests instead of application-level retry logic. This is simpler and leverages well-tested library code. Key changes: - Add `RETRY_STATUS_FORCELIST` = [408, 500, 502, 503, 504] - Add `_build_flags_session()` with POST retries and `status_forcelist` - Update `flags()` to use dedicated flags session - Add tests for retry configuration and session usage The flags session retries on: - Network failures (connect/read errors) - Transient server errors (408, 500, 502, 503, 504) It does NOT retry on: - 429 (rate limit) - need to wait, not hammer - 402 (quota limit) - won't resolve with retries
1 parent b179280 commit 687465b

File tree

2 files changed

+139
-5
lines changed

2 files changed

+139
-5
lines changed

posthog/request.py

Lines changed: 62 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
from io import BytesIO
99
from typing import Any, List, Optional, Tuple, Union
1010

11-
1211
import requests
1312
from dateutil.tz import tzutc
1413
from requests.adapters import HTTPAdapter # type: ignore[import-untyped]
@@ -42,6 +41,9 @@
4241
if hasattr(socket, attr):
4342
KEEP_ALIVE_SOCKET_OPTIONS.append((socket.SOL_TCP, getattr(socket, attr), value))
4443

44+
# Status codes that indicate transient server errors worth retrying
45+
RETRY_STATUS_FORCELIST = [408, 500, 502, 503, 504]
46+
4547

4648
def _mask_tokens_in_url(url: str) -> str:
4749
"""Mask token values in URLs for safe logging, keeping first 10 chars visible."""
@@ -71,20 +73,47 @@ def init_poolmanager(self, *args, **kwargs):
7173

7274

7375
def _build_session(socket_options: Optional[SocketOptions] = None) -> requests.Session:
76+
"""Build a session for general requests (batch, decide, etc.)."""
77+
adapter = HTTPAdapterWithSocketOptions(
78+
max_retries=Retry(
79+
total=2,
80+
connect=2,
81+
read=2,
82+
),
83+
socket_options=socket_options,
84+
)
85+
session = requests.Session()
86+
session.mount("https://", adapter)
87+
return session
88+
89+
90+
def _build_flags_session(
91+
socket_options: Optional[SocketOptions] = None,
92+
) -> requests.Session:
93+
"""
94+
Build a session for feature flag requests with POST retries.
95+
96+
Feature flag requests are idempotent (read-only), so retrying POST
97+
requests is safe. This session retries on transient server errors
98+
(408, 5xx) and network failures.
99+
"""
74100
adapter = HTTPAdapterWithSocketOptions(
75101
max_retries=Retry(
76102
total=2,
77103
connect=2,
78104
read=2,
105+
status_forcelist=RETRY_STATUS_FORCELIST,
106+
allowed_methods=["POST"],
79107
),
80108
socket_options=socket_options,
81109
)
82-
session = requests.sessions.Session()
110+
session = requests.Session()
83111
session.mount("https://", adapter)
84112
return session
85113

86114

87115
_session = _build_session()
116+
_flags_session = _build_flags_session()
88117
_socket_options: Optional[SocketOptions] = None
89118
_pooling_enabled = True
90119

@@ -95,6 +124,12 @@ def _get_session() -> requests.Session:
95124
return _build_session(_socket_options)
96125

97126

127+
def _get_flags_session() -> requests.Session:
128+
if _pooling_enabled:
129+
return _flags_session
130+
return _build_flags_session(_socket_options)
131+
132+
98133
def set_socket_options(socket_options: Optional[SocketOptions]) -> None:
99134
"""
100135
Configure socket options for all HTTP connections.
@@ -103,11 +138,12 @@ def set_socket_options(socket_options: Optional[SocketOptions]) -> None:
103138
from posthog import set_socket_options
104139
set_socket_options([(socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1)])
105140
"""
106-
global _session, _socket_options
141+
global _session, _flags_session, _socket_options
107142
if socket_options == _socket_options:
108143
return
109144
_socket_options = socket_options
110145
_session = _build_session(socket_options)
146+
_flags_session = _build_flags_session(socket_options)
111147

112148

113149
def enable_keep_alive() -> None:
@@ -221,8 +257,29 @@ def flags(
221257
timeout: int = 15,
222258
**kwargs,
223259
) -> Any:
224-
"""Post the `kwargs to the flags API endpoint"""
225-
res = post(api_key, host, "/flags/?v=2", gzip, timeout, **kwargs)
260+
"""
261+
Post the kwargs to the flags API endpoint.
262+
263+
Uses a session configured to retry on transient server errors (408, 5xx)
264+
and network failures. Retries are handled by urllib3 with exponential backoff.
265+
"""
266+
log = logging.getLogger("posthog")
267+
body = kwargs.copy()
268+
body["sentAt"] = datetime.now(tz=tzutc()).isoformat()
269+
url = remove_trailing_slash(host or DEFAULT_HOST) + "/flags/?v=2"
270+
body["api_key"] = api_key
271+
json_data = json.dumps(body, cls=DatetimeSerializer)
272+
log.debug("making request: %s to url: %s", json_data, url)
273+
headers = {"Content-Type": "application/json", "User-Agent": USER_AGENT}
274+
data: Union[str, bytes] = json_data
275+
if gzip:
276+
headers["Content-Encoding"] = "gzip"
277+
buf = BytesIO()
278+
with GzipFile(fileobj=buf, mode="w") as gz:
279+
gz.write(json_data.encode("utf-8"))
280+
data = buf.getvalue()
281+
282+
res = _get_flags_session().post(url, data=data, headers=headers, timeout=timeout)
226283
return _process_response(
227284
res, success_message="Feature flags evaluated successfully"
228285
)

posthog/test/test_request.py

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,15 @@
1212
DatetimeSerializer,
1313
GetResponse,
1414
KEEP_ALIVE_SOCKET_OPTIONS,
15+
RETRY_STATUS_FORCELIST,
1516
QuotaLimitError,
1617
_mask_tokens_in_url,
1718
batch_post,
1819
decide,
1920
determine_server_host,
2021
disable_connection_reuse,
2122
enable_keep_alive,
23+
flags,
2224
get,
2325
set_socket_options,
2426
)
@@ -393,3 +395,78 @@ def test_set_socket_options_is_idempotent():
393395
assert session1 is session2
394396
finally:
395397
set_socket_options(None)
398+
399+
400+
class TestRetryConfiguration(unittest.TestCase):
401+
"""Tests for retry configuration in feature flag requests."""
402+
403+
def test_retry_status_forcelist_contains_expected_codes(self):
404+
"""Verify RETRY_STATUS_FORCELIST contains the expected status codes."""
405+
# 408 Request Timeout - client took too long to send request
406+
# 500 Internal Server Error - transient server issue
407+
# 502 Bad Gateway - upstream issue
408+
# 503 Service Unavailable - temporary overload
409+
# 504 Gateway Timeout - upstream timeout
410+
expected = [408, 500, 502, 503, 504]
411+
self.assertEqual(RETRY_STATUS_FORCELIST, expected)
412+
413+
def test_retry_status_forcelist_excludes_rate_limits(self):
414+
"""Verify 429 (rate limit) is NOT in RETRY_STATUS_FORCELIST."""
415+
# Rate limits shouldn't be retried - need to wait, not hammer
416+
self.assertNotIn(429, RETRY_STATUS_FORCELIST)
417+
418+
def test_retry_status_forcelist_excludes_quota_errors(self):
419+
"""Verify 402 (payment required/quota) is NOT in RETRY_STATUS_FORCELIST."""
420+
# Quota errors won't resolve with retries
421+
self.assertNotIn(402, RETRY_STATUS_FORCELIST)
422+
423+
424+
class TestFlagsSession(unittest.TestCase):
425+
"""Tests for flags session configuration."""
426+
427+
@mock.patch("posthog.request._get_flags_session")
428+
def test_flags_uses_flags_session(self, mock_get_flags_session):
429+
"""flags() uses the dedicated flags session, not the general session."""
430+
mock_response = requests.Response()
431+
mock_response.status_code = 200
432+
mock_response._content = json.dumps(
433+
{
434+
"featureFlags": {"test-flag": True},
435+
"featureFlagPayloads": {},
436+
"errorsWhileComputingFlags": False,
437+
}
438+
).encode("utf-8")
439+
440+
mock_session = mock.MagicMock()
441+
mock_session.post.return_value = mock_response
442+
mock_get_flags_session.return_value = mock_session
443+
444+
result = flags("test-key", "https://test.posthog.com", distinct_id="user123")
445+
446+
self.assertEqual(result["featureFlags"]["test-flag"], True)
447+
mock_get_flags_session.assert_called_once()
448+
mock_session.post.assert_called_once()
449+
450+
@mock.patch("posthog.request._get_flags_session")
451+
def test_flags_no_retry_on_quota_limit(self, mock_get_flags_session):
452+
"""flags() raises QuotaLimitError without retrying (at application level)."""
453+
mock_response = requests.Response()
454+
mock_response.status_code = 200
455+
mock_response._content = json.dumps(
456+
{
457+
"quotaLimited": ["feature_flags"],
458+
"featureFlags": {},
459+
"featureFlagPayloads": {},
460+
"errorsWhileComputingFlags": False,
461+
}
462+
).encode("utf-8")
463+
464+
mock_session = mock.MagicMock()
465+
mock_session.post.return_value = mock_response
466+
mock_get_flags_session.return_value = mock_session
467+
468+
with self.assertRaises(QuotaLimitError):
469+
flags("test-key", "https://test.posthog.com", distinct_id="user123")
470+
471+
# QuotaLimitError is raised after response is received, not retried
472+
self.assertEqual(mock_session.post.call_count, 1)

0 commit comments

Comments
 (0)