fix(capture): reject negative retry counts

.sampo/changesets/valiant-iceseeker-otso.md

@@ -0,0 +1,5 @@
+---
+pypi/posthog: patch
+---
+
+Reject negative capture retry counts

posthog/capture_v1.py

@@ -491,8 +491,10 @@ def _send_v1_batch(
     unchanged. A transport failure re-raises the underlying exception (drops
     collected on an earlier attempt are still tallied in the DEBUG summary).
     ``request_id`` and the batch ``created_at`` are stable across attempts;
-    ``PostHog-Attempt`` increments.
+    ``PostHog-Attempt`` increments. Negative ``max_retries`` values are treated
+    as zero, so delivery is always attempted at least once.
     """
+    max_retries = max(0, max_retries)
     request_id = str(uuid4())
     # Hoisted once so the batch envelope is byte-identical across retry attempts
     # (only the events list shrinks and the attempt header increments).
@@ -606,7 +608,6 @@ def _send_v1_batch(
             continue
         raise v1_error
 
-    # Unreachable in practice (every branch returns or continues), but keeps the
-    # function total if max_retries is somehow negative.
+    # Unreachable in practice (every branch returns or continues).
     if last_exc:
         raise last_exc

posthog/client.py

@@ -503,7 +503,7 @@ def __init__(
             flush_interval: Maximum seconds a background consumer waits before
                 flushing a partial batch.
             gzip: Whether to gzip event upload payloads.
-            max_retries: Number of upload retries for background consumers.
+            max_retries: Number of upload retries. Values below 0 are treated as 0.
             sync_mode: If True, send each event synchronously instead of using
                 background worker threads.
             timeout: HTTP request timeout in seconds for event uploads.
@@ -607,7 +607,7 @@ def __init__(
         self._duplicate_client_registry_key: Optional[tuple[str, str]] = None
         self.gzip = gzip
         self.timeout = timeout
-        self.max_retries = max_retries
+        self.max_retries = max(0, max_retries)
         self._feature_flags: Optional[list[Any]] = (
             None  # private variable to store flags
         )
@@ -766,7 +766,7 @@ def __init__(
             flush_at=flush_at,
             flush_interval=flush_interval,
             gzip=gzip,
-            max_retries=max_retries,
+            max_retries=self.max_retries,
             timeout=timeout,
             historical_migration=historical_migration,
         )

posthog/consumer.py

@@ -77,7 +77,7 @@ def __init__(
         # run() *after* we set it to False in pause... and keep running
         # forever.
         self.running = True
-        self.retries = retries
+        self.retries = max(0, retries)
         self.timeout = timeout
         self.historical_migration = historical_migration
 

posthog/test/test_capture_v1.py

@@ -668,6 +668,17 @@ def test_transport_error_exhausted_reraises_original(self) -> None:
                 )
         self.assertEqual(len(stub.calls), 2)
 
+    def test_negative_max_retries_still_attempts_delivery_once(self) -> None:
+        stub = _PostV1Stub([_results_response({"u-1": "ok"})])
+
+        with mock.patch("posthog.capture_v1._post_v1", stub):
+            _send_v1_batch(
+                "phc_key", "https://app.posthog.com", [_msg("u-1")], max_retries=-1
+            )
+
+        self.assertEqual(len(stub.calls), 1)
+        self.assertEqual(stub.calls[0]["attempt"], 1)
+
     def test_small_retry_after_does_not_shorten_backoff(self) -> None:
         # A Retry-After smaller than the configured backoff must not make the
         # client retry earlier than its own schedule (Retry-After is a minimum).

posthog/test/test_client.py

@@ -3357,6 +3357,58 @@ def test_debug_flag_re_raises_exceptions(self, mock_enqueue):
                 self.assertEqual(str(cm.exception), "Expected error")
 
 
+class TestClientCaptureRetrySemantics(unittest.TestCase):
+    @parameterized.expand(
+        [
+            ("v0_sync", "v0", True),
+            ("v1_sync", "v1", True),
+            ("v0_async", "v0", False),
+            ("v1_async", "v1", False),
+        ]
+    )
+    def test_negative_max_retries_still_attempts_delivery_once(
+        self, _name, capture_mode, sync_mode
+    ):
+        response = mock.Mock(status_code=200, headers={}, text="")
+        response.json.return_value = {"results": {}}
+        client = None
+
+        with (
+            mock.patch("posthog.client.batch_post") as sync_v0_post,
+            mock.patch("posthog.consumer.batch_post") as async_v0_post,
+            mock.patch("posthog.capture_v1._post_v1", return_value=response) as v1_post,
+        ):
+            try:
+                client = Client(
+                    FAKE_TEST_API_KEY,
+                    capture_mode=capture_mode,
+                    sync_mode=sync_mode,
+                    max_retries=-1,
+                    flush_at=1,
+                    flush_interval=0.01,
+                )
+                client.capture("evt", distinct_id="d")
+                if not sync_mode:
+                    client.flush()
+
+                self.assertEqual(client.max_retries, 0)
+                if capture_mode == "v1":
+                    v1_post.assert_called_once()
+                    sync_v0_post.assert_not_called()
+                    async_v0_post.assert_not_called()
+                elif sync_mode:
+                    sync_v0_post.assert_called_once()
+                    async_v0_post.assert_not_called()
+                    v1_post.assert_not_called()
+                else:
+                    async_v0_post.assert_called_once()
+                    sync_v0_post.assert_not_called()
+                    v1_post.assert_not_called()
+            finally:
+                if client is not None and not sync_mode:
+                    client.shutdown()
+
+
 class TestClientSyncCaptureMode(unittest.TestCase):
     """Sync-mode `_enqueue` selects the analytics submitter by `capture_mode`."""
 

posthog/test/test_consumer.py

@@ -149,6 +149,15 @@ def test_request_does_not_retry_client_errors(self) -> None:
     def test_request_fails_when_exceptions_exceed_retries(self) -> None:
         self._run_retry_test(APIError(500, "Internal Server Error"), 4, retries=3)
 
+    def test_negative_retries_still_attempts_delivery_once(self) -> None:
+        consumer = Consumer(None, TEST_API_KEY, retries=-1)
+
+        with mock.patch("posthog.consumer.batch_post") as mock_post:
+            consumer.request([_track_event()])
+
+        self.assertEqual(consumer.retries, 0)
+        mock_post.assert_called_once()
+
     def test_pause(self) -> None:
         consumer = Consumer(None, TEST_API_KEY)
         consumer.pause()

references/public_api_snapshot.txt

@@ -562,7 +562,7 @@ attribute posthog.client.Client.in_app_modules = in_app_modules
 attribute posthog.client.Client.is_server = is_server
 attribute posthog.client.Client.log = logging.getLogger('posthog')
 attribute posthog.client.Client.log_captured_exceptions = log_captured_exceptions
-attribute posthog.client.Client.max_retries = max_retries
+attribute posthog.client.Client.max_retries = max(0, max_retries)
 attribute posthog.client.Client.metrics: PostHogMetrics
 attribute posthog.client.Client.on_error = on_error
 attribute posthog.client.Client.personal_api_key = self.secret_key
@@ -598,7 +598,7 @@ attribute posthog.consumer.Consumer.log = logging.getLogger('posthog')
 attribute posthog.consumer.Consumer.max_msg_size = max_msg_size
 attribute posthog.consumer.Consumer.on_error = on_error
 attribute posthog.consumer.Consumer.queue = queue
-attribute posthog.consumer.Consumer.retries = retries
+attribute posthog.consumer.Consumer.retries = max(0, retries)
 attribute posthog.consumer.Consumer.running = True
 attribute posthog.consumer.Consumer.timeout = timeout
 attribute posthog.consumer.MAX_MSG_SIZE = 900 * 1024