Skip to content

node:net: return false from write() when the send fails at once - #43755

Merged
Jarred-Sumner merged 1 commit into
mainfrom
robobun/c900f15b/net-write-sync-failure-returns-false
Sep 22, 2026
Merged

Jarred-Sumner merged 1 commit into
mainfrom
robobun/c900f15b/net-write-sync-failure-returns-false

Conversation

@robobun

@robobun robobun commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator

Problem

  • net.Socket#write() returns true when the send fails inside the same call: the kernel rejects it (ECONNRESET, EPIPE) or the handle is already closed (EBADF). Node v26.3.0 returns false and sets socket.errored before write() returns.
  • The cause is Socket.prototype._write (src/js/node/net.ts:2968 and :2976). It hands the error to the stream callback with process.nextTick. Writable.write() computes its return value before that tick.

Fix

  • _write calls the stream callback with the error in the same call, in both branches, like Node's afterWriteDispatched (stream_base_commons.js#L158-L159).
  • The stream still runs the write callbacks and destroys the socket on the next tick. The order of write callback, 'error' and 'close' does not change.
  • _write does not call failWrite there. failWrite destroys the socket at once, so a second write in the same tick would get ERR_STREAM_DESTROYED. Node gives both writes the original errno.
  • Verified: test/js/node/net/node-net.test.ts and test/js/node/net/net-syscall-fault.test.ts (8 new tests, all fail on main's src/). Also node's test-net-*, test-http-*, test-tls-* and test-http2-* files.

Background

  • Writable.write() calls _write(chunk, encoding, callback). If callback(err) runs before _write returns, the stream sets errored at once and write() returns false. The user callbacks and the destroy wait for the next tick.
  • The native $write returns a negative errno when send() fails because the peer is gone.

Downsides

Notes

Repro (from a differential run of bun against node):

import net from "node:net";
let accepted, connected, client;
const go = () => {
  if (!accepted || !connected) return;
  accepted.resetAndDestroy();
  const ret = client.write("x", e => console.log("wcb", e ? e.code + " " + e.syscall : "ok"));
  console.log("write() returned", ret, "destroyed=" + client.destroyed, "errored=" + client.errored?.code,
    "writable=" + client.writable, "writableLength=" + client.writableLength);
  console.log("2nd write() returned", client.write("y", e => console.log("wcb2", e?.code, e?.syscall)));
};
const server = net.createServer(s => { s.on("error", () => {}); accepted = s; go(); });
server.listen(0, "127.0.0.1", () => {
  client = net.connect(server.address().port, "127.0.0.1", () => { connected = true; go(); });
  client.on("error", e => console.log("error", e.code, e.syscall));
  client.on("close", hadError => { console.log("close", hadError); server.close(); });
});

node v26.3.0 and this branch (Linux and Windows, line for line):

write() returned false destroyed=false errored=ECONNRESET writable=false writableLength=0
2nd write() returned false
wcb ECONNRESET write
wcb2 ECONNRESET write
error ECONNRESET write
close true

bun 1.4.3 and main:

write() returned true destroyed=false errored=undefined writable=true writableLength=1
2nd write() returned true
wcb ECONNRESET write
wcb2 ECONNRESET write
error ECONNRESET write
close true

The same comparison holds for a unix socket whose peer closed (EPIPE), for socket._handle.close() followed by write() (EBADF, EPIPE on Windows), for cork() / uncork(), and for end(data).

#43705 notes this bug as found on the way and leaves it alone. The two changes are independent.

Why the stream still destroys the socket. The Socket constructor forces autoDestroy: true. Writable's onwrite sees state.sync, so it queues onwriteError with process.nextTick. onwriteError runs the write callback, fails the buffered writes with the same error, and calls destroy(err). This is the path the deferred failWrite call reached before, one tick later.

Tests

  • node-net.test.ts: a unix socket whose peer closed (POSIX, the kernel reports it inside the peer's close(2)), a TCP socket whose peer reset the connection, a handle that was closed directly, and a corked batch that uncork() sends through _writev. The TCP test takes a fresh connection until the kernel rejects the send, because loopback does not promise that the RST is processed before the next send(2). On Linux and Windows the first connection hit in 500 of 500 runs (release builds), so the loop is a guard for other kernels.
  • net-syscall-fault.test.ts: an injected ECONNRESET / EPIPE on the first send(), for the connecting socket and for the accepted socket. These run on ASAN builds only, where the fault hooks exist.
  • With only one of the two branches of the fix applied, the tests of the other branch fail.
  • Windows debug build of this branch: the TCP, closed-handle and corked tests pass, and node-net.test.ts is 114 pass, 0 fail.

Suites run on the Linux debug build: test/js/node/net/*.test.ts, test/js/node/tls/node-tls-{connect,server,upgrade}.test.ts, tls-syscall-fault.test.ts, and the test-net-*, test-http-*, test-https-*, test-tls-*, test-http2-*, test-child-process-*, test-cluster-* files of test/js/node/test. Every failure there also fails without this change: the container resolves localhost to 127.0.0.1 only while Bun.listen("localhost") binds ::1, and a few tests exceed their timeout on a debug build. On Windows, test-net-pingpong.js fails with and without this change (#39723, with #39727 open for it).

TLS. After the peer's raw socket resets, node's tlsSocket.write() also returns false with ECONNRESET. Bun returns true, reports the write as done and closes without an error, with and without this change. The native TLS write never returns the errno, so _write cannot see the failure. #42336 makes the native side report it. With both changes the TLS path reaches the same res < 0 branch.

Not changed: tlsSocketWrite in src/js/node/_http2_upgrade.ts treats any truthy $write result as success. A TLS handle never returns a negative errno today, so that path is unaffected.

Socket.prototype._write deferred a send that the kernel rejects (ECONNRESET,
EPIPE) and a write on a closed handle (EBADF) to the next tick. The stream
was not errored when write() returned, so write() returned true.

Node hands the errno to the stream callback inside the same call
(afterWriteDispatched in lib/internal/stream_base_commons.js). The stream
marks itself errored at once and write() returns false. The stream still
runs the write callback and destroys the socket on the next tick, so the
order of the write callback, 'error' and 'close' does not change.
@coderabbitai

coderabbitai Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

  • Run on-demand review

This review includes 3 billable files and costs up to $0.75.

  • Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 5 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: oven-sh/bun/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 7c79da3b-7eb5-475d-bb48-06ae06dca05e

📥 Commits

Reviewing files that changed from the base of the PR and between 4ada08b and 1efa046.

📒 Files selected for processing (3)
  • src/js/node/net.ts
  • test/js/node/net/net-syscall-fault.test.ts
  • test/js/node/net/node-net.test.ts

Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator Author

Status

  • Reproduced on bun 1.4.3 and on main, Linux and Windows, with the script in the Notes of the PR body. The accepted socket calls resetAndDestroy() and the client writes in the same tick. write() returns true, then the write callback reports ECONNRESET with syscall write. Node v26.3.0 returns false.
  • The 8 new tests fail on a build of main's src/ and pass on this branch.

@Jarred-Sumner
Jarred-Sumner merged commit 46461c3 into main Sep 22, 2026
5 of 7 checks passed
@Jarred-Sumner
Jarred-Sumner deleted the robobun/c900f15b/net-write-sync-failure-returns-false branch September 22, 2026 04:53

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review found no issues

No high-confidence issues detected in this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants