fix(builder): improve error message extraction in task results #10173
Conversation
There was a problem hiding this comment.
Pull request overview
Improves how build task errors are aggregated into a single user-facing string by extracting meaningful fields from non-string error values instead of relying on toString().
Changes:
- Adds structured extraction for error values (string,
Error, and object-like errors withmessage/stack). - Introduces fallbacks (
toString()and a final “unknown format” message) when structured fields aren’t available.
| if (e instanceof Error) { | ||
| return e.message; | ||
| } |
There was a problem hiding this comment.
For Error instances, this returns only e.message, so the stack trace is dropped (and the output also loses the error name). This doesn’t match the PR goal of extracting message + stack and makes debugging harder. Consider using e.stack when available (or e.name + e.message plus stack) instead of message-only.
| return e.message; | ||
| } | ||
| // Handle error objects with message and/or stack properties | ||
| if (e && typeof e === 'object') { | ||
| const errorObj = e as Record<string, any>; | ||
| const hasMessage = 'message' in e && typeof errorObj.message === 'string'; | ||
| const hasStack = 'stack' in e && typeof errorObj.stack === 'string'; | ||
| if (hasMessage && hasStack) { | ||
| return `${errorObj.message}\n${errorObj.stack}`; | ||
| } | ||
| if (hasMessage) { | ||
| return errorObj.message; | ||
| } | ||
| if (hasStack) { | ||
| return errorObj.stack; | ||
| } | ||
| } | ||
| // Try toString as final fallback | ||
| if (typeof (e as any)?.toString === 'function') { | ||
| return (e as any).toString(); | ||
| } | ||
| return `unknown error format: ${JSON.stringify(e)}`; |
There was a problem hiding this comment.
ComponentResult.errors is typed as Array<Error | string> (see scopes/pipelines/builder/types.ts:21), but this function now has runtime handling for non-Error objects (Record<string, any> / 'message' in e). Either widen the errors type to include the supported plain-object shape (or unknown) so callers can legally pass it, or remove this branch to keep the implementation aligned with the public type contract.
| return e.message; | |
| } | |
| // Handle error objects with message and/or stack properties | |
| if (e && typeof e === 'object') { | |
| const errorObj = e as Record<string, any>; | |
| const hasMessage = 'message' in e && typeof errorObj.message === 'string'; | |
| const hasStack = 'stack' in e && typeof errorObj.stack === 'string'; | |
| if (hasMessage && hasStack) { | |
| return `${errorObj.message}\n${errorObj.stack}`; | |
| } | |
| if (hasMessage) { | |
| return errorObj.message; | |
| } | |
| if (hasStack) { | |
| return errorObj.stack; | |
| } | |
| } | |
| // Try toString as final fallback | |
| if (typeof (e as any)?.toString === 'function') { | |
| return (e as any).toString(); | |
| } | |
| return `unknown error format: ${JSON.stringify(e)}`; | |
| // Prefer including stack when available for better diagnostics | |
| if (e.stack) { | |
| return `${e.message}\n${e.stack}`; | |
| } | |
| return e.message; | |
| } | |
| // Fallback for unexpected values; should not occur if types are respected | |
| return String(e); |
| if (typeof (e as any)?.toString === 'function') { | ||
| return (e as any).toString(); | ||
| } | ||
| return `unknown error format: ${JSON.stringify(e)}`; |
There was a problem hiding this comment.
JSON.stringify(e) can throw (e.g., circular references) which would cause error formatting itself to crash and potentially mask the original failure. Use a safe stringification (try/catch with a fallback) or a non-throwing formatter like util.inspect for the fallback message.
1 participant
Handle different error object types in
aggregateTaskErrorsToOneString— extractmessageandstackfrom Error instances and plain objects instead of relying solely ontoString().