End HTTP/2 responses on their final DATA frame

[habdelra]

Node's http2 compat layer — the half that turns an Http2Stream into the (req, res) pair Koa is written against — responds with waitForTrailers: true unconditionally. That moves END_STREAM off the final DATA frame and onto an empty trailers HEADERS frame emitted from a setImmediate (finishSendTrailers in lib/internal/http2/core.js), which drops that frame when the stream is already destroyed. A response whose END_STREAM never arrives leaves its peer holding a body it can neither complete nor fail.

Nothing in this repo sends HTTP trailers, so the deferral buys nothing. endStreamOnFinalDataFrame wraps the request listener createSecureServer is given and declines it, so the last DATA frame carries END_STREAM itself. Response bytes are unchanged.

It wraps the request listener rather than the stream event because the compat listener node registers runs first and responds synchronously for a request the app answers without awaiting — a stream listener added afterwards misses exactly those. allowHTTP1 means it also sees HTTP/1.1 requests, whose res has no backing stream; those pass through untouched.

What this does and does not claim

This started as a fix for the 60s host-test timeouts on shard 16 of run 32796026625. It should not be read that way, and the code comment no longer says so:

• The deferral is real and checkable. Every compat response goes out as {endStream: false, waitForTrailers: true, sendDate: true} on node 24, and the new test pins that declining it works.
• The bridge to a hung peer is not established. 600 responses finishing in one event-loop turn fire wantTrailers 600 times, flush 600 trailers frames, and deliver 600 complete responses — nothing destroyed early. Forcing the window (destroying the stream from a wantTrailers listener registered after the compat layer's) still gives the peer end plus close rst=0. Two independent attempts have failed to reach the state that would strand a peer.
• The peer is not the browser where the flake happens. Under BOXEL_ENVIRONMENT — ci on the host-test shards — Traefik fronts the realm server and negotiates h2 by ALPN to an https://host.docker.internal:<port> upstream (registerService(..., { http2: true })); server.ts says as much, "Traefik is the only client". A truncated response would land on Traefik's Go client first, and whether it forwards a stream it can neither complete nor fail is an open question.

So: this removes a window rather than reasoning about its reachability, at no behavioral cost. Whether it moves the timeouts is unproven.

Diagnostics

Independent of the above, and the part most likely to help next time. describePendingWaiter in host/tests/helpers/setup.ts prefers a token's label over its stack frame, but no call site passed one, so a moment-of-timeout dump repeated the same frame once per pending token. Labelled now:

• fetcher → the method and target of the request in flight, query values replaced with [redacted]
• store-service → the store operation and its subject (persistAndUpdate <id>, create <type> in <realm>, …)
• realm:incremental-indexing → the realm whose index event has not landed

If a 60s timeout recurs, the dump names the stuck request instead of pointing at fetcher.ts.

Known gaps

• describeFetchRequest in host/tests/helpers/setup.ts prints full URLs with query values into the same dump, so the two lines still disagree on redaction policy. Its url is load-bearing for test-realm routing, not only logging, so redacting it is a separate change; noted in the comment rather than done here.
• monaco-test-waiter.ts already receives an operation string per token and drops it at beginAsync() — the same one-call-site-many-operations case, label already in hand. Not touched here.

Verification

• New case in listener-dispatcher-test.ts drives the real createListener and asserts wantTrailers — which node emits only for a response that took the trailers path — never fires. All 12 tests in that module pass; with the wrapper removed only the new one fails, while the response body still arrives intact.
• lint and lint:types clean for realm-server, host, and runtime-common.
• Host and realm-server suites green in CI.

🤖 Generated with Claude Code

https://claude.ai/code/session_016ooR7A4u4BpLufmYBzPnJe

[github-actions]

Preview deployments

• Host staging preview

• Host production preview

Host Test Results

    1 files  ±0      1 suites  ±0   2h 37m 34s ⏱️ + 5m 6s
4 378 tests ±0  4 364 ✅ ±0  14 💤 ±0  0 ❌ ±0 
4 397 runs  ±0  4 383 ✅ ±0  14 💤 ±0  0 ❌ ±0 

Results for commit 948b518. ± Comparison against earlier commit f10888f.

Realm Server Test Results

    1 files  ± 0      1 suites  ±0   12m 30s ⏱️ -22s
2 293 tests +12  2 293 ✅ +12  0 💤 ±0  0 ❌ ±0 
2 376 runs  +12  2 376 ✅ +12  0 💤 ±0  0 ❌ ±0 

Results for commit 948b518. ± Comparison against earlier commit f10888f.

[Copilot]

Pull request overview

This PR addresses intermittent host test timeouts caused by Chromium hanging on HTTP/2 responses that end via Node’s deferred empty-trailers frame (rather than END_STREAM on the final DATA frame). It also improves test-timeout diagnostics by adding per-operation labels to test-waiter tokens so the “pending waiters” dump is actionable.

Changes:

• Wrap the realm-server HTTP/2 request listener to disable waitForTrailers, ensuring END_STREAM is carried on the final DATA frame.
• Add a realm-server test that probes for wantTrailers to verify responses don’t take the trailers path.
• Extend runtime-common test-waiter plumbing to accept per-item labels, and apply labels to fetcher/store/indexing waiters to improve timeout diagnostics.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
packages/runtime-common/test-waiters.ts	Extends waiter API to support per-item labels for better pending-waiter diagnostics.
packages/runtime-common/fetcher.ts	Adds a per-request label to the fetcher waiter token.
packages/realm-server/tests/listener-dispatcher-test.ts	Adds an HTTP/2 regression test asserting no trailers path (wantTrailers) is taken.
packages/realm-server/server.ts	Wraps the HTTP/2 request listener to force END_STREAM on the final DATA frame by disabling waitForTrailers.
packages/host/app/services/store.ts	Labels store-operation waiter tokens to identify which store operation is pending at timeout.
packages/host/app/services/realm.ts	Labels realm incremental-indexing waiter token with the realm URL.
.devcontainer/claude-web-h2-preload.cjs	Updates comment to reflect that realm-server now applies the trailer-avoidance itself.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[backspace]

[Claude Code 🤖] Reviewed the h2 change against node 24.17's http2 compat layer and against the env-mode topology the host shards actually run in, plus every withTestWaiters / beginAsync call site and the moment-of-timeout dump the new labels feed. I did not run the host suite.

No blocking issues. The wrapper is behavior-preserving — nothing in the repo sends trailers, and hosted staging/prod take createListener's cert-less HTTP/1.1 branch, so the blast radius is local dev and CI. What needs work is prose rather than code: the mechanism comment states a causal chain whose middle link I could not reproduce, and it names Chromium as the peer in a topology where Traefik is.

Recommendations:

1. Narrow the endStreamOnFinalDataFrame comment to the part that holds up, or name the stream destroyer that opens the window — thread on that comment block in packages/realm-server/server.ts.
2. Answer whether the mechanism survives the Traefik hop in env mode — same thread. That is what decides whether this PR should claim the 60s host-test timeouts.
3. Fix the describeRequest parenthetical and settle the redaction policy against its twin in host/tests/helpers/setup.ts — thread on packages/runtime-common/fetcher.ts.
4. Give the create waiter label a per-operation discriminator — thread on packages/host/app/services/store.ts.

Adjacent, not asked of this PR:

• packages/host/app/utils/editor/monaco-test-waiter.ts already receives an operation string per token and calls waiter.beginAsync() without it, so monaco-rendering is the same one-call-site-many-operations case with the label already in hand.
• In standard local dev, vite terminates TLS itself (devHttpsConfig in packages/host/vite.config.mjs) and therefore serves h2 from node too; under BOXEL_ENVIRONMENT it is plain HTTP behind Traefik. If the deferral matters, that is the other node h2 server in the picture.