From 5ce1eede6931290e30b4f31218bff90e05c8ff1c Mon Sep 17 00:00:00 2001 From: JasonBuildAI Date: Sun, 11 Oct 2026 20:13:08 +0800 Subject: [PATCH 1/2] ci: bound each Electron install attempt so a stalled download is retried A stalled Electron download neither exits nor emits the structured context the retry wrapper classifies, so the wrapper added in #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 #6063. Generated-by: Codex --- scripts/install-electron-with-retry.mjs | 50 +++++++++++++------ scripts/install-electron-with-retry.test.mjs | 51 +++++++++++++++++++- 2 files changed, 85 insertions(+), 16 deletions(-) diff --git a/scripts/install-electron-with-retry.mjs b/scripts/install-electron-with-retry.mjs index d729c084aa7..b849c52153f 100644 --- a/scripts/install-electron-with-retry.mjs +++ b/scripts/install-electron-with-retry.mjs @@ -26,6 +26,9 @@ import { fileURLToPath } from 'node:url'; const DEFAULT_MAX_ATTEMPTS = 5; const DEFAULT_BASE_DELAY_MS = 2_000; +// A stalled download never errors, so the retry classifier cannot see it; capping +// each attempt keeps the install bounded. MAKA_ELECTRON_INSTALL_ATTEMPT_TIMEOUT_MS raises it. +const DEFAULT_ATTEMPT_TIMEOUT_MS = 10 * 60_000; const transientCodes = new Set([ 'ECONNRESET', 'EAI_AGAIN', @@ -37,6 +40,11 @@ const transientCodes = new Set([ ]); const transientHttpStatuses = new Set([408, 425, 429, 500, 502, 503, 504]); +export function resolveAttemptTimeoutMs(env = process.env) { + const configured = Number.parseInt(env.MAKA_ELECTRON_INSTALL_ATTEMPT_TIMEOUT_MS ?? '', 10); + return Number.isFinite(configured) && configured > 0 ? configured : DEFAULT_ATTEMPT_TIMEOUT_MS; +} + export function isTransientElectronInstallFailure(contextOutput) { return contextOutput.split(/\r?\n/).some((line) => { if (!line) return false; @@ -63,13 +71,16 @@ export async function installWithRetry( ) { for (let attempt = 1; attempt <= maxAttempts; attempt += 1) { const result = await runAttempt(attempt); - if ( - result.status === 0 || - typeof result.status !== 'number' || - result.signal || - result.spawnError || - !isTransientElectronInstallFailure(result.context ?? '') - ) { + if (result.status === 0) { + return result; + } + const retryable = + result.timedOut === true || + (typeof result.status === 'number' && + !result.signal && + !result.spawnError && + isTransientElectronInstallFailure(result.context ?? '')); + if (!retryable) { return result; } if (attempt === maxAttempts) { @@ -78,7 +89,9 @@ export async function installWithRetry( const delayMs = baseDelayMs * 2 ** (attempt - 1); warn( - `[install-electron] transient download failure on attempt ${attempt}/${maxAttempts}; retrying in ${delayMs}ms`, + result.timedOut === true + ? `[install-electron] attempt ${attempt}/${maxAttempts} timed out; retrying in ${delayMs}ms` + : `[install-electron] transient download failure on attempt ${attempt}/${maxAttempts}; retrying in ${delayMs}ms`, ); await wait(delayMs); } @@ -103,17 +116,18 @@ async function main() { const installer = require.resolve('electron/install.js'); const launcher = fileURLToPath(new URL('./run-electron-installer.cjs', import.meta.url)); const repositoryRoot = dirname(dirname(fileURLToPath(import.meta.url))); + const attemptTimeoutMs = resolveAttemptTimeoutMs(); const result = await installWithRetry(() => { - const child = spawnSync( - process.execPath, - [launcher, installer], - electronInstallerSpawnOptions(repositoryRoot), - ); + const child = spawnSync(process.execPath, [launcher, installer], { + ...electronInstallerSpawnOptions(repositoryRoot), + timeout: attemptTimeoutMs, + }); + const timedOut = child.error?.code === 'ETIMEDOUT'; if (child.stderr) { process.stderr.write(child.stderr); } - if (child.error) { + if (child.error && !timedOut) { const message = child.error.stack ?? child.error.message; process.stderr.write(`${message}\n`); return { @@ -124,15 +138,21 @@ async function main() { stderr: `${child.stderr ?? ''}\n${message}`, }; } + if (timedOut) { + process.stderr.write( + `[install-electron] attempt aborted after ${attemptTimeoutMs}ms without completing the download\n`, + ); + } return { status: child.status, signal: child.signal, + timedOut, context: child.output?.[3] ?? '', stderr: child.stderr ?? '', }; }); - if (result.signal) { + if (result.signal && result.timedOut !== true) { process.kill(process.pid, result.signal); } else { process.exitCode = result.status ?? 1; diff --git a/scripts/install-electron-with-retry.test.mjs b/scripts/install-electron-with-retry.test.mjs index 5692d757a17..120c3fa1932 100644 --- a/scripts/install-electron-with-retry.test.mjs +++ b/scripts/install-electron-with-retry.test.mjs @@ -20,7 +20,7 @@ import assert from 'node:assert/strict'; import test from 'node:test'; -import { installWithRetry } from './install-electron-with-retry.mjs'; +import { installWithRetry, resolveAttemptTimeoutMs } from './install-electron-with-retry.mjs'; test('waits through a sustained transient Electron download outage', async () => { const attempts = []; @@ -43,3 +43,52 @@ test('waits through a sustained transient Electron download outage', async () => assert.deepEqual(attempts, [1, 2, 3, 4, 5]); assert.deepEqual(delays, [2_000, 4_000, 8_000, 16_000]); }); + +test('retries an attempt that exceeded the per-attempt timeout', async () => { + const attempts = []; + const delays = []; + + const result = await installWithRetry( + async (attempt) => { + attempts.push(attempt); + return attempt < 3 + ? { status: null, signal: 'SIGTERM', timedOut: true, context: '' } + : { status: 0, context: '' }; + }, + { + wait: async (delay) => delays.push(delay), + warn: () => {}, + }, + ); + + assert.equal(result.status, 0); + assert.deepEqual(attempts, [1, 2, 3]); + assert.deepEqual(delays, [2_000, 4_000]); +}); + +test('keeps terminal signal termination immediate', async () => { + const attempts = []; + + const result = await installWithRetry( + async (attempt) => { + attempts.push(attempt); + return { status: null, signal: 'SIGINT', context: '' }; + }, + { + wait: async () => assert.fail('a terminal signal must not be retried'), + warn: () => {}, + }, + ); + + assert.equal(result.signal, 'SIGINT'); + assert.deepEqual(attempts, [1]); +}); + +test('resolves the per-attempt timeout from the environment', () => { + const variable = 'MAKA_ELECTRON_INSTALL_ATTEMPT_TIMEOUT_MS'; + + assert.equal(resolveAttemptTimeoutMs({}), 600_000); + assert.equal(resolveAttemptTimeoutMs({ [variable]: '15000' }), 15_000); + assert.equal(resolveAttemptTimeoutMs({ [variable]: '0' }), 600_000); + assert.equal(resolveAttemptTimeoutMs({ [variable]: 'soon' }), 600_000); +}); From ba394d7eb0b788137ebf06d1629ab9b061e2f4ec Mon Sep 17 00:00:00 2001 From: JasonBuildAI Date: Sun, 11 Oct 2026 20:37:56 +0800 Subject: [PATCH 2/2] ci: cover the Electron installer timeout abort with a spawn test 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 --- scripts/install-electron-with-retry.mjs | 85 ++++++++++++-------- scripts/install-electron-with-retry.test.mjs | 35 +++++++- 2 files changed, 85 insertions(+), 35 deletions(-) diff --git a/scripts/install-electron-with-retry.mjs b/scripts/install-electron-with-retry.mjs index b849c52153f..13506d4a09c 100644 --- a/scripts/install-electron-with-retry.mjs +++ b/scripts/install-electron-with-retry.mjs @@ -27,7 +27,8 @@ import { fileURLToPath } from 'node:url'; const DEFAULT_MAX_ATTEMPTS = 5; const DEFAULT_BASE_DELAY_MS = 2_000; // A stalled download never errors, so the retry classifier cannot see it; capping -// each attempt keeps the install bounded. MAKA_ELECTRON_INSTALL_ATTEMPT_TIMEOUT_MS raises it. +// each attempt keeps the install bounded (ten minutes covers the ~143 MB artifact +// above ~2 Mbps). MAKA_ELECTRON_INSTALL_ATTEMPT_TIMEOUT_MS raises the cap. const DEFAULT_ATTEMPT_TIMEOUT_MS = 10 * 60_000; const transientCodes = new Set([ 'ECONNRESET', @@ -107,6 +108,47 @@ export function electronInstallerSpawnOptions(repositoryRoot) { }; } +export function runInstallerAttempt({ + installerPath, + launcherPath, + repositoryRoot, + timeoutMs, + spawn = spawnSync, + report = (text) => process.stderr.write(text), +}) { + const child = spawn(process.execPath, [launcherPath, installerPath], { + ...electronInstallerSpawnOptions(repositoryRoot), + timeout: timeoutMs, + }); + const timedOut = child.error?.code === 'ETIMEDOUT'; + if (child.stderr) { + report(child.stderr); + } + if (child.error && !timedOut) { + const message = child.error.stack ?? child.error.message; + report(`${message}\n`); + return { + status: child.status, + signal: child.signal, + spawnError: true, + context: child.output?.[3] ?? '', + stderr: `${child.stderr ?? ''}\n${message}`, + }; + } + if (timedOut) { + report( + `[install-electron] attempt aborted after ${timeoutMs}ms without completing the download\n`, + ); + } + return { + status: child.status, + signal: child.signal, + timedOut, + context: child.output?.[3] ?? '', + stderr: child.stderr ?? '', + }; +} + async function main() { // electron is declared by apps/desktop, not the workspace root, so anchor // resolution at that package. Resolving from this script (under scripts/) @@ -118,39 +160,14 @@ async function main() { const repositoryRoot = dirname(dirname(fileURLToPath(import.meta.url))); const attemptTimeoutMs = resolveAttemptTimeoutMs(); - const result = await installWithRetry(() => { - const child = spawnSync(process.execPath, [launcher, installer], { - ...electronInstallerSpawnOptions(repositoryRoot), - timeout: attemptTimeoutMs, - }); - const timedOut = child.error?.code === 'ETIMEDOUT'; - if (child.stderr) { - process.stderr.write(child.stderr); - } - if (child.error && !timedOut) { - const message = child.error.stack ?? child.error.message; - process.stderr.write(`${message}\n`); - return { - status: child.status, - signal: child.signal, - spawnError: true, - context: child.output?.[3] ?? '', - stderr: `${child.stderr ?? ''}\n${message}`, - }; - } - if (timedOut) { - process.stderr.write( - `[install-electron] attempt aborted after ${attemptTimeoutMs}ms without completing the download\n`, - ); - } - return { - status: child.status, - signal: child.signal, - timedOut, - context: child.output?.[3] ?? '', - stderr: child.stderr ?? '', - }; - }); + const result = await installWithRetry(() => + runInstallerAttempt({ + installerPath: installer, + launcherPath: launcher, + repositoryRoot, + timeoutMs: attemptTimeoutMs, + }), + ); if (result.signal && result.timedOut !== true) { process.kill(process.pid, result.signal); diff --git a/scripts/install-electron-with-retry.test.mjs b/scripts/install-electron-with-retry.test.mjs index 120c3fa1932..284f6d91702 100644 --- a/scripts/install-electron-with-retry.test.mjs +++ b/scripts/install-electron-with-retry.test.mjs @@ -18,9 +18,17 @@ */ import assert from 'node:assert/strict'; +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; import test from 'node:test'; +import { fileURLToPath } from 'node:url'; -import { installWithRetry, resolveAttemptTimeoutMs } from './install-electron-with-retry.mjs'; +import { + installWithRetry, + resolveAttemptTimeoutMs, + runInstallerAttempt, +} from './install-electron-with-retry.mjs'; test('waits through a sustained transient Electron download outage', async () => { const attempts = []; @@ -92,3 +100,28 @@ test('resolves the per-attempt timeout from the environment', () => { assert.equal(resolveAttemptTimeoutMs({ [variable]: '0' }), 600_000); assert.equal(resolveAttemptTimeoutMs({ [variable]: 'soon' }), 600_000); }); + +test('aborts an installer attempt that outlives the timeout', async () => { + const directory = await mkdtemp(join(tmpdir(), 'maka-electron-attempt-')); + const installerPath = join(directory, 'stall-installer.cjs'); + await writeFile(installerPath, 'setInterval(() => {}, 60_000);\n'); + const reports = []; + + try { + const result = runInstallerAttempt({ + installerPath, + launcherPath: fileURLToPath(new URL('./run-electron-installer.cjs', import.meta.url)), + repositoryRoot: process.cwd(), + timeoutMs: 2_000, + report: (text) => reports.push(text), + }); + + assert.equal(result.timedOut, true); + assert.equal(result.status, null); + assert.equal(result.signal, 'SIGTERM'); + assert.equal(result.spawnError, undefined); + assert.match(reports.join(''), /attempt aborted after 2000ms/); + } finally { + await rm(directory, { recursive: true, force: true }); + } +});