fix: Retry flags only on transport errors

.github/workflows/sdk-compliance.yml

@@ -14,8 +14,8 @@ on:
 jobs:
   compliance:
     name: PostHog SDK compliance tests
-    uses: PostHog/posthog-sdk-test-harness/.github/workflows/test-sdk-action.yml@68498bcf3ae95ac941476e6ca3d42d6086e1c7fd
+    uses: PostHog/posthog-sdk-test-harness/.github/workflows/test-sdk-action.yml@02c049e529001d02f37a534745678e057d371fb0
     with:
       adapter-dockerfile: "sdk_compliance_adapter/Dockerfile"
       adapter-context: "."
-      test-harness-version: "0.9.0"
+      test-harness-version: "0.10.0"

posthog/__init__.py

@@ -322,6 +322,9 @@ def get_tags() -> Dict[str, Any]:
         attributed to the person normally.
     feature_flags_request_timeout_seconds: Timeout in seconds for feature flag
         and remote config requests.
+    feature_flags_request_max_retries: Number of retries for feature flag
+        requests after network, transport, or timeout failures. Defaults to 1.
+        Set to 0 to disable retries.
     super_properties: Properties merged into every captured event.
     enable_exception_autocapture: Automatically capture uncaught exceptions.
     log_captured_exceptions: Also log exceptions captured by error tracking.
@@ -365,6 +368,7 @@ def get_tags() -> Dict[str, Any]:
 disable_geoip = True  # type: bool
 is_server = True  # type: bool
 feature_flags_request_timeout_seconds = 3  # type: int
+feature_flags_request_max_retries = 1  # type: int
 super_properties = None  # type: Optional[Dict]
 enable_exception_autocapture = False  # type: bool
 log_captured_exceptions = False  # type: bool
@@ -1156,6 +1160,7 @@ def setup() -> Client:
             disable_geoip=disable_geoip,
             is_server=is_server,
             feature_flags_request_timeout_seconds=feature_flags_request_timeout_seconds,
+            feature_flags_request_max_retries=feature_flags_request_max_retries,
             super_properties=super_properties,
             # TODO: Currently this monitoring begins only when the Client is initialised (which happens when you do something with the SDK)
             # This kind of initialisation is very annoying for exception capture. We need to figure out a way around this,

posthog/client.py

@@ -240,6 +240,7 @@ def __init__(
         is_server=True,
         historical_migration=False,
         feature_flags_request_timeout_seconds=3,
+        feature_flags_request_max_retries=1,
         super_properties=None,
         enable_exception_autocapture=False,
         log_captured_exceptions=False,
@@ -296,6 +297,9 @@ def __init__(
             historical_migration: Mark events as historical migration imports.
             feature_flags_request_timeout_seconds: Timeout in seconds for feature
                 flag and remote config requests.
+            feature_flags_request_max_retries: Number of retries for feature flag
+                requests after network, transport, or timeout failures. Defaults
+                to 1. Set to 0 to disable retries.
             super_properties: Properties merged into every captured event.
             enable_exception_autocapture: Automatically capture uncaught
                 exceptions.
@@ -376,6 +380,9 @@ def __init__(
         self.feature_flags_request_timeout_seconds = (
             feature_flags_request_timeout_seconds
         )
+        self.feature_flags_request_max_retries = max(
+            0, feature_flags_request_max_retries
+        )
         self.poller: Optional[Poller] = None
         self.distinct_ids_feature_flags_reported = SizeLimitedDict(MAX_DICT_SIZE, set)
         self.flag_fallback_cache_url = flag_fallback_cache_url
@@ -899,6 +906,7 @@ def _get_flags_decision(
             self.api_key,
             self.host,
             timeout=self.feature_flags_request_timeout_seconds,
+            max_retries=self.feature_flags_request_max_retries,
             **request_data,
         )
 

posthog/request.py

@@ -2,6 +2,7 @@
 import logging
 import re
 import socket
+import time
 import zlib
 from dataclasses import dataclass
 from datetime import date, datetime, timezone
@@ -42,8 +43,8 @@
     if hasattr(socket, attr):
         KEEP_ALIVE_SOCKET_OPTIONS.append((socket.SOL_TCP, getattr(socket, attr), value))
 
-# Status codes that indicate transient server errors worth retrying
-RETRY_STATUS_FORCELIST = [408, 500, 502, 503, 504]
+_FEATURE_FLAGS_RETRY_BACKOFF_SECONDS = 0.3
+_FEATURE_FLAGS_RETRY_HTTP_STATUSES = {502, 504}
 
 
 def _mask_tokens_in_url(url: str) -> str:
@@ -91,23 +92,13 @@ def _build_session(socket_options: Optional[SocketOptions] = None) -> requests.S
 def _build_flags_session(
     socket_options: Optional[SocketOptions] = None,
 ) -> requests.Session:
-    """
-    Build a session for feature flag requests with POST retries.
+    """Build a session for feature flag requests.
 
-    Feature flag requests are idempotent (read-only), so retrying POST
-    requests is safe. This session retries on transient server errors
-    (408, 5xx) and network failures with exponential backoff
-    (0.5s, 1s delays between retries).
+    /flags retries are handled explicitly in ``flags()`` so that only
+    transport failures and contract-defined transient HTTP responses are retried.
     """
     adapter = HTTPAdapterWithSocketOptions(
-        max_retries=Retry(
-            total=2,
-            connect=2,
-            read=2,
-            backoff_factor=0.5,
-            status_forcelist=RETRY_STATUS_FORCELIST,
-            allowed_methods=["POST"],
-        ),
+        max_retries=Retry(total=0, connect=0, read=0, status=0),
         socket_options=socket_options,
     )
     session = requests.Session()
@@ -310,26 +301,47 @@ def _process_response(
         raise APIError(res.status_code, res.text, retry_after=retry_after)
 
 
+def _feature_flags_retry_delay(failed_attempt: int) -> float:
+    return _FEATURE_FLAGS_RETRY_BACKOFF_SECONDS * (2**failed_attempt)
+
+
 def flags(
     api_key: str,
     host: Optional[str] = None,
     gzip: bool = False,
     timeout: int = 15,
+    max_retries: int = 1,
     **kwargs,
 ) -> Any:
-    """Post the kwargs to the flags API endpoint with automatic retries."""
-    res = post(
-        api_key,
-        host,
-        "/flags/?v=2",
-        gzip,
-        timeout,
-        session=_get_flags_session(),
-        **kwargs,
-    )
-    return _process_response(
-        res, success_message="Feature flags evaluated successfully"
-    )
+    """Post the kwargs to the flags API endpoint with bounded transient retries."""
+    retries = max(0, max_retries)
+    failed_attempt = 0
+
+    while True:
+        try:
+            res = post(
+                api_key,
+                host,
+                "/flags/?v=2",
+                gzip,
+                timeout,
+                session=_get_flags_session(),
+                **kwargs,
+            )
+            return _process_response(
+                res, success_message="Feature flags evaluated successfully"
+            )
+        except (requests.exceptions.ConnectionError, requests.exceptions.Timeout):
+            if failed_attempt >= retries:
+                raise
+        except APIError as exc:
+            if (
+                exc.status not in _FEATURE_FLAGS_RETRY_HTTP_STATUSES
+                or failed_attempt >= retries
+            ):
+                raise
+        time.sleep(_feature_flags_retry_delay(failed_attempt))
+        failed_attempt += 1
 
 
 def remote_config(

posthog/test/test_client.py

@@ -976,6 +976,7 @@ def test_basic_capture_with_feature_flags_returns_active_only(self, patch_flags)
                 "random_key",
                 "https://us.i.posthog.com",
                 timeout=3,
+                max_retries=1,
                 distinct_id="distinct_id",
                 groups={},
                 person_properties={},
@@ -984,6 +985,30 @@ def test_basic_capture_with_feature_flags_returns_active_only(self, patch_flags)
                 device_id=None,
             )
 
+    @parameterized.expand(
+        [
+            ("default", 1),
+            ("disabled", 0),
+            ("two_retries", 2),
+        ]
+    )
+    def test_feature_flags_request_max_retries_is_forwarded(
+        self, _name, expected_max_retries
+    ):
+        with mock.patch("posthog.client.flags") as patch_flags:
+            patch_flags.return_value = {"featureFlags": {}, "featureFlagPayloads": {}}
+            client = Client(
+                FAKE_TEST_API_KEY,
+                feature_flags_request_max_retries=expected_max_retries,
+                personal_api_key=FAKE_TEST_API_KEY,
+            )
+
+            client.get_all_flags("distinct_id")
+
+        self.assertEqual(
+            patch_flags.call_args.kwargs["max_retries"], expected_max_retries
+        )
+
     @mock.patch("posthog.client.flags")
     def test_basic_capture_with_feature_flags_and_disable_geoip_returns_correctly(
         self, patch_flags
@@ -1041,6 +1066,7 @@ def test_basic_capture_with_feature_flags_and_disable_geoip_returns_correctly(
                 "random_key",
                 "https://us.i.posthog.com",
                 timeout=12,
+                max_retries=1,
                 distinct_id="distinct_id",
                 groups={},
                 person_properties={},
@@ -2261,6 +2287,7 @@ def test_disable_geoip_default_on_decide(self, patch_flags):
             "random_key",
             "https://us.i.posthog.com",
             timeout=3,
+            max_retries=1,
             distinct_id="some_id",
             groups={},
             person_properties={"distinct_id": "some_id"},
@@ -2277,6 +2304,7 @@ def test_disable_geoip_default_on_decide(self, patch_flags):
             "random_key",
             "https://us.i.posthog.com",
             timeout=3,
+            max_retries=1,
             distinct_id="feature_enabled_distinct_id",
             groups={},
             person_properties={"distinct_id": "feature_enabled_distinct_id"},
@@ -2291,6 +2319,7 @@ def test_disable_geoip_default_on_decide(self, patch_flags):
             "random_key",
             "https://us.i.posthog.com",
             timeout=3,
+            max_retries=1,
             distinct_id="all_flags_payloads_id",
             groups={},
             person_properties={"distinct_id": "all_flags_payloads_id"},
@@ -2337,6 +2366,7 @@ def test_default_properties_get_added_properly(self, patch_flags):
             "random_key",
             "http://app2.posthog.com",
             timeout=3,
+            max_retries=1,
             distinct_id="some_id",
             groups={"company": "id:5", "instance": "app.posthog.com"},
             person_properties={"distinct_id": "some_id", "x1": "y1"},
@@ -2365,6 +2395,7 @@ def test_default_properties_get_added_properly(self, patch_flags):
             "random_key",
             "http://app2.posthog.com",
             timeout=3,
+            max_retries=1,
             distinct_id="some_id",
             groups={"company": "id:5", "instance": "app.posthog.com"},
             person_properties={"distinct_id": "override"},
@@ -2386,6 +2417,7 @@ def test_default_properties_get_added_properly(self, patch_flags):
             "random_key",
             "http://app2.posthog.com",
             timeout=3,
+            max_retries=1,
             distinct_id="some_id",
             groups={},
             person_properties={"distinct_id": "some_id"},
@@ -2446,7 +2478,11 @@ def test_device_id_is_passed_to_flags_request(
             expected_call["flag_keys_to_evaluate"] = expected_flag_keys
 
         patch_flags.assert_called_with(
-            "random_key", "https://us.i.posthog.com", timeout=3, **expected_call
+            "random_key",
+            "https://us.i.posthog.com",
+            timeout=3,
+            max_retries=1,
+            **expected_call,
         )
 
     @mock.patch("posthog.client.flags")
@@ -2472,6 +2508,7 @@ def test_device_id_from_context_is_used_in_flags_request(self, patch_flags):
                 "random_key",
                 "https://us.i.posthog.com",
                 timeout=3,
+                max_retries=1,
                 distinct_id="some_id",
                 groups={},
                 person_properties={"distinct_id": "some_id"},
@@ -2492,6 +2529,7 @@ def test_device_id_from_context_is_used_in_flags_request(self, patch_flags):
                 "random_key",
                 "https://us.i.posthog.com",
                 timeout=3,
+                max_retries=1,
                 distinct_id="some_id",
                 groups={},
                 person_properties={"distinct_id": "some_id"},
@@ -2521,6 +2559,7 @@ def test_client_set_context_device_id_is_used_in_flags_request(self, patch_flags
             "random_key",
             "https://us.i.posthog.com",
             timeout=3,
+            max_retries=1,
             distinct_id="some_id",
             groups={},
             person_properties={"distinct_id": "some_id"},