diff --git a/src/dualentry_cli/client.py b/src/dualentry_cli/client.py index 59e8178..280f9c9 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,8 +14,72 @@ # Status codes that should be retried (transient errors) _RETRYABLE_STATUS_CODES = {429, 502, 503, 504} -_MAX_RETRIES = 3 +# 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) _RETRY_DELAYS = [1, 2, 4] # Exponential backoff: 1s, 2s, 4s +_MAX_RETRIES = len(_RETRY_DELAYS) + +# 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" + +_MAX_RETRY_AFTER = 60 + + +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_HEADER in response.headers + return response.status_code in _RETRYABLE_STATUS_CODES + + +def _server_detail(response: httpx.Response) -> str: + try: + payload = response.json() + except Exception: + return response.text.strip() + errors = payload.get("errors", payload) if isinstance(payload, dict) else payload + if isinstance(errors, dict): + messages = [] + for field, msgs in errors.items(): + if isinstance(msgs, list): + messages.extend(str(msg) for msg in msgs) + else: + messages.append(f"{field}: {msgs}") + return "; ".join(messages) + return str(errors) + + +def _explain(message: str, detail: str) -> str: + return f"{message} Server said: {detail}" if detail else message class APIError(Exception): @@ -46,7 +111,7 @@ def from_env(cls, api_url: str, *, retry: bool = False) -> DualEntryClient: raise ValueError(msg) return cls(api_url=api_url, api_key=api_key, retry=retry) - def _handle_response(self, response: httpx.Response) -> dict: + def _handle_response(self, response: httpx.Response, *, sent_idempotency_key: bool = False) -> dict: status = response.status_code if status == 401: raise APIError(401, "API key is invalid or expired. Run: dualentry auth login") @@ -61,10 +126,30 @@ def _handle_response(self, response: httpx.Response) -> dict: except Exception: errors = response.text raise APIError(422, f"Validation error: {errors}") + if status == 409: + detail = _server_detail(response) + if _RETRY_AFTER_HEADER in response.headers: + wait = _retry_after_seconds(response) + when = f"Retry in {wait}s with the same key." if wait is not None else "Retry shortly with the same key." + raise APIError(409, _explain(f"The first request with this idempotency key is still being processed. {when}", detail)) + if sent_idempotency_key: + raise APIError( + 409, + _explain( + "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.", + detail, + ), + ) + raise APIError(409, detail or "Conflict.") if status == 429: + wait = _retry_after_seconds(response) + if wait is not None: + raise APIError(429, f"Rate limited. Retry after {wait}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.") + wait = _retry_after_seconds(response) + when = f" The server asked to retry after {wait}s." if wait is not None else "" + raise APIError(status, f"Server error ({status}). The API may be temporarily unavailable.{when}") if status >= 400: try: detail = response.json() @@ -83,30 +168,40 @@ def _handle_response(self, response: httpx.Response) -> dict: return response.json() def _request(self, method: str, path: str, **kwargs) -> dict: + method = method.upper() + keyed = method in _IDEMPOTENCY_METHODS + if keyed: + # 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) + return self._handle_response(response, sent_idempotency_key=keyed) # Retry logic with visible feedback - last_error = None - for attempt in range(_MAX_RETRIES): + for attempt, backoff in enumerate(_RETRY_DELAYS): + retry_after = None try: response = self._client.request(method, path, **kwargs) - if response.status_code not in _RETRYABLE_STATUS_CODES: - 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 - - 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) + if not _is_retryable(response): + return self._handle_response(response, sent_idempotency_key=keyed) + retry_after = _retry_after_seconds(response) + if retry_after is not None and retry_after > _MAX_RETRY_AFTER: + return self._handle_response(response, sent_idempotency_key=keyed) + except _RETRYABLE_EXCEPTIONS: + pass + + # every retry waits, including the one after the loop + delay = retry_after if retry_after is not None else backoff + print(f"\033[33mRetrying in {delay}s... (attempt {attempt + 2}/{_MAX_RETRIES + 1})\033[0m", file=sys.stderr) + time.sleep(delay) # Final attempt response = self._client.request(method, path, **kwargs) - return self._handle_response(response) + return self._handle_response(response, sent_idempotency_key=keyed) def get(self, path: str, params: dict[str, Any] | None = None) -> dict: return self._request("GET", path, params=params) @@ -139,6 +234,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..18974a7 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,384 @@ 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 + + +class TestRetryAfterCeilingAndConflictDetail: + BASE = "https://api.dualentry.com/public/v2" + + @pytest.fixture + def sleeps(self, monkeypatch): + 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) + + @pytest.mark.parametrize("unreadable", ["soon", "2.5", "-5", ""]) + @respx.mock + def test_conflict_with_unreadable_retry_after_still_retries(self, sleeps, unreadable): + route = respx.post(f"{self.BASE}/invoices/").mock( + side_effect=[ + httpx.Response(409, headers={"Retry-After": unreadable}, json={}), + httpx.Response(201, json={"internal_id": 5}), + ] + ) + + data = self._client().post("/invoices/", json={}) + + assert data == {"internal_id": 5} + assert route.call_count == 2 + assert sleeps == [1], f"{unreadable!r} should fall back to the backoff, not cancel the retry" + assert len({c.request.headers["Idempotency-Key"] for c in route.calls}) == 1 + + @pytest.mark.parametrize("status", [409, 429, 503]) + @respx.mock + def test_retry_after_beyond_the_ceiling_is_reported_not_slept_through(self, sleeps, status): + from dualentry_cli.client import APIError + + route = respx.post(f"{self.BASE}/invoices/").mock(return_value=httpx.Response(status, headers={"Retry-After": "3600"}, json={})) + + with pytest.raises(APIError) as exc: + self._client().post("/invoices/", json={}) + + assert route.call_count == 1, "no point retrying on a schedule we refuse to wait for" + assert sleeps == [], "the whole point: we never sleep 3600s" + assert exc.value.status_code == status + assert "3600" in exc.value.detail, "the user still needs to know how long the server asked for" + + @respx.mock + def test_retry_after_at_the_ceiling_is_still_honoured(self, sleeps): + from dualentry_cli.client import _MAX_RETRY_AFTER + + respx.post(f"{self.BASE}/invoices/").mock( + side_effect=[ + httpx.Response(429, headers={"Retry-After": str(_MAX_RETRY_AFTER)}, json={}), + httpx.Response(201, json={"internal_id": 6}), + ] + ) + + self._client().post("/invoices/", json={}) + + assert sleeps == [_MAX_RETRY_AFTER], "the ceiling itself is allowed" + + @respx.mock + def test_conflict_without_a_key_keeps_the_server_message(self, sleeps): + from dualentry_cli.client import APIError + + respx.get(f"{self.BASE}/invoices/").mock(return_value=httpx.Response(409, json={"errors": {"__all__": ["period is closed"]}})) + + with pytest.raises(APIError) as exc: + self._client().get("/invoices/") + + assert exc.value.detail == "period is closed" + assert "256 KB" not in exc.value.detail + assert sleeps == [] + + @respx.mock + def test_conflict_on_a_write_keeps_both_the_guidance_and_the_server_message(self): + from dualentry_cli.client import APIError + + respx.post(f"{self.BASE}/invoices/").mock(return_value=httpx.Response(409, json={"errors": {"__all__": ["response cannot be replayed"]}})) + + with pytest.raises(APIError) as exc: + self._client().post("/invoices/", json={}) + + assert "256 KB" in exc.value.detail, "the write did carry a key, so the guidance applies" + assert "response cannot be replayed" in exc.value.detail, "and the server's own words survive" + + def test_backoff_table_and_retry_count_cannot_drift(self): + from dualentry_cli.client import _MAX_RETRIES, _RETRY_DELAYS + + assert len(_RETRY_DELAYS) == _MAX_RETRIES