Skip to content

fix(sf-core-installer): make postinstall binary download non-fatal#13660

Open
lobowarren0 wants to merge 1 commit into
serverless:mainfrom
lobowarren0:fix/installer-best-effort-postinstall
Open

fix(sf-core-installer): make postinstall binary download non-fatal#13660
lobowarren0 wants to merge 1 commit into
serverless:mainfrom
lobowarren0:fix/installer-best-effort-postinstall

Conversation

@lobowarren0

@lobowarren0 lobowarren0 commented Jun 22, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown

The postinstall script exits 1 when the installer binary can't be downloaded (e.g. behind a corporate proxy/firewall), which aborts the entire npm install and forces users to run npm install --ignore-scripts.

Since the binary is already fetched lazily on first use in run(), the postinstall download is best-effort: it now warns and lets the install succeed, while the runtime path stays strict and still fails if the binary genuinely can't be obtained.

Addresses: obvious bug fix postInstall.js failure should not abort npm install when the binary is recoverable at runtime.

Summary by CodeRabbit

  • Improvements
    • Made the installation process more resilient. If a required component fails to download during installation, the process will now continue and automatically fetch the component when first needed instead of failing immediately.

@lobowarren0
fix(sf-core-installer): make postinstall binary download non-fatal
0cb26a5 
The postinstall script exits 1 when the installer binary can't be
downloaded (e.g. behind a corporate proxy/firewall), which aborts the
entire `npm install` and forces users to run `npm install --ignore-scripts`.

Since the binary is already fetched lazily on first use in run(), the
postinstall download is best-effort: it now warns and lets the install
succeed, while the runtime path stays strict and still fails if the
binary genuinely can't be obtained.
@Mmarzex

Mmarzex commented Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

✅ Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
✅ Open Source Security 0 0 0 0 0 issues
✅ Licenses 0 0 0 0 0 issues
✅ Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@coderabbitai

coderabbitai Bot commented Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6cf7ee3b-77bb-4177-b10a-95d542404e00

📥 Commits

Reviewing files that changed from the base of the PR and between fb7ac39 and 0cb26a5.

📒 Files selected for processing (1)
  • packages/sf-core-installer/binary.js
📝 Walkthrough

Walkthrough

Binary.install gains a bestEffort parameter (default false). On download failure, the partial binary is always removed; if bestEffort is true, a warning is logged and the method resolves silently instead of calling error(). The top-level install() now passes true for both silent and bestEffort.

Changes

Best-effort binary download on install failure

Layer / File(s) Summary
bestEffort parameter, error handling, and top-level wiring
packages/sf-core-installer/binary.js		 Binary.install signature is extended with bestEffort (default false). The .catch branch removes the partial binary unconditionally, then either logs a warning and resolves (when bestEffort is true) or calls error() to fail (when false). The module-level install() is updated to call binary.install(true, true) to enable this behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐇 Hop, hop, the download may fail,
No need to wail or derail!
With best-effort in place,
We log with good grace —
The binary arrives without fail on first trail! 🌟

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: making the postinstall binary download non-fatal by implementing best-effort behavior instead of failing the entire npm install process.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@lobowarren0 @Mmarzex