Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/runtime/node/node_net_binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ pub(crate) fn new_detached_socket(global: &JSGlobalObject, frame: &CallFrame) ->
server_name: JsCell::new(None),
buffered_data_for_node_net: Default::default(),
bytes_written: Cell::new(0),
fatal_write_errno: Cell::new(0),
native_callback: JsCell::new(NativeCallbacks::None),
twin: JsCell::new(None),
verify_error: JsCell::new(None),
Expand Down
5 changes: 5 additions & 0 deletions src/runtime/socket/Listener.rs
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,7 @@ impl Listener {
server_name: JsCell::new(None),
buffered_data_for_node_net: Default::default(),
bytes_written: Cell::new(0),
fatal_write_errno: Cell::new(0),
native_callback: JsCell::new(crate::socket::NativeCallbacks::None),
twin: JsCell::new(None),
verify_error: JsCell::new(None),
Expand Down Expand Up @@ -654,6 +655,7 @@ impl Listener {
server_name: JsCell::new(None),
buffered_data_for_node_net: Default::default(),
bytes_written: Cell::new(0),
fatal_write_errno: Cell::new(0),
native_callback: JsCell::new(crate::socket::NativeCallbacks::None),
twin: JsCell::new(None),
verify_error: JsCell::new(None),
Expand Down Expand Up @@ -1202,6 +1204,7 @@ impl Listener {
ref_pollref_on_connect: Cell::new(true),
buffered_data_for_node_net: Default::default(),
bytes_written: Cell::new(0),
fatal_write_errno: Cell::new(0),
native_callback: JsCell::new(crate::socket::NativeCallbacks::None),
twin: JsCell::new(None),
verify_error: JsCell::new(None),
Expand Down Expand Up @@ -1288,6 +1291,7 @@ impl Listener {
ref_pollref_on_connect: Cell::new(true),
buffered_data_for_node_net: Default::default(),
bytes_written: Cell::new(0),
fatal_write_errno: Cell::new(0),
native_callback: JsCell::new(crate::socket::NativeCallbacks::None),
twin: JsCell::new(None),
verify_error: JsCell::new(None),
Expand Down Expand Up @@ -1530,6 +1534,7 @@ fn connect_finish<const IS_SSL: bool>(
ref_pollref_on_connect: Cell::new(true),
buffered_data_for_node_net: Default::default(),
bytes_written: Cell::new(0),
fatal_write_errno: Cell::new(0),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
native_callback: JsCell::new(crate::socket::NativeCallbacks::None),
twin: JsCell::new(None),
verify_error: JsCell::new(None),
Expand Down
34 changes: 32 additions & 2 deletions src/runtime/socket/socket_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,11 @@
pub(crate) server_name: JsCell<Option<Box<[u8]>>>,
pub(crate) buffered_data_for_node_net: JsCell<Vec<u8>>,
pub(crate) bytes_written: Cell<u64>,
/// First fatal `send()` errno observed by a JS-driven `write()`/`end()`
/// (ECONNRESET/EPIPE while the loop was blocked in JS). `on_end` suppresses
/// the bogus peer-FIN it would otherwise report, and `on_close` reports it
/// as the close error when the later poll delivers only HUP.
Comment thread
robobun marked this conversation as resolved.
Outdated
pub(crate) fatal_write_errno: Cell<i32>,

Check failure on line 283 in src/runtime/socket/socket_body.rs

View check run for this annotation

Claude / Claude Code Review

fatal_write_errno not reset on socket reconnect path

`fatal_write_errno` is never cleared when a `NewSocket` wrapper is reused for reconnect (`detach_for_reconnect`, `connect_finish`'s `maybe_previous` branch, and the Windows named-pipe `prev` branches all skip it). A node:net socket that saw a peer RST, then reconnects — the code comments cite the MongoDB driver as a real user of this path — will on the *new* connection suppress `end` on a clean FIN (line 1593) and fabricate a spurious `ECONNRESET` / `syscall: 'write'` in `on_close` (line 2068) f
Comment thread
claude[bot] marked this conversation as resolved.
Outdated

pub(crate) native_callback: JsCell<NativeCallbacks>,
/// `upgradeTLS` produces two `TLSSocket` wrappers over one
Expand Down Expand Up @@ -1585,6 +1590,12 @@
if this.socket.get().is_detached() {
return;
}
if this.fatal_write_errno.get() != 0 {
// A JS-driven write already observed the peer-gone errno while the
// loop was blocked; the HUP the loop just polled is not a clean
// FIN. Skip the `end` dispatch and let `on_close` report the error.
Comment thread
robobun marked this conversation as resolved.
Outdated
return;
}

Check failure on line 1598 in src/runtime/socket/socket_body.rs

View check run for this annotation

Claude / Claude Code Review

on_end suppression never closes an allowHalfOpen socket — close(ECONNRESET) never fires

For `Bun.connect({ allowHalfOpen: true })`, this early return relies on "let `on_close` report the error", but with `allow_half_open` set loop.c never auto-closes after `on_end` — it only re-arms WRITABLE — so `on_close` never fires and the promised `close(ECONNRESET)` never arrives. The socket instead spins on level-triggered EPOLLHUP (each poll: WRITABLE→`on_writable` no-op, then eof→`on_end` suppressed→re-arm WRITABLE), and pre-PR the user at least got an `end` hook to call `socket.end()` and
Comment thread
robobun marked this conversation as resolved.
let handlers = this.get_handlers();
log!(
"onEnd {}",
Expand Down Expand Up @@ -2054,6 +2065,14 @@
&sys::Error::from_code_int(err, sys::Tag::read),
&global,
);
} else if this.fatal_write_errno.get() != 0 {
// The loop reported a clean close, but a JS-driven write already
// saw the kernel reject the send (peer RST while the loop was
// blocked in JS). Report that errno so the reset is not lost.
Comment thread
robobun marked this conversation as resolved.
Outdated
js_error = <sys::Error as jsc::SysErrorJsc>::to_js(
&sys::Error::from_code_int(this.fatal_write_errno.get(), sys::Tag::write),
&global,
);
}

if let Err(e) = callback.call(&global, this_value, &[this_value, js_error]) {
Expand Down Expand Up @@ -2263,7 +2282,9 @@
Ok(
match this.write_or_end::<false>(global, args.mut_(), false) {
WriteResult::Fail => JSValue::ZERO,
WriteResult::Success { wrote, .. } => JSValue::js_number_from_int32(wrote),
// Fatal send errnos (wrote < -1) are recorded on the socket for
// the close dispatch; the documented native return is -1.
Comment thread
robobun marked this conversation as resolved.
Outdated
WriteResult::Success { wrote, .. } => JSValue::js_number_from_int32(wrote.max(-1)),
},
)
}
Expand Down Expand Up @@ -2439,6 +2460,12 @@
// Kernel rejected the send (peer gone): return the negative errno so
// JS fails the write; never close from under the caller's stack, and
// leave the undeliverable buffer to the caller (aliasing).
// Record the first errno (ECONNRESET precedes the EPIPE tail) so the
// later loop-driven `on_end`/`on_close` can report the reset instead
// of a clean FIN.
Comment thread
robobun marked this conversation as resolved.
Outdated
if self.fatal_write_errno.get() == 0 {
self.fatal_write_errno.set(fatal_errno);
}
return -fatal_errno;
}
let uwrote: usize = usize::try_from(res.max(0)).expect("int cast");
Expand Down Expand Up @@ -3119,7 +3146,7 @@
if wrote >= 0 && usize::try_from(wrote).expect("int cast") == total {
let _ = this.internal_flush();
}
JSValue::js_number(wrote as f64)
JSValue::js_number(f64::from(wrote.max(-1)))
}
};
Ok(result)
Expand Down Expand Up @@ -3505,6 +3532,7 @@
ref_pollref_on_connect: Cell::new(true),
buffered_data_for_node_net: JsCell::new(Vec::new()),
bytes_written: Cell::new(0),
fatal_write_errno: Cell::new(0),
native_callback: JsCell::new(NativeCallbacks::None),
twin: JsCell::new(None),
verify_error: JsCell::new(None),
Expand Down Expand Up @@ -3612,6 +3640,7 @@
ref_pollref_on_connect: Cell::new(true),
buffered_data_for_node_net: JsCell::new(Vec::new()),
bytes_written: Cell::new(0),
fatal_write_errno: Cell::new(0),
native_callback: JsCell::new(NativeCallbacks::None),
twin: JsCell::new(None),
verify_error: JsCell::new(None),
Expand Down Expand Up @@ -4610,6 +4639,7 @@
ref_pollref_on_connect: Cell::new(true),
buffered_data_for_node_net: JsCell::new(Vec::new()),
bytes_written: Cell::new(0),
fatal_write_errno: Cell::new(0),
native_callback: JsCell::new(NativeCallbacks::None),
twin: JsCell::new(None),
verify_error: JsCell::new(None),
Expand Down
93 changes: 93 additions & 0 deletions test/js/bun/net/socket.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3330,3 +3330,96 @@
}
});
});

// Windows: the fatal-send detection in usockets is gated out there (see
// on_writable in socket_body.rs), so the write-side RST path is POSIX-only.
it.concurrent.skipIf(isWindows)(
"native write() on a peer-RST'd socket returns -1 and the close reports ECONNRESET",
async () => {
// The server lives in its own process so the RST arrives while this process
// is inside a synchronous write burst (the loop can't deliver it first).
using dir = tempDir("socket-rst-write", {
"server.mjs": `
const server = Bun.listen({
hostname: "127.0.0.1",
port: 0,
socket: {
open(s) { setTimeout(() => { try { s.terminate(); } catch {} }, 100); },
data() {}, close() {}, error() {}, drain() {},
},
});
console.log("PORT", server.port);
setTimeout(() => process.exit(0), 60000);
`,
});

await using child = Bun.spawn({
cmd: [bunExe(), "server.mjs"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "inherit",
});

let port = 0;
{
const rd = child.stdout.getReader();
let acc = "";
while (!port) {
const { value, done } = await rd.read();
if (done) throw new Error("server exited before reporting its port");
acc += new TextDecoder().decode(value);
const m = acc.match(/PORT (\d+)/);
if (m) port = +m[1];
}
rd.releaseLock();
}

const closed = Promise.withResolvers<{ err: unknown; events: string[] }>();
const events: string[] = [];
const negatives: number[] = [];

const sock = await Bun.connect({
hostname: "127.0.0.1",
port,
socket: {
open() {},
data() {},
drain() {},
error(_s, e) {
events.push("error:" + (e as any)?.code);
},
end() {
events.push("end");
},
close(_s, err) {
events.push("close");
closed.resolve({ err, events });
},
},
});

const chunk = Buffer.alloc(65536, 1);
// Synchronous burst: keep writing until write() reports the dead peer.
// The server RSTs ~100 ms after accept; give the burst a generous deadline.
const deadline = Date.now() + 5000;
while (negatives.length < 3 && Date.now() < deadline) {
const r = sock.write(chunk);
if (r < 0) negatives.push(r);
Bun.sleepSync(5);
}
// Yield so the loop can poll the HUP and close the socket.
const { err: closeErr } = await closed.promise;
child.kill();

// Every negative return is the documented -1 sentinel; the raw errno must
// not leak to JS.
expect(negatives).toEqual([-1, -1, -1]);
// A peer RST is not a clean FIN: `end` must not fire.
expect(events).not.toContain("end");
// The close carries the write-side errno so the reset is observable.
expect(closeErr).toBeInstanceOf(Error);
expect((closeErr as any).code).toBe("ECONNRESET");
expect((closeErr as any).syscall).toBe("write");

Check failure on line 3423 in test/js/bun/net/socket.test.ts

View check run for this annotation

Claude / Claude Code Review

RST test asserts Linux-only errno shape; will fail on macOS CI

The last two assertions (`code === "ECONNRESET"` and `syscall === "write"`) encode Linux errno behavior; on macOS, `send()` to a peer-RST'd socket returns `EPIPE` without consuming `so_error`, so the close error surfaces as either `{code: "EPIPE", syscall: "write"}` or `{code: "ECONNRESET", syscall: "read"}` — the conjunction cannot hold and the test will fail on macOS CI. Either gate these two lines on `isLinux`, or relax to `expect(["ECONNRESET","EPIPE"]).toContain(code)` / `expect(["read","wr
Comment thread
robobun marked this conversation as resolved.
Outdated
},
);
Loading