From be5566fa40ebd72fa3818c2a14bfc4214a21c887 Mon Sep 17 00:00:00 2001 From: Bennytimz Date: Fri, 3 Jul 2026 03:22:41 +0100 Subject: [PATCH] feat(client): exponential backoff retry for transient 5xx errors Adds RetryPolicy and wires it into PaymentTransport and Client. RetryPolicy (src/mpp/client/retry.py): - max_attempts=3 with backoff_delays=[0.5, 1.0] seconds by default - Retries on status codes in retryable_status_codes (500/502/503/504) - Optionally retries on httpx.ConnectError / httpx.RemoteProtocolError - Exported from mpp and mpp.client for easy import PaymentTransport: - New retry_policy: RetryPolicy | None = None parameter - _dispatch() helper runs the retry loop before 402 handling so that transient 5xx errors are resolved before the payment flow begins - asyncio.sleep provides the configurable backoff between attempts - Each retry attempt logs a warning with attempt number and URL - Async-generator bodies (guarded by PR #166) propagate before the retry loop, so no unsafe body replay can occur Client: - retry_policy defaults to RetryPolicy() (retry on by default) - Pass retry_policy=None to get raw no-retry behaviour Tests (tests/test_client.py): - test_5xx_retried_with_backoff: 503x2 then 200; verifies sleep(0.5) and sleep(1.0) - test_5xx_exhausted_returns_last_response: max_attempts=2, always 503 - test_connect_error_retried: ConnectError then 200 - test_retry_disabled_when_policy_is_none: PaymentTransport with retry_policy=None - test_non_retryable_5xx_not_retried: 501 passes through in one attempt - test_client_retry_disabled_when_policy_is_none: Client(retry_policy=None) via HTTPXMock --- src/mpp/__init__.py | 1 + src/mpp/client/__init__.py | 1 + src/mpp/client/retry.py | 28 ++++++++++ src/mpp/client/transport.py | 59 +++++++++++++++++++- tests/test_client.py | 108 +++++++++++++++++++++++++++++++++++- 5 files changed, 192 insertions(+), 5 deletions(-) create mode 100644 src/mpp/client/retry.py diff --git a/src/mpp/__init__.py b/src/mpp/__init__.py index 12ac054..b14e84b 100644 --- a/src/mpp/__init__.py +++ b/src/mpp/__init__.py @@ -412,4 +412,5 @@ def success( from . import _body_digest as BodyDigest # noqa: E402 from . import _expires as Expires # noqa: E402 from . import stores # noqa: E402 +from .client.retry import RetryPolicy # noqa: E402 from .store import MemoryStore, Store # noqa: E402 diff --git a/src/mpp/client/__init__.py b/src/mpp/client/__init__.py index 2b000e2..fcbbeb5 100644 --- a/src/mpp/client/__init__.py +++ b/src/mpp/client/__init__.py @@ -17,6 +17,7 @@ """ from mpp import _expires as Expires +from mpp.client.retry import RetryPolicy from mpp.client.transport import Client, PaymentTransport, get, post, request from mpp.events import ( CHALLENGE_RECEIVED, diff --git a/src/mpp/client/retry.py b/src/mpp/client/retry.py new file mode 100644 index 0000000..5bb9d20 --- /dev/null +++ b/src/mpp/client/retry.py @@ -0,0 +1,28 @@ +"""Retry policy for transient errors in PaymentTransport.""" + +from __future__ import annotations + +from dataclasses import dataclass, field + + +@dataclass +class RetryPolicy: + """Exponential backoff retry policy for transient 5xx errors. + + Example: + transport = PaymentTransport( + methods=[tempo(...)], + retry_policy=RetryPolicy(max_attempts=5, backoff_delays=[0.1, 0.5, 1.0, 2.0]), + ) + + # Disable retry entirely: + async with Client(methods=[tempo(...)], retry_policy=None) as client: + response = await client.get(url) + """ + + max_attempts: int = 3 + backoff_delays: list[float] = field(default_factory=lambda: [0.5, 1.0]) + retryable_status_codes: frozenset[int] = field( + default_factory=lambda: frozenset({500, 502, 503, 504}) + ) + retry_on_connect_error: bool = True diff --git a/src/mpp/client/transport.py b/src/mpp/client/transport.py index 3fa83db..7eefcb3 100644 --- a/src/mpp/client/transport.py +++ b/src/mpp/client/transport.py @@ -9,6 +9,7 @@ from __future__ import annotations +import asyncio import logging from datetime import UTC, datetime from typing import TYPE_CHECKING, Any, Protocol, runtime_checkable @@ -17,6 +18,7 @@ from mpp import Challenge, Credential from mpp._parsing import ParseError +from mpp.client.retry import RetryPolicy from mpp.events import ( CHALLENGE_RECEIVED, CREDENTIAL_CREATED, @@ -90,10 +92,12 @@ def __init__( methods: Sequence[Method], inner: httpx.AsyncBaseTransport | None = None, events: EventDispatcher | None = None, + retry_policy: RetryPolicy | None = None, ) -> None: self._methods = {m.name: m for m in methods} self._inner = inner or httpx.AsyncHTTPTransport() self._events = events or EventDispatcher() + self._retry_policy = retry_policy def on(self, name: str, handler: EventHandler) -> Unsubscribe: """Register a client payment event handler.""" @@ -115,9 +119,49 @@ def on_payment_failed(self, handler: EventHandler) -> Unsubscribe: """Register a handler for failed automatic payment handling.""" return self.on(PAYMENT_FAILED, handler) + async def _dispatch(self, request: httpx.Request) -> httpx.Response: + """Dispatch request to inner transport, retrying on transient 5xx errors.""" + if self._retry_policy is None: + return await self._inner.handle_async_request(request) + + policy = self._retry_policy + + for attempt in range(policy.max_attempts): + is_last = attempt == policy.max_attempts - 1 + delay = policy.backoff_delays[min(attempt, len(policy.backoff_delays) - 1)] + + try: + response = await self._inner.handle_async_request(request) + except (httpx.ConnectError, httpx.RemoteProtocolError) as exc: + if not policy.retry_on_connect_error or is_last: + raise + logger.warning( + "Retrying request (attempt %d/%d) after %s %s", + attempt + 1, + policy.max_attempts, + type(exc).__name__, + request.url, + ) + await asyncio.sleep(delay) + continue + + if response.status_code not in policy.retryable_status_codes or is_last: + return response + + logger.warning( + "Retrying request (attempt %d/%d) after %s %s", + attempt + 1, + policy.max_attempts, + response.status_code, + request.url, + ) + await asyncio.sleep(delay) + + raise AssertionError("unreachable: max_attempts must be >= 1") # pragma: no cover + async def handle_async_request(self, request: httpx.Request) -> httpx.Response: """Handle request, automatically retrying on 402 with credentials.""" - response = await self._inner.handle_async_request(request) + response = await self._dispatch(request) if response.status_code != 402: return response @@ -288,8 +332,17 @@ class Client: response = await client.get("https://api.example.com/resource") """ - def __init__(self, methods: Sequence[Method]) -> None: - self._transport = PaymentTransport(methods) + # Sentinel so that Client() defaults to RetryPolicy() while Client(retry_policy=None) disables retry. + _RETRY_DEFAULT: Any = object() + + def __init__( + self, + methods: Sequence[Method], + retry_policy: RetryPolicy | None = _RETRY_DEFAULT, # type: ignore[assignment] + ) -> None: + if retry_policy is Client._RETRY_DEFAULT: + retry_policy = RetryPolicy() + self._transport = PaymentTransport(methods, retry_policy=retry_policy) self._client = httpx.AsyncClient(transport=self._transport) def on(self, name: str, handler: EventHandler) -> Unsubscribe: diff --git a/tests/test_client.py b/tests/test_client.py index 2164399..8a03d73 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1,12 +1,12 @@ """Tests for client-side transport.""" -from unittest.mock import AsyncMock +from unittest.mock import AsyncMock, patch import httpx import pytest from pytest_httpx import HTTPXMock -from mpp import Challenge, Credential +from mpp import Challenge, Credential, RetryPolicy from mpp.client import Client, PaymentTransport, get, post, request from tests import make_credential @@ -430,6 +430,99 @@ async def handle_async_request(self, request: httpx.Request) -> httpx.Response: assert events == ["failed:test-id:RuntimeError"] + @pytest.mark.asyncio + async def test_5xx_retried_with_backoff(self) -> None: + """Should retry on retryable 5xx responses with exponential backoff.""" + inner = MockTransport( + [ + httpx.Response(503), + httpx.Response(503), + httpx.Response(200, content=b'{"data": "ok"}'), + ] + ) + policy = RetryPolicy(max_attempts=3, backoff_delays=[0.5, 1.0]) + transport = PaymentTransport(methods=[], inner=inner, retry_policy=policy) + + with patch("mpp.client.transport.asyncio.sleep", new_callable=AsyncMock) as mock_sleep: + response = await transport.handle_async_request( + httpx.Request("GET", "https://example.com") + ) + + assert response.status_code == 200 + assert len(inner.requests) == 3 + mock_sleep.assert_any_call(0.5) + mock_sleep.assert_any_call(1.0) + + @pytest.mark.asyncio + async def test_5xx_exhausted_returns_last_response(self) -> None: + """Should return the last 5xx response when max_attempts is exhausted.""" + inner = MockTransport( + [ + httpx.Response(503), + httpx.Response(503), + ] + ) + policy = RetryPolicy(max_attempts=2, backoff_delays=[0.5]) + transport = PaymentTransport(methods=[], inner=inner, retry_policy=policy) + + with patch("mpp.client.transport.asyncio.sleep", new_callable=AsyncMock): + response = await transport.handle_async_request( + httpx.Request("GET", "https://example.com") + ) + + assert response.status_code == 503 + assert len(inner.requests) == 2 + + @pytest.mark.asyncio + async def test_connect_error_retried(self) -> None: + """Should retry on ConnectError and succeed if a later attempt returns 200.""" + + class ConnectErrorThenOkTransport(MockTransport): + async def handle_async_request(self, request: httpx.Request) -> httpx.Response: + self.requests.append(request) + if len(self.requests) == 1: + raise httpx.ConnectError("Connection refused") + return httpx.Response(200, content=b'{"data": "ok"}') + + inner = ConnectErrorThenOkTransport([]) + policy = RetryPolicy(max_attempts=3, backoff_delays=[0.1]) + transport = PaymentTransport(methods=[], inner=inner, retry_policy=policy) + + with patch("mpp.client.transport.asyncio.sleep", new_callable=AsyncMock): + response = await transport.handle_async_request( + httpx.Request("GET", "https://example.com") + ) + + assert response.status_code == 200 + assert len(inner.requests) == 2 + + @pytest.mark.asyncio + async def test_retry_disabled_when_policy_is_none(self) -> None: + """Should not retry when retry_policy=None; first 503 is returned immediately.""" + inner = MockTransport([httpx.Response(503)]) + transport = PaymentTransport(methods=[], inner=inner, retry_policy=None) + + response = await transport.handle_async_request( + httpx.Request("GET", "https://example.com") + ) + + assert response.status_code == 503 + assert len(inner.requests) == 1 + + @pytest.mark.asyncio + async def test_non_retryable_5xx_not_retried(self) -> None: + """Should not retry 501 which is not in retryable_status_codes by default.""" + inner = MockTransport([httpx.Response(501)]) + policy = RetryPolicy() + transport = PaymentTransport(methods=[], inner=inner, retry_policy=policy) + + response = await transport.handle_async_request( + httpx.Request("GET", "https://example.com") + ) + + assert response.status_code == 501 + assert len(inner.requests) == 1 + class TestClient: @pytest.mark.asyncio @@ -493,6 +586,17 @@ async def test_on_delegates_to_transport(self) -> None: assert callable(client.on_credential_created(lambda payload: None)) assert callable(client.on_payment_response(lambda payload: None)) + @pytest.mark.asyncio + async def test_client_retry_disabled_when_policy_is_none( + self, httpx_mock: HTTPXMock + ) -> None: + """Client(retry_policy=None) should not retry; first 503 is returned as-is.""" + httpx_mock.add_response(url="https://example.com/test", status_code=503) + + async with Client(methods=[], retry_policy=None) as client: + response = await client.get("https://example.com/test") + assert response.status_code == 503 + class TestConvenienceFunctions: @pytest.mark.asyncio