diff --git a/README.md b/README.md index c03a4a8..c78736a 100644 --- a/README.md +++ b/README.md @@ -454,9 +454,26 @@ from well-known status codes. The `trailers()` read-only property of the `Response` interface returns a promise that resolves to either `null` or a `Headers` structure that contains the HTTP/2 or /3 trailing headers. -Note that this will never resolve if you don't also consume the body in some way. +**This does not resolve until the body has been consumed**, because trailers arrive after the body +ends. Read the body first — `text()`, `bytes()`, `json()`, `blob()`, or the `body` stream — and then +await the trailers: -Custom to Fáith. This was once in the spec but was removed as it wasn't implemented by any browser. +```javascript +const res = await fetch(url); +const body = await res.text(); +const trailers = await res.trailers; // resolves +``` + +Awaiting the trailers on their own, without ever reading the body, waits forever: there is nothing +to end the body and produce them. That is the behaviour the current spec proposal describes +([whatwg/fetch#1940](https://github.com/whatwg/fetch/pull/1940)), not a quirk of Fáith. Holding the +promise while something else reads the body is fine, and costs nothing while it is pending. + +`discard()` counts as consuming the body, but discards its trailers along with it: the promise then +resolves to `null` rather than waiting for trailers that can no longer arrive. + +Custom to Fáith. This was once in the spec but was removed as it wasn't implemented by any browser; +the proposal above is the current effort to bring it back. ### `Response.type: string` diff --git a/index.d.ts b/index.d.ts index d7f868b..7ceea50 100644 --- a/index.d.ts +++ b/index.d.ts @@ -236,7 +236,15 @@ json(): Promise * * This was once in the spec as a getter but was removed as it wasn't implemented by any browser. * - * Note that this will never resolve if you don't also consume the body in some way. + * Trailers only exist once the body has ended, so this does not resolve until the body + * has been consumed — by `text()`, `bytes()`, `json()`, `blob()`, or reading the `body` + * stream. Awaiting it first, on its own, waits forever: that is the behaviour the fetch + * spec's trailers proposal describes (), not + * a quirk of Fáith. Holding the promise while something else reads the body is fine, and + * costs nothing while it is pending. + * + * `discard()` counts as consuming the body but discards its trailers with it, so this + * then resolves to `null` rather than waiting for trailers that can no longer arrive. * * This is an async fn as an internal implementation detail and the wrapper makes it a property. */ diff --git a/src/response.rs b/src/response.rs index 219a8b7..080f2f4 100644 --- a/src/response.rs +++ b/src/response.rs @@ -19,7 +19,7 @@ use napi_derive::napi; use reqwest::{StatusCode, Url, Version, header::HeaderMap}; use serde_json; use stream_shared::SharedStream; -use tokio::{sync::RwLock, task::yield_now}; +use tokio::sync::watch; use crate::{ agent::InnerAgentStats, @@ -44,7 +44,7 @@ pub struct FaithResponse { pub(crate) redirected: bool, pub(crate) stats: Arc, pub(crate) status_code: StatusCode, - pub(crate) trailers: Arc>, + pub(crate) trailers: Arc, pub(crate) url: Url, pub(crate) version: Version, } @@ -62,7 +62,7 @@ pub struct PeerInformation { pub certificate: Option>, } -#[derive(Debug, Default)] +#[derive(Clone, Debug, Default)] pub enum Trailers { #[default] NotYet, @@ -70,6 +70,58 @@ pub enum Trailers { Some(HeaderMap), } +/// Where the trailers land: written by whoever finishes the body, awaited by `trailers()`. +/// +/// A watch channel, rather than a lock read in a loop. Per the fetch spec's trailers +/// proposal () this promise is *meant* not to +/// resolve until the body has been consumed, so the wait is unbounded by design -- which is +/// precisely why polling was the wrong shape for it. Awaiting trailers without reading the +/// body now leaves an idle pending promise rather than a pegged core, and the future can be +/// cancelled while it waits. +#[derive(Debug)] +pub struct TrailersSlot(watch::Sender); + +impl Default for TrailersSlot { + fn default() -> Self { + Self(watch::channel(Trailers::NotYet).0) + } +} + +impl TrailersSlot { + /// Record trailers that arrived, waking whoever is waiting. + fn arrived(&self, trailers: HeaderMap) { + self.0.send_replace(Trailers::Some(trailers)); + } + + /// Record that the body ended, if no trailers frame got there first. + /// + /// `send_if_modified` so the read and the write are one step, and so waiters are woken + /// only by the call that actually settled it. + fn ended(&self) { + self.0.send_if_modified(|state| { + if matches!(state, Trailers::NotYet) { + *state = Trailers::None; + true + } else { + false + } + }); + } + + /// Wait until the body has settled the question. + async fn settled(&self) -> Trailers { + let mut rx = self.0.subscribe(); + // `wait_for` tests the current value before waiting, so trailers that already + // arrived return without yielding. Its error case is the sender being gone, which + // means the response was dropped and nothing can ever set this -- no trailers is + // the only answer left. + match rx.wait_for(|state| !matches!(state, Trailers::NotYet)).await { + Ok(state) => state.clone(), + Err(_) => Trailers::None, + } + } +} + #[napi] impl FaithResponse { /// The `headers` read-only property of the `Response` interface contains the `Headers` object @@ -281,8 +333,7 @@ impl FaithResponse { Err(err) => Some(Err(err.to_string())), Ok(frame) => match frame.into_trailers() { Ok(trailers) => { - let mut t = trailers_lock.write().await; - *t = Trailers::Some(trailers); + trailers_lock.arrived(trailers); None } Err(frame) => Some( @@ -295,10 +346,7 @@ impl FaithResponse { } }) .chain(stream::once(async move { - let mut t = trailers_finish.write().await; - if matches!(*t, Trailers::NotYet) { - *t = Trailers::None; - } + trailers_finish.ended(); // Track that we've finished consuming a body stats_finish.bodies_finished.fetch_add(1, Ordering::Relaxed); // Mark body as drained so Drop doesn't try to drain again @@ -359,6 +407,7 @@ impl FaithResponse { let body = self.body.body.clone(); let drained_flag = self.body.drained.clone(); let is_multiplexed = self.body.is_multiplexed(); + let trailers = self.trailers.clone(); faith_promise(env, async move { if let Some(arc) = body { if is_multiplexed { @@ -372,6 +421,12 @@ impl FaithResponse { } } drained_flag.store(true, Ordering::SeqCst); + // Discarding the body discards its trailers: on a multiplexed connection the + // stream was cancelled before any could arrive, and draining an HTTP/1 body + // here bypasses the stream that would have collected them. Settling this as + // "none" rather than leaving it pending is the point -- a caller who discarded + // the body and then awaited trailers used to wait forever. + trailers.ended(); Ok(()) }) } @@ -448,32 +503,33 @@ impl FaithResponse { /// /// This was once in the spec as a getter but was removed as it wasn't implemented by any browser. /// - /// Note that this will never resolve if you don't also consume the body in some way. + /// Trailers only exist once the body has ended, so this does not resolve until the body + /// has been consumed — by `text()`, `bytes()`, `json()`, `blob()`, or reading the `body` + /// stream. Awaiting it first, on its own, waits forever: that is the behaviour the fetch + /// spec's trailers proposal describes (), not + /// a quirk of Fáith. Holding the promise while something else reads the body is fine, and + /// costs nothing while it is pending. + /// + /// `discard()` counts as consuming the body but discards its trailers with it, so this + /// then resolves to `null` rather than waiting for trailers that can no longer arrive. /// /// This is an async fn as an internal implementation detail and the wrapper makes it a property. #[napi] pub async fn trailers(&self) -> Option> { - let t = Arc::clone(&self.trailers); - loop { - match &*t.read().await { - Trailers::NotYet => { - yield_now().await; - continue; - } - Trailers::None => break None, - Trailers::Some(h) => { - break Some( - h.iter() - .filter_map(|(name, value)| { - value - .to_str() - .ok() - .map(|v| (name.to_string(), v.to_string())) - }) - .collect(), - ); - } - } + match self.trailers.settled().await { + // NotYet cannot come back from `settled`, which is what it waits on. + Trailers::NotYet | Trailers::None => None, + Trailers::Some(headers) => Some( + headers + .iter() + .filter_map(|(name, value)| { + value + .to_str() + .ok() + .map(|v| (name.to_string(), v.to_string())) + }) + .collect(), + ), } } diff --git a/test/conformance/matrix.json b/test/conformance/matrix.json new file mode 100644 index 0000000..8f6b97c --- /dev/null +++ b/test/conformance/matrix.json @@ -0,0 +1,635 @@ +{ + "kind": "realised", + "cells": [ + { + "server": "node-h1", + "dimension": "trailers", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "node-h1", + "dimension": "chunked bodies", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "node-h1", + "dimension": "gzip", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "node-h1", + "dimension": "conditional GET", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "node-h1", + "dimension": "protocol negotiation", + "status": "skip", + "reason": "lacks alpnMulti", + "outcome": "skipped" + }, + { + "server": "node-h1", + "dimension": "connection reuse", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "node-h1", + "dimension": "oversized headers", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "node-h1", + "dimension": "h2 GOAWAY", + "status": "skip", + "reason": "lacks goaway, h2", + "outcome": "skipped" + }, + { + "server": "node-h1", + "dimension": "HTTP/3", + "status": "skip", + "reason": "lacks h3", + "outcome": "skipped" + }, + { + "server": "node-h1", + "dimension": "HTTP/3 upgrade", + "status": "skip", + "reason": "lacks altsvc", + "outcome": "skipped" + }, + { + "server": "node-h2", + "dimension": "trailers", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "node-h2", + "dimension": "chunked bodies", + "status": "skip", + "reason": "lacks chunked", + "outcome": "skipped" + }, + { + "server": "node-h2", + "dimension": "gzip", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "node-h2", + "dimension": "conditional GET", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "node-h2", + "dimension": "protocol negotiation", + "status": "skip", + "reason": "lacks alpnMulti", + "outcome": "skipped" + }, + { + "server": "node-h2", + "dimension": "connection reuse", + "status": "skip", + "reason": "lacks keepaliveLimit", + "outcome": "skipped" + }, + { + "server": "node-h2", + "dimension": "oversized headers", + "status": "skip", + "reason": "lacks headerLimits", + "outcome": "skipped" + }, + { + "server": "node-h2", + "dimension": "h2 GOAWAY", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "node-h2", + "dimension": "HTTP/3", + "status": "skip", + "reason": "lacks h3", + "outcome": "skipped" + }, + { + "server": "node-h2", + "dimension": "HTTP/3 upgrade", + "status": "skip", + "reason": "lacks altsvc", + "outcome": "skipped" + }, + { + "server": "caddy", + "dimension": "trailers", + "status": "skip", + "reason": "lacks trailers", + "outcome": "skipped" + }, + { + "server": "caddy", + "dimension": "chunked bodies", + "status": "skip", + "reason": "lacks chunked", + "outcome": "skipped" + }, + { + "server": "caddy", + "dimension": "gzip", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "caddy", + "dimension": "conditional GET", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "caddy", + "dimension": "protocol negotiation", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "caddy", + "dimension": "connection reuse", + "status": "skip", + "reason": "lacks keepaliveLimit", + "outcome": "skipped" + }, + { + "server": "caddy", + "dimension": "oversized headers", + "status": "skip", + "reason": "lacks headerLimits", + "outcome": "skipped" + }, + { + "server": "caddy", + "dimension": "h2 GOAWAY", + "status": "skip", + "reason": "lacks goaway", + "outcome": "skipped" + }, + { + "server": "caddy", + "dimension": "HTTP/3", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "caddy", + "dimension": "HTTP/3 upgrade", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "nginx", + "dimension": "trailers", + "status": "skip", + "reason": "lacks trailers", + "outcome": "skipped" + }, + { + "server": "nginx", + "dimension": "chunked bodies", + "status": "skip", + "reason": "lacks chunked", + "outcome": "skipped" + }, + { + "server": "nginx", + "dimension": "gzip", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "nginx", + "dimension": "conditional GET", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "nginx", + "dimension": "protocol negotiation", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "nginx", + "dimension": "connection reuse", + "status": "skip", + "reason": "lacks keepaliveLimit", + "outcome": "skipped" + }, + { + "server": "nginx", + "dimension": "oversized headers", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "nginx", + "dimension": "h2 GOAWAY", + "status": "skip", + "reason": "lacks goaway", + "outcome": "skipped" + }, + { + "server": "nginx", + "dimension": "HTTP/3", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "nginx", + "dimension": "HTTP/3 upgrade", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "apache-h1", + "dimension": "trailers", + "status": "skip", + "reason": "lacks trailers", + "outcome": "skipped" + }, + { + "server": "apache-h1", + "dimension": "chunked bodies", + "status": "skip", + "reason": "lacks chunked", + "outcome": "skipped" + }, + { + "server": "apache-h1", + "dimension": "gzip", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "apache-h1", + "dimension": "conditional GET", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "apache-h1", + "dimension": "protocol negotiation", + "status": "skip", + "reason": "lacks alpnMulti", + "outcome": "skipped" + }, + { + "server": "apache-h1", + "dimension": "connection reuse", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "apache-h1", + "dimension": "oversized headers", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "apache-h1", + "dimension": "h2 GOAWAY", + "status": "skip", + "reason": "lacks goaway, h2", + "outcome": "skipped" + }, + { + "server": "apache-h1", + "dimension": "HTTP/3", + "status": "skip", + "reason": "lacks h3", + "outcome": "skipped" + }, + { + "server": "apache-h1", + "dimension": "HTTP/3 upgrade", + "status": "skip", + "reason": "lacks altsvc", + "outcome": "skipped" + }, + { + "server": "apache-h2", + "dimension": "trailers", + "status": "skip", + "reason": "lacks trailers", + "outcome": "skipped" + }, + { + "server": "apache-h2", + "dimension": "chunked bodies", + "status": "skip", + "reason": "lacks chunked", + "outcome": "skipped" + }, + { + "server": "apache-h2", + "dimension": "gzip", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "apache-h2", + "dimension": "conditional GET", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "apache-h2", + "dimension": "protocol negotiation", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "apache-h2", + "dimension": "connection reuse", + "status": "skip", + "reason": "lacks keepaliveLimit", + "outcome": "skipped" + }, + { + "server": "apache-h2", + "dimension": "oversized headers", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "apache-h2", + "dimension": "h2 GOAWAY", + "status": "skip", + "reason": "lacks goaway", + "outcome": "skipped" + }, + { + "server": "apache-h2", + "dimension": "HTTP/3", + "status": "skip", + "reason": "lacks h3", + "outcome": "skipped" + }, + { + "server": "apache-h2", + "dimension": "HTTP/3 upgrade", + "status": "skip", + "reason": "lacks altsvc", + "outcome": "skipped" + }, + { + "server": "haproxy-h1", + "dimension": "trailers", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "haproxy-h1", + "dimension": "chunked bodies", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "haproxy-h1", + "dimension": "gzip", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "haproxy-h1", + "dimension": "conditional GET", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "haproxy-h1", + "dimension": "protocol negotiation", + "status": "skip", + "reason": "lacks alpnMulti", + "outcome": "skipped" + }, + { + "server": "haproxy-h1", + "dimension": "connection reuse", + "status": "skip", + "reason": "lacks keepaliveLimit", + "outcome": "skipped" + }, + { + "server": "haproxy-h1", + "dimension": "oversized headers", + "status": "skip", + "reason": "lacks headerLimits", + "outcome": "skipped" + }, + { + "server": "haproxy-h1", + "dimension": "h2 GOAWAY", + "status": "skip", + "reason": "lacks goaway, h2", + "outcome": "skipped" + }, + { + "server": "haproxy-h1", + "dimension": "HTTP/3", + "status": "skip", + "reason": "lacks h3", + "outcome": "skipped" + }, + { + "server": "haproxy-h1", + "dimension": "HTTP/3 upgrade", + "status": "skip", + "reason": "lacks altsvc", + "outcome": "skipped" + }, + { + "server": "haproxy-h2", + "dimension": "trailers", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "haproxy-h2", + "dimension": "chunked bodies", + "status": "skip", + "reason": "lacks chunked", + "outcome": "skipped" + }, + { + "server": "haproxy-h2", + "dimension": "gzip", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "haproxy-h2", + "dimension": "conditional GET", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "haproxy-h2", + "dimension": "protocol negotiation", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "haproxy-h2", + "dimension": "connection reuse", + "status": "skip", + "reason": "lacks keepaliveLimit", + "outcome": "skipped" + }, + { + "server": "haproxy-h2", + "dimension": "oversized headers", + "status": "skip", + "reason": "lacks headerLimits", + "outcome": "skipped" + }, + { + "server": "haproxy-h2", + "dimension": "h2 GOAWAY", + "status": "skip", + "reason": "lacks goaway", + "outcome": "skipped" + }, + { + "server": "haproxy-h2", + "dimension": "HTTP/3", + "status": "skip", + "reason": "lacks h3", + "outcome": "skipped" + }, + { + "server": "haproxy-h2", + "dimension": "HTTP/3 upgrade", + "status": "skip", + "reason": "lacks altsvc", + "outcome": "skipped" + }, + { + "server": "quiche", + "dimension": "trailers", + "status": "skip", + "reason": "lacks trailers", + "outcome": "skipped" + }, + { + "server": "quiche", + "dimension": "chunked bodies", + "status": "skip", + "reason": "lacks chunked", + "outcome": "skipped" + }, + { + "server": "quiche", + "dimension": "gzip", + "status": "skip", + "reason": "lacks gzip", + "outcome": "skipped" + }, + { + "server": "quiche", + "dimension": "conditional GET", + "status": "skip", + "reason": "lacks conditional", + "outcome": "skipped" + }, + { + "server": "quiche", + "dimension": "protocol negotiation", + "status": "skip", + "reason": "lacks alpnMulti", + "outcome": "skipped" + }, + { + "server": "quiche", + "dimension": "connection reuse", + "status": "skip", + "reason": "lacks keepaliveLimit", + "outcome": "skipped" + }, + { + "server": "quiche", + "dimension": "oversized headers", + "status": "skip", + "reason": "lacks headerLimits", + "outcome": "skipped" + }, + { + "server": "quiche", + "dimension": "h2 GOAWAY", + "status": "skip", + "reason": "lacks goaway, h2", + "outcome": "skipped" + }, + { + "server": "quiche", + "dimension": "HTTP/3", + "status": "run", + "reason": null, + "outcome": "pass" + }, + { + "server": "quiche", + "dimension": "HTTP/3 upgrade", + "status": "skip", + "reason": "lacks altsvc", + "outcome": "skipped" + } + ] +} diff --git a/test/trailers.test.js b/test/trailers.test.js new file mode 100644 index 0000000..dd8cf6d --- /dev/null +++ b/test/trailers.test.js @@ -0,0 +1,160 @@ +/** + * Response trailers, and the ordering the fetch spec requires of them. + * + * Trailers only exist once a body has ended, so `response.trailers` does not resolve + * until the body has been consumed — see https://github.com/whatwg/fetch/pull/1940. + * That makes "await trailers, then read the body" a deadlock the caller wrote, which + * is why the pending case is asserted here rather than smoothed over: the promise has + * to stay cheap and cancellable while it waits, and it has to resolve the moment the + * body ends. + */ + +const test = require("tape"); +const http = require("node:http"); + +const { Agent } = require("../index.js"); +const { fetch } = require("../wrapper.js"); + +const PAYLOAD = "trailers-payload"; +const TRAILER = "x-checksum"; +const VALUE = "abc123"; + +/** + * `/trailers` sends one, `/plain` sends none. + * + * Cleartext HTTP/1.1 deliberately: trailers have nothing to do with TLS, and the + * shared test CA is generated by whichever openssl the platform ships -- which is a + * source of failures of its own, since rustls rejected that certificate for its + * extended key usage on macOS and Windows while accepting it on Linux. An OS-assigned + * port, since there is no UDP listener to coordinate with here. + */ +async function origin() { + const sockets = new Set(); + const server = http.createServer((req, res) => { + res.setHeader("content-type", "text/plain"); + if (req.url === "/trailers") { + res.setHeader("trailer", TRAILER); + res.write(PAYLOAD); + res.addTrailers({ [TRAILER]: VALUE }); + res.end(); + return; + } + res.end(PAYLOAD); + }); + server.on("connection", (socket) => { + sockets.add(socket); + socket.on("close", () => sockets.delete(socket)); + }); + + await new Promise((resolve, reject) => { + server.once("error", reject); + server.listen(0, "127.0.0.1", resolve); + }); + + return { + url: `http://127.0.0.1:${server.address().port}`, + agent: new Agent(), + close: () => + new Promise((resolve) => { + // The agent pools its connection, so a bare close() never settles. + for (const socket of sockets) socket.destroy(); + sockets.clear(); + server.close(resolve); + setTimeout(resolve, 500).unref(); + }), + }; +} + +test("trailers: arrive once the body has been read", async (t) => { + const server = await origin(); + try { + const res = await fetch(`${server.url}/trailers`, { agent: server.agent, timeout: 10000 }); + t.equal(await res.text(), PAYLOAD, "the body reads back"); + + const trailers = await res.trailers; + t.ok(trailers, "and the trailers resolve"); + t.equal(trailers.get(TRAILER), VALUE, "carrying what the server sent"); + } finally { + await server.close(); + t.end(); + } +}); + +test("trailers: a body with none resolves to null", async (t) => { + const server = await origin(); + try { + const res = await fetch(`${server.url}/plain`, { agent: server.agent, timeout: 10000 }); + await res.text(); + t.equal( + await res.trailers, + null, + "null rather than an empty Headers, so a caller can tell none arrived", + ); + } finally { + await server.close(); + t.end(); + } +}); + +test("trailers: pending until the body is consumed, and idle while pending", async (t) => { + const server = await origin(); + try { + const res = await fetch(`${server.url}/trailers`, { agent: server.agent, timeout: 10000 }); + + // Held, not awaited: awaiting here is the deadlock the spec describes. + const pending = res.trailers; + + const before = process.cpuUsage(); + const raced = await Promise.race([ + pending.then(() => "resolved"), + new Promise((resolve) => setTimeout(() => resolve("pending"), 500)), + ]); + const spentMs = (process.cpuUsage(before).user + process.cpuUsage(before).system) / 1000; + + t.equal(raced, "pending", "does not resolve before the body is consumed, per the spec"); + // The bug this replaces was a `yield_now` loop: half a second of waiting cost half + // a second of CPU, kept Node's event loop alive, and survived a test timeout. A + // parked future spends about a millisecond, so the threshold is loose on purpose: + // it only has to sit well under the ~500ms a spin costs, and stay clear of what a + // slow or noisy runner might attribute to this process. + t.ok( + spentMs < 200, + `waiting is idle, not a spin: ${spentMs.toFixed(0)}ms of CPU across 500ms of waiting`, + ); + + // And the same promise settles as soon as the body ends. + t.equal(await res.text(), PAYLOAD, "the body is still there to read"); + const trailers = await pending; + t.equal( + trailers && trailers.get(TRAILER), + VALUE, + "the promise held from before the read resolves once the body ends", + ); + } finally { + await server.close(); + t.end(); + } +}); + +test("trailers: a discarded body settles them as none", async (t) => { + const server = await origin(); + try { + const res = await fetch(`${server.url}/trailers`, { agent: server.agent, timeout: 10000 }); + + // Held first, so this also covers the case that used to wedge: a waiter already + // parked when the body gets thrown away. + const pending = res.trailers; + await res.discard(); + + t.equal( + await pending, + null, + "discarding the body discards its trailers, rather than waiting for trailers " + + "that can no longer arrive", + ); + t.equal(await res.trailers, null, "and asking again says the same thing"); + } finally { + await server.close(); + t.end(); + } +}); diff --git a/wrapper.d.ts b/wrapper.d.ts index d0cc20e..e6aab90 100644 --- a/wrapper.d.ts +++ b/wrapper.d.ts @@ -309,9 +309,17 @@ export class Response { * resolves to either `null` or a `Headers` structure that contains the HTTP/2 or /3 trailing * headers. * - * This was once in the spec but was removed as it wasn't implemented by any browser. + * This was once in the spec but was removed as it wasn't implemented by any browser; + * https://github.com/whatwg/fetch/pull/1940 is the current effort to bring it back. * - * Note that this will never resolve if you don't also consume the body in some way. + * Trailers arrive after the body ends, so this does not resolve until the body has been + * consumed — by `text()`, `bytes()`, `json()`, `blob()`, or reading the `body` stream. + * Awaiting it on its own, without ever reading the body, waits forever; that is what + * the spec above describes. Holding the promise while something else reads the body is + * fine, and costs nothing while it is pending. + * + * `discard()` counts as consuming the body but discards its trailers with it: this then + * resolves to `null`. */ readonly trailers: Promise;