🤖 Found while building the conformance harness, which needed a trailers test — the first thing in the suite to exercise response.trailers at all.
The behaviour
response.trailers never settles until the response body has been fully consumed. FaithResponse::trailers polls a Trailers::NotYet state in a loop with yield_now(), and that state is only resolved by an arriving trailers frame or by a sentinel appended after the last body chunk:
pub async fn trailers(&self) -> Option<Vec<(String, String)>> {
let t = Arc::clone(&self.trailers);
loop {
match &*t.read().await {
Trailers::NotYet => {
yield_now().await;
continue;
}
...
So this hangs forever:
const res = await fetch(url);
const trailers = await res.trailers; // never resolves
const body = await res.text();
while this works:
const res = await fetch(url);
const body = await res.text();
const trailers = await res.trailers; // fine
Three things make it worse than an ordinary ordering requirement:
- There is no error path. It doesn't reject, warn or time out — it spins. The failure presents as a wedged process, not a diagnosable error.
timeout does not cover it. The request-level timeout option is reqwest's, applied to the request and body; the trailers() future is separate and outside it. So the usual backstop doesn't apply.
- It burns CPU and resists being killed. Because it's a busy
yield_now loop rather than a parked future, the pending native future also keeps Node's event loop alive. In the harness this was reproduced as an indefinite hang; even adding a test-framework timeout reported the failure and then still failed to exit, until the runner was made to force process.exit.
That last point is the sharp part: a test framework's own timeout cannot rescue you, so in CI this reads as a job timeout with no diagnosis.
Why it's easy to hit
The ordering is not obvious from the API. response.trailers looks like a property you can read whenever you like — headers is — and nothing in the README, wrapper.d.ts or the getter's own docs mentions that the body must be drained first. Reading trailers before the body is also the intuitive order for anyone thinking "get the metadata, then decide whether to read the body".
Possible fixes, roughly in order of how much they help
- Reject instead of spinning, if the body is dropped or the response is disturbed without the trailers ever arriving. An error beats a hang, and this is the case that turns a mistake into a wedged CI job.
- Document the ordering on the
trailers getter, in wrapper.d.ts and in the README's Response.trailers section — cheap, and would have saved the harness work an hour.
- Park rather than spin.
yield_now() in a loop is a busy-wait; a notify/watch would at least stop burning a core and would let the future be cancelled properly.
- Consider making it cover the request timeout, so the existing backstop applies, or document explicitly that it does not.
Even 2 alone would be worth doing. 1 and 3 together would make the failure mode diagnosable instead of fatal.
For reference, the harness works around this by draining the body first and carrying a comment explaining why, since a future reader would otherwise "tidy" the ordering and reintroduce the hang.
🤖 Found while building the conformance harness, which needed a trailers test — the first thing in the suite to exercise
response.trailersat all.The behaviour
response.trailersnever settles until the response body has been fully consumed.FaithResponse::trailerspolls aTrailers::NotYetstate in a loop withyield_now(), and that state is only resolved by an arriving trailers frame or by a sentinel appended after the last body chunk:So this hangs forever:
while this works:
Three things make it worse than an ordinary ordering requirement:
timeoutdoes not cover it. The request-leveltimeoutoption is reqwest's, applied to the request and body; thetrailers()future is separate and outside it. So the usual backstop doesn't apply.yield_nowloop rather than a parked future, the pending native future also keeps Node's event loop alive. In the harness this was reproduced as an indefinite hang; even adding a test-framework timeout reported the failure and then still failed to exit, until the runner was made to forceprocess.exit.That last point is the sharp part: a test framework's own timeout cannot rescue you, so in CI this reads as a job timeout with no diagnosis.
Why it's easy to hit
The ordering is not obvious from the API.
response.trailerslooks like a property you can read whenever you like —headersis — and nothing in the README,wrapper.d.tsor the getter's own docs mentions that the body must be drained first. Reading trailers before the body is also the intuitive order for anyone thinking "get the metadata, then decide whether to read the body".Possible fixes, roughly in order of how much they help
trailersgetter, inwrapper.d.tsand in the README'sResponse.trailerssection — cheap, and would have saved the harness work an hour.yield_now()in a loop is a busy-wait; a notify/watch would at least stop burning a core and would let the future be cancelled properly.Even 2 alone would be worth doing. 1 and 3 together would make the failure mode diagnosable instead of fatal.
For reference, the harness works around this by draining the body first and carrying a comment explaining why, since a future reader would otherwise "tidy" the ordering and reintroduce the hang.