Skip to content

Comments

fix: prevent WebpackLogger 'done hook' error when callback throws#703

Open
Srushti-33 wants to merge 2 commits intowebpack:mainfrom
Srushti-33:fix-499-webpacklogger
Open

fix: prevent WebpackLogger 'done hook' error when callback throws#703
Srushti-33 wants to merge 2 commits intowebpack:mainfrom
Srushti-33:fix-499-webpacklogger

Conversation

@Srushti-33
Copy link
Contributor

@Srushti-33 Srushti-33 commented Feb 11, 2026

Fixes #499 – use finally block to always call callback(), preventing WebpackLogger "done hook" error.

alexander-akait
alexander-akait reviewed Feb 11, 2026
View reviewed changes
Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test case or provide how we can reproduce this problem? Thanks

@Srushti-33
Copy link
Contributor Author

Srushti-33 commented Feb 11, 2026

@alexander-akait The issue is already covered by the existing test test/issue-499.test.js (added in 2022).

Before this PR:
Running npx jest test/issue-499.test.js would fail with:

No such label 'done hook' for WebpackLogger.timeEnd()

With this PR:
The same test passes cleanly:

PASS  test/issue-499.test.js
  Issue #499
    ✓ should not cause WebpackLogger error when callback throws (13xx ms)

The test already throws an error inside compiler.run() and verifies that the WebpackLogger error does not occur. No new test is needed, but if you'd like a more explicit assertion (e.g., capturing console.error), I'm happy to add it.

Let me know if you need anything else

@alexander-akait
Copy link
Member

alexander-akait commented Feb 12, 2026

@Srushti-33 Can we improve a test case to catch that we don't output No such label 'done hook' for WebpackLogger.timeEnd(), also can you rebase?

@Srushti-33 Srushti-33 force-pushed the fix-499-webpacklogger branch 9 times, most recently from 53bcad7 to e7fb190 Compare February 12, 2026 11:30
@Srushti-33
Copy link
Contributor Author

Srushti-33 commented Feb 12, 2026

@alexander-akait Rebased and test improved to explicitly assert the WebpackLogger error is not logged.

@alexander-akait
Copy link
Member

alexander-akait commented Feb 12, 2026

CI failed

@Srushti-33 Srushti-33 force-pushed the fix-499-webpacklogger branch 2 times, most recently from 4049092 to 2a532a8 Compare February 12, 2026 14:30
alexander-akait
alexander-akait reviewed Feb 12, 2026
View reviewed changes
test/issue-499.test.js Outdated
done();
}, 1000);
});
});
Copy link
Member

@alexander-akait alexander-akait Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move this test into test/plugins.js under new it

@Srushti-33 Srushti-33 force-pushed the fix-499-webpacklogger branch 4 times, most recently from 83865fd to b540f86 Compare February 12, 2026 15:59
alexander-akait
alexander-akait reviewed Feb 12, 2026
View reviewed changes
test/analyzer.js
});
});
// The rest of your analyzer tests (Webpack 5, etc.) remain as they are.
});
Copy link
Member

@alexander-akait alexander-akait Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't touch this file, do you know how to write tests without AI?

@Srushti-33 Srushti-33 force-pushed the fix-499-webpacklogger branch 5 times, most recently from 415feb7 to aad9dfe Compare February 12, 2026 18:38
@Srushti-33
fix: prevent WebpackLogger 'done hook' error when callback throws
492f49b 
- Replace try/catch with try/finally to always call callback()
- Improve test to explicitly assert WebpackLogger error is not logged
@Srushti-33 Srushti-33 force-pushed the fix-499-webpacklogger branch from aad9dfe to 492f49b Compare February 12, 2026 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexander-akait alexander-akait alexander-akait left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

No such label 'done hook' for WebpackLogger.timeEnd()

2 participants

@Srushti-33 @alexander-akait