From 457728dfa53591365289b034aba5c576b2ab57d8 Mon Sep 17 00:00:00 2001 From: Datata1 Date: Sun, 12 Jul 2026 15:14:28 +0200 Subject: [PATCH] feat(client): opt-in retry with backoff honoring Retry-After Implements plans/refactor/07-retry-support.md (final ticket): - RetryConfig frozen dataclass (max_retries=0, backoff_factor=0.5, retry_statuses={429,502,503,504}, retry_methods={GET,HEAD,PUT,DELETE}), exported from the top-level package and accepted by CodesphereSDK(retry=...) - Retry loop in APIHttpClient.request: idempotent methods only by default (POST opt-in via retry_methods), retries on the configured statuses and on httpx timeout/connect errors; delay honors Retry-After (delta-seconds or HTTP-date, past dates clamp to 0) and falls back to exponential backoff with jitter; exhausted retries raise exactly what the non-retry path would (RateLimitError with retry_after populated, TimeoutError, ...); each retry is logged at debug level - Plain ~40-line loop, no retry dependency reintroduced - Default max_retries=0 means zero behavior change unless opted in (tested: single request on failure with the default config) - 17 respx tests: Retry-After honored (429->200 with exactly 2 requests and >=1s sleep), exhaustion, exponential backoff bounds, non-retryable 500, POST gating and opt-in, transport-error retries, SDK wire-through, Retry-After parsing matrix - README documents RetryConfig --- README.md | 24 +++++ src/codesphere/__init__.py | 2 + src/codesphere/client.py | 4 +- src/codesphere/config.py | 21 ++++ src/codesphere/http_client.py | 102 ++++++++++++++++---- tests/test_retry.py | 177 ++++++++++++++++++++++++++++++++++ 6 files changed, 310 insertions(+), 20 deletions(-) create mode 100644 tests/test_retry.py diff --git a/README.md b/README.md index 4c239cd..22e946d 100644 --- a/README.md +++ b/README.md @@ -58,6 +58,30 @@ If no token is provided at all, `CodesphereSDK()` raises an `AuthenticationError` at construction (importing the package never requires any environment variable). +### Retries + +Retries are off by default. Opt in with a `RetryConfig`: + +```python +from codesphere import CodesphereSDK, RetryConfig + +sdk = CodesphereSDK( + token="your-api-token", + retry=RetryConfig(max_retries=3), +) +``` + +Requests using an idempotent method (`GET`, `HEAD`, `PUT`, `DELETE`) are then +retried on `429`, `502`, `503`, and `504` responses as well as on +connect/timeout errors. The delay honors the server's `Retry-After` header +(seconds or HTTP-date) when present, otherwise exponential backoff with +jitter (`backoff_factor * 2**attempt`). `POST` is never retried unless you +add it to `retry_methods` explicitly: + +```python +RetryConfig(max_retries=3, retry_methods=frozenset({"GET", "POST"})) +``` + ## Quick Start ```python diff --git a/src/codesphere/__init__.py b/src/codesphere/__init__.py index a312e4b..c62a026 100644 --- a/src/codesphere/__init__.py +++ b/src/codesphere/__init__.py @@ -22,6 +22,7 @@ import logging from .client import CodesphereSDK +from .config import RetryConfig from .exceptions import ( APIError, AuthenticationError, @@ -75,6 +76,7 @@ "NetworkError", "NotFoundError", "RateLimitError", + "RetryConfig", # Resources "Team", "TeamBase", diff --git a/src/codesphere/client.py b/src/codesphere/client.py index 1da6d7f..30ada2a 100644 --- a/src/codesphere/client.py +++ b/src/codesphere/client.py @@ -3,7 +3,7 @@ import httpx from pydantic import SecretStr -from .config import Settings +from .config import RetryConfig, Settings from .exceptions import AuthenticationError from .http_client import APIHttpClient from .resources.metadata import MetadataResource @@ -30,6 +30,7 @@ def __init__( token: str | SecretStr | None = None, base_url: str | None = None, timeout: httpx.Timeout | None = None, + retry: RetryConfig | None = None, http_client: APIHttpClient | None = None, ): if http_client is not None: @@ -53,6 +54,7 @@ def __init__( token=resolved_token, base_url=resolved_base_url, timeout=resolved_timeout, + retry=retry, ) self.teams = TeamsResource(self._http_client) diff --git a/src/codesphere/config.py b/src/codesphere/config.py index 250e788..242e556 100644 --- a/src/codesphere/config.py +++ b/src/codesphere/config.py @@ -1,9 +1,30 @@ +from dataclasses import dataclass + from pydantic import HttpUrl, SecretStr from pydantic_settings import BaseSettings, SettingsConfigDict DEFAULT_BASE_URL = "https://codesphere.com/api" +@dataclass(frozen=True, slots=True) +class RetryConfig: + """Opt-in retry behavior for transient failures. + + Disabled by default (``max_retries=0``). When enabled, requests using a + method in ``retry_methods`` are retried on the statuses in + ``retry_statuses`` and on connect/timeout errors. The delay honors a + ``Retry-After`` response header (seconds or HTTP-date) when present, + otherwise exponential backoff with jitter is used. Only idempotent + methods are retried by default; add ``"POST"`` to ``retry_methods`` + explicitly if your endpoints tolerate it. + """ + + max_retries: int = 0 + backoff_factor: float = 0.5 + retry_statuses: frozenset[int] = frozenset({429, 502, 503, 504}) + retry_methods: frozenset[str] = frozenset({"GET", "HEAD", "PUT", "DELETE"}) + + class Settings(BaseSettings): """Environment-driven defaults (``CS_*`` variables, optional ``.env``). diff --git a/src/codesphere/http_client.py b/src/codesphere/http_client.py index 7f2e102..46057c5 100644 --- a/src/codesphere/http_client.py +++ b/src/codesphere/http_client.py @@ -1,28 +1,54 @@ +import asyncio import logging +import random from contextlib import AbstractAsyncContextManager +from datetime import UTC, datetime +from email.utils import parsedate_to_datetime from types import TracebackType from typing import Any, cast import httpx from pydantic import SecretStr +from .config import RetryConfig from .exceptions import NetworkError, TimeoutError, raise_for_status log = logging.getLogger(__name__) +def _parse_retry_after(value: str | None) -> float | None: + """Parse a Retry-After header: delta-seconds or an HTTP-date.""" + if not value: + return None + if value.isdigit(): + return float(value) + try: + retry_at = parsedate_to_datetime(value) + except (TypeError, ValueError): + return None + return max(0.0, (retry_at - datetime.now(UTC)).total_seconds()) + + class APIHttpClient: """Thin async transport around httpx. Receives its full configuration from the caller (the SDK resolves arguments, environment variables, and defaults) and owns nothing but - the connection lifecycle and error mapping. + the connection lifecycle, error mapping, and opt-in retries. """ - def __init__(self, *, token: SecretStr, base_url: str, timeout: httpx.Timeout): + def __init__( + self, + *, + token: SecretStr, + base_url: str, + timeout: httpx.Timeout, + retry: RetryConfig | None = None, + ): self._token = token.get_secret_value() self._base_url = base_url self._client: httpx.AsyncClient | None = None + self._retry = retry if retry is not None else RetryConfig() self._timeout_config = timeout self._client_config: dict[str, Any] = { @@ -92,28 +118,66 @@ async def request( log.debug(f"Request: {method} {endpoint}") log.debug(f"Request kwargs: {kwargs}") - try: - response = await client.request(method, endpoint, **kwargs) + retryable_method = method.upper() in self._retry.retry_methods + attempts = self._retry.max_retries + 1 if retryable_method else 1 + + for attempt in range(attempts): + last_attempt = attempt == attempts - 1 + try: + response = await client.request(method, endpoint, **kwargs) + except httpx.TimeoutException as e: + if not last_attempt: + await self._sleep_before_retry(attempt, None, method, endpoint) + continue + log.error(f"Request timeout for {method} {endpoint}: {e}") + raise TimeoutError(f"Request to {endpoint} timed out.") from e + except httpx.ConnectError as e: + if not last_attempt: + await self._sleep_before_retry(attempt, None, method, endpoint) + continue + log.error(f"Connection error for {method} {endpoint}: {e}") + raise NetworkError( + f"Failed to connect to the API: {e}", + original_error=e, + ) from e + except httpx.RequestError as e: + log.error(f"Network error for {method} {endpoint}: {e}") + raise NetworkError( + f"A network error occurred: {e}", + original_error=e, + ) from e + log.debug( f"Response: {response.status_code} {response.reason_phrase} " f"for {method} {endpoint}" ) + if response.status_code in self._retry.retry_statuses and not last_attempt: + await self._sleep_before_retry(attempt, response, method, endpoint) + continue + raise_for_status(response) return response - except httpx.TimeoutException as e: - log.error(f"Request timeout for {method} {endpoint}: {e}") - raise TimeoutError(f"Request to {endpoint} timed out.") from e - except httpx.ConnectError as e: - log.error(f"Connection error for {method} {endpoint}: {e}") - raise NetworkError( - f"Failed to connect to the API: {e}", - original_error=e, - ) from e - except httpx.RequestError as e: - log.error(f"Network error for {method} {endpoint}: {e}") - raise NetworkError( - f"A network error occurred: {e}", - original_error=e, - ) from e + raise AssertionError("unreachable: retry loop must return or raise") + + async def _sleep_before_retry( + self, + attempt: int, + response: httpx.Response | None, + method: str, + endpoint: str, + ) -> None: + delay = None + if response is not None: + delay = _parse_retry_after(response.headers.get("Retry-After")) + if delay is None: + delay = self._retry.backoff_factor * (2**attempt) + # Jitter to avoid thundering herds; not security-relevant. + delay += random.uniform(0, self._retry.backoff_factor / 2) # nosec B311 + + log.debug( + f"Retrying {method} {endpoint} in {delay:.2f}s " + f"(attempt {attempt + 1}/{self._retry.max_retries})" + ) + await asyncio.sleep(delay) diff --git a/tests/test_retry.py b/tests/test_retry.py new file mode 100644 index 0000000..b0058f0 --- /dev/null +++ b/tests/test_retry.py @@ -0,0 +1,177 @@ +"""Opt-in retry behavior (refactor ticket 07), tested against respx.""" + +from unittest.mock import AsyncMock, patch + +import httpx +import pytest +import respx +from pydantic import SecretStr + +from codesphere import APIError, CodesphereSDK, RateLimitError, RetryConfig +from codesphere.exceptions import TimeoutError +from codesphere.http_client import APIHttpClient, _parse_retry_after + +BASE = "https://retry.test/api" + + +def make_transport(retry: RetryConfig | None = None) -> APIHttpClient: + return APIHttpClient( + token=SecretStr("t"), + base_url=BASE, + timeout=httpx.Timeout(5.0), + retry=retry, + ) + + +@pytest.fixture +def api(): + with respx.mock(base_url=BASE, assert_all_called=False) as router: + yield router + + +@pytest.fixture +def sleep_mock(): + with patch("asyncio.sleep", new_callable=AsyncMock) as mock: + yield mock + + +class TestRetryOnStatus: + async def test_429_then_success_honors_retry_after(self, api, sleep_mock): + route = api.get("/x") + route.side_effect = [ + httpx.Response(429, headers={"Retry-After": "1"}), + httpx.Response(200, json={"ok": True}), + ] + async with make_transport(RetryConfig(max_retries=2)) as transport: + response = await transport.request("GET", "/x") + + assert response.status_code == 200 + assert route.call_count == 2 + assert sleep_mock.await_count == 1 + assert sleep_mock.await_args.args[0] >= 1.0 + + async def test_exhausted_retries_raise_rate_limit_with_retry_after( + self, api, sleep_mock + ): + route = api.get("/x").respond(429, headers={"Retry-After": "7"}, json={}) + async with make_transport(RetryConfig(max_retries=2)) as transport: + with pytest.raises(RateLimitError) as exc_info: + await transport.request("GET", "/x") + + assert exc_info.value.retry_after == 7 + assert route.call_count == 3 # initial + 2 retries + assert sleep_mock.await_count == 2 + + async def test_backoff_grows_exponentially_without_header(self, api, sleep_mock): + route = api.get("/x").respond(503, json={}) + config = RetryConfig(max_retries=2, backoff_factor=0.5) + async with make_transport(config) as transport: + with pytest.raises(APIError): + await transport.request("GET", "/x") + + assert route.call_count == 3 + delays = [call.args[0] for call in sleep_mock.await_args_list] + assert 0.5 <= delays[0] < 0.75 # 0.5 * 2**0 (+ jitter < 0.25) + assert 1.0 <= delays[1] < 1.25 # 0.5 * 2**1 (+ jitter) + + async def test_non_retryable_status_fails_immediately(self, api, sleep_mock): + route = api.get("/x").respond(500, json={}) + async with make_transport(RetryConfig(max_retries=3)) as transport: + with pytest.raises(APIError): + await transport.request("GET", "/x") + + assert route.call_count == 1 # 500 not in default retry_statuses + sleep_mock.assert_not_awaited() + + +class TestRetryMethodGating: + async def test_post_is_not_retried_by_default(self, api, sleep_mock): + route = api.post("/x") + route.side_effect = [httpx.Response(502), httpx.Response(200)] + async with make_transport(RetryConfig(max_retries=3)) as transport: + with pytest.raises(APIError): + await transport.request("POST", "/x") + + assert route.call_count == 1 + sleep_mock.assert_not_awaited() + + async def test_post_can_be_opted_in(self, api, sleep_mock): + route = api.post("/x") + route.side_effect = [httpx.Response(502), httpx.Response(200)] + config = RetryConfig( + max_retries=3, + retry_methods=frozenset({"GET", "HEAD", "PUT", "DELETE", "POST"}), + ) + async with make_transport(config) as transport: + response = await transport.request("POST", "/x") + + assert response.status_code == 200 + assert route.call_count == 2 + + +class TestRetryOnTransportErrors: + async def test_connect_timeout_is_retried(self, api, sleep_mock): + route = api.get("/x") + route.side_effect = [ + httpx.ConnectTimeout("slow"), + httpx.Response(200, json={}), + ] + async with make_transport(RetryConfig(max_retries=1)) as transport: + response = await transport.request("GET", "/x") + + assert response.status_code == 200 + assert route.call_count == 2 + + async def test_exhausted_timeouts_raise_sdk_timeout(self, api, sleep_mock): + api.get("/x").mock(side_effect=httpx.ConnectTimeout("slow")) + async with make_transport(RetryConfig(max_retries=1)) as transport: + with pytest.raises(TimeoutError): + await transport.request("GET", "/x") + + assert sleep_mock.await_count == 1 + + +class TestDefaultsAreOff: + async def test_default_config_performs_zero_retries(self, api, sleep_mock): + route = api.get("/x").respond(503, json={}) + async with make_transport() as transport: # no RetryConfig + with pytest.raises(APIError): + await transport.request("GET", "/x") + + assert route.call_count == 1 + sleep_mock.assert_not_awaited() + + async def test_sdk_wires_retry_through(self, api, sleep_mock): + route = api.get("/teams") + route.side_effect = [ + httpx.Response(429, headers={"Retry-After": "2"}), + httpx.Response(200, json=[]), + ] + sdk = CodesphereSDK(token="t", base_url=BASE, retry=RetryConfig(max_retries=1)) + async with sdk: + teams = await sdk.teams.list() + + assert teams == [] + assert route.call_count == 2 + assert sleep_mock.await_args.args[0] >= 2.0 + + +class TestParseRetryAfter: + def test_delta_seconds(self): + assert _parse_retry_after("12") == 12.0 + + def test_http_date_in_the_future(self): + from datetime import UTC, datetime, timedelta + from email.utils import format_datetime + + future = datetime.now(UTC) + timedelta(seconds=30) + delay = _parse_retry_after(format_datetime(future, usegmt=True)) + assert delay is not None + assert 28.0 < delay <= 30.0 + + def test_http_date_in_the_past_clamps_to_zero(self): + assert _parse_retry_after("Mon, 01 Jan 2001 00:00:00 GMT") == 0.0 + + @pytest.mark.parametrize("value", [None, "", "soon", "-5"]) + def test_unparseable_values_return_none(self, value): + assert _parse_retry_after(value) is None