usockets: report a peer reset behind unread data as ECONNRESET, not 'end'

packages/bun-usockets/src/loop.c

@@ -588,6 +588,9 @@ void us_internal_dispatch_ready_poll(struct us_poll_t *p, int error, int eof, in
             struct us_loop_t* loop = s->group->loop;
             /* Captured before the read loop folds recv()==0 into `eof`; error events keep the error path. */
             const int hangup = (eof & LIBUS_POLL_HANGUP) && !error;
+            /* Set once recv() returns 0 below: the only proof that the peer's stream ended with a FIN. The
+             * eof hint this dispatch was called with (EPOLLHUP, kqueue's EV_EOF) also rides on a reset. */
+            int read_fin = 0;
             if (events & LIBUS_SOCKET_WRITABLE && !error) {
                 s->flags.last_write_failed = 0;
                 #ifdef LIBUS_USE_KQUEUE
@@ -669,9 +672,6 @@ void us_internal_dispatch_ready_poll(struct us_poll_t *p, int error, int eof, in
                 }
 
                 size_t repeat_recv_count = 0;
-                /* Whether this dispatch's read loop delivered any bytes; see the
-                 * hung-up drain and its error handling below. */
-                int read_any = 0;
 
                 do {
                     #ifdef _WIN32
@@ -748,7 +748,6 @@ void us_internal_dispatch_ready_poll(struct us_poll_t *p, int error, int eof, in
                                    : us_dispatch_data(s, loop->data.recv_buf + LIBUS_RECV_BUFFER_PADDING, length);
                         /* After socket adoption, track the new socket; the old one becomes invalid */
                         s = us_internal_socket_follow_adopted(s);
-                        read_any = 1;
                         // loop->num_ready_polls isn't accessible on Windows.
                         #ifndef WIN32
                         // rare case: we're reading a lot of data, there's more to be read, and either:
@@ -820,21 +819,17 @@ void us_internal_dispatch_ready_poll(struct us_poll_t *p, int error, int eof, in
                         #endif
                     } else if (!length) {
                         eof = LIBUS_POLL_EOF; // lets handle EOF in the same place
+                        read_fin = 1;
                         break;
                     } else if (length == LIBUS_SOCKET_ERROR && !bsd_would_block()) {
-                        if (eof && read_any) {
-                            /* The hangup drain above already delivered this
-                             * connection's final data and then hit the error queued
-                             * behind its FIN (an RST from the peer tearing down right
-                             * after, often provoked by our own teardown writes). The
-                             * orderly EOF was announced and nothing readable remains:
-                             * report end-of-stream like a reader that stopped at the
-                             * FIN, not a read error. A hard error on the FIRST read of
-                             * a hung-up event (a pure RST: the kernel discards the
-                             * receive queue) still takes the error path below. */
-                            break;
-                        }
-                        /* Peer-initiated TCP error (RST etc.) — go straight to
+                        /* A read error closes with its errno even when the drain above
+                         * delivered data first, which is what libuv reports to node as
+                         * well: a reset behind unread data (the kernel keeps the receive
+                         * queue, so recv() returns the data and then the error) never
+                         * reached a FIN, so there is no end of stream to report. The eof
+                         * hint this event carried is the reset's own (EPOLLHUP; EV_EOF
+                         * with the error in fflags); a FIN is recv()==0 above.
+                         * Peer-initiated TCP error (RST etc.) — go straight to
                          * raw-close. us_socket_close() would route through
                          * us_internal_ssl_close() now that s->ssl is the
                          * discriminator, and that path fires
@@ -882,6 +877,17 @@ void us_internal_dispatch_ready_poll(struct us_poll_t *p, int error, int eof, in
                  * the read loop drain it, instead of closing over the unread tail. */
                 eof = 0;
             }
+            if (eof && error && !read_fin) {
+                /* An error event whose read loop did not reach a FIN (the socket is
+                 * paused, or on_data paused it mid-drain): the eof hint next to the
+                 * error flag is the reset taking both directions down (EPOLLHUP beside
+                 * EPOLLERR; EV_EOF with the error in fflags), not an end of stream, so
+                 * it must not take the end path below. That path dispatched on_end for
+                 * a reset, and a TLS socket's on_end closes with a clean code itself,
+                 * so the error close never ran. A FIN this dispatch did read still
+                 * delivers its end first; the error close follows either way. */
+                eof = 0;
+            }
             if(eof && s) {
                 if (UNLIKELY(us_socket_is_closed(s))) {
                     // Do not call on_end after the socket has been closed

test/js/node/tls/node-tls-server.test.ts

@@ -1,6 +1,6 @@
 import crypto from "crypto";
 import { readFileSync, realpathSync } from "fs";
-import { bunEnv, bunExe, tls as cert1, isDebug } from "harness";
+import { bunEnv, bunExe, tls as cert1, isDebug, isWindows } from "harness";
 import https from "https";
 import net, { AddressInfo } from "net";
 import { createTest } from "node-harness";
@@ -2457,3 +2457,104 @@ describe("deferred spill-close", () => {
     }
   });
 });
+
+describe.each(["tls", "net"])("%s server socket whose peer resets the connection behind unread data", transport => {
+  // The peer fills both kernel buffers while the accepted socket is paused, then
+  // resets the connection (RST) instead of ending it. Node reports that as a read
+  // error and never emits 'end'. allowHalfOpen keeps the accepted socket open
+  // after an 'end', so a reset misreported as an orderly end strands it.
+  async function acceptPausedSocketAndFill() {
+    const accepted = Promise.withResolvers<net.Socket>();
+    const onConnection = (socket: net.Socket) => {
+      socket.pause();
+      accepted.resolve(socket);
+    };
+    const server =
+      transport === "tls"
+        ? createServer({ ...COMMON_CERT, allowHalfOpen: true }, onConnection)
+        : net.createServer({ allowHalfOpen: true }, onConnection);
+    await once(server.listen(0, "127.0.0.1"), "listening");
+
+    const peerReady = Promise.withResolvers<void>();
+    const peer = await Bun.connect({
+      hostname: "127.0.0.1",
+      port: (server.address() as AddressInfo).port,
+      tls: transport === "tls" ? { ca: COMMON_CERT.cert } : undefined,
+      socket: {
+        open() {
+          if (transport !== "tls") peerReady.resolve();
+        },
+        handshake(_peer, success, verifyError) {
+          if (success) peerReady.resolve();
+          else peerReady.reject(verifyError ?? new Error("handshake failed"));
+        },
+        data() {},
+        close() {},
+        error(_peer, error) {
+          peerReady.reject(error);
+        },
+        connectError(_peer, error) {
+          peerReady.reject(error);
+        },
+      },
+    });
+    const socket = await accepted.promise;
+    await peerReady.promise;
+
+    const events: string[] = [];
+    let bytes = 0;
+    // Settles on 'close', or on the 'end' that must not happen, so a failure does
+    // not wait for the test timeout on a socket left half-open.
+    const settled = Promise.withResolvers<void>();
+    socket.on("data", chunk => (bytes += chunk.length));
+    socket.on("end", () => {
+      events.push("end");
+      settled.resolve();
+    });
+    socket.on("error", error => events.push(`error ${(error as NodeJS.ErrnoException).code}`));
+    socket.on("close", hadError => {
+      events.push(`close hadError=${hadError}`);
+      settled.resolve();
+    });
+    // Paused again: the 'data' listener above switched the stream to flowing.
+    socket.pause();
+
+    // A short write means the peer's send buffer and the server's receive buffer
+    // are both full: everything written so far is queued ahead of the reset.
+    const chunk = Buffer.alloc(64 * 1024, "r");
+    while (peer.write(chunk) === chunk.length) {}
+
+    return {
+      socket,
+      peer,
+      events,
+      bytesRead: () => bytes,
+      settled: settled.promise,
+      [Symbol.dispose]() {
+        socket.destroy();
+        server.close();
+      },
+    };
+  }
+
+  it("reports the reset that arrives while the socket is paused as ECONNRESET, not 'end'", async () => {
+    using t = await acceptPausedSocketAndFill();
+    t.peer.terminate();
+    await t.settled;
+    expect(t.events).toEqual(["error ECONNRESET", "close hadError=true"]);
+  });
+
+  it("delivers the data queued ahead of the reset and then reports ECONNRESET, not 'end'", async () => {
+    using t = await acceptPausedSocketAndFill();
+    // Both happen before the event loop runs again, so the socket's next read
+    // event carries the queued data and the reset together: the read loop drains
+    // the data and then gets the error from recv().
+    t.socket.resume();
+    t.peer.terminate();
+    await t.settled;
+    expect(t.events).toEqual(["error ECONNRESET", "close hadError=true"]);
+    // Windows discards the receive queue on a reset. Linux and macOS keep it, and
+    // like node, the data is delivered before the error.
+    if (!isWindows) expect(t.bytesRead()).toBeGreaterThan(0);
+  });
+});