Skip to content

ci: bound each Electron install attempt so a stalled download is retried - #6064

Open
JasonBuildAI wants to merge 2 commits into
apache:mainfrom
JasonBuildAI:fix/electron-install-stall-7q2x
Open

JasonBuildAI wants to merge 2 commits into
apache:mainfrom
JasonBuildAI:fix/electron-install-stall-7q2x

Conversation

@JasonBuildAI

@JasonBuildAI JasonBuildAI commented Oct 11, 2026 •

Copy link
Copy Markdown

Summary

Bound every Electron installer attempt (10 minutes by default, MAKA_ELECTRON_INSTALL_ATTEMPT_TIMEOUT_MS to override) and treat a timed-out attempt as retryable, so npm ci / npm install can no longer hang forever when the release CDN stops progressing mid-download. scripts/install-electron-with-retry.mjs previously only retried failures that exited with a structured context line; a stalled download produces neither an exit nor a context line, so the retry wrapper added in #2594 never fired and spawnSync blocked indefinitely. Installations on such networks now get the existing five-attempt backoff, and if every attempt stalls the install fails with a diagnostic instead of hanging.

Fixes #6063

Verification

  • Failing-before / passing-after repro (node --test against the wrapper with a timed-out attempt): before the change the wrapper returned the killed attempt immediately (AssertionError: null !== 0); after the change it retries and the attempt succeeds.
  • node --test scripts/install-electron-with-retry.test.mjs — 5/5 pass. Besides the retry classification, the suite now aborts a real installer process that outlives a 2 s cap and asserts the reported result (timedOut: true, status: null, signal: SIGTERM), and guards that terminal signals (e.g. Ctrl+C) stay immediate.
  • End-to-end with a local HTTP server that trickles a few bytes forever (no external network involved): before the change the wrapper sat silent past 120 s with the download stalled and had to be killed externally; with the change and the timeout lowered to 15 s for the demo it aborted each attempt, retried with backoff, and exited 1 after 105.6 s:
[install-electron] attempt aborted after 15000ms without completing the download
[install-electron] attempt 1/5 timed out; retrying in 2000ms
...
[install-electron] attempt 4/5 timed out; retrying in 16000ms
[BOUNDED] exited code=1 signal=null after 105.6s
  • Healthy path unchanged: a cold-cache download and extract of the real 143.2 MB artifact from a mirror exits 0.
  • Machine checks: npm run lint, npm run format:check, npm run build, npm run typecheck pass. The CI-planner step (node --test --test-concurrency=1 scripts/ci-test-plan.test.mjs scripts/ci-workflow-policy.test.mjs scripts/verify-windows-harness.test.mjs scripts/run-workspace-tests-parallel.test.mjs scripts/install-electron-with-retry.test.mjs) reports 140 tests: 139 pass, 0 fail, 1 skipped. The skip needs a real bash; the WSL relay on this host fails that test on unmodified main as well.

Known tradeoffs

  • The cap is wall-clock, not progress-aware: a slow-but-progressing download below roughly 2 Mbps would now fail after the five attempts instead of eventually completing. The default covers the 143 MB artifact comfortably above that, and MAKA_ELECTRON_INSTALL_ATTEMPT_TIMEOUT_MS raises the cap.
  • A killed attempt leaves its partial electron-download-* directory in the OS temp directory; the artifact cache itself is never polluted (only checksum-verified downloads are moved into it).

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex (OpenAI) drafted the change, its tests, and this description under my direction and review.

Generated-by: Codex

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

A stalled Electron download neither exits nor emits the structured context the
retry wrapper classifies, so the wrapper added in apache#2594 could hang `npm install`
indefinitely on networks where the release CDN stops progressing.

Cap each installer attempt at ten minutes (overridable with
MAKA_ELECTRON_INSTALL_ATTEMPT_TIMEOUT_MS), classify a timed-out attempt as
retryable inside the existing five-attempt backoff, and exit with a diagnostic
instead of re-raising the wrapper's own kill signal.

Fixes apache#6063.

Generated-by: Codex
@github-actions github-actions Bot added the effort/M Under 500 readable lines label Oct 11, 2026
JasonBuildAI added a commit to JasonBuildAI/maka that referenced this pull request Oct 11, 2026
The ETIMEDOUT classification and the aborted attempt's exit behavior only
had manual coverage, so a regression there would silently reintroduce the
unbounded hang. Extract a single attempt into runInstallerAttempt so the
test can abort a real installer process that outlives a 2 s cap.

Generated-by: Codex

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

I reviewed ba394d7eb0b788137ebf06d1629ab9b061e2f4ec and found no verified P0–P2 issue.

The change bounds each installer subprocess, classifies its ETIMEDOUT as retryable, and preserves the existing five-attempt limit and 2/4/8/16-second backoff (scripts/install-electron-with-retry.mjs:72–97,111–151). Exhausting timeout retries exits with failure rather than re-raising the wrapper's own kill signal; ordinary terminal signals and spawn failures remain non-retryable (:169–178). Only the installer script and its tests change: there is no .github permission, workflow-trigger, dependency or protocol change.

All five installer tests and all 140 tests in the affected CI-planner/workflow-policy/Windows-harness/workspace-runner group passed locally, with no skips. Syntax, formatting and diff checks passed. I also ran the complete installer entry point against a local server that continuously trickled an unfinished response, using the real Electron installer and launcher. With a 700 ms cap, it made five requests, timed out five times, used all four backoffs and exited with code 1 after 33.58 seconds. Additional checks covered timeout-then-success and non-retryable failures. No external artifact server was used.

The documented tradeoffs remain: this is a wall-clock limit, not an inactivity limit; sufficiently slow healthy downloads can be interrupted, and aborted attempts leave partial temporary directories. The local merge with main 9a32d423b is clean and retains epoch 217. CI and Windows recovery are still running for this head. I did not test a real artifact download/extraction, run the full monorepo suite, or independently exercise this on Windows or macOS.

This branch has not been deployed

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

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

build: npm install hangs when the Electron download stalls

2 participants