fix(build): address PR review feedback#1142
Closed
jdalton wants to merge 1 commit intochore/build-rename-configsfrom
Closed
fix(build): address PR review feedback#1142jdalton wants to merge 1 commit intochore/build-rename-configsfrom
jdalton wants to merge 1 commit intochore/build-rename-configsfrom
Conversation
- runBuild() rethrows after catching so Promise.allSettled can detect rejections - download-assets: handle rejected promises in allSettled, not just ok:false - esbuild.cli.mjs: fix plugin order so envVarReplacement runs after unicode transform - build.mjs: fix watch mode error message to use fallback exit code 1 - package.json: fix e2e:sea indentation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| logger.error(`Build failed: ${description || 'Unknown'}`) | ||
| logger.error(e) | ||
| process.exitCode = 1 | ||
| throw e |
There was a problem hiding this comment.
Rethrown error causes unhandled rejection in standalone callers
Medium Severity
Adding throw e in runBuild's catch block enables Promise.allSettled detection in esbuild.build.mjs, which properly handles rejections. However, esbuild.cli.mjs (line 203) and esbuild.index.mjs (line 23) call runBuild() without a .catch() handler. On build failure, this now produces an unhandled promise rejection in those standalone scripts, resulting in duplicate error output and a noisy Node.js crash trace. The esbuild.cli.mjs script is specifically spawned as a child process during watch mode.
Additional Locations (1)
Contributor
Author
|
Superseded by #1140 which already includes these changes. |
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
1 participant
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.
Summary
Follow-up fixes after review of chore/build-rename-configs:
runBuild()now rethrows after catching soPromise.allSettledcan detect rejectionsdownload-assets: handle rejected promises fromallSettled, not justok: falseesbuild.cli.mjs: fix plugin order soenvVarReplacementruns after unicode transformbuild.mjs: fix watch mode error message to use fallback exit code 1package.json: fixe2e:seaindentationTest plan
pnpm run buildsucceedsNote
Medium Risk
Touches CLI build tooling and asset download flow; incorrect error handling or plugin order could cause builds to succeed incorrectly or fail unexpectedly in CI.
Overview
Fixes several Socket CLI build reliability issues.
Reorders the
esbuildplugins inesbuild.cli.mjssounicodeTransformPlugin()runs beforeenvVarReplacementPlugin()as intended. Tightens failure handling by rethrowing errors fromrunBuild()(soPromise.allSettledcan observe rejections), improvingdownload-assetsreporting/exit codes for rejected downloads, and ensuring watch-mode errors include a?? 1fallback exit code. Also normalizes thee2e:seascript entry inpackage.json.Written by Cursor Bugbot for commit 16a85cf. Configure here.