node:http: defer ServerResponse.destroy() 'close' and stop rewriting completed requests as aborted

src/js/node/_http_server.ts

@@ -833,8 +833,13 @@ Server.prototype[kRealListen] = function (tls, port, host, socketPath, reusePort
 
         if (handle.finished || didFinish) {
           handle = undefined;
-          http_res[kCloseCallback] = undefined;
-          http_res.detachSocket(socket);
+          // A destroyed-but-not-closed response: leave the socket attached so
+          // the socket close path (#onClose) can emit res 'close' and destroy
+          // the request (Node.js: socketOnClose → onServerResponseClose).
+          if (http_res._closed || !http_res.destroyed) {
+            http_res[kCloseCallback] = undefined;
+            http_res.detachSocket(socket);
+          }
           return;
         }
         if (http_res.socket) {
@@ -1112,13 +1117,9 @@ const NodeHTTPServerSocket = class Socket extends Duplex {
     this[kHandle] = undefined;
     handle.onclose = this.#onCloseForDestroy.bind(this, callback, err);
     handle.close();
-    // lets sync check and destroy the request if it's not complete
-    const message = this._httpMessage;
-    const req = message?.req;
-    if (req && !req.complete) {
-      // at this point the handle is not destroyed yet, lets destroy the request
-      req.destroy();
-    }
+    // Do not req.destroy() here: #onClose (scheduled as a task from the native
+    // close path) does it after pending nextTicks, so a fully-received request
+    // is not rewritten as aborted (Node.js's socketOnClose → abortIncoming).
   }
   #onClose() {
     this[kHandle] = null;
@@ -2416,11 +2417,18 @@ ServerResponse.prototype.destroy = function (err?: Error) {
   if (handle) {
     handle.abort();
   }
-  this?.socket?.destroy(err);
-  if (!this._closed) {
-    // res.closed must already be true inside the 'close' listeners.
-    this._closed = true;
-    this.emit("close");
+  // Writable.destroy semantics: 'close' is emitted on a later tick. Read the
+  // storage directly: the `socket` getter auto-creates a FakeSocket that does
+  // not route back to emitCloseNT.
+  const socket = this[fakeSocketSymbol];
+  if (socket) {
+    socket.destroy(err);
+  }
+  // Native server path: the socket's #onClose schedules emitCloseNT(this).
+  // Without a handle (standalone response) schedule it here; emitCloseNT is
+  // guarded by _closed so a later socket close emission is a no-op.
+  if (!handle) {
+    process.nextTick(emitCloseNT, this);
   }
   return this;
 };

test/js/node/http/node-http-server-response-destroy.test.ts

@@ -0,0 +1,92 @@
+/**
+ * ServerResponse.destroy() must follow Writable.destroy semantics: 'close' is
+ * emitted on a later tick (res.closed is still false when destroy() returns),
+ * and the server tearing down its own response must not retroactively rewrite
+ * an already fully-received request as a client abort.
+ *
+ * These tests also pass in Node.js.
+ */
+import { describe, expect, it } from "bun:test";
+import { once } from "node:events";
+import { createServer, IncomingMessage, ServerResponse } from "node:http";
+import type { AddressInfo } from "node:net";
+import { connect } from "node:net";
+
+describe.each([
+  ["before any write", (_res: ServerResponse) => {}, undefined],
+  [
+    "after writeHead + partial body",
+    (res: ServerResponse) => {
+      res.writeHead(200, { "content-length": "50" });
+      res.write("xx");
+    },
+    undefined,
+  ],
+  [
+    "with an error argument",
+    (res: ServerResponse) => {
+      res.writeHead(200, { "content-length": "50" });
+      res.write("xx");
+    },
+    Object.assign(new Error("boom"), { code: "EBOOM" }),
+  ],
+])("ServerResponse.destroy() %s", (_name, setup, err) => {
+  it("defers 'close' and leaves a fully-received request complete (not aborted)", async () => {
+    const events: string[] = [];
+    let closedAtReturn: boolean | undefined;
+    let reqRef!: IncomingMessage;
+
+    const { promise: resClosed, resolve: resolveResClose } = Promise.withResolvers<void>();
+    const { promise: reqClosed, resolve: resolveReqClose } = Promise.withResolvers<void>();
+
+    await using server = createServer((req, res) => {
+      reqRef = req;
+      // Consume the (empty) body so 'end' is due on this request.
+      req.on("data", () => {});
+      req.on("aborted", () => events.push("req.aborted"));
+      req.on("end", () => events.push("req.end"));
+      req.on("close", () => {
+        events.push("req.close");
+        resolveReqClose();
+      });
+      res.on("close", () => {
+        events.push("res.close");
+        resolveResClose();
+      });
+      setup(res);
+      events.push("call-destroy");
+      res.destroy(err);
+      closedAtReturn = res.closed;
+      events.push("destroy-returned");
+    }).listen(0, "127.0.0.1");
+    await once(server, "listening");
+
+    const { promise: clientClosed, resolve: resolveClientClosed } = Promise.withResolvers<void>();
+    const client = connect((server.address() as AddressInfo).port, "127.0.0.1", () => {
+      client.write("GET /x HTTP/1.1\r\nHost: h\r\n\r\n");
+    });
+    // The server force-closes the connection; an ECONNRESET here is expected
+    // and must not fail the test (once(client, "close") would reject on it).
+    client.on("error", () => {});
+    client.on("close", () => resolveClientClosed());
+    await Promise.all([clientClosed, resClosed, reqClosed]);
+
+    // Writable.destroy semantics: 'close' is emitted on a later tick.
+    expect(closedAtReturn).toBe(false);
+    expect(events.indexOf("destroy-returned")).toBeLessThan(events.indexOf("res.close"));
+    // The server destroying its own response is not a client abort: the
+    // already-received request still gets 'end' and stays complete.
+    expect(events).toEqual(["call-destroy", "destroy-returned", "req.end", "req.close", "res.close"]);
+    expect({ aborted: reqRef.aborted, complete: reqRef.complete }).toEqual({ aborted: false, complete: true });
+  });
+});
+
+it("standalone ServerResponse.destroy() defers 'close' to a later tick", async () => {
+  const res = new ServerResponse(new IncomingMessage(undefined as any));
+  const { promise: closed, resolve } = Promise.withResolvers<void>();
+  res.on("close", resolve);
+  res.destroy();
+  expect({ destroyed: res.destroyed, closed: res.closed }).toEqual({ destroyed: true, closed: false });
+  await closed;
+  expect(res.closed).toBe(true);
+});