From c256859854db39e241046c59b3be9e2b62025151 Mon Sep 17 00:00:00 2001 From: root Date: Tue, 8 Sep 2026 22:41:56 +0200 Subject: [PATCH] Make a dying E2E run fail loudly instead of exiting 0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A run whose chromium died stopped mid-step and exited with code 0, silently reporting success for checks it never ran: CDP.send() left its promise pending when the socket closed, and an unsettled promise does not keep node alive. Waiting on the server had the mirror problem – a bare fetch keeps its socket open, so a stalled response hung the run forever. Every wait can now time out, the socket rejects what it can no longer answer, and reaching process exit without a summary reports RUN DIED with exit 1. Chromium's stderr and exit signal are kept, so "killed by SIGKILL" is visible rather than guessed at. Co-Authored-By: Claude Opus 5 --- README.md | 2 +- tools/lib/harness.mjs | 105 ++++++++++++++++++++++++++++++++++++++--- tools/lib/projects.mjs | 6 ++- 3 files changed, 104 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 4d52923a..ef6db536 100644 --- a/README.md +++ b/README.md @@ -403,7 +403,7 @@ npm run lint # ESLint, type-aware; --fix for the fixable half npm run format # Prettier over src, tests and tools npm run check # typecheck + lint + format:check + tests, what CI would run npm run build # writes dist/ and regenerates index.html -node tools/seed-e2e.mjs && node tools/e2e.mjs # 258 editor checks, screenshots in /tmp/gm-e2e-*.png +node tools/seed-e2e.mjs && node tools/e2e.mjs # 267 editor checks, screenshots in /tmp/gm-e2e-*.png node tools/seed-jumprun.mjs && node tools/smoke-jumprun.mjs # 24 checks: the example game, played end to end ``` diff --git a/tools/lib/harness.mjs b/tools/lib/harness.mjs index 32038527..81086f5d 100644 --- a/tools/lib/harness.mjs +++ b/tools/lib/harness.mjs @@ -9,12 +9,20 @@ * * The harness always waits for chromium to actually exit (a SwiftShader GPU process kept * rendering at full CPU once when only SIGTERM was sent) and exits with code 1 on failures. + * + * A run that ends without printing its summary exits 1 as well. Every wait here can time out for + * that reason: chromium dies under memory pressure on this machine, and a run that stops halfway + * used to exit 0 – silently reporting success for checks it never ran. */ import { spawn } from 'node:child_process'; import { writeFileSync } from 'node:fs'; const sleep = (ms) => new Promise((r) => setTimeout(r, ms)); +/** Generous: navigation and screenshots under swiftshader are slow, a wedged chromium is forever. */ +const CDP_TIMEOUT_MS = 30_000; +const SERVER_TIMEOUT_MS = 20_000; + /** Structural equality for the small plain values the checks compare (arrays of ids, selections). */ export const same = (a, b) => JSON.stringify(a) === JSON.stringify(b); @@ -50,12 +58,36 @@ class CDP { for (const l of this.listeners) l(msg); } }); + ws.addEventListener('close', () => this.failAll('the CDP socket closed – chromium is gone')); + ws.addEventListener('error', () => this.failAll('the CDP socket errored')); + } + + /** + * Rejects every waiting `send`. Without this a dead socket leaves them pending forever, and + * since an unsettled promise is not a reason for node to stay alive, the run exits mid-step + * with code 0 – a dead run that reports success. + */ + failAll(reason) { + this.dead = reason; + for (const { reject } of this.pending.values()) reject(new Error(reason)); + this.pending.clear(); } send(method, params = {}) { + if (this.dead) return Promise.reject(new Error(`${this.dead} (while sending ${method})`)); const id = ++this.id; this.ws.send(JSON.stringify({ id, method, params })); - return new Promise((resolve, reject) => this.pending.set(id, { resolve, reject })); + return new Promise((resolve, reject) => { + const timer = setTimeout(() => { + this.pending.delete(id); + reject(new Error(`CDP ${method} got no answer within ${CDP_TIMEOUT_MS / 1000}s`)); + }, CDP_TIMEOUT_MS); + const settle = (fn) => (value) => { + clearTimeout(timer); + fn(value); + }; + this.pending.set(id, { resolve: settle(resolve), reject: settle(reject) }); + }); } on(fn) { @@ -66,7 +98,9 @@ class CDP { async function waitForChrome(port) { for (let i = 0; i < 50; i++) { try { - const res = await fetch(`http://127.0.0.1:${port}/json/version`); + const res = await fetch(`http://127.0.0.1:${port}/json/version`, { + signal: AbortSignal.timeout(2000), + }); if (res.ok) return res.json(); } catch { // Chromium has not opened the debugging port yet – retry. @@ -209,7 +243,16 @@ function makeTools({ cdp, projectId, baseUrl, consoleLog, check, shotPrefix }) { // --- server ---------------------------------------------------------------------------------- - const api = async (route) => (await fetch(`${baseUrl}api/index.php?r=${route}`)).json(); + // Timeout, not convenience: a stalled response keeps its socket open, so node stays alive and + // the whole run hangs on this line instead of failing. + const api = async (route) => { + const url = `${baseUrl}api/index.php?r=${route}`; + try { + return await (await fetch(url, { signal: AbortSignal.timeout(SERVER_TIMEOUT_MS) })).json(); + } catch (error) { + throw new Error(`request to ${route} failed: ${error.message}`, { cause: error }); + } + }; const sceneOf = (doc) => doc.project.assets.find((a) => a.type === 'scene'); const exceptions = () => consoleLog.filter((l) => l.startsWith('EXCEPTION')); /** Opens the project and asserts the page came up cleanly with the test hook installed. */ @@ -271,25 +314,70 @@ export async function runChecks(options, body) { `--remote-debugging-port=${port}`, `--window-size=${width},${height}`, '--hide-scrollbars', + // Memory on this machine is tight and a long run is where chromium dies. None of this + // serves a headless check: the stderr was full of GCM sign-in retries before. + '--disable-background-networking', + '--disable-sync', + '--disable-extensions', + '--no-first-run', + '--mute-audio', 'about:blank', ], - { stdio: 'ignore' }, + { stdio: ['ignore', 'ignore', 'pipe'] }, ); const consoleLog = []; let failures = 0; + let checkCount = 0; function check(name, ok, detail = '') { console.log(`${ok ? 'PASS' : 'FAIL'} ${name}${detail ? ' – ' + detail : ''}`); + checkCount++; if (!ok) failures++; } + // Chromium dies on this machine now and then (memory pressure). Its own last words name the + // reason, and a signal here is the difference between "a check failed" and "the browser was + // killed" – questions the check output alone cannot tell apart. + const chromeErr = []; + chrome.stderr.on('data', (b) => chromeErr.push(String(b))); + let chromeDeath = ''; + chrome.once('exit', (code, signal) => { + chromeDeath = signal ? `killed by ${signal}` : `exited with code ${code}`; + }); + + // Last line of defence: reaching the end of the run without printing a summary means the + // process walked out early. Whatever the cause, it must never look like success. + let reportedSummary = false; + process.on('exit', (code) => { + if (reportedSummary) return; + console.log( + `\nRUN DIED after ${checkCount} checks – node exited with code ${code} before the summary` + + `${chromeDeath ? `. chromium ${chromeDeath}` : ''}.`, + ); + if (chromeErr.length) console.log('--- chromium stderr ---\n' + chromeErr.join('').trim().slice(-2000)); + process.exitCode = 1; + }); + try { await waitForChrome(port); const target = await ( - await fetch(`http://127.0.0.1:${port}/json/new?about:blank`, { method: 'PUT' }) + await fetch(`http://127.0.0.1:${port}/json/new?about:blank`, { + method: 'PUT', + signal: AbortSignal.timeout(SERVER_TIMEOUT_MS), + }) ).json(); const ws = new WebSocket(target.webSocketDebuggerUrl); - await new Promise((r) => ws.addEventListener('open', r)); + await new Promise((resolve, reject) => { + const timer = setTimeout(() => reject(new Error('the CDP socket never opened')), 15_000); + ws.addEventListener('open', () => { + clearTimeout(timer); + resolve(); + }); + ws.addEventListener('error', () => { + clearTimeout(timer); + reject(new Error('the CDP socket failed to open')); + }); + }); const cdp = new CDP(ws); await cdp.send('Page.enable'); await cdp.send('Runtime.enable'); @@ -312,6 +400,8 @@ export async function runChecks(options, body) { ws.close(); } catch (error) { console.error(error); + if (chromeDeath) console.error(`chromium ${chromeDeath}`); + if (chromeErr.length) console.error('--- chromium stderr ---\n' + chromeErr.join('').trim().slice(-2000)); failures++; } finally { await new Promise((resolve) => { @@ -325,6 +415,7 @@ export async function runChecks(options, body) { }); } if (consoleLog.length) console.log('--- console ---\n' + consoleLog.slice(0, 20).join('\n')); - console.log(failures ? `${failures} FAILED` : 'ALL PASSED'); + reportedSummary = true; + console.log(failures ? `${failures} FAILED` : `ALL PASSED – ${checkCount} checks`); process.exit(failures ? 1 : 0); } diff --git a/tools/lib/projects.mjs b/tools/lib/projects.mjs index 187d34cd..709fe120 100644 --- a/tools/lib/projects.mjs +++ b/tools/lib/projects.mjs @@ -33,7 +33,10 @@ export function cliArgs(script, defaultId) { /** Overwrites a project, keeping the name it already has on the server. */ export async function writeProject(projectId, baseUrl, project) { const url = `${baseUrl}api/index.php?r=projects/${projectId}`; - const current = await fetch(url); + // A request without a timeout keeps its socket open, so a stalled server hangs the seed + // instead of failing it – and the run that follows never starts. + const timeout = () => AbortSignal.timeout(20_000); + const current = await fetch(url, { signal: timeout() }); if (!current.ok) { console.error(`GET failed: ${current.status} – create the project first (POST ?r=projects)`); process.exit(1); @@ -47,6 +50,7 @@ export async function writeProject(projectId, baseUrl, project) { method: 'PUT', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body), + signal: timeout(), }); const result = await res.json(); if (!res.ok) {