node:net: fail the write when the flush at the end of the open dispatch gets a fatal send error

RO
2mo ago
+213-29

Problem

  • A node:net write made inside a 'connection' or 'connect' listener can be reported as delivered when it was not. The write callback succeeds and 'finish' fires, with no 'error'. Repro: the listener writes 16 MB and the peer resets before the listener returns. Node fails the write with ECONNRESET. Bun reports success with 2.6 MB sent.
  • The cause is the flush at the tail of on_open (src/runtime/socket/socket_body.rs:1623): let _ = this.internal_flush();. On a fatal send() errno, internal_flush drops buffered_data_for_node_net and returns the errno. The tail discarded it, saw the buffer empty, and dispatched the drain callback.

Fix

  • Move the POSIX fatal-flush delivery of on_writable (#32488) into fail_fatal_flush. Call it from the tail of on_open too. It calls the error handler with a write errno, then closes the socket.
  • write_or_end no longer queues the bytes of a write whose first send was fatal. JS fails that write, but the tail flush sent its bytes again.
  • Windows keeps the legacy drain contract, as on_writable documents.
  • Verified: test/js/node/net/net-syscall-fault.test.ts (6 new cases), test/js/node/net/node-net-server.test.ts (real peer reset, no injector), and the Node suite's test-net-*, test-http-*, test-tls-*. Self-reviewed: 3 concerns raised, 3 addressed.

Background

  • buffered_data_for_node_net holds the unsent rest of a short send(). JS parks the write callback until the native drain callback arrives.
  • internal_flush sends that buffer again. It returns 0, or the errno of a fatal send (EPIPE, ECONNRESET).
  • The open dispatch is the native on_open call. Its JS open handler emits 'connection' or 'connect'. A write made there is flushed when the handler returns.
Notes

Real-kernel repro (no fault injection, no timers). The server's 'connection' listener writes 16 MB (a real short send), calls end(), then stays in the listener until the peer has reset the connection. The peer signals the reset through a marker file.

node v26.3.0:  ["write:ECONNRESET","error:ECONNRESET:write","close:true"]
bun main:      ["write:ok","finish","close:false"]   bytesWritten 2634240 of 16777216
this branch:   ["write:ECONNRESET","error:ECONNRESET:write","close:true"]

bun main is release 1.4.3-canary.1+c6b7fcb5b and a debug build of 630e921db0. Both give the same result.

Schedule matrix (no injection, the listener does not block). The writer queues 16 MB inside its 'connection' listener or 'connect' callback. A node v26 peer checks every byte and resets at a set point: in its callback, after 0 to 40 ms, after 1 B to 12 MB read, N ms after its first data, or a close with unread data. A run fails when the peer got less than everything and the writer saw no error.

release main c6b7fcb5b:      16 of 30 runs end in write ok + 'finish' + close:false
debug build of main:         10 of 30, and 30 of 32 with resets timed from the peer's first data
debug build of this branch:  0 of 152
node v26.3.0:                0 of 30

The peer always received a prefix of the stream. A trace of a failing run on release main shows that the failing send() is the second one, in the same loop iteration as the accept, with no epoll_pwait in between. That send is the flush at the tail of on_open. About 10 ms pass between the two sends while the 14 MB rest is copied into the buffer, so a reset that arrives in that window is enough. The listener does not have to block.

47.173 ms  accept4 -> fd 10
50.898 ms  send fd 10 len 16777216 -> 2681856
61.631 ms  send fd 10 len 14095360 -> -1 (errno 104)
62.437 ms  epoll_pwait enter

Syscall-level repro (LD_PRELOAD shim on send, accepted fd only). net.createServer(c => { 16 x c.write(4 KiB); c.end(); }), plan: send #2 short (1 byte), send #3 EPIPE.

main:         send #4 = 57344 bytes on the same fd. Peer gets 61441 of 65536 bytes,
              spliced at offset 4097. Server: no 'error', 16 write callbacks ok, 'finish'.
this branch:  no send after #3. Peer gets a clean 4097-byte prefix.
              Server: error:EPIPE:write, close:true, writes 1..15 fail with EPIPE.

Why the second change is needed. When the first send of a write is fatal, write_maybe_corked returns -errno and JS schedules failWrite on the next tick. write_or_end still appended the whole chunk to buffered_data_for_node_net, so the tail of on_open sent it a second time. Two effects:

  • On main, with a one-shot injected ECONNRESET, the second send succeeds. The peer receives 200 bytes of a write that JS was told failed. The new test is not sent again covers this.
  • With only the first change, a real reset before accept makes the second send fail with EPIPE, and fail_fatal_flush reports it before failWrite runs. 'error' then carries EPIPE while Node and main report ECONNRESET. Real-kernel check, reset before accept, two writes in the listener:
node / main / this branch:  ["write1:ECONNRESET","write2:ECONNRESET","error:ECONNRESET:write","close:true"]
first change only:          ["write1:ECONNRESET","write2:EPIPE","error:EPIPE:write","close:true"]

Only wrote < -1 (a fatal send) skips the queue. -1 is the legacy closed or rejected-TLS result, which JS treats as would-block. Its behavior does not change.

Other callers of internal_flush that discard the errno. end_buffered and end call it only when wrote == total, so the buffer is empty. The public flush() has no socket-handle caller in src/js. None of them dispatches drain.

History. #33803 made ENOBUFS/ENOMEM transient. #32488 made on_writable fail the write on POSIX. #33506 covered the same ground as #32488 and was closed as stale. The tail of on_open was the remaining caller.

Overlap. #41785 changes the same if buffer_unwritten_data line to wrote >= 0 for a different bug (Bun.connect end(data)). The second PR to land needs a one-line conflict resolution.

Not part of this change. SocketHandlers.error in net.ts (the handler table for net.connect({ fd }), which includes each connection a cluster worker accepts) fails the parked write callback and then also emits 'error' directly. That gives two 'error' events for one failed flush. It reproduces on main through on_writable, so this PR does not change it. Tracked in #43030.

Suites run on the debug build. net-syscall-fault.test.ts (27 pass), node-net-server.test.ts (27 pass), socket-syscall-fault.test.ts, tls-syscall-fault.test.ts, node-net.test.ts, and the Node suite files test-net-*.js (139), test-http-*.js (386), test-tls-*.js (186). Every failure there also fails on a debug build of main without this change (test-http-agent-keepalive, test-http-client-timeout-option, test-tls-client-allow-partial-trust-chain, two 1-byte-send timeouts in node-http-syscall-fault.test.ts, ten localhost cases in node-net.test.ts). cargo check --workspace --target x86_64-pc-windows-msvc passes.

no test proof · iteration 2 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/net/node-net-server.test.ts, test/js/node/net/net-syscall-fault.test.ts