Require aiohttp 3.15 and read request.timeout directly - #23
Draft
rodrigobnogueira wants to merge 28 commits into
Draft
rodrigobnogueira wants to merge 28 commits into
rodrigobnogueira wants to merge 28 commits into
Conversation
Add RateLimitMiddleware, a token-bucket client middleware that throttles outgoing requests to a configurable rate and burst, with optional per-domain buckets and numeric Retry-After handling on HTTP 429. Promoted from the example in aiohttp#11969 (which was moved here): drop the demo and module-level logging config, keep a strong reference to the scheduler task so it cannot be garbage collected mid-run, and add tests and documentation.
- Never sleep on a non-finite or non-positive Retry-After, and clamp it to a configurable max_retry_after (default 300s), so a server cannot stall or hang the client with "inf"/"nan"/huge values. - Clear the scheduler reference in a finally block so a bucket whose scheduler was cancelled mid-sleep (e.g. its event loop was torn down) restarts cleanly on the next acquire instead of deadlocking on reuse. - On a cancelled acquire, drop the waiter from the queue and hand its slot to the next waiter rather than wasting an interval. - Validate rate > 0 and burst >= 1 at construction (fail fast). - Document that the per-domain bucket cache is unbounded. Adds regression tests for each fix (kept at 100% branch coverage) plus per-domain isolation, drift-cap, and cross-loop reuse.
The code-quality scanner flagged three bare ``await <task>`` statements (inside ``pytest.raises``) as having no effect. Replace them with a ``_cancel_and_join`` helper that cancels the task, waits for it to finish unwinding, and asserts it was cancelled -- a call expression rather than a bare await, and a slightly stronger check. Behaviour and coverage are unchanged.
Round out the constructor validation: rate and burst already raise on out-of-range values, but max_retry_after did not, so a nan/negative value would silently misbehave in the Retry-After clamp. Reject anything that is not None or a non-negative finite number. Clarify the docs to distinguish this config validation from the (separate) handling of hostile server-sent Retry-After values.
… cap Three robustness gaps found while re-reviewing before marking the PR ready: - A scheduler task cancelled before its first step (loop torn down right after an acquire), or stranded by a loop that was closed without cancelling tasks, never runs the finally that clears _scheduler_task. The stale reference then blocks every restart and all later acquires hang silently, on any loop. Restart scheduling whenever the stored task is done or belongs to another loop, drop waiters that were queued on an abandoned loop (their events can never be set from the current loop), and make the finally clear only its own registration so a garbage collected zombie cannot clobber a live replacement. - rate=nan/inf passed the rate <= 0 check and a subnormal rate overflowed 1.0 / rate to inf; each silently disabled throttling. Require a positive finite rate whose interval is also finite. - The default max_retry_after of 300s equalled aiohttp's default ClientTimeout total (300s), so a fully honored Retry-After was guaranteed to surface as a timeout error with the 429 lost. Lower the default cap to 60s, treat a 0.0 cap as disabling the sleep, and log at debug level. Document the ClientTimeout interplay, middleware ordering, and per-host bucket semantics. Also rescale the two tightest timing tests (the drift-cap bounds sat 5ms and 2ms from flaking) and relax three roundtrip upper bounds to 0.5s.
The PyPI-facing README still described the package as digest-auth only. In the API reference, spell out the details a user needs to avoid surprises: rate must be finite; per-domain buckets are keyed on host only; Retry-After is honored on 429 only, delays just the request that received it, and its sleep counts against the session ClientTimeout (hence the 60s default cap); configuration is fixed at construction; and the rate limiter should be listed last so internal retries by other middlewares are also throttled.
docs/code/index.py backs index.rst (the two quickstart examples merge into one session showing middleware composition and ordering) and docs/code/api.py backs the RateLimitMiddleware usage block in api.rst, replacing the inline code block.
aiohttp raises InvalidUrlClientError for host-less URLs before any middleware runs, on the initial request and on every redirect hop (client.py checks raw_host before constructing ClientRequest), so the 'unknown' fallback could never be reached; it existed only because URL.host is Optional in the type.
TokenBucket.acquire() now just computes: it refills fractionally from elapsed time, takes a token (the count may go negative, queueing callers in arrival order) and returns the exact delay to sleep, so the middleware owns the sleep. The waiter queue, events, scheduler task, loop rebinding and their hardening tests are all deleted. acquire() takes an optional timeout and raises asyncio.TimeoutError, handing the token back, when the wait alone would exceed it. The middleware feeds it the request's total timeout when aiohttp exposes one; 3.x carries only the timer context, so the bound engages on newer versions. A caller cancelled mid-sleep returns its slot through the new release(). The bucket is injectable -- RateLimitMiddleware(TokenBucket(...)) -- leaving room for alternative algorithms; per_domain still builds per-host buckets from rate/burst and rejects a single injected bucket. TokenBucket is exported. Bucket tests run on a fake clock as pure math, with wall-clock sleeps confined to the integration tests.
…After - Add the RateLimiter ABC defining acquire()/release(), with the async wait() (sleep plus cancellation handback) shared by all implementations - RateLimitMiddleware now requires the limiter: an instance, or a zero-argument factory with per_domain=True; rate/burst live only on TokenBucket - Drop the Retry-After handling as a potentially separate feature - Docs: two separate quickstart examples included by pyobject from the page-named files - Prefer the public request.timeout attribute once aiohttp exposes it
- The timeout check moves from each implementation's acquire() into the shared RateLimiter.wait(), which hands the slot back through release() and raises asyncio.TimeoutError; acquire() is now just reserve-and- return-delay - Replace the per-domain factory callable with a clone() abstractmethod: the middleware always takes a RateLimiter instance, and per_domain=True clones it once per target host
aiohttp exposes the request ClientTimeout to client middlewares as a read-only property since aio-libs/aiohttp#13176, so the private _timeout fallback is gone and the docs name the version (3.15) instead of 'newer versions'. On 3.12-3.14 the attribute is absent and the limiter keeps waiting without a budget, as before.
Retry-After handling was dropped from the middleware during review, but the README still advertised it. Describe what actually ships instead: a pluggable limiter with a token bucket included. The rate-limit example installs a single middleware, so the aside about ordering was describing something the example never showed.
acquire() and release() are deliberately synchronous. That is what makes concurrent callers on one event loop reserve slots atomically, in arrival order, and release() additionally runs from a cancellation handler, where an awaiting implementation can be cut short and lose the slot. A limiter that needs I/O to reserve a slot overrides wait() instead. It is already a coroutine and is the only method the middleware calls, so nothing about the base class stands in the way of a Redis- or database-backed implementation.
ClientRequest.timeout is a read-only property, so assigning it through the ClientRequest annotation needs an ignore whose code depends on the installed aiohttp: attr-defined before the property existed, misc after. Setting it inside the helper, while the autospec mock is still untyped, needs no ignore on either.
ClientRequest.timeout is public and read-only from aiohttp 3.15 (aio-libs/aiohttp#13176), and absent on every release available today, so the attribute cannot be resolved statically here. Annotating the result of the getattr() gives mypy the value type to check against instead of Any, which is most of what dropping the getattr() would buy. Verified against the installed 3.14.1 and against the current 3.15 branch: 249 tests and a clean mypy run on both, so the branch survives 3.15 landing without anyone touching it.
The 3.15 change Dreamsorcerer asked for is written and verified in #23; it waits only on the aiohttp release, so the comment says where it is.
rodrigobnogueira
force-pushed
the
rate-limit-aiohttp-315
branch
from
August 9, 2026 01:33
0cc5c31 to
e9e2e1e
Compare
Three things a pre-merge review turned up, all cheap now and awkward after 0.2.0 ships: per_domain was a writable public attribute that nothing read. The limiters are built once in __init__, so assigning it afterwards was silently inert in both directions. It is a read-only property now, which keeps it introspectable without pretending it is a switch. The docstrings told an I/O-backed limiter to override wait() rather than acquire(), but acquire() and clone() are abstract, so a class following that advice literally could not be instantiated. Both docs now say the two still have to be defined, and why clone() is the one that matters. The arrival-order guarantee was stated absolutely. A slot handed back by release() after a cancellation frees capacity that queued callers already hold fixed delays against, so two of them can send in the same instant. That is inherent to the synchronous-acquire design and not worth changing, but the docs should not overpromise.
The getattr() fallback existed because ClientRequest carried no timeout before aio-libs/aiohttp#13176. With the floor at 3.15 the attribute is always there, so the middleware reads it directly and mypy checks it. The None guard goes with it: the property is typed ClientTimeout, never Optional, so the check is statically dead. Verified against the 3.15 branch (the #13211 backport tree): 249 passed, and mypy is clean on this package, tests and docs.
rodrigobnogueira
force-pushed
the
rate-limit-aiohttp-315
branch
from
August 9, 2026 02:01
e9e2e1e to
eb3f19c
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What do these changes do?
Raises the floor to
aiohttp >= 3.15and readsrequest.timeout.totaldirectly in
RateLimitMiddleware, replacing thegetattr()fallback that#12 carries.
This is the change you asked for in #12. It cannot land until aiohttp 3.15
is on PyPI (latest today is 3.14.3), so it is parked here as a draft rather
than holding #12 red. Based on
add-rate-limit-middleware, so the diff isthe one commit; GitHub will retarget it to
masterwhen #12 merges.Are there changes in behavior for the user?
No behavior change on 3.15. The
Noneguard that goes away was only everreachable on older aiohttp, where the attribute did not exist. Installs
resolving aiohttp below 3.15 will no longer get this package.
Related issue number
Follows the review on #12. The attribute comes from aio-libs/aiohttp#13176,
backported to the 3.15 branch as aio-libs/aiohttp#13211.
Checklist
pytest).pre-commit run --all-filesandmypypass.CHANGES/folderVerification
Checked against the current
3.15branch, which already carries theClientRequest.timeoutproperty, imported ahead of the installed aiohttp:The three failures are
AttributeErroronrequest.timeout, which is thewhole reason this is not merged yet.
Before merging