node:net: let unhandled socket 'error' throw (stop swallowing ECONNRESET)

packages/bun-usockets/src/context.c

@@ -799,7 +799,10 @@ void us_internal_socket_after_open(struct us_socket_t *s, int error) {
         if (c) {
             for (struct us_socket_t *next = c->connecting_head; next; next = next->connect_next) {
                 if (next != s) {
-                    us_socket_close(next, LIBUS_SOCKET_CLOSE_CODE_CONNECTION_RESET, 0);
+                    /* A losing attempt the peer already accepted should see a clean
+                     * FIN, not a reset: a net.Server that accepted it would otherwise
+                     * observe ECONNRESET on a connection that never sent anything. */
+                    us_socket_close(next, LIBUS_SOCKET_CLOSE_CODE_CLEAN_SHUTDOWN, 0);
                 }
             }
             /* Attach TLS now that we know which candidate won. */

src/js/node/http2.ts

@@ -6805,9 +6805,9 @@ Http2Server.prototype[EventEmitter.captureRejectionSymbol] = function (err, even
 
 function onErrorSecureServerSession(err, socket) {
   if (!this.emit("clientError", err, socket)) {
-    // The handshake-failed socket has no 'error' listener yet; destroying it with the error
-    // would crash the process with an uncaught exception. The failure has already been
-    // surfaced through 'tlsClientError'/'clientError'.
+    // No error argument: tlsClientError already reported it, and net's
+    // onServerSocketTLSError listener is inert once the tlsClientError guard is
+    // set so destroy(err) would be swallowed.
     if (!socket.destroyed) socket.destroy();
   }
 }

src/js/node/net.ts

@@ -149,6 +149,7 @@ const kSNIError = Symbol("kSNIError");
 const kALPNError = Symbol("kALPNError");
 const kPerfHooksNetConnectContext = Symbol("kPerfHooksNetConnectContext");
 const khandshakeTimer = Symbol("khandshakeTimer");
+const ktlsClientErrorEmitted = Symbol("ktlsClientErrorEmitted");
 const kUserUnrefed = Symbol("kUserUnrefed");
 // Set when pause() dropped the handle's hold on the loop, so the read paths
 // only restore a hold they actually removed - re-refing a handle that never
@@ -188,19 +189,11 @@ function failWrite(self, negErrno, callback) {
       self.destroy(er);
     }
   } else if (!self.destroyed) {
-    if (self.listenerCount("error") > 0) {
-      // The consumer can detach its listener between now and destroy()'s
-      // deferred 'error' emission - the same last-resort guard
-      // SocketEmitEndNT uses for read errors.
-      self.once("error", () => {});
-      self.destroy(er);
-    } else {
-      // No write callback and no 'error' listener: a failed flush on an
-      // orphaned socket (an h2 teardown racing the peer's reset - routine on
-      // Windows, where the reset completes the send first) is teardown noise.
-      // Same silent-close policy as SocketEmitEndNT's no-listener case.
-      self.destroy();
-    }
+    // No write callback: Node's onWriteComplete delivers the error to the
+    // stream (errorOrDestroy). An unhandled 'error' throws, as with any
+    // EventEmitter - that is how a missing .on("error") surfaces.
+    self._hadError = true;
+    self.destroy(er);
   }
 }
 function endNT(socket, callback, err) {
@@ -517,12 +510,7 @@ function SocketEmitEndNT(self, _err?) {
   // A read error delivered with the close (e.g. a received RST surfacing as
   // ECONNRESET) is not a clean EOF — Node destroys the socket with the error
   // ("read ECONNRESET") instead of emitting a graceful 'end'. Guard on
-  // !destroyed so an already-torn-down socket isn't re-destroyed, and on an
-  // 'error' listener so callers that opted into error handling get Node's
-  // behavior while those that did not keep the previous silent EOF (a server
-  // hard-closing after a clean response would otherwise surface here as an
-  // unhandled error across the proxy/http2/fetch suites under ASAN/baseline
-  // timing).
+  // !destroyed so an already-torn-down socket isn't re-destroyed.
   // A reset that lands after the exchange already finished in BOTH
   // directions (clean EOF delivered and nothing left being written) is
   // teardown noise - a peer hard-closing once the exchange completed - not
@@ -539,13 +527,8 @@ function SocketEmitEndNT(self, _err?) {
   // _hadError: the failure already reached JS through the error dispatch
   // (native on_error / a fatal write); node emits a socket error exactly
   // once, so the close that follows it is delivered plain.
-  if (_err && !self.destroyed && !self._hadError && !teardownNoise && self.listenerCount("error") > 0) {
-    // The consumer can detach its 'error' listener between this close
-    // callback and destroy()'s deferred 'error' emission (a request that
-    // finished just as the reset arrived); a last-resort no-op listener keeps
-    // that race from surfacing as an uncaught exception - the no-listener
-    // case is already a documented silent close.
-    self.once("error", () => {});
+  if (_err && !self.destroyed && !self._hadError && !teardownNoise) {
+    self._hadError = true;
     let errErrno;
     if (_err.code === undefined && typeof (errErrno = _err.errno) === "number" && errErrno !== 0) {
       // A codeless close error that still carries the errno (Windows IOCP
@@ -583,10 +566,9 @@ function SocketEmitEndNT(self, _err?) {
     self[kended] = true;
     self.push(null);
   } else if (_err && !self.destroyed) {
-    // An error excluded from the synthesis above (teardown noise, or no
-    // listener attached): nothing more is coming, but the socket still has to
-    // finish its lifecycle - close it quietly instead of leaving it open with
-    // no further events.
+    // Teardown noise or an already-reported error: nothing more is coming, but
+    // the socket still has to finish its lifecycle - close it quietly instead
+    // of leaving it open with no further events.
     self.destroy();
   }
   // A write that was waiting on the native drain can never complete once the
@@ -835,6 +817,7 @@ const ServerHandlers: SocketHandler<NetSocket> = {
         err = tlsHandshakeError(verifyError);
       }
       self.emit("_tlsError", err);
+      self[ktlsClientErrorEmitted] = true;
       server?.emit("tlsClientError", err, self);
       self._hadError = true;
       // error before handshake on the server side will only be emitted using tlsClientError
@@ -862,8 +845,9 @@ const ServerHandlers: SocketHandler<NetSocket> = {
           server?.emit("tlsClientError", verifyError, self);
           // if we reject we still need to emit secure
           self.emit("secure", self);
-          // No error argument: the socket has no 'error' listener yet, so destroy(err)
-          // would surface as an uncaught exception.
+          // No error argument: tlsClientError already reported it, and the
+          // onServerSocketTLSError listener is inert once _secureEstablished is
+          // set so destroy(verifyError) would be swallowed.
           self.destroy();
           return;
         }
@@ -946,6 +930,18 @@ const ServerHandlers: SocketHandler<NetSocket> = {
   binaryType: "buffer",
 } as const;
 
+// Node's onSocketTLSError (lib/_tls_wrap.js): attached to every server-
+// accepted TLSSocket. Before 'secureConnection' the socket is still owned by
+// the TLS machinery, so an error is reported through 'tlsClientError' instead
+// of as an unhandled 'error'. After 'secureConnection' the listener is inert,
+// but its presence keeps the default EventEmitter throw from firing.
+function onServerSocketTLSError(err) {
+  if (!this._secureEstablished && !this[ktlsClientErrorEmitted]) {
+    this[ktlsClientErrorEmitted] = true;
+    this._server?.emit("tlsClientError", err, this);
+  }
+}
+
 // Node.js-compatible onconnection: assigned to server._handle.onconnection in
 // kRealListen and invoked from ServerHandlers.open with `this` bound to the
 // listener handle. Kept as a standalone function so tests/cluster can wrap it.
@@ -990,6 +986,9 @@ function onconnection(err, clientHandle) {
         remoteFamily: _socket.remoteFamily || "IPv4",
       };
       clientHandle.end();
+      // destroy() so a RST from the dropped peer reaches a destroyed socket
+      // (SocketEmitEndNT short-circuits) instead of one with no 'error' listener.
+      _socket.destroy();
       self.emit("drop", data);
       return;
     }
@@ -1005,6 +1004,7 @@ function onconnection(err, clientHandle) {
     };
 
     clientHandle.end();
+    _socket.destroy();
     self.emit("drop", data);
     return;
   }
@@ -1026,6 +1026,15 @@ function onconnection(err, clientHandle) {
   _socket.server = self;
   _socket._server = self;
 
+  // Node's tlsConnectionListener attaches onSocketTLSError to every accepted
+  // TLSSocket: pre-handshake it routes to 'tlsClientError'; post-handshake it
+  // is inert but its presence keeps an unhandled 'error' from throwing, since
+  // user code only sees the socket at 'secureConnection'. A plain TCP server's
+  // accepted socket gets no such listener (Node throws on unhandled 'error').
+  if (isTLS) {
+    _socket.on("error", onServerSocketTLSError);
+  }
+
   if (pauseOnConnect) {
     _socket.pause();
   }
@@ -1046,6 +1055,7 @@ function onconnection(err, clientHandle) {
       _socket[khandshakeTimer] = undefined;
       const err = $ERR_TLS_HANDSHAKE_TIMEOUT();
       _socket._hadError = true;
+      _socket[ktlsClientErrorEmitted] = true;
       self.emit("tlsClientError", err, _socket);
       if (!_socket.destroyed) _socket.destroy();
     }, handshakeTimeout);
@@ -1178,11 +1188,15 @@ const SocketHandlers2: SocketHandler<NonNullable<import("node:net").Socket["_han
     // socket's current handle: connection attempts that lost the
     // family-autoselection race and raw sockets handed off during a TLS
     // upgrade also report errors on close, and those must keep ending
-    // cleanly.
-    if (err && !self.destroyed && socket === self._handle && self.listenerCount("error") > 0) {
-      // Same late-detach guard as SocketEmitEndNT: the listener seen at
-      // close-time can be gone by the deferred 'error' emission.
-      self.once("error", () => {});
+    // cleanly. Same teardownNoise/_hadError guards as SocketEmitEndNT.
+    if (
+      err &&
+      !self.destroyed &&
+      !self._hadError &&
+      socket === self._handle &&
+      !(self[kended] && self.writableFinished)
+    ) {
+      self._hadError = true;
       if (err.code === undefined || err.code === "ECONNRESET") {
         // Shape it like Node's errnoException(UV_ECONNRESET, 'read').
         const er = new ConnResetException("read ECONNRESET") as Error & { errno?: number; syscall?: string };
@@ -1200,6 +1214,12 @@ const SocketHandlers2: SocketHandler<NonNullable<import("node:net").Socket["_han
       }
       return;
     }
+    if (err && !self.destroyed && socket === self._handle) {
+      // Teardown noise or already-reported error (SocketEmitEndNT's fall-through):
+      // nothing more is coming, close quietly so 'close' still fires.
+      self.destroy();
+      return;
+    }
     self[kended] = true;
     if (!self.allowHalfOpen) self.write = writeAfterFIN;
     self.push(null);

test/js/bun/http/proxy.test.ts

@@ -387,6 +387,7 @@ test("unsupported protocol", async () => {
 async function createAuthCapturingProxy() {
   const capturedAuths: string[] = [];
   const server = net.createServer((clientSocket: net.Socket) => {
+    clientSocket.on("error", () => {});
     clientSocket.once("data", data => {
       const request = data.toString();
       const lines = request.split("\r\n");
@@ -711,6 +712,7 @@ test("HTTPS proxy tunnel keep-alive does not share tunnel across different crede
   const sockets = new Set<net.Socket>();
   const upstreamSockets = new Set<net.Socket>();
   const proxy = net.createServer(clientSocket => {
+    clientSocket.on("error", () => {});
     sockets.add(clientSocket);
     clientSocket.once("data", data => {
       const req = data.toString();
@@ -1231,6 +1233,7 @@ describe.concurrent("proxy object format with headers", () => {
     // Create a proxy server that captures headers
     const capturedHeaders: string[] = [];
     const proxyServerWithCapture = net.createServer((clientSocket: net.Socket) => {
+      clientSocket.on("error", () => {});
       clientSocket.once("data", data => {
         const request = data.toString();
         // Capture headers
@@ -1306,6 +1309,7 @@ describe.concurrent("proxy object format with headers", () => {
     // Create a proxy server that captures headers
     const capturedHeaders: string[] = [];
     const proxyServerWithCapture = net.createServer((clientSocket: net.Socket) => {
+      clientSocket.on("error", () => {});
       clientSocket.once("data", data => {
         const request = data.toString();
         // Capture headers
@@ -1446,6 +1450,7 @@ describe.concurrent("proxy object format with headers", () => {
   test("proxy object with headers as Headers instance", async () => {
     const capturedHeaders: string[] = [];
     const proxyServerWithCapture = net.createServer((clientSocket: net.Socket) => {
+      clientSocket.on("error", () => {});
       clientSocket.once("data", data => {
         const request = data.toString();
         const lines = request.split("\r\n");
@@ -1514,6 +1519,7 @@ describe.concurrent("proxy object format with headers", () => {
   test("user-provided Proxy-Authorization header overrides URL credentials", async () => {
     const capturedHeaders: string[] = [];
     const proxyServerWithCapture = net.createServer((clientSocket: net.Socket) => {
+      clientSocket.on("error", () => {});
       clientSocket.once("data", data => {
         const request = data.toString();
         const lines = request.split("\r\n");
@@ -1835,6 +1841,7 @@ describe.concurrent("NO_PROXY with explicit proxy option", () => {
           // origin-form and forward to the endpoint.
           let proxyHits = 0;
           const proxy = net.createServer(client => {
+            client.on("error", () => {});
             client.once("data", data => {
               proxyHits++;
               const text = data.toString();