Skip to content

Commit fd3ab0a

Browse files
fix(plugin-hono-server): keep a graceful close from cutting off a response still flushing to a slow client (#31255)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1X6PYHRe3yiUSd4YRysSoxcj)_ Fixes #31208 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:服务优雅关闭(`HonoHttpServer.close()`)时,一个已经调用 `end()`、但还在往慢客户端发送的响应,不再在关闭开始那一刻被掐断;空闲的 keep-alive 连接改为在没有响应仍在发送时才统一释放。 - 为什么改:每次 `os serve` / 运行时关停或重新部署时,正在下载大响应的客户端会收到被截断的内容(实测 32 MiB 只收到 3,919,702 字节),原因是 Node 自己的 `server.close()` 把「已 end 未发完」的连接当作空闲连接销毁了。 - 风险与回滚:只改 `@objectstack/plugin-hono-server` 一个文件,patch 级;关闭窗口的强制关闭兜底(默认 10 秒)原样保留,所以最坏情况是空闲连接晚一点释放,不会让关停卡住;回滚即还原本 PR。 - 先看:`packages/plugins/plugin-hono-server/src/adapter.ts` 的 `closeIdleConnectionsOnceFlushed`(什么时候推迟清扫),再看 `tryListen` 里把它装到 server 实例上的那三行,以及 `trackForDrain` 里响应 `'close'` 时补跑被推迟的清扫。 - 两分钟验证:`pnpm --filter @objectstack/plugin-hono-server exec vitest run src/adapter-drain.test.ts`,预期 9 个用例全绿。 - 需要你做的:无,席位落地。 ## What changes `packages/plugins/plugin-hono-server/src/adapter.ts`, the drain start of `HonoHttpServer.close()`: - **What destroyed the response, measured.** On Node 22.23.4 (`.nvmrc` is `22`, `engines` is `>=22.0.0`, and every CI job reads `.nvmrc` or pins `'22'`), a `Socket.prototype.destroy` probe during the reordered slow-reader case recorded two destroys of the flushing socket, in this order: first `Server.closeIdleConnections` called from `httpServerPreClose` from `Server.close` (`adapter.ts` `server.close(...)`), then the adapter's own `server.closeIdleConnections()` call on the next line. So Node's own `server.close()` sweep destroys it first; the adapter's call would have done the same. Both skip a connection only while `socket._httpMessage.finished` is false, and `end()` sets `finished` before the bytes are flushed. - **The guard.** `tryListen` puts a `closeIdleConnections` on the server instance that wraps Node's: `closeIdleConnectionsOnceFlushed` runs Node's sweep only when no tracked response is `writableEnded && !writableFinished`. Otherwise it holds the sweep back. Node's `httpServerPreClose` calls `server.closeIdleConnections()` on the instance (the probe's stack shows it), so both sweeps at drain start go through the guard, and `server.close()` still stops accepting and clears Node's connection-check interval at once. - **Each flushing connection closes after its own `'finish'`.** That path is PR #31189's, unchanged: the response's `'close'` `destroySoon()`s its own socket once it has finished and nothing else is owed on that connection. - **The held-back sweep then runs.** Each tracked response's `'close'` (in `trackForDrain`) re-asks the guard when a sweep is held back, so idle keep-alive connections are released as soon as no response is flushing, not a keep-alive window later. Node's raw server-wide sweep is still never run from a response's `'close'` (the PR #31189 M1 hazard): the only sweep there is the guarded one, which runs only when nothing is flushing. - **The backstop is untouched.** The `closeAllConnections()` timer and its window are byte-identical. A reader that never drains is now held until the window ends instead of being destroyed at drain start. **Why this route and not the other one the card names.** Stopping the accept without the sweep means calling `net.Server.prototype.close` instead of `server.close()`. That skips the other half of `httpServerPreClose`, `clearInterval(server[kConnectionsCheckingInterval])`, an interval only reachable through a private symbol, so it would keep the closed server alive in a timer; and calling `server.close()` later to clear it emits `'close'` a second time when the connections have already drained. Deferring the whole `server.close()` keeps accepting new connections during the flush. The instance guard keeps Node's `close()` whole and changes only when its sweep runs, on every Node the CI runs (22) and any with the sweep (19 and later). ## Tests `packages/plugins/plugin-hono-server/src/adapter-drain.test.ts`, three new cases: - **The reordered slow-reader pin.** A 32 MiB response calls `end()` to a paused raw socket before `close()` starts; the case asserts the premise (`writableEnded` true, `writableFinished` false), then the client receives every byte against `Content-Length`. An idle keep-alive connection sits beside it and must be released within 1 s of the flush ending (the keep-alive timer would hold it 5 s). **Red on `main`** (`df22adc3`, adapter unchanged since `714dda8f`): `every byte of the body arrived: expected 3919702 to be 33554432`. - **Control: an idle keep-alive connection is released at drain start.** Nothing is flushing, so the sweep runs at once: `close()` and the connection end within 1 s. - **Control: the backstop still force-closes a reader that never drains.** A paused reader of a 32 MiB response with a 300 ms window: `close()` takes at least 250 ms (held for the window, not swept at drain start) and under 2 s, and the reader gets fewer than 32 MiB. <details><summary>Ablations</summary> Through `scripts/ablation-replace.mjs` on the committed head, anchor hit once, restore proven by blob == HEAD and an empty `git diff HEAD`. The suite imports `./adapter` from `src/`, so no build sits on the path. - A1 (head `ab2845c8`), the guard's flushing check disabled (`if (false && res.writableEnded && !res.writableFinished)`): 2 failed / 7 passed. `a response that ended before close() but is still flushing to a slow reader arrives whole`: `expected 3919702 to be 33554432`; `the drain window still force-closes a reader that never drains`: `expected 1 to be greater than or equal to 250`. - A2 (head `1502afc5`), the held-back sweep never re-run (the `if (sweep) this.closeIdleConnectionsOnceFlushed(sweep);` line deleted): 1 failed / 8 passed, the slow-reader case `Test timed out in 5000ms` (the idle keep-alive connection holds `close()` open). </details> ## Acceptance notes - **The guard relies on Node calling the instance method.** `httpServerPreClose` reads `server.closeIdleConnections()` dynamically in Node 22 (measured by the probe's stack). If a future Node called the prototype directly, the slow-reader pin goes red again; it does not fail silently. - **Idle connections wait while something flushes.** While an ended response is still flushing, the sweep is held back, so an idle keep-alive connection stays open until the flush ends (then it is released at once) or the window ends. A request arriving on it meanwhile is answered `Connection: close` by PR #31189's request-time line. Under a stream of new slow flushes the backstop decides, as before. - **A pipelined response queued behind another counts as flushing once it has ended** (its bytes sit in `outputData`), which only holds the sweep back until it is written. A dead connection's responses drop out with the connection's entry, so they cannot hold the sweep. ## Verification Gate union derived by `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` on the final head `ab2845c8` (`origin/main` merged once, at `733efbd4`): 63 families, each exit code captured before any pipe. - **63 run, exit 0 each.** `--ran`: "63 derived famil(ies) accounted for — 63 run, 0 NOT-MEASURED". Three first read exit 3 (prerequisite, not measured) and were re-run green: `check-plugin-teardown-shape --self-test` after `git fetch --depth=1 origin 621a487` for its fixture, and `check:dual-build-cjs-loads` and `check:lean-entry-closure` after a full `pnpm turbo run build --concurrency=2` under the lock (VERDICT command-exit 0). `check:dts-closure` and `check:published-files` were read again on the full build, exit 0. - `pnpm --filter @objectstack/plugin-hono-server build && typecheck && test` under the lock on `ab2845c8`: VERDICT command-exit 0, Test Files 38 passed (38), Tests 484 passed (484). The package `tsconfig.json` includes `src/**/*`, so `typecheck` compiles the test file. - `pnpm lint` scoped: `eslint --no-inline-config` over the two touched source files, exit 0. - No dogfood battery: no `!` in the title and no `(narrowing)` arm. - CI families are read from this PR's check-runs, not re-run here. --- _Generated by [Claude Code](https://claude.ai/code/session_01FEkQ5puKTX6kQoJs8iKrFr)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 455229e commit fd3ab0a

3 files changed

Lines changed: 151 additions & 11 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@objectstack/plugin-hono-server': patch
3+
---
4+
5+
fix(plugin-hono-server): a graceful `close()` no longer cuts off a response that has ended but is still being sent to a slow client
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
When `HonoHttpServer.close()` started, Node's own `server.close()` released idle keep-alive connections first thing, and counted a response as done as soon as it had called `end()`, before its bytes were sent. A large response still flushing to a slow client at that moment was destroyed mid-body, so the client got a truncated download. The release of idle connections now waits until no ended response is still flushing; each such response closes its own connection once every byte has been sent, and idle connections are released right after. The drain window still force-closes whatever outlasts it.

‎packages/plugins/plugin-hono-server/src/adapter-drain.test.ts‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,88 @@ describe('HonoHttpServer — graceful close drains in-flight requests (P1-3)', (
185185
await closeP;
186186
});
187187

188+
it('a response that ended before close() but is still flushing to a slow reader arrives whole', async () => {
189+
// Node counts a response as done once it has called `end()`, before its
190+
// bytes are flushed. `server.close()` sweeps idle connections first thing
191+
// (Node 19 and later), so without the guard this response's connection is
192+
// destroyed at drain start: 3,919,702 of 33,554,432 bytes arrived on main.
193+
// An idle keep-alive connection alongside it must still be released by the
194+
// drain, once the flush is done, rather than holding `close()` open until
195+
// the server's keep-alive timer fires.
196+
const BIG = new Uint8Array(32 * 1024 * 1024).fill(0x61);
197+
let bigResponse: ServerResponse | undefined;
198+
const server = new HonoHttpServer(0, undefined, 10_000);
199+
server.getRawApp().get('/big', (c) => {
200+
bigResponse = (c.env as { outgoing: ServerResponse }).outgoing;
201+
return c.body(BIG);
202+
});
203+
server.getRawApp().get('/probe', (c) => c.text('idle-after-this'));
204+
await server.listen(0);
205+
const port = server.getPort();
206+
207+
const idle = await rawConnection(port);
208+
idle.socket.write('GET /probe HTTP/1.1\r\nHost: 127.0.0.1\r\n\r\n');
209+
const slowReader = await rawConnection(port);
210+
slowReader.socket.pause(); // reads nothing until resumed below
211+
slowReader.socket.write('GET /big HTTP/1.1\r\nHost: 127.0.0.1\r\n\r\n');
212+
await sleep(100);
213+
expect(splitResponse(idle.received()).headers.connection).toBe('keep-alive');
214+
// The premise: the big response has called end() and is still flushing.
215+
expect(bigResponse?.writableEnded, 'the big response has called end()').toBe(true);
216+
expect(bigResponse?.writableFinished, 'the big response is still flushing').toBe(false);
217+
218+
const closeP = server.close();
219+
await sleep(50);
220+
slowReader.socket.resume();
221+
await slowReader.ended;
222+
const flushed = Date.now();
223+
const { head, headers, body } = splitResponse(slowReader.received());
224+
expect(head).toMatch(/^HTTP\/1\.1 200/);
225+
expect(Number(headers['content-length'])).toBe(BIG.length);
226+
expect(body.length, 'every byte of the body arrived').toBe(BIG.length);
227+
228+
await idle.ended;
229+
await closeP;
230+
// The keep-alive timer would hold it 5 s; the release takes milliseconds.
231+
expect(Date.now() - flushed).toBeLessThan(1000);
232+
});
233+
234+
it('an idle keep-alive connection is released at drain start', async () => {
235+
const server = new HonoHttpServer(0, undefined, 10_000);
236+
server.getRawApp().get('/probe', (c) => c.text('idle-after-this'));
237+
await server.listen(0);
238+
const idle = await rawConnection(server.getPort());
239+
idle.socket.write('GET /probe HTTP/1.1\r\nHost: 127.0.0.1\r\n\r\n');
240+
await sleep(50);
241+
expect(splitResponse(idle.received()).headers.connection).toBe('keep-alive');
242+
243+
const t0 = Date.now();
244+
await server.close();
245+
await idle.ended;
246+
// Nothing is flushing, so the sweep runs at once: no keep-alive wait.
247+
expect(Date.now() - t0).toBeLessThan(1000);
248+
});
249+
250+
it('the drain window still force-closes a reader that never drains', async () => {
251+
const BIG = new Uint8Array(32 * 1024 * 1024).fill(0x61);
252+
const server = new HonoHttpServer(0, undefined, 300);
253+
server.getRawApp().get('/big', (c) => c.body(BIG));
254+
await server.listen(0);
255+
const stalled = await rawConnection(server.getPort());
256+
stalled.socket.pause(); // never resumed
257+
stalled.socket.write('GET /big HTTP/1.1\r\nHost: 127.0.0.1\r\n\r\n');
258+
await sleep(100);
259+
260+
const t0 = Date.now();
261+
await server.close();
262+
const elapsed = Date.now() - t0;
263+
expect(elapsed).toBeGreaterThanOrEqual(250); // held for the window, not swept at drain start
264+
expect(elapsed).toBeLessThan(2000); // the 300 ms window, not the reader
265+
stalled.socket.resume(); // read what made it out, to see the connection end
266+
await stalled.ended; // the backstop destroyed it
267+
expect(stalled.received().length).toBeLessThan(BIG.length);
268+
});
269+
188270
it('forgets a pipelined response whose connection died before it was written', async () => {
189271
// Pipelined requests queue behind the first one's response; the queued
190272
// responses have no socket of their own, so if the connection dies they

‎packages/plugins/plugin-hono-server/src/adapter.ts‎

Lines changed: 59 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,12 @@ export class HonoHttpServer implements IHttpServer {
595595
* another on a connection that died) is not held past its connection.
596596
*/
597597
private inFlightResponses = new Map<Socket, OwedResponses>();
598+
/**
599+
* Node's idle-connection sweep, held back because a response was still
600+
* flushing when it was asked for. {@link trackForDrain} runs it once none
601+
* is — see {@link closeIdleConnectionsOnceFlushed}.
602+
*/
603+
private deferredIdleSweep: (() => void) | undefined;
598604
/**
599605
* Every `(method, pattern)` pair registered through this server, kept so
600606
* the `notFound` handler can answer "the path exists but the method is
@@ -1849,6 +1855,12 @@ export class HonoHttpServer implements IHttpServer {
18491855
resolve();
18501856
});
18511857
this.server = server;
1858+
// On the instance, so `server.close()` reaches it too: Node's
1859+
// `httpServerPreClose` calls `server.closeIdleConnections()`.
1860+
if ('closeIdleConnections' in server) {
1861+
const sweepIdle: () => void = server.closeIdleConnections.bind(server);
1862+
server.closeIdleConnections = () => this.closeIdleConnectionsOnceFlushed(sweepIdle);
1863+
}
18521864
// Prepended so it runs before the Hono listener can write a
18531865
// synchronous response's headers.
18541866
server.prependListener('request', (req: IncomingMessage, res: ServerResponse) => {
@@ -1880,10 +1892,13 @@ export class HonoHttpServer implements IHttpServer {
18801892
* handed to the socket — and no other response on that connection is
18811893
* pending. `destroySoon()` flushes what is queued before it closes.
18821894
*
1883-
* ⛔ Never the server-wide `closeIdleConnections()` from here: Node counts
1884-
* a response as done once it has called `end()`, before its bytes are
1885-
* flushed, so a sweep when one response ends destroys any other connection
1886-
* whose response is ended but still flushing to a slow reader.
1895+
* ⛔ Never Node's own server-wide sweep from here: Node counts a response
1896+
* as done once it has called `end()`, before its bytes are flushed, so a
1897+
* sweep when one response ends destroys any other connection whose
1898+
* response is ended but still flushing to a slow reader. The only sweep
1899+
* run here is one `close()` asked for and
1900+
* {@link closeIdleConnectionsOnceFlushed} held back, and it still runs only
1901+
* once no response is flushing.
18871902
*
18881903
* A connection's entry is dropped when the connection closes, not only
18891904
* when each response does: a response pipelined behind another never gets
@@ -1898,12 +1913,42 @@ export class HonoHttpServer implements IHttpServer {
18981913
owed.add(res);
18991914
res.once('close', () => {
19001915
owed.delete(res);
1901-
if (!this.draining || owed.size > 0 || !res.writableFinished) return;
1902-
if (socket.destroyed || socket.writableEnded) return;
1903-
socket.destroySoon();
1916+
if (this.draining && owed.size === 0 && res.writableFinished
1917+
&& !socket.destroyed && !socket.writableEnded) {
1918+
socket.destroySoon();
1919+
}
1920+
const sweep = this.deferredIdleSweep;
1921+
if (sweep) this.closeIdleConnectionsOnceFlushed(sweep);
19041922
});
19051923
}
19061924

1925+
/**
1926+
* Node's `closeIdleConnections()`, but never while a response is ended and
1927+
* still flushing.
1928+
*
1929+
* Node counts a connection as idle once its request has been read, and
1930+
* skips it only while its response has not called `end()`. `end()` comes
1931+
* before the bytes are flushed, so a sweep destroys a connection that is
1932+
* still sending an ended response to a slow reader, and the client gets a
1933+
* truncated body. `server.close()` runs that sweep first thing (Node 19 and
1934+
* later), so the drain's force-close backstop never gets a say. Such a
1935+
* connection closes on its own once flushed ({@link trackForDrain}); the
1936+
* sweep is held back until no response is flushing, then run for the idle
1937+
* connections it is for.
1938+
*/
1939+
private closeIdleConnectionsOnceFlushed(sweep: () => void) {
1940+
for (const owed of this.inFlightResponses.values()) {
1941+
for (const res of owed) {
1942+
if (res.writableEnded && !res.writableFinished) {
1943+
this.deferredIdleSweep = sweep;
1944+
return;
1945+
}
1946+
}
1947+
}
1948+
this.deferredIdleSweep = undefined;
1949+
sweep();
1950+
}
1951+
19071952
/** The {@link inFlightResponses} entry for one connection, made on its first request and dropped when it closes. */
19081953
private responsesOwedOn(socket: Socket): OwedResponses {
19091954
const known: OwedResponses | undefined = this.inFlightResponses.get(socket);
@@ -1931,10 +1976,12 @@ export class HonoHttpServer implements IHttpServer {
19311976
// Graceful drain (P1-3): stop accepting new connections and let in-flight
19321977
// requests finish rather than force-killing them mid-response.
19331978
// `closeIdleConnections()` releases idle keep-alive sockets so the process
1934-
// can exit promptly; active requests keep running until they complete or
1935-
// the drain window elapses, and each one's connection closes as soon as
1936-
// its response ends (`trackForDrain`), so the drain finishes with the
1937-
// last in-flight request rather than a keep-alive window after it.
1979+
// can exit promptly, once no ended response is still flushing
1980+
// (`closeIdleConnectionsOnceFlushed`); active requests keep running
1981+
// until they complete or the drain window elapses, and each one's
1982+
// connection closes as soon as its response ends (`trackForDrain`), so
1983+
// the drain finishes with the last in-flight request rather than a
1984+
// keep-alive window after it.
19381985
this.draining = true;
19391986
for (const owed of this.inFlightResponses.values()) {
19401987
for (const res of owed) {
@@ -1963,6 +2010,7 @@ export class HonoHttpServer implements IHttpServer {
19632010
});
19642011
this.server = undefined;
19652012
this.draining = false;
2013+
this.deferredIdleSweep = undefined;
19662014
this.inFlightResponses.clear();
19672015
}
19682016
}

0 commit comments

Comments
 (0)