diff --git a/.changeset/server-startup-cleanup.md b/.changeset/server-startup-cleanup.md new file mode 100644 index 0000000000..771b92c688 --- /dev/null +++ b/.changeset/server-startup-cleanup.md @@ -0,0 +1,5 @@ +--- +"webpack-dev-server": patch +--- + +Reject occupied TCP and IPC startup attempts, clean up only owned WebSocket upgrade listeners, wait for pending plugin startup during shutdown, and report the configured Bonjour protocol correctly. diff --git a/lib/Server.js b/lib/Server.js index 38577856c9..1731bfbc84 100644 --- a/lib/Server.js +++ b/lib/Server.js @@ -2570,17 +2570,6 @@ class Server { }); }, ); - - /** @type {S} */ - (this.server).on( - "error", - /** - * @param {Error} error error - */ - (error) => { - throw error; - }, - ); } /** @@ -3005,7 +2994,7 @@ class Server { if (this.options.bonjour) { const bonjourProtocol = /** @type {BonjourOptions} */ - (this.options.bonjour).type || this.isTlsServer ? "https" : "http"; + (this.options.bonjour).type || (this.isTlsServer ? "https" : "http"); this.logger.info( `Broadcasting "${bonjourProtocol}" with subtype of "webpack" via ZeroConf DNS (Bonjour)`, @@ -3514,8 +3503,13 @@ class Server { * @returns {Promise} */ async start() { - await this.setup(); - await this.listen(); + try { + await this.setup(); + await this.listen(); + } catch (error) { + await this.stop(); + throw error; + } } /** @@ -3558,7 +3552,8 @@ class Server { socket.connect( { path: /** @type {string} */ (this.options.ipc) }, () => { - throw new Error(`IPC "${this.options.ipc}" is already used`); + socket.destroy(); + reject(new Error(`IPC "${this.options.ipc}" is already used`)); }, ); }) @@ -3586,9 +3581,11 @@ class Server { : { host: this.options.host, port: this.options.port }; await /** @type {Promise} */ ( - new Promise((resolve) => { - /** @type {S} */ - (this.server).listen(listenOptions, () => { + new Promise((resolve, reject) => { + const server = /** @type {S} */ (this.server); + server.once("error", reject); + server.listen(listenOptions, () => { + server.removeListener("error", reject); resolve(); }); }) @@ -3748,6 +3745,8 @@ class Server { /** @type {Promise | undefined} */ let setupPromise; + /** @type {Promise | undefined} */ + let listenPromise; let inWatchMode = false; let listening = false; let stopped = false; @@ -3787,10 +3786,12 @@ class Server { hooks.done.tap(pluginName, () => { // `done` also fires for a one-shot `compiler.run()` build, where no // `watchRun` ran; staying passive lets that build finish and exit. - if (listening || !inWatchMode) return; + if (listening || !inWatchMode || stopped) return; listening = true; - ensureSetup() - .then(() => this.listen()) + listenPromise = ensureSetup() + .then(() => { + if (!stopped) return this.listen(); + }) .catch((error) => { this.logger.error(error); }); @@ -3802,6 +3803,9 @@ class Server { const onShutdown = async () => { if (stopped) return; stopped = true; + // Startup errors are reported by watchRun or the done handler above. + // Wait for pending startup before releasing the resources it creates. + await Promise.allSettled([setupPromise, listenPromise]); await this.stop(); }; diff --git a/lib/servers/WebsocketServer.js b/lib/servers/WebsocketServer.js index cc3f7b635b..e07b5dc1dd 100644 --- a/lib/servers/WebsocketServer.js +++ b/lib/servers/WebsocketServer.js @@ -29,15 +29,16 @@ export default class WebsocketServer extends BaseServer { this.implementation = new WsServer(options); - /** @type {import("http").Server} */ - (this.server.server).on( - "upgrade", + if (isNoServerMode) { + const httpServer = /** @type {import("http").Server} */ ( + this.server.server + ); /** * @param {import("http").IncomingMessage} req request * @param {import("stream").Duplex} sock socket * @param {Buffer} head head */ - (req, sock, head) => { + const handleUpgrade = (req, sock, head) => { if (!this.implementation.shouldHandle(req)) { return; } @@ -45,8 +46,13 @@ export default class WebsocketServer extends BaseServer { this.implementation.handleUpgrade(req, sock, head, (connection) => { this.implementation.emit("connection", connection, req); }); - }, - ); + }; + + httpServer.on("upgrade", handleUpgrade); + this.implementation.on("close", () => { + httpServer.removeListener("upgrade", handleUpgrade); + }); + } this.implementation.on( "error", diff --git a/test/e2e/api.test.js b/test/e2e/api.test.js index f8928ded60..2f72ef936c 100644 --- a/test/e2e/api.test.js +++ b/test/e2e/api.test.js @@ -2,7 +2,7 @@ import path from "node:path"; import { afterEach, beforeEach, describe, it, mock } from "node:test"; import { fileURLToPath } from "node:url"; import { expect } from "expect"; -import { fn } from "jest-mock"; +import { fn, spyOn } from "jest-mock"; import webpack from "webpack"; import Server from "../../lib/Server.js"; import config from "../fixtures/client-config/webpack.config.js"; @@ -193,6 +193,43 @@ describe("API", () => { }); }); + it("should clean up initialized resources when listening fails", async () => { + const compiler = webpack(config); + const server = new Server({ port }, compiler); + const listenError = new Error("listen failed"); + const listenSpy = spyOn(server, "listen").mockRejectedValue(listenError); + const stopSpy = spyOn(server, "stop"); + + await expect(server.start()).rejects.toBe(listenError); + + expect(stopSpy).toHaveBeenCalledTimes(1); + expect(server.server).toBeUndefined(); + + listenSpy.mockRestore(); + stopSpy.mockRestore(); + }); + + it("should clean up initialized resources when setup fails", async () => { + const compiler = webpack(config); + const server = new Server({ port }, compiler); + const setupError = new Error("setup failed"); + const close = fn((callback) => callback()); + const setupSpy = spyOn(server, "setup").mockImplementation(async () => { + server.server = { close }; + throw setupError; + }); + const stopSpy = spyOn(server, "stop"); + + await expect(server.start()).rejects.toBe(setupError); + + expect(stopSpy).toHaveBeenCalledTimes(1); + expect(close).toHaveBeenCalledTimes(1); + expect(server.server).toBeUndefined(); + + setupSpy.mockRestore(); + stopSpy.mockRestore(); + }); + it("should work when using configured manually", async (t) => { const compiler = webpack({ ...config,