Conversation
Looks like we need to explicitly call `core.setFailed` before `process.exit(1)` (aka what we use if anything goes wrong) if we want this to be readable in Actions logs.
There was a problem hiding this comment.
Pull request overview
This PR enhances error logging in GitHub Actions bootstrap files by adding core.setFailed() calls before process.exit(1) to ensure error messages are human-readable in Actions logs. It also includes code formatting improvements (removing spaces in destructuring imports, using arrow functions without parentheses for single parameters, and removing trailing newlines).
Changes:
- Added
core.setFailed()calls beforeprocess.exit(1)in error handlers for npm ci and build failures - Added informational logging with
core.info()when starting the main action script - Applied consistent code formatting across all bootstrap files
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| .github/actions/fix/bootstrap.js | Added core.setFailed() for npm ci and build errors, added info logging, applied formatting updates |
| .github/actions/find/bootstrap.js | Added error logging for npm ci and build errors, added info logging, applied formatting updates (but uses console.error instead of core.setFailed) |
| .github/actions/file/bootstrap.js | Added core.setFailed() for npm ci and build errors, added info logging, applied formatting updates |
| .github/actions/auth/bootstrap.js | Added core.setFailed() for npm ci and build errors, added info logging, applied formatting updates |
Comments suppressed due to low confidence (1)
.github/actions/find/bootstrap.js:55
- This should use
core.setFailed()instead ofconsole.error()for consistency with the other three bootstrap files (fix, file, and auth) and to properly mark the action as failed in GitHub Actions logs. The PR description specifically mentions the need to callcore.setFailedbeforeprocess.exit(1)for human-readable error messages in Actions logs.
console.error(`npm run build (TypeScript compilation) failed: ${error}`)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }) | ||
| } catch { | ||
| } catch (error) { | ||
| console.error(`npm ci failed: ${error}`) |
There was a problem hiding this comment.
This should use core.setFailed() instead of console.error() for consistency with the other three bootstrap files (fix, file, and auth) and to properly mark the action as failed in GitHub Actions logs. The PR description specifically mentions the need to call core.setFailed before process.exit(1) for human-readable error messages in Actions logs.
This issue also appears on line 55 of the same file.
Root cause: PR #143 added `import * as core from '@actions/core'` as a top-level static import. Since node_modules isn't pre-installed, Node.js fails to resolve this import before any code (including npm ci) runs. Fix: Replace the static top-level import with a dynamic import inside the `finally` block where node_modules is guaranteed to exist. For the npm ci failure case, use console.error since @actions/core isn't available yet. Co-authored-by: JoyceZhu <6251669+JoyceZhu@users.noreply.github.com>
1 participant
Looks like we need to explicitly call
core.setFailedbeforeprocess.exit(1)(akawhat we use if anything goes wrong) if we want
this to be human-readable in Actions logs.