diff --git a/.sampo/changesets/capture-v1-retry-404-429.md b/.sampo/changesets/capture-v1-retry-404-429.md new file mode 100644 index 000000000..7e1956f17 --- /dev/null +++ b/.sampo/changesets/capture-v1-retry-404-429.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: patch +--- + +With `capture_mode="v1"`, capture now retries `404` and `429` responses like `408` and `500`, `502`, `503`, `504`, with the same backoff and `Retry-After` handling. Before, both failed the batch at once. diff --git a/AGENTS.md b/AGENTS.md index 60e388a4b..31dfbfb4b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -24,7 +24,7 @@ Follow [Public API changes](./CONTRIBUTING.md#public-api-changes). As an agent, Before changing capture configuration, serialization, routing, or retries, read the relevant implementation and tests. -Preserve v0 defaults/compatibility; strictly typed v1 options and `$set`/`$set_once` relocation; v1-only compression (zlib-wrapped deflate, optional zstd); partial-only per-event retries with stable identity; accumulated drop reporting even on 2xx; terminal v1 `429`; `Retry-After` as a minimum bounded by the shared 30s ceiling; and inline blocking retries with `sync_mode=True`. +Preserve v0 defaults/compatibility; strictly typed v1 options and `$set`/`$set_once` relocation; v1-only compression (zlib-wrapped deflate, optional zstd); partial-only per-event retries with stable identity; accumulated drop reporting even on 2xx; v1 retries on `404` and `429` like `408` and 5xx; `Retry-After` as a minimum bounded by the shared 30s ceiling; and inline blocking retries with `sync_mode=True`. ## Mirror and build safety diff --git a/posthog/capture_v1.py b/posthog/capture_v1.py index 1ae31ce71..8f29cd706 100644 --- a/posthog/capture_v1.py +++ b/posthog/capture_v1.py @@ -81,10 +81,9 @@ _RESULT_DROP = "drop" _RESULT_RETRY = "retry" -# HTTP status classification. 429 is terminal in v1 (unlike v0, where it is -# retried) — the backend signals overload via retryable 5xx + Retry-After. -_RETRYABLE_STATUSES = frozenset({408, 500, 502, 503, 504}) -_TERMINAL_STATUSES = frozenset({400, 401, 402, 413, 415, 429}) +# HTTP status classification. 404 and 429 can come from a proxy in front of capture. +_RETRYABLE_STATUSES = frozenset({404, 408, 429, 500, 502, 503, 504}) +_TERMINAL_STATUSES = frozenset({400, 401, 402, 413, 415}) # Single ceiling (seconds) for the retry backoff: caps the exponential schedule # and clamps a server ``Retry-After`` to the same value. Keeps the max retry @@ -591,7 +590,7 @@ def _send_v1_batch( continue # Non-2xx. Retryable transient statuses back off; everything else - # (400/401/402/413/415/429/...) is terminal. Any drops collected from a + # (400/401/402/413/415/...) is terminal. Any drops collected from a # prior 2xx attempt ride along so on_error still sees them. v1_error = CaptureV1Error( parsed.status_code, diff --git a/posthog/test/test_capture_v1.py b/posthog/test/test_capture_v1.py index f4809d501..56c05af36 100644 --- a/posthog/test/test_capture_v1.py +++ b/posthog/test/test_capture_v1.py @@ -664,7 +664,7 @@ def test_malformed_2xx_is_terminal(self) -> None: self.assertEqual(len(stub.calls), 1) self.assertEqual(exc.status, 200) - @parameterized.expand([("bad_request", 400), ("rate_limited", 429)]) + @parameterized.expand([("bad_request", 400), ("billing_limit", 402)]) def test_terminal_status_raises_immediately(self, _name, status) -> None: stub, exc = self._run_expecting_error( [_msg("u-1")], @@ -674,23 +674,32 @@ def test_terminal_status_raises_immediately(self, _name, status) -> None: self.assertEqual(len(stub.calls), 1) # not retried self.assertEqual(exc.status, status) - def test_retryable_status_then_success(self) -> None: + @parameterized.expand( + [("not_found", 404), ("rate_limited", 429), ("unavailable", 503)] + ) + def test_retryable_status_then_success(self, _name, status) -> None: stub = self._run( [_msg("u-1")], [ - _FakeResponse(503, headers={"Retry-After": "2"}), + _FakeResponse(status, headers={"Retry-After": "2"}), _results_response({"u-1": "ok"}), ], ) self.assertEqual(len(stub.calls), 2) self.sleep.assert_called_once_with(2.0) # honored Retry-After - def test_retryable_status_exhausted_raises(self) -> None: + @parameterized.expand( + [("not_found", 404), ("rate_limited", 429), ("unavailable", 503)] + ) + def test_retryable_status_exhausted_raises(self, _name, status) -> None: stub, exc = self._run_expecting_error( - [_msg("u-1")], [_FakeResponse(503), _FakeResponse(503)], max_retries=1 + [_msg("u-1")], [_FakeResponse(status), _FakeResponse(status)], max_retries=1 ) self.assertEqual(len(stub.calls), 2) - self.assertEqual(exc.status, 503) + self.assertEqual(exc.status, status) + self.assertEqual(exc.attempts, 2) + # One 1s backoff between the two attempts, none after the final one. + self.assertEqual([c.args[0] for c in self.sleep.call_args_list], [1]) def test_transport_error_then_success(self) -> None: stub = self._run(