mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
Sami
2d864889f5
A 429 with a `retry in {N}s` hint followed by a network error kept the
hint for the next sleep, but only because the sole assignment to
`retry_hint_secs` sat in the Ok-retriable arm. The behaviour was correct
and entirely incidental: moving that line, or adding an arm, would have
silently changed a policy nobody had written down.
Make the match an expression whose value is the hint governing the next
sleep, so every outcome names its own policy. The network-error arm now
yields `retry_hint_secs` explicitly, with a comment giving the reason
(a network error says nothing about the quota window) and the cost
asymmetry that picks the direction: over-sleeping wastes time we were
told to wait anyway, while under-sleeping earns a fresh denial that
still costs a counter increment, because the limiter's INCR runs on
refused checks too.
The comment also fences the one nuance a future reader would trip on:
the kept hint is exactly right only for the sleep immediately after the
429, and subtracting elapsed time to "fix" the over-sleep would
reintroduce under-sleep via clock arithmetic — the expensive direction.
Spelling note: the literal `retry_hint_secs = retry_hint_secs` in that
arm is a deny-by-default clippy error (clippy::self_assignment), so the
policy is expressed by making every arm yield its hint instead.
Covered by a behavioral test, because the arm is otherwise invisible:
it holds the value it already had, so yielding `None` instead is a
silent revert that all four pure `rest_retry_delay` tests stay green
under, as does the existing 429->200 wiring test (which never produces
a network error). The new test serves one 429 with a 51s hint and then
drops the listener, so the ladder sleeps hint/hint/hint (~153s) when
the hint is carried and hint/1s/2s (~54s) when it is dropped, and
asserts into the gap. Mutating the arm to `None` fails it at 53.8s;
across the whole 688-test lib run that mutant is caught by exactly one
test, so the coverage claim is measured rather than assumed.
Verified at rustc 1.95.0: buzz-acp 688 + 9 integration, buzz-cli 322,
0 failed; cargo fmt --all --check and clippy -p buzz-acp -p buzz-cli
--all-targets -D warnings both clean.
Co-authored-by: Sami <f4a42a97e594b77bdbd8ee35191c8b28a94a4cb871d96f32921558275421fb68@buzz.block.builderlab.xyz>
Signed-off-by: Sami <f4a42a97e594b77bdbd8ee35191c8b28a94a4cb871d96f32921558275421fb68@buzz.block.builderlab.xyz>