Skip to content

fix: prevent postinstall infinite loop on bun install #1064

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

Merged
Kitenite merged 1 commit into from
Jan 31, 2026
+17 −1
Merged

fix: prevent postinstall infinite loop on bun install #1064

Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
"format:check": "biome format .",
"typecheck": "turbo typecheck",
"ui-add": "turbo run ui-add",
"postinstall": "sherif; bun run --filter=@superset/desktop install:deps",
"postinstall": "./scripts/postinstall.sh",
"clean": "git clean -xdf node_modules",
"clean:workspaces": "turbo clean",
"release:desktop": "./apps/desktop/create-release.sh",
Expand Down
16 changes: 16 additions & 0 deletions scripts/postinstall.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#!/bin/bash
# Prevent infinite recursion during postinstall
# electron-builder install-app-deps can trigger nested bun installs
# which would re-run postinstall, spawning hundreds of processes

if [ -n "$SUPERSET_POSTINSTALL_RUNNING" ]; then
exit 0
fi

export SUPERSET_POSTINSTALL_RUNNING=1

# Run sherif for workspace validation
sherif

# Install native dependencies for desktop app
bun run --filter=@superset/desktop install:deps
Comment on lines +1 to +16
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jan 30, 2026

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

🌐 Web query:

bash strict mode set -e set -u set -o pipefail best practices error handling

💡 Result:

What “strict mode” does (and what it doesn’t)

Common pattern:

set -euo pipefail
  • set -e (errexit): exit when an unhandled command fails, but it is intentionally skipped in several contexts (tests in if/while/until, most &&/|| lists, non-last pipeline elements unless pipefail, and when status is inverted with !). So you still need explicit checks where you expect failures. [2]
  • set -u (nounset): treat unset variables as an error. [1]
  • set -o pipefail: a pipeline fails if any element fails (specifically: returns the last non‑zero status, or 0 if all succeed). Disabled by default. [1]

Best-practice pattern (robust + debuggable errors)

1) Prefer explicit error handling + a trap, not blind reliance on set -e

Use a single error handler that prints context:

#!/usr/bin/env bash
set -Eeuo pipefail

die() { echo "ERROR: $*" >&2; exit 1; }
trap 'die "Command failed (exit=$?) at ${BASH_SOURCE[0]}:${LINENO}: ${BASH_COMMAND}"' ERR

Why -E? It helps ERR traps propagate into functions/subshell contexts (without it, you’ll miss failures in places you thought were covered). [2]

2) When failure is expected, handle it explicitly

Under -e, don’t “probe” with commands that might fail unless you capture/branch on the status:

grep -q pattern file || echo "not found (this is ok)"
# or
if ! grep -q pattern file; then
  echo "not found"
fi

This aligns with “always check return values” guidance. [3]

3) Be careful with pipefail + early-terminating consumers (head, grep -q)

With pipefail, producers can die with SIGPIPE when the downstream command exits early; that can abort your script even though the overall behavior is fine. You may need to explicitly allow it, or restructure:

# allow SIGPIPE-ish failures if you truly don’t care:
set +e
big_producer | head -n 1
set -e

(Or avoid such pipelines / use tools designed for it.)

4) Avoid “hidden” errexit holes ShellCheck warns about

Example: ! cmd can suppress errexit unexpectedly unless you explicitly exit/check status. [4]

set -e
! some_check || exit 1

5) If you need POSIX sh, don’t use pipefail

pipefail is not POSIX; either require bash, or implement an alternative. [5]

Key takeaway

set -euo pipefail is a good baseline, but robust Bash comes from: (a) explicit checks where failure is expected, (b) an ERR trap for actionable diagnostics, and (c) knowing the documented contexts where errexit/pipefail don’t behave like exceptions. [1][2][3]

Sources: GNU Bash manual for pipefail/nounset and errexit/ERR-trap exceptions [1][2], Google Shell Style Guide on checking return values and error reporting [3], ShellCheck wiki on ! vs errexit and portability of pipefail [4][5].

🏁 Script executed:

# First, let's see the full postinstall.sh file to understand the complete context
cat -n scripts/postinstall.sh

Repository: superset-sh/superset

Length of output: 609

🏁 Script executed:

# Let's check if there are other bash scripts in the repo to see patterns for error handling
fd -e sh -e bash . --type f | head -20

Repository: superset-sh/superset

Length of output: 205

🏁 Script executed:

# Check if postinstall.sh is referenced in package.json or other build configs
rg -i "postinstall" --max-count 10

Repository: superset-sh/superset

Length of output: 653

🏁 Script executed:

# Check if other scripts in the same location use set -e or strict mode
head -5 scripts/lint.sh

Repository: superset-sh/superset

Length of output: 204

🏁 Script executed:

# Check the desktop build script for comparison
head -10 apps/desktop/create-release.sh

Repository: superset-sh/superset

Length of output: 407

Fail fast on sherif/bun errors (enable strict mode).

Without set -e (and ideally -u -o pipefail), a failed sherif command will not terminate the script, allowing subsequent commands to execute even after validation fails. Since this script runs as an npm/bun postinstall hook, such failures should halt the installation. Add strict mode to catch errors early.

♻️ Proposed change 
 #!/bin/bash
+set -euo pipefail
+
 # Prevent infinite recursion during postinstall
 # electron-builder install-app-deps can trigger nested bun installs
 # which would re-run postinstall, spawning hundreds of processes
📝 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
#!/bin/bash
# Prevent infinite recursion during postinstall
# electron-builder install-app-deps can trigger nested bun installs
# which would re-run postinstall, spawning hundreds of processes
if [ -n "$SUPERSET_POSTINSTALL_RUNNING" ]; then
exit 0
fi
export SUPERSET_POSTINSTALL_RUNNING=1
# Run sherif for workspace validation
sherif
# Install native dependencies for desktop app
bun run --filter=@superset/desktop install:deps
#!/bin/bash
set -euo pipefail
# Prevent infinite recursion during postinstall
# electron-builder install-app-deps can trigger nested bun installs
# which would re-run postinstall, spawning hundreds of processes
if [ -n "$SUPERSET_POSTINSTALL_RUNNING" ]; then
exit 0
fi
export SUPERSET_POSTINSTALL_RUNNING=1
# Run sherif for workspace validation
sherif
# Install native dependencies for desktop app
bun run --filter=@superset/desktop install:deps
🤖 Prompt for AI Agents 
In `@scripts/postinstall.sh` around lines 1 - 16, Enable strict shell error
handling at the top of the script so failures in sherif or the bun command abort
the postinstall: add POSIX-safe strict mode (set -euo pipefail) before any
commands that can fail, keep the SUPERSET_POSTINSTALL_RUNNING guard and export
as-is, and ensure sherif and the bun install line run under that strict mode so
any non-zero exit will terminate the script immediately.