Repository navigation
fix(plugin-hono-server): end a graceful close with its last in-flight request; cut or budget seven slow Test Core cases - #31189
Conversation
…ponse ends during a drain close() released idle keep-alive sockets only at the start of the drain, so a connection whose request was still running became an idle keep-alive socket afterwards and server.close() waited for the client to drop it (3 s for fetch, 4 s for the node:http(s) agent). During a drain a response whose headers are not yet written now goes out with Connection: close, and one that already sent keep-alive headers releases its socket through closeIdleConnections() once it ends. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ihL2ebbZkr2QAEG1x7UC8
- metadata-protocol: the purity case derives once and compares it with the module-load singleton instead of deriving twice. - client: the shared auth server and its better-auth instance are built at module scope, out of the first case's clock. - driver-sql, plugin-security, plugin-sharing, cli census: an explicit per-case budget of 3x the slowest Test Core reading, rounded up to the next second, with the readings and the arithmetic beside it. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ihL2ebbZkr2QAEG1x7UC8
…st-core-slow-cases
📓 Docs Drift CheckThis PR changes 1 package(s): 3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
What this run could not see
Coarse fallback — 9 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin edf77727516c530e813ee26b289d218d6243ba83 && git checkout edf77727516c530e813ee26b289d218d6243ba83
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 658d84f5756e616d6418cb0ecf38630e04b4e341 586338ffbdd25e1fa9c86965dcfa3534e8993ad0 && git checkout -B drift-repro 658d84f5756e616d6418cb0ecf38630e04b4e341 && git merge --no-ff 586338ffbdd25e1fa9c86965dcfa3534e8993ad0
node scripts/docs-audit/affected-docs.mjs --json 658d84f5756e616d6418cb0ecf38630e04b4e341
|
|
domain:devx#2 · marchtian · session_017ihL2ebbZkr2QAEG1x7UC8 Contract reviewServed-tier: ① Derived judgmentsSections ① to ③ are the round-1 review of the earlier head Read against Gates on this head (the check-runs are the verdicts). Green: The drain fix (
|
…ction, and track drain state per connection The round-1 drain called the server-wide closeIdleConnections() whenever a keep-alive response ended. Node treats a response as done once it has called end(), before its bytes are flushed, so that sweep destroyed any other connection whose response was ended but still flushing. The ending response now destroySoon()s its own socket, once it has finished and no other response on that connection is pending. The tracked responses are now keyed by connection and dropped when the connection closes: a response pipelined behind another never gets a socket if the connection dies, never emits 'close', and was held until close(). Pins: a slow reader's 32 MiB body arrives whole while another stream ends during the drain; pipelined responses on a destroyed connection are forgotten; a request whose headers finish after close() began is answered Connection: close. The driver-sql budget comment states the rule's over-half threshold and why a reading 1 ms under it takes a budget. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ihL2ebbZkr2QAEG1x7UC8
… the tenant-audit census can place its receiver Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ihL2ebbZkr2QAEG1x7UC8
|
domain:devx#2 · marchtian · session_017ihL2ebbZkr2QAEG1x7UC8 维护者速读
席位意见:可以落地。第一轮的两项必须修都是真问题,第二轮修复后已对照 Node 22 源码核实,三条新用例都做了反向对照(去掉修复即变红)。 席位核对 — ACCEPT ·
|
Requested by 田騏 · project thread
Fixes #30920
Clause-②: no
Metadata-protocol: none
维护者速读
Connection: close,已发 keep-alive 头的在全部字节发完后只关自己这条连接,不碰别的连接);关闭期间的连接记账按连接存放、连接断开即清掉;另外七条慢用例里两条砍掉重复工作、四条按「CI 最慢读数 ×3」给了单独超时,一条经测量不需要改。plugin-hono-server的关闭路径;一条响应结束时只释放它自己的连接,别的连接上还在发送的大响应不受影响(有慢读者 32 MiB 用例钉住),流水线请求在连接断开后不再被留在内存里;drain 超时强关的兜底不变;其余都是测试文件。回滚就是 revert 本 PR。packages/plugins/plugin-hono-server/src/adapter.ts的trackForDrain、responsesOwedOn与close();adapter-drain.test.ts新增的慢读者、流水线断连与迟到请求头三条用例;packages/client/src/auth-rotated-session-token.test.ts模块顶层新增的fileServer()预建。pnpm --filter @objectstack/plugin-hono-server exec vitest run src/adapter-drain.test.ts src/adapter-tls-listener.test.ts --reporter=verbose,应看到 12 条全过,两条 drain 用例约 0.2 秒(改前 3.2 秒与 4.2 秒)。What changes
The drain (
packages/plugins/plugin-hono-server/src/adapter.ts, patch changeset).close()callscloseIdleConnections()once, when the drain starts. A connection whose request is still running at that moment is skipped, and Node never runs the release again. When the response ends, the connection becomes an idle keep-alive socket, andserver.close()waits until the client drops it. The client drops it after the server'sKeep-Alive: timeout=5hint minus its own margin: 3 s forfetch(undici) and 4 s for thenode:http(s)agent. Measured with a socket trace (see Tests): the response finishes at about 220 ms, the server socket waits with a 6000 ms timer, andclose()resolves only when the client sends FIN, at 3232 ms (fetch) and 4216 ms (agent).During a drain the adapter now does two things:
Connection: close, so Node ends the socket once it is sent and the client knows not to reuse it. This covers responses in flight whenclose()starts, and requests whose headers finish arriving on an open connection after the drain began;destroySoon()once it has finished (every byte handed to the socket) and no other response on that connection is pending. It never calls the server-widecloseIdleConnections(): Node counts a response as done once it has calledend(), before its bytes are flushed, so a sweep at that moment would destroy any other connection whose response is ended but still flushing to a slow reader.The drain bookkeeping is kept per connection (
responsesOwedOn) and dropped when the connection closes. A response pipelined behind another never gets a socket if the connection dies, so it never emits'close'; keying by connection keeps it from being held untilclose(). The drain window still force-closes whatever outlasts it.The seven other cases. Each budget follows the rule PR #30919 used: 3x the slowest
Test Corereading on the card, rounded up to the next second, with the readings and the arithmetic in a comment at the case. No global timeout, no skip, no retry.metadata-protocoltwo derivations of the same schemas agree exactlyclientlogin → enable → verifyTotp → disable → deleteUserdriver-sqlsql-driver.ts and the modules split out of it contain none of themplugin-securityposture single/group/isolated hidden-column casesplugin-sharinga refused upgrade write does not block the readclithe census reaches the live treeclithe boot this sweep reads from really did compose the deploymentTests
adapter-drain.test.tspins each part:Connection: close;close()completes, andclose()resolves less than 1000 ms after its body ends (the defect waits about 3000 ms);close()starts, the rest arrives: the answer isConnection: close;end()during the drain and is still flushing (asserted) when another keep-alive stream ends: every byte arrives;Ablation, one part at a time, through
scripts/ablation-replace.mjson head586338ffbd(the tests import./adapterfrom source, so no build is involved; each restore verified as blob == HEAD with an emptygit diff HEAD):closeIdleConnections()in place of the own-socket release: the slow-reader case goes red,expected 3919730 to be 33554432bytes;'close': the pipelined case goes red,expected 3 to be +0;Connection: closeline: the late-headers case goes red,expected 'keep-alive' to be 'close'.The whole new test file against the round-1
adapter.ts(2ff6bb0735): the slow-reader case fails (expected 3919730 to be 33554432) and the pipelined case fails (the round-1 set holds responses, so its retained count readsNaNagainst the expected 4); the late-headers case passes, as the round-1 code had that line.Socket trace of close() with one request in flight (before / after)
Before (
fetch):Before (
node:httpagent, keepAlive): FIN from the client andclose()resolved at 4216 ms. Withagent: false(no keep-alive)close()resolved at 215 ms, which isolates the keep-alive wait as the cause.After (
fetch):Drain cases, local, before / after
adapter-tls-listener.test.tsdrains on close exactly as the plain listener doesadapter-drain.test.tslets an in-flight request complete instead of aborting itAcceptance notes
cliunmanaged-tables "the boot this sweep reads from really did compose the deployment" is not changed. Timing trace (instrumented locally, not committed): the case body takes 1-2 ms, itsafterEach3 ms, and itsbeforeEach10038-10378 ms, nearly all of it the firstbootSchemaStackof the worker. vitest reports hooks inside a case's duration, but the 5 stestTimeoutclocks only the body; thisbeforeEachdeclares its own180_000. So the 5 s default cannot time this case out, and a per-case timeout on a 2 ms body would change nothing. The later cases in the file boot in 90-145 ms. The local readings are on a box shared with other builds; the card's CI reading is 3295 ms.clientkeeps no budget, by the file's own rule. Its header says a recurrence is answered by cutting the case's own work, ⛔ not by a timeout, and its pin ④ refuses a numeric per-case timeout. The cut above moves arrangement out of the clock; what is left is the probe itself (a sign-up and five password or TOTP checks).end()but is still flushing at the momentclose()STARTS is destroyed mid-body. Node's ownserver.close()sweeps idle connections (Node 19+), and the adapter's call at drain start does the same; both treat an ended response as done before its bytes are flushed. Reproduced on this branch with the slow-reader case reordered so the big response ends beforeclose():expected 3919702 to be 33554432bytes. That path is unchanged frommain; reported to the seat for a card.patchchangeset; the test-only changes in the other six packages publish nothing.Verification
Head
586338ffbd(round 2, on the round-1 merge oforigin/mainat8ef4d30905; the delta againstmainis these nine files).plugin-hono-server,client,metadata-protocol,plugin-security,plugin-sharing,driver-sql,cli: all exit 0.plugin-hono-server481/481 on round 2 (478/478 on round 1),client712/712,metadata-protocol22525 passed / 26 skipped,plugin-security5820 passed / 45 skipped,plugin-sharing1445/1445,driver-sql3867 passed / 218 skipped,cli --project unit5603/5603. Thecliintegration layer is left to CI.node scripts/pm/dispatch-gates.mjs --commandson586338ffbd: 69 families, 69 run, all exit 0;--ranreconciliation: 0 NOT-MEASURED, 0 UNRUN.check:tenant-audit-censusfirst flagged the new set's.delete()receiver; the set is now a named type in the adapter, and the census passes.check:dts-closureandcheck:dual-build-cjs-loadsfirst read adist/that was being rebuilt and went red on missing files; rerun on the finished build, both exit 0.🤖 Generated with Claude Code