Repository navigation
Conversation
The h2 frame parser latched a flag on a fatal send() and dropped the errno, then closed the transport with CloseCode::Normal on the next deferred tick. That close reaches JS as a clean EOF, and it runs before the loop can poll the read side, so a request whose HEADERS write hit the peer's RST ended with no 'error' and no 'response'. Keep the errno and close the socket with it, which is how the uSockets loop closes a socket whose recv() failed. The close handlers in net.ts then shape it like Node's read ECONNRESET, and the session takes its existing transport-error path: ECONNRESET on the session and on the stream, with rstCode INTERNAL_ERROR.
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe change preserves fatal transport write errors through HTTP/2 handling, normalizes peer-reset send errors to ChangesHTTP/2 transport error handling
Suggested reviewers: Priority: ⬇️ Low Merge Risk: 🔵 Low · up to
|
StatusTwo faces of one bug, both reproduced on main and both fixed by this branch for client sessions. Two more cases are listed under "Not covered": this branch leaves them as they are on main. An idle session whose loop was busy while the reset arrived (1.4.3 and main, 3/3 on linux-x64 and on Windows x64). The peer writes the HTTP/2 frames by hand, sends an RST while the client's event loop is blocked, and the client then calls The first request of a connection that the peer resets at the preface (main, which has #40048). The connect flush sends the preface and then the queued HEADERS, and the RST lands between the two sends. With the socket as the only handle, 12/12 on a debug build: In both, a With this change a client session and its request report Two things a maintainer should know before merging, both in the PR notes:
|
One peer reset gives a different send errno per platform: linux reports ECONNRESET, darwin reports EPIPE. The read side reports ECONNRESET on every platform, and that is the code node surfaces for a peer that vanished, so close the dead transport with ECONNRESET for the peer-gone errnos that mean the same thing. Every other errno keeps its identity. The new test injects each errno through the socket fault harness, so the code the session and the stream report is pinned on every platform.
|
Updated 6:10 PM PT - Oct 13th, 2026
✅ @robobun, your commit 341384fa3d6d7df2c84d7b9707f20c2659cb7d33 passed in 🧪 To try this PR locally: bunx bun-pr 42182That installs a local version of the PR into your bun-42182 --bun |
|
Update after the darwin CI lane. macOS returns So a dead transport now closes with Both errnos are pinned on every platform now: Also shortened the doc comments the comment linter flagged. |
…2-write-side-reset
Since the client preface waits for 'connect', the connect flush sends the preface and then the queued request's HEADERS back to back. A peer that resets at the preface can land its RST between the two sends, so the first request of a fresh connection takes the fatal-send path. Before this branch that closed the transport as a clean EOF from the deferred tick. The only JS work that leaves behind is a process.nextTick, which the deferred task queue runs after the microtask drain, so with the socket as the only handle the process exited with no event at all. Add that case with a real reset and with an injected errno on the second send. The clients start no timer and report when the process exits on its own, so every case also checks that the events arrive before exit. Drop a sentence from a comment: errno 1 never reaches close_after_fatal_send, because a -1 write result is not latched.
…te that fills the cork us_socket_raw_writev returned 0 for a failed writev(), so a request body above one DATA frame never reached note_transport_write_fatal. It now reports the errno like us_socket_write_check_error. Both use one classifier, us_internal_classify_failed_send. A fatal send made while the cork buffer fills latches while the cork holds the auto-flush registration. uncork() then cancelled the deferred tick that closes the transport, and the later failing send did not register again. uncork() now keeps the registration while a fatal write is latched. Tests: the busy client blocks on stdin until the parent has reset the connection, in place of a fixed wait. New cases: a request body above one frame, an injected errno on a body that fills the cork, and an injected errno on the writev().
|
About the vectored write, the finding outside the diff in the last review: 48d3419 addresses it. I measured the previous head again after I posted this comment, and the first version of it was wrong about what that head did. This is the corrected account.
|
…2-write-side-reset Conflicts: main added us_socket_writev, a caller of us_socket_raw_writev, and this branch added the fatal_write_error parameter to the latter. us_socket_writev passes NULL. A caller that passes NULL keeps main's behaviour: the failed writev is not classified and writable is re-armed. socket_body.rs: both sides added a function at the same place.
EPIPE names no cause. On linux a send fails with it when the peer sent its FIN first and then reset the data that followed. The failed send takes the socket error, and a read of that socket is a clean EOF. Node reports a clean close there. The earlier fold reported ECONNRESET. macOS and FreeBSD fail every send on a disconnected socket with EPIPE and keep the cause as the socket error, which is what a read returns. There the socket error is read and reported as it is, so a reset is still reported. linux does not read it: after such a send SO_ERROR can only hold a soft error, which a read does not return. ECONNABORTED keeps its identity on POSIX, as a read reports it. The comments that state the close-code contract name close_after_fatal_send, and its doc says that it does not read what is queued ahead of the error. Tests: ECONNABORTED is reported as itself, and an injected EPIPE reports no error. The subprocess cases take a longer timeout on a debug build, where each child needs several seconds to load node:http2.
test/CLAUDE.md asks for the runner's default. The cases need about one second on a release build.
Both made the close after a fatal send run before the loop's next read in shapes where main lets the loop read first. A response or a GOAWAY that was queued ahead of the peer's reset was then dropped when the request body was above one DATA frame. An h2c server that calls server.close() in its handler ended with an uncaught ERR_SOCKET_CLOSED when the client had already left. us_socket_raw_writev, flush_batch_vectored and uncork() are main's again. A client session still closes with the errno where main already closes in the deferred tick. The other cases need the close to read the queue first. Tests: the cases for a body that fills the cork buffer, for a body above one frame and for an injected writev error go with the code they pinned.
…2-write-side-reset # Conflicts: # src/uws_sys/socket.rs # test/js/node/http2/node-http2.test.js
Problem
node:http2client whose write is the first operation to see a peer RST gets a clean close (rstCode8, no'error'). With only the socket alive, the process exits 0 silently.note_transport_write_fatal(src/runtime/api/bun/h2_frame_parser.rs:3027) latches a rejectedsend()and drops the errno. The deferred tick then closes the transport withCloseCode::Normal.Fix
recv(). Server sessions keep the plain close, as in Node.EPIPEstays a plain close (macOS: the socket's own error).node-http2.test.js,node-http2-syscall-fault.test.ts) andtest/js/node/http2/.Background
H2FrameParseris the native engine of anHttp2Session. A deferred tick flushes its cork buffer and closes a dead transport.Downsides
ECONNRESETwhere Node closes cleanly, in two cases that need a maintainer's yes: a request made inside a read callback, and a reset behind an unread GOAWAY, which this close does not read (as on main).EPIPEon linux). TLS has its own path (tls: consolidate the open TLS fixes (both engines, node:tls, node:https, WebSocket, SQL) #44618).Notes
Where this differs from Node
Node reports a reset only when its read side sees it. It drops the status of a failed write:
Http2Session::ClearOutgoingcallsDone(0)underTODO(addaleax): Pass status instead of 0, so that we actually error out with the error from the write to the underlying protocol(src/node_http2.cc, v26.3.0). On linux the failedsend()also consumes the socket error, so theread()that follows returns a clean EOF. Which side sees the reset first depends on where the request is made: Node defers a write made from a timer until after the next poll, and writes at once from inside a read callback. bun writes at once everywhere.Since #40048 the connect flush sends the preface and then the queued HEADERS, so a reset at the preface hits the first request of a connection (the first row).
linux-x64, node v26.3.0, same fixtures as the tests, the socket is the only handle:
ECONNRESET, rstCode 2 (3/3)ECONNRESET, rstCode 2request()from a timer (the original report)ECONNRESET, rstCode 2 (8/8, one clean close in an earlier 3)ECONNRESET, rstCode 2ECONNRESET, rstCode 2 (12/13 for 20000 bytes, one clean close)request()inside'remoteSettings'ECONNRESET, rstCode 2'error'The
'remoteSettings'row is the divergence: bun reports a reset that Node, by its own TODO, means to report and does not. The request did fail, and a client that is told nothing cannot retry it. I did not measure Node on darwin or Windows. The injected-errno tests pin what bun reports there.The last row is the next section. The third row is "Two changes that were taken out". The other rows were measured at earlier heads, and the tests pin them at this one.
A peer that closes first and then resets
A server that accepts a connection and closes it at once (
net.createServer(s => s.destroy())) sends its FIN before the client's preface arrives. The kernel then answers the client's data with an RST. Measured on linux 6.1 with a raw socket: the secondsend()fails withEPIPE,SO_ERRORis empty after it, andrecv()returns EOF. Whenrecv()is the first call after the RST it also returns EOF, and theEPIPEstays inSO_ERROR. No read of that socket reports an error, so Node reports a clean close.Earlier heads of this PR reported every
EPIPEasECONNRESET. The fold was there for darwin, where a send on a reset socket fails withEPIPE. On linux it reported an error that Node and the read side of the kernel do not have, and it moved the cells where main already equals Node (any other live handle) away from Node. NowEPIPEis a plain close on linux. macOS and FreeBSD fail every send on a disconnected socket withEPIPEand keep the cause as the socket error (sosendcheckreturnsEPIPEbefore it looks atso_error), and a read returns that error. Thereclose_after_fatal_sendreadsSO_ERRORand reports it as it is. linux does not read it: after a send that failed withEPIPE,SO_ERRORcan only hold a soft error (sk_err_soft), which a read does not return. I could not run macOS. Its CI lanes run the three real-reset tests, which fail if this reading is wrong. They pass there.The client makes one request with
'error','end'and'close'listeners and holds nothing but the socket. linux-x64, the client and the peer run the same binary:request end, request close, session closesession error ECONNRESET, session close, request error ECONNRESET, request close(3/3)request end, request close, session close(3/3)The
pipelineandfor awaitforms of the same request agree with these rows. With the loop held open the branch prints Node's words in Node's order. The plain close of this case is made inside the deferred task queue, so its events are lost when the socket is the last handle. That is the delivery bug of "The silent exit" below, and #42485 owns it. This PR does not fix this case and does not change it.Server sessions
The first version of this PR closed server sessions with the errno too. A raw client that sends one request and resets when the server's
'stream'handler starts made an h2c server with no'error'listener exit with an uncaughtECONNRESET, 4/4, with no loop blocking: the response write fails, the session is destroyed with the error, and the still-open stream emits it. Node and main close quietly (stream,stream.close(rstCode=0),session.close).close_transport_after_fatal_writenow passes the errno for!is_serveronly, anda server session whose response write fails closes quietlypins it (it fails on the earlier head withuncaught:ECONNRESET). A server with a stream still open dies with an uncaughtERR_SOCKET_CLOSEDon main and on this branch alike. That is #42197's bug, and this PR does not change it.From Sep 21 to head 690be7d the server seat moved a second time, through
uncork(): see "Two changes that were taken out".Two changes that were taken out
Two more changes were in this PR from Sep 21 until head 690be7d. Both are removed at b6a8fb0.
The latch on a failed
writev(). A request body above one DATA frame leaves with its corked HEADERS in onewritev()(flush_batch_vectored). On mainus_socket_raw_writevreturns 0 for a failedwritev(), so the parser sees backpressure and no failure. The change reported the errno and latched it.uncork()kept the deferred tick. A fatal send made while the cork buffer fills latches while the cork holds the auto-flush registration. The nextflush()callsuncork(), which unregisters, so nothing closes the transport in the deferred tick. The change kept the registration while the latch was set.Both made the close run in the deferred tick, before the loop's next read, in shapes where main leaves the socket to the loop. On main the loop then reads what is queued, and the close comes after it. A maintainer's test run against 690be7d found what that cost:
uncork()did not ask whether the session is a client. A client that sent a POST and left ended an h2c server that callsserver.close()in its handler, with an uncaughtERR_SOCKET_CLOSED. Main closes quietly there.My runs of those two cases at b6a8fb0 (linux-x64, debug build): the queued response or GOAWAY is heard in 4 of 4 cases, with main's words (
response 200, session error EBADF, ...andgoaway 0, request end, request close, rstCode 8, session close), and 0 of 3 servers are ended.So a write that overflows the cork buffer, or a body above one DATA frame, keeps main's path in this PR. From a timer with nothing queued: bodies of 0, 100 and 16340 bytes report
ECONNRESET, rstCode 2. Bodies of 16360 and 16374 bytes end in a clean close. Bodies of 16375, 20000 and 70000 bytes print no event. To report the reset in those cases the close has to read the queue first and then carry the errno: see "Frames queued ahead of the reset are not read".One face this PR does not cover: a small write on a stream that is already open
The stream is opened inside
'remoteSettings', so its HEADERS leave at once. After the loop was busy while the reset arrived, a timer callsreq.end(Buffer.alloc(100)). The transport closes with the errno (us_socket_close(errno 104)in the trace), but the socket emits'close'before'error', so the session tears down cleanly first:stream.close(rstCode=8),session.close, no'error'. The cause is the deferred task queue, not this change. The close runs inside that queue,net.tsemits'error'fromprocess.nextTickand'close'fromsetImmediate, and no checkpoint follows the queue, so here the immediate runs first. A 20000-byte body takes main's path (see "Two changes that were taken out").req.end(body)from a timer on an open streamECONNRESET1/3ECONNRESET, rstCode 2 (at 53b7b94c)ECONNRESET1/3Node is not deterministic here, so this is not a parity break. #42485 moves the close out of the deferred queue into an event loop task, and that fixes the order for the 100-byte case. I merged #42485 at 366003a into an earlier head of this branch (53b7b94c, which still had the two changes that are now removed): the merge was clean by text, and the tests of both PRs passed together. That pair was not built again at this head.
Frames queued ahead of the reset are not read
The loop reads everything that is queued before it closes a socket for an error (#39860). The close in the deferred tick does not: not the plain close on main, and not
close_after_fatal_send. Measured on linux 6.1 with a raw socket: the peer writes 17 bytes and resets, the client'ssend()fails withECONNRESET, andrecv()then returns the 17 bytes and after them EOF. So where the deferred tick closes (a send that fits the cork), the frames that arrived ahead of the reset are dropped, on main and here. What this PR changes in that case is the report:ECONNRESETwhere main closes quietly, and an unread GOAWAY does not make it quiet throughisEconnresetAfterGoawayinhttp2.ts. Node reads the frames first.I tried to keep the plain close whenever bytes are queued (
us_socket_queued_input). That is wrong the other way: in the main case of this PR aSETTINGSACK is usually queued ahead of the reset, and the tests then report nothing. The right step is to read the queue and then close with the errno. That read cannot run where the close runs today: the deferred task queue also runs underon_native_read, where the loop's receive buffer and the parser are in use. #42485 moves the close to an event loop task, which has a clean stack, so the read belongs on top of it. With that in place the latch can cover the cork overflow and thewritev()again.Since this PR was opened, main gave the TLS engine that shape (#44618, #45124). A rejected send ends the writes only. The socket stays open while input is queued, and the close after the read carries the error (
ssl_close_after_rejected_sendinopenssl.c). The parser no longer latches a TLS write:NativeSocketWrite for &TLSSocketclamps the result. Plain TCP could take the same shape, with the writable event as the clean stack in place of #42485's task. I have not built that, and this PR does not do it.The silent exit
Traced with the
H2FrameParser,uwsandSocketdebug scopes on main: the deferred close does run.us_socket_close(s, normal)dispatcheson_close, thenet.tsclose handler takes the clean-EOF branch (finishSocketEnd:push(null)andunref()), and the only JS work that leaves behind is aprocess.nextTick.drain_microtasks_with_global(src/jsc/event_loop.rs) drains microtasks first and runs the deferred task queue after, so that tick is still queued when the loop finds nothing alive and exits. Closing with the errno takesdestroy(err)instead, which registers asetImmediatefor'close'and holds the loop. So a close that carries an errno is delivered, as a side effect and not at the layer that owns the bug. A plain close made inside the deferred queue still waits for the next loop activity: a server session, and a client whose send failed withEPIPEon a socket that holds no error. The layer fix (one more checkpoint after the deferred queue, or running the close as an event loop task the wayFileSink::run_pending_laterdoes) is independent of this PR and is tracked separately. Control: a peer that sends a real FIN at the preface delivers every event on main.How the failure reaches JS
Nothing new decides it.
NewSocket::close_after_fatal_sendcloses with the errno (us_socket_closenow takes the code as anintin Rust, as it already does in C),on_closeturns a code above theCloseCoderange into the error, thenet.tsclose handlers shape it like Node'sread ECONNRESETand keep their teardown-noise rules, and the session takes its existing transport-error path.dead_transport_close_codegives a close code only for an errno that a read of the socket would also return:ECONNRESET,ECONNABORTED,ENOTCONN,ETIMEDOUT,ENETDOWN,ENETUNREACH,EHOSTUNREACH. That is the peer-gone set ofus_socket_write_check_errorwithoutEPIPE, and each keeps its identity. On Windows the code is a WSA code, andread_error_from_close_codereportsWSAECONNABORTEDasECONNRESET, as it does for a failed read.EPIPEis not in the set: it is a plain close on linux, and on macOS and FreeBSD the error the socket holds is reported in its place, so a reset, a timeout or an unreachable host keeps its code there. Every other send errno, for exampleEPROTOTYPEafter the bounded retry window, keeps the plain close it has on main (sustained errnointest/js/bun/net/socket-syscall-fault.test.tspins that). An earlier head closed with every latched errno. A client then reportedread EPROTOTYPE, and whether that test saw it depended on the order of the socket's'error'and'close'events. Thenode:netwrite path is untouched. An error dispatch followed by a close was the other option: it delivers the socket error twice and skips theteardownNoiserule inSocketEmitEndNT.The session
'error'is emitted before the stream's, where Node emits the stream's first. That order is not specific to this path: a read-side RST with an open stream already reportssession.error,session.close,stream.error,stream.closeon 1.4.3.Self-review
The push of 382ca9d (merge with main, the
EPIPErule): 8 concerns raised, 5 survived. Three were fixed: the socket error behind anEPIPEis read only on macOS and FreeBSD and is reported as it is, the comments that state the close-code contract name the new producer, andECONNABORTEDhas a test. One is a stated limit of a test: the injectedEPIPEcase proves that no close with an errno was made and does not prove delivery. One was the missing read of the queue. It was filed as a Downside, and one day later a maintainer's test run showed that it was a regression against main for large bodies. That is why the two changes are out.The push of 48d3419 (the two changes that are now removed): two concerns were raised and neither survived. Neither review asked what main does with the queue in those shapes.
Tests
node-http2.test.js, describe "a client session reports a peer reset that one of its writes sees first": a busy idle session withrequest()from a timer, the same withrequest()inside'remoteSettings', and a reset at the preface. The client blocks infs.readSync(0)and the parent writes that byte only afterresetAndDestroy()returns, so the RST is always there before the request is written.node-http2-syscall-fault.test.ts, describe "node:http2 transport write errors":ECONNRESET,ECONNABORTEDandETIMEDOUTinjected on a later request (each keeps its identity),ECONNRESETon the second send of the connect flush, an injectedEPIPE, and the quiet server. TheEPIPEcase checks that no'error'is reported. It proves that no close with an errno was made, and it does not prove delivery: in that child no event arrives before the process exits, which is the gap #42485 closes. The faulted request is made from a zero timer and not inside'remoteSettings': a plain close made under a read reportsEBADFon main (4 of 4 runs of theEPIPEcase), which is also #42485's.Seven client cases fail on main's
src/and pass here. TheEPIPEcase and the server case pass both ways: they pin what must not be reported. With theEPIPEfold restored theEPIPEcase reportsECONNRESET. No client starts a timer to wait and each reports atprocess.on('exit'), so every case that expects an error also checks that the events arrive before the process exits.Related open PRs
NewSocketfor the nativeBun.connectwrite path. If it lands, the parser'stransport_write_errnofield can go andclose_after_fatal_sendcan read the socket's record. If it reports the record fromon_closefor every socket, h2 server sessions need an opt-out, for the reason above.on_auto_flushblock. node:http2: count a frame written after the transport closed as sent #42197 addsBunSocket::Closedin the same file. Both rebase over this without a semantic conflict.Suites
linux-x64, debug build, head 341384f, merged with main @c69a636b.
test/js/node/http2/: 625 pass, 0 fail, 6 skip, 2 todo.test/js/bun/net/socket-syscall-fault.test.tsandtest/js/node/tls/tls-syscall-fault.test.ts: 58 pass. All with--timeout 60000: a debug child needs about 3 s to loadnode:http2in my container, so the subprocess tests of that directory exceed the default 5 s there (the existing ones and the nine new ones).bun checkreports no type error in the two test files. Not run at this head: Node'stest-http2-*files (261 passed at 53b7b94c), Windows and macOS, andrust:check-all(it passed for Windows, macOS and FreeBSD at 382ca9d, and the code that differs per platform is unchanged since).Cost: the parser gets one
Cell<i32>.socket.candlibusockets.hare main's. The send path has no new branch: the errno is stored where the latch is set, and it is read where the transport is closed.no test proof · iteration 4 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/http2/node-http2-syscall-fault.test.ts