fix: print error when internal cli error happens (more helpful than ERROR: null) #5344
Conversation
There have been issues where the reported error is "ERROR: null", which is very unhelpful. Here is an analysis of a specific example: mochajs#5048 (comment) Although the trigger for that error was fixed by mochajs#5074, the unhelpfulness of "ERROR: null" was not addressed. To help with debugging, this patch prints the original error when this stage is unexpectedly reached.
|
Switching to draft pending #5078 (comment). We can't review without a reproduction, and it might be that all buggy behavior was already fixed? Thanks for the PR in the interim though! |
For clarity, this is not about fixing whatever that triggered the error, but about improving the logged diagnostics information when an error (unexpectedly) happens. While the specific trigger was fixed, it is still possible to end up in this case if some other internal error occurs. If you want to reproduce locally, you could throw an error from Line 377 in abf3dd9 There are probably other ways to trigger this issue, but I haven't looked. If you are concerned about logging too much information (when an error message is already present), I could add a check to only log the error when the message is null. |
There was a problem hiding this comment.
Pull Request Overview
This PR enhances error reporting in the CLI by printing the original error object to stderr when an internal error occurs, making the “ERROR: null” message more informative.
- Prints the caught error object after showing help and the generic error message.
- Exits with status code 1 after logging the full error.
Comments suppressed due to low confidence (1)
lib/cli/cli.js:64
- Add a test case that simulates an internal CLI error to verify that the original error message (or stack) is printed as expected.
console.error(err);
| @@ -61,6 +61,7 @@ exports.main = (argv = process.argv.slice(2), mochaArgs) => { | |||
| debug('caught error sometime before command handler: %O', err); | |||
| yargs.showHelp(); | |||
| console.error(`\n${symbols.error} ${pc.red('ERROR:')} ${msg}`); | |||
| console.error(err); | |||
There was a problem hiding this comment.
Consider logging the error stack (e.g., console.error(err.stack || err)) instead of the error object itself to provide full context for debugging.
| console.error(err); | |
| console.error(err.stack || err); |
Copilot uses AI. Check for mistakes.
PR Checklist
ERROR: nullpart of it)status: accepting prsOverview
There have been issues where the reported error is "ERROR: null", which is very unhelpful. Here is an analysis of a specific example: #5048 (comment)
Although the trigger for that error was fixed by #5074, the unhelpfulness of "ERROR: null" was not addressed.
To help with debugging, this patch prints the original error when this stage is unexpectedly reached.