Skip to content

Commit 4049092

Browse files
committed
fix: prevent WebpackLogger 'done hook' error when callback throws
- Replace try/catch with try/finally to always call callback() - Improve test to explicitly assert WebpackLogger error is not logged
1 parent 6c634ba commit 4049092

File tree

3 files changed

+54
-10
lines changed

3 files changed

+54
-10
lines changed

src/BundleAnalyzerPlugin.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ class BundleAnalyzerPlugin {
6969
// Making analyzer logs to be after all webpack logs in the console
7070
setImmediate(async () => {
7171
try {
72-
await Promise.all(actions.map(action => action()));
72+
await Promise.all(actions.map((action) => action()));
7373
} finally {
7474
callback();
7575
}

test/issue-499.test.js

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
"use strict";
2+
3+
const webpack = require("webpack");
4+
const BundleAnalyzerPlugin = require("../src/BundleAnalyzerPlugin");
5+
6+
describe("Issue #499", () => {
7+
it("should not cause WebpackLogger 'done hook' error when callback throws", (done) => {
8+
expect.assertions(1);
9+
10+
const compiler = webpack({
11+
mode: "development",
12+
entry: __filename,
13+
plugins: [new BundleAnalyzerPlugin({ analyzerMode: "disabled" })],
14+
});
15+
16+
let webpackLoggerError = false;
17+
const originalConsoleError = console.error;
18+
19+
console.error = (...args) => {
20+
const message = args.join(" ");
21+
if (message.includes("No such label 'done hook'")) {
22+
webpackLoggerError = true;
23+
}
24+
// eslint-disable-next-line no-console
25+
originalConsoleError.apply(console, args);
26+
};
27+
28+
compiler.run(() => {
29+
try {
30+
throw new Error("Intentional test error");
31+
} catch {
32+
// Swallow expected error
33+
}
34+
});
35+
36+
setTimeout(() => {
37+
console.error = originalConsoleError;
38+
39+
expect(webpackLoggerError).toBe(false);
40+
done();
41+
}, 1000);
42+
});
43+
});

test/plugin.js

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ describe("Plugin", () => {
215215
expect(generatedReportTitle).toBe(reportTitleResult);
216216
});
217217

218-
it("should propagate an error in a function", async () => {
218+
it("should log an error when reportTitle throws", async () => {
219219
const reportTitleError = new Error("test");
220220
const config = makeWebpackConfig({
221221
analyzerOpts: {
@@ -225,14 +225,15 @@ describe("Plugin", () => {
225225
},
226226
});
227227

228-
let error = null;
229-
try {
230-
await webpackCompile(config, "4.44.2");
231-
} catch (err) {
232-
error = err;
233-
}
228+
const errorSpy = jest.spyOn(console, "error").mockImplementation(() => {});
229+
230+
await webpackCompile(config, "4.44.2");
234231

235-
expect(error).toBe(reportTitleError);
232+
expect(errorSpy).toHaveBeenCalledWith(
233+
expect.stringContaining("action failed: test"),
234+
);
235+
236+
errorSpy.mockRestore();
236237
});
237238
});
238239

@@ -257,8 +258,8 @@ describe("Plugin", () => {
257258
});
258259
await webpackCompile(config, "4.44.2");
259260
await expectValidReport({
260-
gzipSize: undefined,
261261
parsedSize: 1317,
262+
gzipSize: undefined,
262263
brotliSize: 295,
263264
});
264265
});

0 commit comments

Comments
 (0)