fix: bound the response body by the total on the httpx family - #28
Merged
Conversation
TimeoutConfig.total stopped at the response headers on httpx and httpx2:
the engine returns at handle_async_request, so a body that kept dripping
after the headers escaped the wall clock entirely and a 1 s total let a
4 s body through. The response stream already passes through the
family's timed-stream wrapper for the body-duration metric, so the
deadline now travels with it. The async wrapper bounds every chunk by
the remaining budget and raises the adapter's DeadlineExceededError when
the budget is gone; the sync wrapper cannot interrupt a blocked read and
instead refuses to start the next one, late by at most one chunk or one
read timeout. Both hand on_done an Outcome(kind=total_timeout).
The declaration is a new Capability.DEADLINE_COVERS_BODY rather than
boundary=FULL: DurationBoundary says where the call duration metric
closes, and that is still the headers. The httpx family declares it
emulated and reports it in the build; aiohttp, requests and urllib3
declare it absent with notes. The normalizer contract's wrap_stream
gains the deadline argument, and OriginServer gets a
/drip/{count}/{interval} route for the scenario.
Closes #25
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
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.
Branched from master; independent of #26 (both touch
engine/aio.py, in different places).TimeoutConfig.totalstopped at the response headers on the httpx family. The engine sits in the transport and returns whenhandle_async_requestdoes, so a body that kept dripping after the headers was outside the wall clock:total=1.0against a server sending one byte every 0.5 s returned a full body after 4 s, same as bare httpx withtimeout=1.0, while the same config against a server that stalls before the headers correctly raised at 1.0 s. The one thing a caller cannot tell from the outside is exactly the thing the total did not cover.The response stream already passes through the family's
AsyncTimedStreamMixin/SyncTimedStreamMixinfor the body-duration metric, so the deadline now travels with it:run()builds theDeadline(it was built a few microseconds later in_admitted) and hands it towrap_stream(response, on_done, deadline). The async wrapper bounds every chunk withasyncio.timeout(deadline.remaining())and, when the scope fires or the budget is already gone before the next read, raises the adapter'sDeadlineExceededErrorthrough the adapter's translator (bound into the normalizer, so_httpx_sharedstays SDK-free); httpcore closes the connection on the cancellation and httpx'ssend()closes the response on the way out. The sync wrapper cannot interrupt a blocked read, so it re-checks the deadline before starting the next one, the way the sync engine re-checks at attempt boundaries: a chunk that would start after the deadline is refused, late by at most one chunk or one read timeout. Both handon_doneanOutcome(kind=total_timeout).Where this diverges from the issue is the declaration.
DurationBoundaryis documented (agents page, Prometheus help text) as where the call duration closes, andcall_endstill fires at the headers after this change; an honestFULLwould moverequests_total,request_durationand the in-flight gauge to the end of the body for every httpx user, and an abandoned stream would leak in flight. That is a different change. Soboundarystaysheadersand the coverage is a newCapability.DEADLINE_COVERS_BODY:emulatedon the httpx family (hard on async, soft on sync, in the note) and listed in the build report'semulated;absenton aiohttp, requests and urllib3 with notes saying where the body streams instead. Consequence stated plainly, in the docs too: a body-phase deadline shows in the exception, inbody_durationand in the outcome the wrapper reports, not inrequests_total{outcome}, because that metric closed at the headers.aiohttp stays at headers, declared. The middleware hands back a
ClientResponsewhose body the caller reads through aiohttp'sStreamReader, which aiohttp relies on for release, EOF and connection reuse; wrapping it is private surgery, and aiohttp's owntotaltimer would cover the body but wraps the whole middleware chain including our retries, which is why the adapter zeroes it. An integration test pins the declared divergence the way the syncread_timeoutparity test does.Two knock-ons.
wrap_streamon the normalizer protocols gains thedeadlineargument; the protocols live belowclientwright.core, which the reference page marks internal, so no semver break, but a third-party adapter implementingAsyncNormalizer/SyncNormalizerneeds the extra parameter (none exist in-tree). AndOriginServergets a/drip/{count}/{interval}route, documented in the testing guide and on the agents page, so the scenario is a real integration test.Rejected:
boundary=FULL(above); wrapping aiohttp'sStreamReader; leaving the sync client's body unbounded when a check between chunks costs nothing and matches how the sync total already behaves.Docs: timeouts guide (new paragraph on the body), sync-and-async, the httpx and aiohttp adapter pages (the aiohttp page also claimed body duration was measured via trace hooks, which the capability record and the agents page both deny; corrected while there), agents page (
Capabilityrow, rule 10, adapter bullets, origin routes), testing guide. The changelog belongs to release-please.The reporter's
01_read_timeout.pyfrom the blog lab, before and after (the last line is the one that changes):Negative control: with the two wrapper loops put back to their unbounded form (signatures kept), the dripping-body tests fail (
5 failed, 2 passed, the two passing ones being the bodies that finish inside the budget, which must not change); with the branch they pass. Gate:make checkclean (ruff, ruff format, mypy, import-linter);make test818 passed, 42 skipped, coverage 99.60 % against the 97 % floor;uv.lockuntouched. The branch merges cleanly with #26 (git merge-treereports no conflicts), so either can land first.Closes #25