Skip to content

build(mac): split bundle step and capture stderr for diagnostics #9965

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
asheshv wants to merge 1 commit into
base: master
Choose a base branch
from
+23 −2
Open

build(mac): split bundle step and capture stderr for diagnostics #9965

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions pkg/mac/build-functions.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,29 @@ _complete_bundle() {
pushd "${SOURCE_DIR}/web" > /dev/null || exit
yarn set version berry
yarn set version 4
yarn install
yarn run bundle
yarn install 2>&1

# Record the source commit hash before the heavy lint/webpack
# steps. `yarn run` needs node_modules so this runs after install,
# but it's a pure `git log` redirect (see web/package.json
# "git:hash") that costs ~nothing, so doing it up front means the
# commit_hash file is captured even if webpack later bails out.
echo "==> Recording git hash..."
yarn run git:hash
Comment on lines +294 to +302
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Add error handling for yarn install and yarn run git:hash.

Both commands lack explicit error handling (e.g., || exit 1). If yarn install fails, subsequent build steps will operate on broken or incomplete dependencies, producing unusable artifacts. The git:hash step is less critical but should still fail fast if it cannot complete.

Other critical commands in this file (lines 92–93) follow the || exit 1 pattern. Apply the same here to maintain consistency and ensure build failures are caught immediately.

🛡️ Proposed fix to add error handling 
-        yarn install 2>&1
+        yarn install 2>&1 || exit 1
 
         # Record the source commit hash before the heavy lint/webpack
         # steps. `yarn run` needs node_modules so this runs after install,
         # but it's a pure `git log` redirect (see web/package.json
         # "git:hash") that costs ~nothing, so doing it up front means the
         # commit_hash file is captured even if webpack later bails out.
         echo "==> Recording git hash..."
-        yarn run git:hash
+        yarn run git:hash || exit 1
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
yarn install 2>&1
# Record the source commit hash before the heavy lint/webpack
# steps. `yarn run` needs node_modules so this runs after install,
# but it's a pure `git log` redirect (see web/package.json
# "git:hash") that costs ~nothing, so doing it up front means the
# commit_hash file is captured even if webpack later bails out.
echo "==> Recording git hash..."
yarn run git:hash
yarn install 2>&1 || exit 1
# Record the source commit hash before the heavy lint/webpack
# steps. `yarn run` needs node_modules so this runs after install,
# but it's a pure `git log` redirect (see web/package.json
# "git:hash") that costs ~nothing, so doing it up front means the
# commit_hash file is captured even if webpack later bails out.
echo "==> Recording git hash..."
yarn run git:hash || exit 1
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/mac/build-functions.sh` around lines 294 - 302, Add explicit failure
handling to the two commands that currently lack it: ensure the "yarn install
2>&1" invocation and the "yarn run git:hash" invocation both propagate errors
(e.g., append "|| exit 1") so the script fails fast on install or git:hash
errors; update the calls where found in build-functions.sh to match the existing
error-handling pattern used elsewhere (see the earlier commands that use "||
exit 1") so subsequent build steps don't run on a broken state.


# Split the "bundle" script into its underlying steps and merge
# stderr into stdout, so a crash inside lint/webpack (e.g. an OOM
# kill or native-module load failure) leaves a trace in the
# Jenkins console instead of an empty gap before the trap fires.
# Env vars match the top-level "bundle" npm script (see web/package.json)
# so behavior is identical to `yarn run bundle`.
export NODE_ENV=production
export NODE_OPTIONS=--max-old-space-size=3072
echo "==> Running ESLint..."
yarn run linter 2>&1
echo "==> Running webpack bundle..."
yarn run webpacker 2>&1
Comment on lines +313 to +315
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Add error handling for yarn run linter and yarn run webpacker.

If either linting or webpack bundling fails, the build should stop immediately. Without explicit error handling, the script will continue after failures and produce incomplete or broken build artifacts.

Linting failures indicate code quality issues that should block release, and webpack failures mean the frontend bundle is missing or corrupted. Both are critical build gates.

🛡️ Proposed fix to add error handling 
         echo "==> Running ESLint..."
-        yarn run linter 2>&1
+        yarn run linter 2>&1 || exit 1
         echo "==> Running webpack bundle..."
-        yarn run webpacker 2>&1
+        yarn run webpacker 2>&1 || exit 1
         unset NODE_ENV NODE_OPTIONS
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/mac/build-functions.sh` around lines 313 - 315, The yarn commands for
linting and webpacking currently run without checking failure: ensure the build
stops on errors by adding explicit error handling for the two invocations (the
lines running "yarn run linter" and "yarn run webpacker"): either enable a
fail-fast shell option (e.g., set -e at the top of the script) or append a check
to each command so that if "yarn run linter" or "yarn run webpacker" exits
non‑zero you print a clear error and exit 1 (e.g., use "yarn run linter" ... ||
{ echo 'Lint failed'; exit 1; } and similarly for "yarn run webpacker") so the
build cannot continue with failed lint or bundle steps.

unset NODE_ENV NODE_OPTIONS

curl https://curl.se/ca/cacert.pem -o cacert.pem -s
popd > /dev/null || exit
Expand Down
Loading