fix(http): publish server response finish diagnostics - #23
Merged
Merged
Conversation
Publish Node-compatible request, response, socket, and server identities before response detachment and pipeline advancement. Let the finish listener own synchronous response detachment so nextTick subscribers observe the same socket state. Cover native and injected sockets, queued HEAD responses, subscription timing, and aborts. Validation: 14 new cases pass on Node 24.21.0 and the release build; 8 subscribed cases fail on the post-sync baseline. Diagnostics: 26 pass, 3 todo. HTTP qualification: 914 pass, 1 pre-existing Node-reference failure, 11 skip, 3 todo. Prettier, lint, and independent review pass.
…use-halfclose # Conflicts: # CHANGELOG.md
steipete
added a commit
that referenced
this pull request
Sep 30, 2026
Merge upstream Bun through `ba3f27d1d1ce359d4eed842c135f0f6fba1acb00` into fork main, retaining upstream and contributor history. Land with a merge commit to retain upstream ancestry. All fork fixes #15, #7, #14, and #27 are incorporated and qualified. The five conflict resolutions preserve the fork’s behavior while integrating upstream changes: - Keep macOS `ProcessRetry` alongside upstream’s `Tty` event-loop flag. - Keep the richer worker `execArgv` record and parser for preloads, TLS trust, CPU profiling, and addon/FFI restrictions; incorporate upstream’s invalid process-only flag reporting and its C++ error channel. - Keep literal `?` file-URL handling in the module loader alongside upstream’s string-code-generation guards. - Retain one upstream-positioned `ERR_WORKER_INVALID_EXEC_ARGV` mapping; both histories added it, and the automatic merge initially duplicated it. The fork fixes landed since the previous sync `ee83b78b18` remain present: file-URL preloads (#22), response-finish diagnostics (#23), and macOS silent-run signal/stdio handling (#24). Earlier fork changes are retained by the merge. Upstream’s addon/FFI worker restrictions overlap the fork’s broader implementation; the shared behavior is retained in one parser, with upstream’s new invalid-flag handling added. WebKit remains `f20ce7744553c910bcf16a33faf976af208de091` on both sides. Direct source inspection confirms that `DFGSSALoweringPhase.cpp` lowers `StringAt`, `StringCharCodeAt`, and `StringCodePointAt` to independent `CheckInBounds` nodes, and the pin includes `JSTests/stress/string-index-dce-bounds-check.js`. This is the upstream replacement for closed oven-sh/WebKit#578, so no pin rollback is needed. The release documentation now reflects that fix. Final integration also retained both sides of the changelog conflict and added contributor credit for @SebTardif and @RomneyDa. The runtime plugin cache format remains version 34; the frozen upstream uses 33. Native qualification exposed a pre-existing fork mismatch with two new upstream idle-sweep tests: cached idle state stays false until message timing clears, even after a bodyless response ends inside its handler. Explicit Node HTTP idle sweeps now derive response availability while preserving incomplete TLS handshakes, application-owned parser-error sockets, request bodies, partial heads, queued responses, and tunnels. Both upstream one-read/split-head tests pass unchanged, as does the existing parser-error ownership test. A TLS 1.2 relay control proves a sweep cannot close a handshake in progress. Node 26.10.0 confirms the bodyless close behavior. Qualified head: `41dffc47212456d5ac80fc6fdddda08c4e4bfccc`, macOS arm64 release build (Bun 1.4.3, WebKit `f20ce7744553c910bcf16a33faf976af208de091`). No local SDK signpost patch was needed. Binary, hardening, duplicate-symbol, formatting, and applicable hosted checks passed. Codex review has no remaining accepted/actionable P0–P2 findings; the rejected non-regular-copy concern is ruled out by unchanged regular-file guards and eight matching baseline/candidate FIFO/device controls. | File | Pass | Skip | Todo | Fail | | --- | ---: | ---: | ---: | ---: | | `test/js/bun/terminal/terminal.test.ts` | 97 | 2 | 0 | 0 | | `test/js/bun/terminal/terminal-spawn.test.ts` | 23 | 2 | 0 | 0 | | `test/js/bun/spawn/spawn.test.ts` | 165 | 24 | 0 | 0 | | `test/js/node/child_process/child-process-stdio.test.js` | 9 | 0 | 0 | 0 | | `test/js/node/http/node-http.test.ts` | 277 | 1 | 0 | 0 | | `test/js/node/diagnostics_channel/diagnostics_channel.test.ts` | 26 | 0 | 3 | 0 | | `test/js/node/sqlite/node-sqlite.test.ts` | 143 | 4 | 0 | 0 | | `test/js/node/worker_threads/worker_threads.test.ts` | 172 | 0 | 0 | 0 | | `test/cli/run/preload-test.test.js` | 2 | 0 | 3 | 0 | | `test/cli/run/no-orphans.test.ts` | 15 | 9 | 0 | 0 | | `test/cli/install/bun-install-lifecycle-scripts.test.ts` | 70 | 0 | 0 | 0 | | `test/js/node/fs/fs.test.ts` | 660 | 16 | 0 | 0 | | `test/js/node/fs/cp.test.ts` | 54 | 6 | 0 | 0 | | `test/cli/run/transpiler-cache.test.ts` | 27 | 0 | 0 | 0 | | `test/cli/test/isolation.test.ts` | 41 | 0 | 0 | 0 | | `test/js/bun/plugin/plugins.test.ts` | 47 | 0 | 0 | 0 | | `test/js/bun/plugin/plugin-namespace-drive-letter.test.ts` | 1 | 0 | 0 | 0 | | `test/js/node/disallow-code-generation-from-strings.test.ts` | 28 | 0 | 0 | 0 | | `test/js/node/http/node-http-server-abort-events.test.ts` | 105 | 0 | 0 | 0 | | `test/js/node/http/node-http-server-close-drain.test.ts` | 31 | 0 | 0 | 0 | | `test/js/node/http/node-http-connect.test.ts` | 80 | 1 | 2 | 0 | | `test/js/node/http/node-http-upgrade-body.test.ts` | 11 | 2 | 0 | 0 | | `test/js/node/http/node-http-req-socket-pause.test.ts` | 40 | 0 | 0 | 0 | | `test/js/node/tls/node-tls-server.test.ts` | 99 | 0 | 0 | 0 | | `test/js/node/tls/node-tls-wrapped-socket-close.test.ts` | 15 | 0 | 0 | 0 | | `test/cli/install/bun-install-patch.test.ts` | 32 | 0 | 0 | 0 | | `test/js/bun/io/bun-write.test.js (stream fallback only)` | 1 | 0 | 0 | 0 | | `test/bundler/compile-argv.test.ts (compiled code-generation flags)` | 3 | 0 | 0 | 0 | | **Total** | **2274** | **67** | **8** | **0** | Filesystem and isolation suites used outer concurrency 2; nested stress work, assertions, and timeouts were unchanged. Lifecycle, TLS, and compiled-argument tests enabled the existing internal-test API at process startup (`BUN_FEATURE_FLAG_INTERNAL_FOR_TESTING=1`, `BUN_GARBAGE_COLLECTOR_LEVEL=0`). Initial lifecycle/TLS invocations without those startup flags could not load `bun:internal-for-testing`. Earlier runs encountered recursive-readdir and copy-stress timeouts; the same readdir workload exceeded its deadline on the pre-sync baseline. Both full filesystem suites passed on the final head. The terminal ESRCH test missed its real kernel race window once (its path-reached assertion failed); the unchanged complete file passed on retry. The known broader Bun.write Response timeout was reproduced on a separately built pre-change baseline during #27 qualification and is not claimed as a passing full-suite run here; the added stream-fallback regression passes. Additional proof: normal/strict data/blob imports and Workers behaved as expected in all 16 probes; the compiled-argument tests preserve quoted strict-mode floors. OpenClaw smoke with the built runtime passed: `OpenClaw 2026.9.6 (23ad3a5)`. No tag or release is part of this PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Publishes Node's
http.server.response.finishdiagnostics event from node:http servers. The payload is{ request, response, socket, server }, sent only when the channel has subscribers, with Node's timing (Node 24resOnFinish). It covers native and injected connections, including responses that finish while queued behind an earlier one on a pipelined socket.Timing detail: Node publishes before it detaches the socket and advances queued responses. Bun's dispatcher detached a synchronously finished response itself, before the deferred
finishran. So a subscriber added in anextTicksaw a detached response. The finish listener now owns detachment and pipeline advancement for every response, and the dispatcher's early detach and itskDispatcherDetachedguard are gone.Why now: after the upstream sync (#21), OpenClaw's HTTP rejection transport routes new-Bun requests through its Node path, where a queued HEAD rejection can wait on this event to follow earlier responses. OpenClaw's change (openclaw/openclaw#160329) no longer requires the event on Bun, but publishing it closes the Node compatibility gap.
How did you verify your code works?
macOS arm64 release build on
openclaw/mainee83b78b18:test/js/node/diagnostics_channel/diagnostics_channel.test.ts. They cover payload identities, one event per finished response, pipelined ordering, queued HEAD, subscription timing includingnextTicksubscribers, no event without subscribers, and aborts. All pass on Node 24.21.0 and on this build; the post-sync build fails all 8 cases that subscribe.http-request-lifecycle.test.tsis 31/31 on this build.