diff --git a/src/dualentry_cli/client.py b/src/dualentry_cli/client.py index 59e8178..9c5fbf0 100644 --- a/src/dualentry_cli/client.py +++ b/src/dualentry_cli/client.py @@ -5,6 +5,7 @@ import os import sys import time +import uuid from typing import Any import httpx @@ -13,9 +14,50 @@ # Status codes that should be retried (transient errors) _RETRYABLE_STATUS_CODES = {429, 502, 503, 504} +# Transient transport failures. Deliberately excludes LocalProtocolError, +# UnsupportedProtocol, DecodingError and TooManyRedirects: those fail the same +# way every time, so retrying only delays the error the user needs to see. +_RETRYABLE_EXCEPTIONS = (httpx.TimeoutException, httpx.NetworkError, httpx.RemoteProtocolError) _MAX_RETRIES = 3 _RETRY_DELAYS = [1, 2, 4] # Exponential backoff: 1s, 2s, 4s +# The API replays the original response for a repeated Idempotency-Key instead of +# running the operation again, so a retried write cannot create a duplicate record. +# https://docs.dualentry.com/developers/release-notes/2026-08-12 +_IDEMPOTENCY_HEADER = "Idempotency-Key" +_IDEMPOTENCY_METHODS = frozenset({"POST", "PUT", "PATCH", "DELETE"}) + +# 429 and the in-flight 409 both report exactly how long to wait. +# https://docs.dualentry.com/developers/guides/rate-limiting +_RETRY_AFTER_HEADER = "Retry-After" + + +def _retry_after_seconds(response: httpx.Response) -> int | None: + """Seconds from the Retry-After header, or None if absent or unusable.""" + raw = response.headers.get(_RETRY_AFTER_HEADER) + if raw is None: + return None + try: + # RFC 9110 delay-seconds is a non-negative integer + seconds = int(raw.strip()) + except (TypeError, ValueError): + return None + return seconds if seconds >= 0 else None + + +def _is_retryable(response: httpx.Response) -> bool: + """ + Whether this response should be retried with the same idempotency key. + + 409 means two different things, told apart by Retry-After: + with the header the first request is still running and we should retry; + without it the original response was too large to store, the write did not run again + https://docs.dualentry.com/developers/guides/idempotency-and-write-validation + """ + if response.status_code == 409: + return _retry_after_seconds(response) is not None + return response.status_code in _RETRYABLE_STATUS_CODES + class APIError(Exception): def __init__(self, status_code: int, detail: str): @@ -61,7 +103,17 @@ def _handle_response(self, response: httpx.Response) -> dict: except Exception: errors = response.text raise APIError(422, f"Validation error: {errors}") + if status == 409: + wait = _retry_after_seconds(response) + if wait is not None: + raise APIError(409, f"The first request with this idempotency key is still being processed. Retry in {wait:g}s with the same key.") + raise APIError( + 409, "The original response is too large to replay (over 256 KB). The write was not repeated - check whether the record already exists before sending it again." + ) if status == 429: + wait = _retry_after_seconds(response) + if wait is not None: + raise APIError(429, f"Rate limited. Retry after {wait:g}s.") raise APIError(429, "Rate limited. Please wait and try again.") if status >= 500: raise APIError(status, f"Server error ({status}). The API may be temporarily unavailable.") @@ -83,26 +135,33 @@ def _handle_response(self, response: httpx.Response) -> dict: return response.json() def _request(self, method: str, path: str, **kwargs) -> dict: + method = method.upper() + if method in _IDEMPOTENCY_METHODS: + # One key per logical request, deliberately generated here rather than + # per attempt: reusing it across retries is what makes a retry safe. + headers = dict(kwargs.pop("headers", None) or {}) + headers.setdefault(_IDEMPOTENCY_HEADER, str(uuid.uuid4())) + kwargs["headers"] = headers + if not self._retry: response = self._client.request(method, path, **kwargs) return self._handle_response(response) # Retry logic with visible feedback - last_error = None for attempt in range(_MAX_RETRIES): + retry_after = None try: response = self._client.request(method, path, **kwargs) - if response.status_code not in _RETRYABLE_STATUS_CODES: + if not _is_retryable(response): return self._handle_response(response) - # Retryable error - will retry - last_error = APIError(response.status_code, f"Temporary error ({response.status_code})") - except httpx.RequestError as e: - last_error = e + retry_after = _retry_after_seconds(response) + except _RETRYABLE_EXCEPTIONS: + pass - if attempt < _MAX_RETRIES - 1: - delay = _RETRY_DELAYS[attempt] - print(f"\033[33mRetrying in {delay}s... (attempt {attempt + 2}/{_MAX_RETRIES})\033[0m", file=sys.stderr) - time.sleep(delay) + # every retry waits, including the one after the loop + delay = retry_after if retry_after is not None else _RETRY_DELAYS[attempt] + print(f"\033[33mRetrying in {delay:g}s... (attempt {attempt + 2}/{_MAX_RETRIES + 1})\033[0m", file=sys.stderr) + time.sleep(delay) # Final attempt response = self._client.request(method, path, **kwargs) @@ -139,6 +198,9 @@ def post(self, path: str, json: dict[str, Any] | None = None) -> dict: def put(self, path: str, json: dict[str, Any] | None = None) -> dict: return self._request("PUT", path, json=json) + def patch(self, path: str, json: dict[str, Any] | None = None) -> dict: + return self._request("PATCH", path, json=json) + def delete(self, path: str) -> dict: return self._request("DELETE", path) diff --git a/tests/test_client.py b/tests/test_client.py index a26ba2e..675a0d5 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1,3 +1,6 @@ +import uuid +from types import SimpleNamespace + import httpx import pytest import respx @@ -117,3 +120,291 @@ def test_context_manager_closes_client(self): with DualEntryClient(api_url="https://api.dualentry.com", api_key="test_key") as client: assert client._client is not None assert client._client.is_closed + + +class TestIdempotencyKey: + """ + Writes carry an Idempotency-Key so a retry cannot duplicate a record. + + The API replays the original response for a repeated key rather than running + the operation again: https://docs.dualentry.com/developers/release-notes/2026-08-12 + """ + + BASE = "https://api.dualentry.com/public/v2" + + @pytest.fixture + def no_backoff(self, monkeypatch): + """Collapse the retry backoff so retry tests stay fast.""" + monkeypatch.setattr("dualentry_cli.client._RETRY_DELAYS", [0, 0, 0]) + + @staticmethod + def _client(*, retry=False): + from dualentry_cli.client import DualEntryClient + + return DualEntryClient(api_url="https://api.dualentry.com", api_key="test_key", retry=retry) + + @pytest.mark.parametrize( + ("method", "call"), + [ + ("post", lambda c: c.post("/invoices/", json={"customer_id": 1})), + ("put", lambda c: c.put("/invoices/1/", json={"memo": "x"})), + ("patch", lambda c: c.patch("/customer-payments/1/", json={"memo": "x"})), + ("delete", lambda c: c.delete("/invoices/1/")), + ], + ) + @respx.mock + def test_write_methods_send_an_idempotency_key(self, method, call): + route = getattr(respx, method)(url__startswith=self.BASE).mock(return_value=httpx.Response(200, json={"ok": True})) + + call(self._client()) + + key = route.calls[0].request.headers.get("Idempotency-Key") + assert key is not None, f"{method.upper()} must send an Idempotency-Key" + # Documented as "a unique value (a UUID works well)", max length 255. + assert uuid.UUID(key) + assert len(key) <= 255 + + @respx.mock + def test_get_does_not_send_an_idempotency_key(self): + route = respx.get(f"{self.BASE}/invoices/").mock(return_value=httpx.Response(200, json={"items": [], "count": 0})) + + self._client().get("/invoices/") + + assert "Idempotency-Key" not in route.calls[0].request.headers + + @pytest.mark.usefixtures("no_backoff") + @respx.mock + def test_retry_reuses_the_same_key_across_attempts(self): + """The whole point: a retried POST must not create a second record.""" + route = respx.post(f"{self.BASE}/invoices/").mock( + side_effect=[ + httpx.Response(502, text="bad gateway"), + httpx.Response(201, json={"internal_id": 1}), + ] + ) + + data = self._client(retry=True).post("/invoices/", json={"customer_id": 1}) + + assert data == {"internal_id": 1} + assert route.call_count == 2 + keys = {c.request.headers["Idempotency-Key"] for c in route.calls} + assert len(keys) == 1, f"retry must reuse the original key, got {keys}" + + @pytest.mark.usefixtures("no_backoff") + @respx.mock + def test_every_retry_attempt_carries_the_key(self): + from dualentry_cli.client import APIError + + route = respx.post(f"{self.BASE}/invoices/").mock(return_value=httpx.Response(502, text="bad gateway")) + + with pytest.raises(APIError): + self._client(retry=True).post("/invoices/", json={"customer_id": 1}) + + # 4, not 3: the loop runs _MAX_RETRIES times and then issues one more + # request after it. That off-by-one is tracked separately; it is harmless + # here precisely because every attempt replays the same key. + assert route.call_count == 4 + keys = {c.request.headers["Idempotency-Key"] for c in route.calls} + assert len(keys) == 1, f"every attempt must reuse one key, got {keys}" + + @respx.mock + def test_separate_requests_use_different_keys(self): + route = respx.post(f"{self.BASE}/invoices/").mock(return_value=httpx.Response(201, json={"internal_id": 1})) + client = self._client() + + client.post("/invoices/", json={"customer_id": 1}) + client.post("/invoices/", json={"customer_id": 2}) + + keys = [c.request.headers["Idempotency-Key"] for c in route.calls] + assert keys[0] != keys[1], "each logical request needs its own key" + + @respx.mock + def test_caller_supplied_key_is_not_overwritten(self): + route = respx.post(f"{self.BASE}/invoices/").mock(return_value=httpx.Response(201, json={"internal_id": 1})) + + self._client()._request("POST", "/invoices/", json={}, headers={"Idempotency-Key": "caller-supplied-key"}) + + assert route.calls[0].request.headers["Idempotency-Key"] == "caller-supplied-key" + + @respx.mock + def test_key_is_sent_even_when_retry_is_disabled(self): + """Protects against retries outside our control (proxies, user re-runs are new keys).""" + route = respx.post(f"{self.BASE}/invoices/").mock(return_value=httpx.Response(201, json={"internal_id": 1})) + + self._client(retry=False).post("/invoices/", json={}) + + assert "Idempotency-Key" in route.calls[0].request.headers + + +class TestRetryAfterAndConflicts: + """ + Retry timing follows the server, and the two meanings of 409 are separated. + + https://docs.dualentry.com/developers/guides/rate-limiting + https://docs.dualentry.com/developers/guides/idempotency-and-write-validation + """ + + BASE = "https://api.dualentry.com/public/v2" + + @pytest.fixture + def sleeps(self, monkeypatch): + """Record what the client would sleep, without actually sleeping.""" + recorded = [] + monkeypatch.setattr("dualentry_cli.client.time", SimpleNamespace(sleep=recorded.append)) + return recorded + + @staticmethod + def _client(): + from dualentry_cli.client import DualEntryClient + + return DualEntryClient(api_url="https://api.dualentry.com", api_key="test_key", retry=True) + + @respx.mock + def test_conflict_with_retry_after_is_retried(self, sleeps): + """A 409 with Retry-After means the first request is still running, so retry with the same key.""" + route = respx.post(f"{self.BASE}/invoices/").mock( + side_effect=[ + httpx.Response(409, headers={"Retry-After": "2"}, json={"errors": {"__all__": ["still processing"]}}), + httpx.Response(201, json={"internal_id": 7}), + ] + ) + + data = self._client().post("/invoices/", json={"customer_id": 1}) + + assert data == {"internal_id": 7} + assert route.call_count == 2 + assert sleeps == [2], "must wait exactly as long as Retry-After says" + keys = {c.request.headers["Idempotency-Key"] for c in route.calls} + assert len(keys) == 1, "the retry must reuse the original key" + + @respx.mock + def test_conflict_without_retry_after_is_not_retried(self, sleeps): + """A 409 without Retry-After means the response was too large to replay; the write did not run again.""" + from dualentry_cli.client import APIError + + route = respx.post(f"{self.BASE}/invoices/").mock(return_value=httpx.Response(409, json={"errors": {"__all__": ["original response cannot be replayed"]}})) + + with pytest.raises(APIError) as exc: + self._client().post("/invoices/", json={"customer_id": 1}) + + assert route.call_count == 1 + assert sleeps == [] + assert exc.value.status_code == 409 + assert "256 KB" in exc.value.detail + + @respx.mock + def test_rate_limit_waits_for_retry_after_not_the_hardcoded_backoff(self, sleeps): + """On 429 the server says how long to wait, and that wins over _RETRY_DELAYS.""" + respx.get(f"{self.BASE}/invoices/").mock( + side_effect=[ + httpx.Response(429, headers={"Retry-After": "7"}, json={"errors": {"__all__": ["slow down"]}}), + httpx.Response(200, json={"items": [], "count": 0}), + ] + ) + + self._client().get("/invoices/") + + assert sleeps == [7], "Retry-After must win over _RETRY_DELAYS[0] (1s)" + + @respx.mock + def test_rate_limit_without_retry_after_falls_back_to_backoff(self, sleeps): + """Without the header there is nothing to follow, so the exponential backoff is used.""" + from dualentry_cli.client import APIError + + respx.get(f"{self.BASE}/invoices/").mock(return_value=httpx.Response(429, json={"errors": {"__all__": ["slow down"]}})) + + with pytest.raises(APIError): + self._client().get("/invoices/") + + assert sleeps == [1, 2, 4], "no header, so use the exponential backoff" + + @respx.mock + def test_the_last_attempt_also_waits_for_retry_after(self, sleeps): + """Every request after a failure waits, including the final one sent after the loop.""" + from dualentry_cli.client import APIError + + route = respx.get(f"{self.BASE}/invoices/").mock(return_value=httpx.Response(429, headers={"Retry-After": "3"}, json={})) + + with pytest.raises(APIError): + self._client().get("/invoices/") + + assert sleeps == [3, 3, 3], "the request after the loop must wait too" + assert route.call_count == len(sleeps) + 1 + + @pytest.mark.parametrize("bad_value", ["next tuesday", "inf", "Infinity", "1e9", "2.5", "-5", ""]) + @respx.mock + def test_unparsable_retry_after_falls_back_to_backoff(self, sleeps, bad_value): + """Values int() cannot use fall back to the backoff; "inf" must never reach time.sleep().""" + respx.get(f"{self.BASE}/invoices/").mock( + side_effect=[ + httpx.Response(429, headers={"Retry-After": bad_value}, json={}), + httpx.Response(200, json={"items": [], "count": 0}), + ] + ) + + self._client().get("/invoices/") + + assert sleeps == [1], f"{bad_value!r} should fall back to the backoff" + + @pytest.mark.parametrize( + "error", + [ + httpx.ConnectTimeout, + httpx.ReadTimeout, + httpx.WriteTimeout, + httpx.PoolTimeout, + httpx.ConnectError, + httpx.ReadError, + httpx.WriteError, + httpx.CloseError, + httpx.RemoteProtocolError, + ], + ) + @respx.mock + def test_transient_transport_error_is_retried(self, sleeps, error): + """These can succeed on a second attempt, so they are retried and then propagate.""" + route = respx.get(f"{self.BASE}/invoices/").mock(side_effect=error("boom")) + + with pytest.raises(error): + self._client().get("/invoices/") + + assert route.call_count == 4 + assert sleeps == [1, 2, 4] + + @pytest.mark.parametrize( + "error", + [ + httpx.LocalProtocolError, + httpx.UnsupportedProtocol, + httpx.ProxyError, + httpx.DecodingError, + httpx.TooManyRedirects, + ], + ) + @respx.mock + def test_non_transient_transport_error_is_not_retried(self, sleeps, error): + """These fail the same way every time, so they are reported at once.""" + route = respx.get(f"{self.BASE}/invoices/").mock(side_effect=error("broken")) + + with pytest.raises(error): + self._client().get("/invoices/") + + assert route.call_count == 1 + assert sleeps == [] + + @pytest.mark.usefixtures("sleeps") + @respx.mock + def test_storage_unavailable_is_retried_with_the_same_key(self): + """A 503 from idempotency storage should be retried with the same key, as the guide asks.""" + route = respx.post(f"{self.BASE}/invoices/").mock( + side_effect=[ + httpx.Response(503, json={"errors": {"__all__": ["retry with the same key"]}}), + httpx.Response(201, json={"internal_id": 9}), + ] + ) + + data = self._client().post("/invoices/", json={"customer_id": 1}) + + assert data == {"internal_id": 9} + keys = {c.request.headers["Idempotency-Key"] for c in route.calls} + assert len(keys) == 1