Fix function build errors not rendering actual failure reason#7216
Fix function build errors not rendering actual failure reason#7216davejcameron wants to merge 1 commit into
Conversation
When a Shopify Function build fails (e.g. cargo/rust not installed),
the app-event-watcher only wrote error.message ("Failed to build
function.") to stderr, ignoring error.tryMessage which contains the
actual failure details. Now also writes tryMessage when present.
There was a problem hiding this comment.
Pull request overview
Improves the developer experience when Shopify Function builds fail during dev by ensuring the underlying failure reason (stored in tryMessage) is written to stderr instead of only showing a generic error message.
Changes:
- Print
error.tryMessageto stderr inAppEventWatcherwhen present, to surface the actual build failure reason. - Add a dev watcher test to verify
tryMessagecontent is included in stderr output. - Add build service tests to verify build command failure details are preserved in
AbortError.tryMessage.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/app/src/cli/services/dev/app-events/app-event-watcher.ts | Writes tryMessage to stderr on non-esbuild build errors. |
| packages/app/src/cli/services/dev/app-events/app-event-watcher.test.ts | Adds coverage asserting tryMessage details appear in stderr output. |
| packages/app/src/cli/services/build/extension.test.ts | Adds coverage asserting build command failures propagate details via tryMessage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| this.options.stderr.write( | ||
| typeof error.tryMessage === 'string' ? error.tryMessage : String(error.tryMessage), | ||
| ) |
There was a problem hiding this comment.
When printing error.message and then error.tryMessage, the two writes are concatenated with no separator, so users may see a single glued string (e.g., Failed to build function.Command failed ...). Consider writing a newline (or \n\n) between them, or only adding a separator if error.message doesn’t already end with one, so the output is readable.
| this.options.stderr.write( | |
| typeof error.tryMessage === 'string' ? error.tryMessage : String(error.tryMessage), | |
| ) | |
| const tryMessage = | |
| typeof error.tryMessage === 'string' ? error.tryMessage : String(error.tryMessage) | |
| if (!String(error.message).endsWith('\n')) { | |
| this.options.stderr.write('\n') | |
| } | |
| this.options.stderr.write(tryMessage) |
| if (error.tryMessage) { | ||
| this.options.stderr.write( | ||
| typeof error.tryMessage === 'string' ? error.tryMessage : String(error.tryMessage), | ||
| ) | ||
| } |
There was a problem hiding this comment.
AbortError.tryMessage is typed as a TokenItem | null (not necessarily a plain string). Using String(error.tryMessage) can yield [object Object] for tokenized messages. Since this file already imports from @shopify/cli-kit/node/output, consider using itemToString(error.tryMessage) (or equivalent) to reliably render token items to a human-readable string before writing to stderr.
|
This PR seems inactive. If it's still relevant, please add a comment saying so. Otherwise, take no action. |
|
Copilot messages make sense, let's fix those and we can merge this! |
|
This PR seems inactive. If it's still relevant, please add a comment saying so. Otherwise, take no action. |
Summary
app-event-watcheronly wroteerror.message("Failed to build function.") to stderr, completely hiding the actual failure reasonerror.tryMessage(e.g. "Command failed with exit code 127: cargo build --release") but never renderedtryMessageto stderr when present, so users see why their build actually failedTest plan
app-event-watcher.test.tsverifyingtryMessagecontent is written to stderrextension.test.tsverifyingtryMessageis preserved when wrappingExternalErrorand generic errors🤖 Generated with Claude Code