Skip to content

Commit a45e0eb

Browse files
authored
fix!: validate retry and budget config, case-insensitive retry_methods, Retry-After give-up event (#177)
* fix!: validate Retry, AsyncRetry and RetryBudget config at construction * fix: match retry_methods case-insensitively * fix: emit retry.giving_up when Retry-After exceeds max_delay * fix: treat a Retry-After integer too large for a float as an infinite wait
1 parent a0c1a87 commit a45e0eb

8 files changed

Lines changed: 264 additions & 29 deletions

File tree

‎docs/observability.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ The logger and event names below are a stable public contract:
1313

1414
Each log record has an `event` field holding the event name, such as `event="circuit.opened"`, which you can filter on in your log aggregator. `circuit.rejected` is emitted at most once per second per circuit, so an open circuit under heavy traffic does not flood your logs. The first rejection of each outage is always emitted, `suppressed` counts the rejections left out since the previous one, and `circuit.closed` reports the outage's total in `rejected`. Events from `AsyncKeyedCircuitBreaker` and `KeyedCircuitBreaker` also carry `circuit_key`, the circuit they belong to. [Resilience](resilience.md) describes when each event fires.
1515

16+
`retry.giving_up` fires when a retryable failure is re-raised without another attempt, and its `reason` attribute says why. `max_attempts` means every attempt failed. `retry_after_exceeds_max_delay` means the server's `Retry-After` asked for a longer wait than `max_delay` allows; these events also carry `retry_after` and `max_delay` in seconds, and `retry_after` is `inf` when the header held an integer too large to fit in a float. Both kinds carry `attempts`, `method`, `url`, `last_status` and `last_exception_type`.
17+
1618
Events that carry a `url` attribute get it redacted first: httpware strips `user:pass@` userinfo and replaces every query-parameter value with `REDACTED`, keeping the keys. A URL logged as `https://api.test/items?page=REDACTED&sort=REDACTED` came from `https://api.test/items?page=2&sort=name`.
1719

1820
```python

‎docs/resilience.md‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,11 @@ from httpware.middleware.resilience import AsyncRetry
2424
| Parameter | Default | Effect |
2525
|---|---|---|
2626
| `max_attempts` | `3` | Total tries, including the first. `1` disables retries; `<1` raises `ValueError`. |
27-
| `base_delay` | `0.1` (s) | Floor for the full-jitter exponential backoff. |
28-
| `max_delay` | `5.0` (s) | Ceiling for backoff. |
27+
| `base_delay` | `0.1` (s) | Floor for the full-jitter exponential backoff. Must be `>= 0`; a negative or NaN value raises `ValueError`. |
28+
| `max_delay` | `5.0` (s) | Ceiling for backoff and the longest `Retry-After` honored. Must be finite and `>= 0`; a negative, NaN or infinite value raises `ValueError`. |
2929
| `retry_status_codes` | `frozenset({408, 429, 502, 503, 504})` | Status codes considered retryable. |
30-
| `retry_methods` | `frozenset({"GET", "HEAD", "OPTIONS", "PUT", "DELETE"})` | Idempotent methods only. To retry POST, pass a frozenset that includes `"POST"`. |
31-
| `respect_retry_after` | `True` | When a retryable response has a `Retry-After` header, wait that long instead of the jittered backoff. If the value is over `max_delay`, `AsyncRetry` gives up and re-raises the `StatusError` with a PEP 678 note: `httpware: Retry-After (Ns) exceeded max_delay (Ms); giving up`. To avoid this, set `respect_retry_after=False` or raise `max_delay`. |
30+
| `retry_methods` | `frozenset({"GET", "HEAD", "OPTIONS", "PUT", "DELETE"})` | Idempotent methods only. To retry POST, pass a frozenset that includes `"POST"`. Matching ignores case, so `"post"` works too. |
31+
| `respect_retry_after` | `True` | When a retryable response has a `Retry-After` header, wait that long instead of the jittered backoff. If the value is over `max_delay`, `AsyncRetry` gives up, emits `retry.giving_up` with `reason="retry_after_exceeds_max_delay"`, and re-raises the `StatusError` with a PEP 678 note: `httpware: Retry-After (Ns) exceeded max_delay (Ms); giving up`. To avoid this, set `respect_retry_after=False` or raise `max_delay`. |
3232
| `budget` | `RetryBudget()` | The token bucket. Pass one shared `RetryBudget` to several clients to give them a joint budget. |
3333

3434
To bound the total time across all attempts, put [`AsyncTimeout`](#asynctimeout) first in the chain. To bound a single request, set `httpx2.Timeout` on the client or pass `timeout=` per request.
@@ -40,7 +40,7 @@ To bound the total time across all attempts, put [`AsyncTimeout`](#asynctimeout)
4040
- integer seconds: `Retry-After: 30` waits 30 seconds.
4141
- an HTTP date (RFC 5322): `Retry-After: Wed, 21 Oct 2026 07:28:00 GMT` waits until that time, or not at all if it has passed.
4242

43-
Either form over `max_delay` makes `AsyncRetry` give up as described above. Negative integers count as 0. Malformed values are ignored and the jittered backoff is used instead.
43+
Either form over `max_delay` makes `AsyncRetry` give up as described above. An integer too large to fit in a float counts as an infinite wait, so it always exceeds `max_delay`. Negative integers count as 0. Malformed values are ignored and the jittered backoff is used instead.
4444

4545
### Streaming-body refusal
4646

@@ -70,9 +70,11 @@ A token bucket in the style of Finagle's retry budget. Each request deposits a t
7070

7171
| Parameter | Default | Effect |
7272
|---|---|---|
73-
| `ttl` | `10.0` (s) | Sliding window over which deposits and withdrawals count. |
74-
| `min_retries_per_sec` | `10.0` | Floor: at least this many retries per second are allowed, whatever the deposit rate. |
75-
| `percent_can_retry` | `0.2` | Fraction of recent deposits that can convert to retries (above the floor). |
73+
| `ttl` | `10.0` (s) | Sliding window over which deposits and withdrawals count. Must be finite and `> 0`. |
74+
| `min_retries_per_sec` | `10.0` | Floor: at least this many retries per second are allowed, whatever the deposit rate. Must be finite and `>= 0`. |
75+
| `percent_can_retry` | `0.2` | Fraction of recent deposits that can convert to retries (above the floor). Must be finite and `>= 0`; values above `1.0` allow more retries than requests. |
76+
77+
An out-of-range value raises `ValueError` when the budget is constructed.
7678

7779
### The token-bucket formula
7880

‎src/httpware/middleware/resilience/budget.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,20 @@
1717
from collections.abc import Callable
1818

1919

20+
_TTL_INVALID = "ttl must be a finite number > 0"
21+
_MIN_RETRIES_PER_SEC_INVALID = "min_retries_per_sec must be a finite number >= 0"
22+
_PERCENT_CAN_RETRY_INVALID = "percent_can_retry must be a finite number >= 0"
23+
24+
25+
def _validate_budget_config(*, ttl: float, min_retries_per_sec: float, percent_can_retry: float) -> None:
26+
if not (ttl > 0 and math.isfinite(ttl)):
27+
raise ValueError(_TTL_INVALID)
28+
if not (min_retries_per_sec >= 0 and math.isfinite(min_retries_per_sec)):
29+
raise ValueError(_MIN_RETRIES_PER_SEC_INVALID)
30+
if not (percent_can_retry >= 0 and math.isfinite(percent_can_retry)):
31+
raise ValueError(_PERCENT_CAN_RETRY_INVALID)
32+
33+
2034
class RetryBudget:
2135
"""Token-bucket budget bounding retry rate to prevent retry storms.
2236
@@ -33,6 +47,7 @@ def __init__(
3347
percent_can_retry: float = 0.2,
3448
_now: Callable[[], float] = time.monotonic,
3549
) -> None:
50+
_validate_budget_config(ttl=ttl, min_retries_per_sec=min_retries_per_sec, percent_can_retry=percent_can_retry)
3651
self._ttl = ttl
3752
self._min_retries_per_sec = min_retries_per_sec
3853
self._percent_can_retry = percent_can_retry

‎src/httpware/middleware/resilience/retry.py‎

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import datetime as dt
1818
import email.utils
1919
import logging
20+
import math
2021
import time
2122
from collections.abc import Awaitable, Callable
2223
from http import HTTPStatus
@@ -56,6 +57,8 @@
5657
_RETRYABLE_EXCEPTIONS = (StatusError, NetworkError, TimeoutError)
5758

5859
_MAX_ATTEMPTS_INVALID = "max_attempts must be >= 1"
60+
_BASE_DELAY_INVALID = "base_delay must be >= 0"
61+
_MAX_DELAY_INVALID = "max_delay must be a finite number >= 0"
5962
_STREAMING_BODY_REFUSAL_NOTE = (
6063
"httpware: not retrying because the request body is a stream that cannot replay across attempts"
6164
)
@@ -66,12 +69,27 @@
6669
_LOGGER = logging.getLogger("httpware.retry")
6770

6871

72+
def _validate_retry_config(*, max_attempts: int, base_delay: float, max_delay: float) -> None:
73+
if max_attempts < 1:
74+
raise ValueError(_MAX_ATTEMPTS_INVALID)
75+
if not base_delay >= 0:
76+
raise ValueError(_BASE_DELAY_INVALID)
77+
if not (max_delay >= 0 and math.isfinite(max_delay)):
78+
raise ValueError(_MAX_DELAY_INVALID)
79+
80+
6981
def _parse_retry_after(value: str) -> float | None:
70-
"""Parse a Retry-After header value. Returns None on malformed input."""
82+
"""Parse a Retry-After header value. Returns None on malformed input, inf for integers too large for a float."""
7183
try:
72-
return max(0.0, float(int(value))) # clamp: negative integers are malformed servers
73-
except (ValueError, OverflowError):
74-
pass
84+
seconds = int(value)
85+
except ValueError:
86+
if value.strip().isdigit() and value.isascii():
87+
return math.inf # past int()'s digit limit
88+
else:
89+
try:
90+
return max(0.0, float(seconds)) # clamp: negative integers are malformed servers
91+
except OverflowError:
92+
return math.inf if seconds > 0 else 0.0
7593
try:
7694
parsed = email.utils.parsedate_to_datetime(value)
7795
except (TypeError, ValueError, OverflowError):
@@ -105,13 +123,12 @@ def __init__( # noqa: PLR0913 — retry policy has many orthogonal knobs; a dat
105123
respect_retry_after: bool,
106124
budget: RetryBudget | None,
107125
) -> None:
108-
if max_attempts < 1:
109-
raise ValueError(_MAX_ATTEMPTS_INVALID)
126+
_validate_retry_config(max_attempts=max_attempts, base_delay=base_delay, max_delay=max_delay)
110127
self.max_attempts = max_attempts
111128
self.base_delay = base_delay
112129
self.max_delay = max_delay
113130
self.retry_status_codes = retry_status_codes
114-
self.retry_methods = retry_methods
131+
self.retry_methods = frozenset(method.upper() for method in retry_methods)
115132
self.respect_retry_after = respect_retry_after
116133
self.budget = budget if budget is not None else RetryBudget()
117134

@@ -128,7 +145,7 @@ def decide( # noqa: C901 — complexity budget: classification + streaming-body
128145
exception. `exc` is the currently-handled exception (one of
129146
_RETRYABLE_EXCEPTIONS); see the class docstring for chaining semantics.
130147
"""
131-
method_eligible = request.method.upper() in self.retry_methods
148+
method_eligible = request.method in self.retry_methods
132149
if isinstance(exc, StatusError):
133150
retryable_status = exc.response.status_code in self.retry_status_codes
134151
if not method_eligible or not retryable_status:
@@ -168,6 +185,7 @@ def decide( # noqa: C901 — complexity budget: classification + streaming-body
168185
"url": str(request.url),
169186
"last_status": last_response.status_code if last_response is not None else None,
170187
"last_exception_type": type(exc).__qualname__,
188+
"reason": "max_attempts",
171189
},
172190
)
173191
raise exc
@@ -185,6 +203,22 @@ def decide( # noqa: C901 — complexity budget: classification + streaming-body
185203
max_delay=self.max_delay,
186204
),
187205
)
206+
_emit_event(
207+
_LOGGER,
208+
"retry.giving_up",
209+
level=logging.WARNING,
210+
message=f"retry gave up after {attempt + 1} attempts: Retry-After exceeded max_delay",
211+
attributes={
212+
"attempts": attempt + 1,
213+
"method": request.method,
214+
"url": str(request.url),
215+
"last_status": last_response.status_code if last_response is not None else None,
216+
"last_exception_type": type(exc).__qualname__,
217+
"reason": "retry_after_exceeds_max_delay",
218+
"retry_after": retry_after,
219+
"max_delay": self.max_delay,
220+
},
221+
)
188222
raise exc
189223

190224
if not self.budget.try_withdraw():

‎tests/test_budget.py‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,11 @@
44
so they cannot be perturbed by other tests sharing the same module.
55
"""
66

7+
import math
78
import time
89

10+
import pytest
11+
912
from httpware.middleware.resilience.budget import RetryBudget
1013

1114

@@ -106,3 +109,30 @@ def test_default_now_is_time_monotonic() -> None:
106109
"""When _now is not passed, the budget uses time.monotonic by default."""
107110
budget = RetryBudget()
108111
assert budget._now is time.monotonic # noqa: SLF001
112+
113+
114+
@pytest.mark.parametrize("ttl", [0.0, -1.0, math.nan, math.inf])
115+
def test_invalid_ttl_rejected(ttl: float) -> None:
116+
with pytest.raises(ValueError, match="ttl must be a finite number > 0"):
117+
RetryBudget(ttl=ttl)
118+
119+
120+
@pytest.mark.parametrize("min_retries_per_sec", [-1.0, math.nan, math.inf])
121+
def test_invalid_min_retries_per_sec_rejected(min_retries_per_sec: float) -> None:
122+
with pytest.raises(ValueError, match="min_retries_per_sec must be a finite number >= 0"):
123+
RetryBudget(min_retries_per_sec=min_retries_per_sec)
124+
125+
126+
@pytest.mark.parametrize("percent_can_retry", [-0.1, math.nan, math.inf])
127+
def test_invalid_percent_can_retry_rejected(percent_can_retry: float) -> None:
128+
with pytest.raises(ValueError, match="percent_can_retry must be a finite number >= 0"):
129+
RetryBudget(percent_can_retry=percent_can_retry)
130+
131+
132+
def test_boundary_budget_values_accepted() -> None:
133+
clock = _Clock()
134+
budget = RetryBudget(ttl=0.001, min_retries_per_sec=0.0, percent_can_retry=2.0, _now=clock.now)
135+
budget.deposit()
136+
assert budget.try_withdraw() is True
137+
assert budget.try_withdraw() is True
138+
assert budget.try_withdraw() is False

‎tests/test_retry.py‎

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,18 @@ async def test_retries_post_when_method_explicitly_included() -> None:
128128
assert handler.calls == 2 # noqa: PLR2004 — "2" is intentional literal in test assertion
129129

130130

131+
async def test_retry_methods_match_case_insensitively() -> None:
132+
sleeper = _SleepRecorder()
133+
handler = _ResponseSequence([HTTPStatus.SERVICE_UNAVAILABLE, HTTPStatus.OK])
134+
client = _client(
135+
handler,
136+
retry=AsyncRetry(_sleep=sleeper, retry_methods=frozenset({"post"}), base_delay=0.01, max_delay=0.02),
137+
)
138+
response = await client.post("https://example.test/x", json={"x": 1})
139+
assert response.status_code == HTTPStatus.OK
140+
assert handler.calls == 2 # noqa: PLR2004 — initial attempt + 1 retry
141+
142+
131143
async def test_max_attempts_one_means_no_retries() -> None:
132144
sleeper = _SleepRecorder()
133145
handler = _ResponseSequence([HTTPStatus.SERVICE_UNAVAILABLE])
@@ -143,6 +155,16 @@ def test_max_attempts_zero_rejected() -> None:
143155
AsyncRetry(max_attempts=0)
144156

145157

158+
def test_negative_base_delay_rejected() -> None:
159+
with pytest.raises(ValueError, match="base_delay must be >= 0"):
160+
AsyncRetry(base_delay=-1)
161+
162+
163+
def test_negative_max_delay_rejected() -> None:
164+
with pytest.raises(ValueError, match="max_delay must be a finite number >= 0"):
165+
AsyncRetry(max_delay=-1)
166+
167+
146168
async def test_budget_exhausted_raises_retry_budget_exhausted_error() -> None:
147169
# NOTE: lives here for coverage of the AsyncRetry loop's budget-exhaustion branch.
148170
# Task 11 adds the broader budget-gate + sharing tests (carry-through behavior,
@@ -625,6 +647,28 @@ async def test_retry_giving_up_emits_observability_event(caplog: pytest.LogCaptu
625647
assert record.method == "GET" # ty: ignore[unresolved-attribute]
626648
assert record.last_status == HTTPStatus.SERVICE_UNAVAILABLE # ty: ignore[unresolved-attribute]
627649
assert record.last_exception_type == "ServiceUnavailableError" # ty: ignore[unresolved-attribute]
650+
assert record.reason == "max_attempts" # ty: ignore[unresolved-attribute]
651+
652+
653+
async def test_retry_after_exceeding_max_delay_emits_giving_up_event(caplog: pytest.LogCaptureFixture) -> None:
654+
sleeper = _SleepRecorder()
655+
handler = _ResponseSequenceWithHeaders([(HTTPStatus.SERVICE_UNAVAILABLE, {"Retry-After": "9999"})])
656+
client = _client(handler, retry=AsyncRetry(_sleep=sleeper, base_delay=0.01, max_delay=2.5))
657+
658+
with caplog.at_level(logging.WARNING, logger="httpware.retry"), pytest.raises(ServiceUnavailableError):
659+
await client.get("https://example.test/x")
660+
661+
records = [r for r in caplog.records if r.name == "httpware.retry" and r.event == "retry.giving_up"] # ty: ignore[unresolved-attribute]
662+
assert len(records) == 1
663+
record = records[0]
664+
assert record.levelno == logging.WARNING
665+
assert record.reason == "retry_after_exceeds_max_delay" # ty: ignore[unresolved-attribute]
666+
assert record.attempts == 1 # ty: ignore[unresolved-attribute]
667+
assert record.method == "GET" # ty: ignore[unresolved-attribute]
668+
assert record.last_status == HTTPStatus.SERVICE_UNAVAILABLE # ty: ignore[unresolved-attribute]
669+
assert record.last_exception_type == "ServiceUnavailableError" # ty: ignore[unresolved-attribute]
670+
assert record.retry_after == 9999.0 # noqa: PLR2004 — matches the header above # ty: ignore[unresolved-attribute]
671+
assert record.max_delay == 2.5 # noqa: PLR2004 — matches max_delay above # ty: ignore[unresolved-attribute]
628672

629673

630674
async def test_retry_budget_refused_emits_observability_event(caplog: pytest.LogCaptureFixture) -> None:
@@ -735,8 +779,7 @@ async def test_retry_after_exceeds_max_delay_does_not_consume_budget_token() ->
735779
assert len(budget._withdrawn) == 0 # noqa: SLF001 — implementation-detail access for invariant
736780

737781

738-
async def test_retry_after_huge_digit_string_does_not_crash() -> None:
739-
"""A very large Retry-After integer string must fall back to jitter, not crash with OverflowError."""
782+
async def test_retry_after_huge_digit_string_gives_up() -> None:
740783
sleeper = _SleepRecorder()
741784
handler = _ResponseSequenceWithHeaders(
742785
[
@@ -745,10 +788,11 @@ async def test_retry_after_huge_digit_string_does_not_crash() -> None:
745788
]
746789
)
747790
client = _client(handler, retry=AsyncRetry(_sleep=sleeper, base_delay=0.01, max_delay=0.05))
748-
response = await client.get("https://example.test/x")
749-
assert response.status_code == HTTPStatus.OK
750-
assert len(sleeper.calls) == 1
751-
assert 0.0 <= sleeper.calls[0] <= 0.05 # noqa: PLR2004 — 0.05 matches max_delay literal above
791+
with pytest.raises(ServiceUnavailableError) as exc_info:
792+
await client.get("https://example.test/x")
793+
assert any("exceeded max_delay" in n for n in getattr(exc_info.value, "__notes__", []))
794+
assert sleeper.calls == []
795+
assert handler.calls == 1
752796

753797

754798
async def test_method_ineligible_with_streaming_body_does_not_attach_streaming_note() -> None:

0 commit comments

Comments
 (0)