diff --git a/.changeset/plugin-listen-error.md b/.changeset/plugin-listen-error.md new file mode 100644 index 0000000000..5e419958bd --- /dev/null +++ b/.changeset/plugin-listen-error.md @@ -0,0 +1,5 @@ +--- +"webpack-dev-server": patch +--- + +In plugin mode, a failure to start the server (e.g. `EADDRINUSE` when the port is taken) is now passed to the `watch()` callback, for a `MultiCompiler` too, instead of only being logged while the watch build keeps running. diff --git a/lib/Server.js b/lib/Server.js index a36bec7e35..dbf5f7a85b 100644 --- a/lib/Server.js +++ b/lib/Server.js @@ -4551,6 +4551,8 @@ class Server { let setupPromise; /** @type {Promise | undefined} */ let listenPromise; + /** @type {Promise | undefined} */ + let pendingListenPromise; let inWatchMode = false; let listening = false; let stopped = false; @@ -4579,7 +4581,8 @@ class Server { // is how we know a one-shot `compiler.run()` build is not in play. // - `done` on a `MultiCompiler` is the aggregate `SyncHook` that fires once // after every child finishes, so the server starts exactly once. Being a - // `SyncHook`, it can only be `tap`ped; `listen()` runs detached. + // `SyncHook`, it can only be `tap`ped, so `listen()` is awaited from the + // `done` hook of each `Compiler` below. const { hooks } = /** @type {Compiler} */ (compiler); hooks.watchRun.tapPromise(pluginName, () => { @@ -4592,15 +4595,44 @@ class Server { // `watchRun` ran; staying passive lets that build finish and exit. if (listening || !inWatchMode || stopped) return; listening = true; - listenPromise = ensureSetup() - .then(() => { - if (!stopped) return this.listen(); - }) - .catch((error) => { - this.logger.error(error); - }); + listenPromise = ensureSetup().then(() => { + if (!stopped) return this.listen(); + }); + pendingListenPromise = listenPromise; }); + // A `Compiler`'s own `done` is an `AsyncSeriesHook`, so a `listen()` failure + // (e.g. `EADDRINUSE`) rejects the user's `watch()` callback, as `start()` + // does. On a `MultiCompiler` the aggregate `done` above fires from inside + // the last child's `done` (its tap is added when the `MultiCompiler` is + // created, before any plugin), so that child awaits the listen started there. + const doneCompilers = /** @type {MultiCompiler} */ (compiler).compilers || [ + compiler, + ]; + + /** + * @param {Stats} stats stats + * @param {(err?: Error) => void} callback callback + */ + const awaitPendingListen = (stats, callback) => { + if (!pendingListenPromise) { + callback(); + return; + } + + const promise = pendingListenPromise; + + pendingListenPromise = undefined; + promise.then(() => callback(), callback); + }; + + for (const doneCompiler of doneCompilers) { + /** @type {Compiler} */ (doneCompiler).hooks.done.tapAsync( + pluginName, + awaitPendingListen, + ); + } + /** * @returns {Promise} promise */ diff --git a/test/e2e/api-plugin.test.js b/test/e2e/api-plugin.test.js index 64279fc1ea..f0bd4f56e6 100644 --- a/test/e2e/api-plugin.test.js +++ b/test/e2e/api-plugin.test.js @@ -31,6 +31,25 @@ const cssFilePath = path.resolve( "../fixtures/reload-config-3/main.css", ); +const occupyPort = (portToOccupy) => + new Promise((resolve, reject) => { + const blocker = http.createServer(); + + blocker.once("error", reject); + blocker.listen(portToOccupy, "127.0.0.1", () => resolve(blocker)); + }); + +const watchError = (compiler) => + new Promise((resolve, reject) => { + compiler.watch({}, (err) => { + if (err) { + resolve(err); + } else { + reject(new Error("expected listen to fail")); + } + }); + }); + describe("API (plugin)", () => { it("should work with plugin API", async (t) => { const compiler = webpack(config); @@ -440,40 +459,25 @@ describe("API (plugin)", () => { }); }); - it("should log a listen() failure instead of throwing", async () => { - // The server is started from the `done` hook, which is a `SyncHook`, so - // `listen()` runs detached. A failure there must be logged, not surface as - // an unhandled rejection. + it("should propagate listen errors via the watch callback", async () => { + const blocker = await occupyPort(port); const compiler = webpack(config); - const server = new Server({ port }); + const server = new Server({ host: "127.0.0.1", port }); + server.apply(compiler); - const listenError = new Error("listen failed"); - const listenSpy = spyOn(server, "listen").mockImplementation(() => - Promise.reject(listenError), - ); - const errorSpy = spyOn(server.logger, "error"); + try { + const error = await watchError(compiler); - await new Promise((resolve, reject) => { - const timer = setTimeout( - () => reject(new Error("listen error was not logged")), - 30000, - ); - errorSpy.mockImplementation(() => { - clearTimeout(timer); - resolve(); + expect(error.code).toBe("EADDRINUSE"); + } finally { + await new Promise((resolve) => { + compiler.close(resolve); }); - compiler.watch({}, () => {}); - }); - - expect(errorSpy).toHaveBeenCalledTimes(1); - expect(errorSpy).toHaveBeenCalledWith(listenError); - - listenSpy.mockRestore(); - errorSpy.mockRestore(); - await new Promise((resolve) => { - compiler.close(resolve); - }); + await new Promise((resolve) => { + blocker.close(resolve); + }); + } }); describe("plugin in webpack config", () => { @@ -645,6 +649,27 @@ describe("API (plugin)", () => { }); }); + it("should propagate listen errors via the watch callback", async () => { + const blocker = await occupyPort(port); + const compiler = webpack(multiCompilerConfig); + const server = new Server({ host: "127.0.0.1", port }); + + server.apply(compiler); + + try { + const error = await watchError(compiler); + + expect(error.code).toBe("EADDRINUSE"); + } finally { + await new Promise((resolve) => { + compiler.close(resolve); + }); + await new Promise((resolve) => { + blocker.close(resolve); + }); + } + }); + it("should stop the server only once when all child compilers shut down", async () => { const compiler = webpack(multiCompilerConfig); const server = new Server({ port });