Repository navigation
fix: prevent WebpackLogger 'done hook' error when callback throws - #703
Srushti-33 wants to merge 2 commits into
Conversation
alexander-akait
left a comment
There was a problem hiding this comment.
Can you add a test case or provide how we can reproduce this problem? Thanks
|
@alexander-akait The issue is already covered by the existing test Before this PR: With this PR: The test already throws an error inside Let me know if you need anything else |
|
@Srushti-33 Can we improve a test case to catch that we don't output |
53bcad7 to
e7fb190
Compare
|
@alexander-akait Rebased and test improved to explicitly assert the WebpackLogger error is not logged. |
|
CI failed |
4049092 to
2a532a8
Compare
| done(); | ||
| }, 1000); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Please move this test into test/plugins.js under new it
83865fd to
b540f86
Compare
| }); | ||
| }); | ||
| // The rest of your analyzer tests (Webpack 5, etc.) remain as they are. | ||
| }); |
There was a problem hiding this comment.
Don't touch this file, do you know how to write tests without AI?
415feb7 to
aad9dfe
Compare
- Replace try/catch with try/finally to always call callback() - Improve test to explicitly assert WebpackLogger error is not logged
aad9dfe to
492f49b
Compare
ronakmaheshwari
left a comment
There was a problem hiding this comment.
The codebase should be synced with the latest changes and linted before this is merged. Please make the necessary updates based on the current code.
More importantly, Issue #499 is hardcoded here. We should avoid hardcoding issue/PR references in the implementation, as this creates an unnecessary coupling to a specific issue. Could you please remove this and make the implementation independent of the issue number?
Fixes #499 – use finally block to always call callback(), preventing WebpackLogger "done hook" error.