diff --git a/scripts/install-electron-with-retry.mjs b/scripts/install-electron-with-retry.mjs index d729c084aa..13506d4a09 100644 --- a/scripts/install-electron-with-retry.mjs +++ b/scripts/install-electron-with-retry.mjs @@ -26,6 +26,10 @@ 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 (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', 'EAI_AGAIN', @@ -37,6 +41,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 +72,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 +90,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); } @@ -94,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/) @@ -103,36 +158,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), - ); - if (child.stderr) { - process.stderr.write(child.stderr); - } - if (child.error) { - 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}`, - }; - } - return { - status: child.status, - signal: child.signal, - context: child.output?.[3] ?? '', - stderr: child.stderr ?? '', - }; - }); + const result = await installWithRetry(() => + runInstallerAttempt({ + installerPath: installer, + launcherPath: launcher, + repositoryRoot, + timeoutMs: attemptTimeoutMs, + }), + ); - 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 5692d757a1..284f6d9170 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 } 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 = []; @@ -43,3 +51,77 @@ 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); +}); + +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 }); + } +});