diff --git a/src/js/node/_http_server.ts b/src/js/node/_http_server.ts index 6c66f7904a12..14a443bcfec6 100644 --- a/src/js/node/_http_server.ts +++ b/src/js/node/_http_server.ts @@ -121,23 +121,34 @@ const DateNow = Date.now; let cluster; -function emitCloseServer(self: Server) { +// Whether the pending close() is complete. If it is, its 'close' is due and the pending state is cleared. +function takePendingClose(self: Server) { // A native close promise belongs to one listener; Node's Server 'close' spans overlapping listeners and handed-off sockets (net.js _emitCloseIfDrained). - if (!self[kPendingDrainClose]) return; + if (!self[kPendingDrainClose]) return false; const pendingGenerations = self[kPendingCloseGenerations]; let drainedAtClose = true; for (const generation of pendingGenerations) { - if (!generation.nativeClosed) return; + if (!generation.nativeClosed) return false; drainedAtClose &&= generation.drainedAtClose; } const relistened = !!self[serverSymbol]; // Node queues 'close' inside a close() that leaves no connection open. A listen() after it, or a connection to that listener, cannot hold the event back. - if ((relistened || self[kTrackedConnections].size > 0) && !drainedAtClose) return; + if ((relistened || self[kTrackedConnections].size > 0) && !drainedAtClose) return false; self[kPendingDrainClose] = false; pendingGenerations.clear(); if (!relistened) self[kListenerGeneration] = undefined; + return true; +} +function emitCloseServer(self: Server) { + if (takePendingClose(self)) self.emit("close"); +} +function emitServerClose(self: Server) { self.emit("close"); } +// Node emits a plaintext socket's 'error' ahead of the server's 'close': https://github.com/nodejs/node/blob/v26.3.0/lib/net.js#L906-L918 +function emitCloseServerAfterSocketError(self: Server) { + if (takePendingClose(self)) process.nextTick(emitServerClose, self); +} function markListenerGenerationClosed(self: Server, generation) { generation.nativeClosed = true; emitCloseServer(self); @@ -1848,8 +1859,11 @@ function getNodeHTTPServerSocket() { if (tracked) { tracked.delete(this); if (tracked.size === 0 && server[kPendingDrainClose]) { - // The server 'close' follows this socket's own 'close' event. - this.once("close", () => process.nextTick(emitCloseServer, server)); + // Not a listener on the socket. Over TLS Node destroys the counted raw socket before the 'error': https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L684-L687 + process.nextTick( + (errored || this.#closeError) && !this.encrypted ? emitCloseServerAfterSocketError : emitCloseServer, + server, + ); } } const timer = this[kSocketTimeoutTimer]; diff --git a/test/js/node/http/node-http-server-close-drain.test.ts b/test/js/node/http/node-http-server-close-drain.test.ts index 1ddba31ce6ce..bfac715b650e 100644 --- a/test/js/node/http/node-http-server-close-drain.test.ts +++ b/test/js/node/http/node-http-server-close-drain.test.ts @@ -1,10 +1,11 @@ import { expect, test } from "bun:test"; -import { bunEnv, bunExe, isWindows, tls as tlsCert } from "harness"; +import { bunEnv, bunExe, isASAN, isCI, isDebug, isWindows, tls as tlsCert } from "harness"; import { once } from "node:events"; import { createServer } from "node:http"; import { Agent as HttpsAgent, createServer as createHttpsServer, get as httpsGet } from "node:https"; import type { AddressInfo } from "node:net"; import { connect, createServer as createNetServer } from "node:net"; +import path from "node:path"; import { connect as tlsConnect } from "node:tls"; // Node's net.Server#close callback (and the 'close' event) only fires once @@ -1011,3 +1012,88 @@ test("no 'close' is emitted on a re-listened server when an earlier connection e server.closeAllConnections(); } }); + +// Node queues the server's 'close' when the last socket is destroyed, not from that socket's events: https://github.com/nodejs/node/blob/v26.3.0/lib/net.js#L906-L918 +// Node v26.3.0 prints the same line for each row, except where a comment gives its line. +test( + "server.close(cb) calls back, whatever the program does to the events of the last connection", + async () => { + await using proc = Bun.spawn({ + cmd: [bunExe(), path.join(import.meta.dir, "node-http-server-close-last-connection-fixture.js")], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(JSON.parse(stdout)).toEqual({ + "a response 'close' listener throws": ["server close", "close callback", "response close", "uncaught boom"], + "a response 'close' listener throws (https)": [ + "server close", + "close callback", + "response close", + "uncaught boom", + ], + "a socket 'close' listener throws": ["server close", "close callback", "uncaught boom"], + "the 'close' listener of an upgraded socket throws": ["server close", "close callback", "uncaught boom"], + // Node: server close, close callback, uncaught boom. Node emits this socket's 'close' a turn of the loop after its 'error'. + "the 'close' listener of a socket that sent one byte throws": ["uncaught boom", "server close", "close callback"], + // Node: server close, disposed, uncaught boom. Node emits the socket's 'close' a turn of the loop later. + "server[Symbol.asyncDispose]() waits and a socket 'close' listener throws": [ + "server close", + "uncaught boom", + "disposed", + ], + "an 'aborted' listener removes the socket's 'close' listeners": ["server close", "close callback"], + "the program's socket.emit swallows 'close'": ["server close", "close callback"], + "the 'error' listener of a socket destroyed with an error throws": [ + "uncaught boom", + "server close", + "close callback", + ], + "closeAllConnections() closes the idle last connection": ["server close", "close callback", "uncaught boom"], + "closeIdleConnections() closes the idle last connection": ["server close", "close callback", "uncaught boom"], + "the peer closes the idle last connection": ["server close", "close callback", "uncaught boom"], + // Node: server close, close callback, uncaught boom. Here the listener's own close is delivered after this socket's. + "closeIdleConnections() and then socket.destroy() close the idle last connection": [ + "uncaught boom", + "server close", + "close callback", + ], + "a socket 'end' listener removes every listener of the socket": ["server close", "close callback"], + "a socket 'error' listener removes the 'close' listeners at the peer's reset": [ + "socket error ECONNRESET", + "server close", + "close callback", + ], + "the peer ends the idle last connection": ["socket end", "server close", "close callback", "socket close"], + "the peer ends the idle last connection (https)": [ + "socket end", + "server close", + "close callback", + "socket close", + ], + // Node: server close, close callback, socket close. Here the listener's own close is delivered after this socket's. + "close() itself closes the idle last connection": ["socket close", "server close", "close callback"], + "a tunnel that is idle after the peer's FIN is destroyed": ["server close", "close callback"], + "the response is destroyed with an error": ["clientError", "server close", "close callback"], + "the response is destroyed with an error (https)": ["server close", "close callback", "clientError"], + "the peer resets a connection that sent part of a request head": [ + "clientError ECONNRESET", + "server close", + "close callback", + ], + "the peer resets a connection that sent part of a request head (https)": [ + "server close", + "close callback", + "clientError ECONNRESET", + ], + "a 'clientError' listener listens again": ["server close", "close callback", "listening again"], + "the socket's 'error' listener listens again": ["server close", "close callback", "listening again"], + "the socket's 'close' listener listens again": ["server close", "close callback", "listening again"], + }); + expect(exitCode).toBe(0); + }, + // CI has its own time for each test. A local debug or ASAN build needs several seconds to start the fixture. + (isASAN || isDebug) && !isCI ? 90_000 : undefined, +); diff --git a/test/js/node/http/node-http-server-close-last-connection-fixture.js b/test/js/node/http/node-http-server-close-last-connection-fixture.js new file mode 100644 index 000000000000..d6d2f5d95f49 --- /dev/null +++ b/test/js/node/http/node-http-server-close-last-connection-fixture.js @@ -0,0 +1,303 @@ +// Each row has its own server. server.close() waits for the last connection of that server, and the program does +// something to that connection. A step follows an event or a round trip, never a timer. +// Prints one JSON object: the name of each row and its events in order. Node.js runs this file too. +const http = require("node:http"); +const https = require("node:https"); +const net = require("node:net"); +const tls = require("node:tls"); +const fs = require("node:fs"); +const path = require("node:path"); + +const keys = path.join(__dirname, "..", "test", "fixtures", "keys"); +const tlsOptions = { + key: fs.readFileSync(path.join(keys, "agent1-key.pem")), + cert: fs.readFileSync(path.join(keys, "agent1-cert.pem")), +}; +const get = "GET / HTTP/1.1\r\nHost: x\r\n\r\n"; +const upgrade = "GET / HTTP/1.1\r\nHost: x\r\nConnection: Upgrade\r\nUpgrade: x\r\n\r\n"; +const switching = "HTTP/1.1 101 Switching Protocols\r\nConnection: Upgrade\r\nUpgrade: x\r\n\r\n"; + +const results = {}; +process.on("uncaughtException", err => { + if (!err.events) throw err; + err.events.push("uncaught " + err.message); +}); +process.on("exit", () => console.log(JSON.stringify(results))); + +function row(name, protocol, run) { + const events = (results[name] = []); + const secure = protocol === "https"; + const server = secure ? https.createServer(tlsOptions) : http.createServer(); + // No idle timer of the server closes a connection of a row. + server.keepAliveTimeout = 0; + server.on("close", () => events.push("server close")); + server.listen(0, "127.0.0.1", () => { + const { port } = server.address(); + const open = request => { + const socket = secure + ? tls.connect({ port, host: "127.0.0.1", rejectUnauthorized: false }, () => socket.write(request)) + : net.connect(port, "127.0.0.1", () => socket.write(request)); + socket.on("error", () => {}); + return socket; + }; + // Calls then(raw socket) when the server has read a part of a request head from that connection. + const partial = (head, then) => { + const raw = net.connect(port, "127.0.0.1", () => { + const peer = secure ? tls.connect({ socket: raw, rejectUnauthorized: false }, () => send(peer)) : send(raw); + peer.on("error", () => {}); + }); + raw.on("error", () => {}); + const send = peer => { + peer.write(head); + // The bytes were written first, so the server has read them when it answers this. + const barrier = open("GET / HTTP/1.1\r\nHost: x\r\nConnection: close\r\n\r\n"); + barrier.resume(); + barrier.on("close", () => then(raw)); + return peer; + }; + }; + run({ + server, + events, + open, + partial, + close: () => server.close(() => events.push("close callback")), + boom() { + throw Object.assign(new Error("boom"), { events }); + }, + }); + }); +} + +// The response is destroyed one round trip after close(), from outside the parse of its connection. +function responseCloseThrows({ server, events, close, boom, open }) { + server.on("request", (req, res) => { + res.on("close", () => { + events.push("response close"); + boom(); + }); + close(); + res.write("x"); + client.once("data", () => res.destroy()); + }); + const client = open(get); +} +row("a response 'close' listener throws", "http", responseCloseThrows); +row("a response 'close' listener throws (https)", "https", responseCloseThrows); + +row("a socket 'close' listener throws", "http", ({ server, close, boom, open }) => { + server.on("request", (req, res) => { + req.socket.on("close", boom); + close(); + res.write("x"); + client.once("data", () => res.destroy()); + }); + const client = open(get); +}); + +row("the 'close' listener of an upgraded socket throws", "http", ({ server, close, boom, open }) => { + server.on("upgrade", (req, socket) => { + socket.on("close", boom); + close(); + socket.write(switching); + client.once("data", () => socket.destroy()); + }); + const client = open(upgrade); +}); + +row("the 'close' listener of a socket that sent one byte throws", "http", ({ server, close, boom, partial }) => { + server.once("connection", socket => socket.on("close", boom)); + server.on("request", (req, res) => res.end("ok")); + partial("G", socket => { + close(); + socket.destroy(); + }); +}); + +row("server[Symbol.asyncDispose]() waits and a socket 'close' listener throws", "http", context => { + const { server, events, boom, open } = context; + server.on("upgrade", (req, socket) => { + socket.on("close", boom); + server[Symbol.asyncDispose]().then(() => events.push("disposed")); + socket.write(switching); + client.once("data", () => socket.destroy()); + }); + const client = open(upgrade); +}); + +row("an 'aborted' listener removes the socket's 'close' listeners", "http", ({ server, close, open }) => { + server.on("request", (req, res) => { + req.on("error", () => {}); + req.on("aborted", () => req.socket.removeAllListeners("close")); + close(); + res.write("x"); + client.once("data", () => res.destroy()); + }); + const client = open(get); +}); + +row("the program's socket.emit swallows 'close'", "http", ({ server, close, open }) => { + server.on("upgrade", (req, socket) => { + const emit = socket.emit; + socket.emit = function (name) { + if (name === "close") return false; + return emit.apply(this, arguments); + }; + close(); + socket.write(switching); + client.once("data", () => socket.destroy()); + }); + const client = open(upgrade); +}); + +row("the 'error' listener of a socket destroyed with an error throws", "http", ({ server, close, boom, open }) => { + server.on("upgrade", (req, socket) => { + socket.on("error", boom); + close(); + socket.write(switching); + client.once("data", () => socket.destroy(new Error("destroyed"))); + }); + const client = open(upgrade); +}); + +// The last connection is idle after its response. Each row closes it in another way, and its 'close' listener throws. +function idleConnection(closeIt) { + return ({ server, close, boom, open }) => { + server.on("request", (req, res) => { + req.socket.on("close", boom); + close(); + res.end("ok"); + client.once("data", () => closeIt(server, client, req.socket)); + }); + const client = open(get); + }; +} +row( + "closeAllConnections() closes the idle last connection", + "http", + idleConnection(server => server.closeAllConnections()), +); +row( + "closeIdleConnections() closes the idle last connection", + "http", + idleConnection(server => server.closeIdleConnections()), +); +row( + "the peer closes the idle last connection", + "http", + idleConnection((server, client) => client.end()), +); +// The native handle is closed and its close is not delivered yet when destroy() runs. +row( + "closeIdleConnections() and then socket.destroy() close the idle last connection", + "http", + idleConnection((server, client, socket) => { + server.closeIdleConnections(); + socket.destroy(); + }), +); + +row("a socket 'end' listener removes every listener of the socket", "http", ({ server, close, open }) => { + server.on("request", (req, res) => { + req.socket.on("end", () => req.socket.removeAllListeners()); + close(); + res.end("ok"); + client.once("data", () => client.end()); + }); + const client = open(get); +}); + +row("a socket 'error' listener removes the 'close' listeners at the peer's reset", "http", context => { + const { server, events, close, open } = context; + server.on("request", (req, res) => { + req.socket.on("error", err => { + events.push("socket error " + err.code); + req.socket.removeAllListeners("close"); + }); + close(); + res.end("ok"); + client.once("data", () => client.resetAndDestroy()); + }); + const client = open(get); +}); + +// The order of the server's 'close' and the events of the last socket. +function endBeforeServerClose({ server, events, close, open }) { + server.on("request", (req, res) => { + req.socket.on("end", () => events.push("socket end")); + req.socket.on("close", () => events.push("socket close")); + close(); + res.end("ok"); + client.once("data", () => client.end()); + }); + const client = open(get); +} +row("the peer ends the idle last connection", "http", endBeforeServerClose); +row("the peer ends the idle last connection (https)", "https", endBeforeServerClose); + +row("close() itself closes the idle last connection", "http", ({ server, events, close, open }) => { + server.on("request", (req, res) => { + req.socket.on("close", () => events.push("socket close")); + res.end("ok"); + client.once("data", close); + }); + const client = open(get); +}); + +row("a tunnel that is idle after the peer's FIN is destroyed", "http", ({ server, close, open }) => { + server.on("upgrade", (req, socket) => { + socket.on("end", () => { + close(); + socket.write(switching); + client.once("data", () => socket.destroy()); + }); + socket.resume(); + client.end(); + }); + const client = open(upgrade); +}); + +// A close that carries an error. On http Node emits the socket's 'error' before the server's 'close', on https after it. +function destroyWithError({ server, events, close, open }) { + server.on("clientError", () => events.push("clientError")); + server.on("request", (req, res) => { + close(); + res.write("x"); + client.once("data", () => res.destroy(new Error("destroyed"))); + }); + const client = open(get); +} +row("the response is destroyed with an error", "http", destroyWithError); +row("the response is destroyed with an error (https)", "https", destroyWithError); + +function peerReset({ server, events, close, partial }) { + server.on("request", (req, res) => res.end("ok")); + server.on("clientError", (err, socket) => { + events.push("clientError " + err.code); + socket.destroy(); + }); + partial("GET / HT", socket => { + close(); + socket.resetAndDestroy(); + }); +} +row("the peer resets a connection that sent part of a request head", "http", peerReset); +row("the peer resets a connection that sent part of a request head (https)", "https", peerReset); + +// The close carries an error, and a listener that runs on the way listens again. +function listenAgain(listenFrom) { + return ({ server, events, close, open }) => { + const again = () => server.listen(0, "127.0.0.1", () => events.push("listening again") && server.unref()); + server.on("clientError", () => listenFrom === "clientError" && again()); + server.on("request", (req, res) => { + if (listenFrom !== "clientError") req.socket.on(listenFrom, again); + close(); + res.write("x"); + client.once("data", () => res.destroy(new Error("destroyed"))); + }); + const client = open(get); + }; +} +row("a 'clientError' listener listens again", "http", listenAgain("clientError")); +row("the socket's 'error' listener listens again", "http", listenAgain("error")); +row("the socket's 'close' listener listens again", "http", listenAgain("close")); diff --git a/test/js/node/http/node-http.test.ts b/test/js/node/http/node-http.test.ts index 26142da2ae36..ec5f667f596a 100644 --- a/test/js/node/http/node-http.test.ts +++ b/test/js/node/http/node-http.test.ts @@ -5492,7 +5492,13 @@ describe("HTTP server transport shutdown", () => { await release.promise; res.end("done"); }); - server.on("connection", socket => socket.once("close", () => events.push("socket close"))); + const socketClosed = Promise.withResolvers(); + server.on("connection", socket => + socket.once("close", () => { + events.push("socket close"); + socketClosed.resolve(); + }), + ); server.listen(0, "127.0.0.1"); await once(server, "listening"); const { port } = server.address() as AddressInfo; @@ -5533,9 +5539,9 @@ describe("HTTP server transport shutdown", () => { client.destroy(); expect(await closed.promise).toBeUndefined(); - await clientClosed.promise; - expect(events[0]).toBe("socket close"); - expect(events.indexOf("socket close")).toBeLessThan(events.indexOf("close callback")); + await Promise.all([clientClosed.promise, socketClosed.promise]); + // The order Node v26.3.0 prints: net.Server queues its 'close' when the last socket is destroyed, ahead of that socket's own 'close'. + expect(events).toEqual(["close callback", "socket close"]); expect(wire).toContain("Content-Length: 4\r\n"); expect(wire).toEndWith("\r\n\r\ndone"); } finally {