fix: surface the root cause of wrapped errors in CLI output - #126
Open
maelvls wants to merge 2 commits into
Open
fix: surface the root cause of wrapped errors in CLI output#126maelvls wants to merge 2 commits into
maelvls wants to merge 2 commits into
Conversation
errorMessage() only printed err.message, so a network failure like Node's fetch throwing "fetch failed" hid the actual reason (e.g. an AggregateError of ECONNREFUSED entries) in err.cause. Walk the cause chain, including AggregateError.errors, so users see e.g. "fetch failed: connect ECONNREFUSED 127.0.0.1:9090" instead of just "fetch failed".
There was a problem hiding this comment.
Pull request overview
This PR improves CLI error output by including underlying .cause details (including AggregateError.errors) so wrapped runtime/network failures (notably Node fetch() failures) surface actionable root causes, and makes the formatter unit-testable by exporting it.
Changes:
- Enhanced
errorMessage()to walk the.causechain and flattenAggregateError.errorsinto the rendered message. - Exported
errorMessage()fromsrc/cli/context.tsso it can be tested directly. - Added unit tests covering plain errors, non-Error inputs, single-cause errors,
AggregateErrorcauses, and multi-level cause chains.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/cli/context.ts |
Exported and extended errorMessage() to include chained causes / aggregate causes in CLI-visible output. |
test/error-message.test.ts |
Added unit tests validating errorMessage() formatting across common wrapped-error shapes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+49
to
+65
| const { message: rootMessage, cause: rootCause } = err; | ||
| let message = rootMessage; | ||
| let cause: unknown = rootCause; | ||
| while (cause !== undefined && cause !== null) { | ||
| if (cause instanceof AggregateError && cause.errors.length > 0) { | ||
| message += `: ${cause.errors.map((e) => (e instanceof Error ? e.message : String(e))).join("; ")}`; | ||
| break; | ||
| } | ||
| if (cause instanceof Error) { | ||
| const { message: causeMessage, cause: nextCause } = cause; | ||
| message += `: ${causeMessage}`; | ||
| cause = nextCause; | ||
| continue; | ||
| } | ||
| message += `: ${String(cause)}`; | ||
| break; | ||
| } |
Author
There was a problem hiding this comment.
Good catch — fixed in 6af7204. errorMessage() now tracks visited causes in a Set and stops as soon as it sees a repeat, so a cyclic chain (e.g. err.cause === err) can't hang the process. Added tests for a self-referential cause and a longer a↔b cycle. (written by Claude)
errorMessage() could loop forever on a cyclic cause chain (e.g. err.cause === err), hanging the process while rendering a CLI error. Track visited causes in a Set and stop once a repeat is seen.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I got annoyed by the unhelpful error message that you get anytime there is some form of network problem:
With this patch, the actual error will be shown. For example, in my case, it was:
other example when I was testing
HTTPS_PROXYin #125:Much more useful!
I manually tested this with Node, but not with the Bun-produced binary.
(code by Claude)