From dfa33294282fead15094a3d343ebc0715afaef31 Mon Sep 17 00:00:00 2001 From: Robert DeLuca Date: Wed, 11 Nov 2026 13:58:08 -0600 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Explain=20cancelled=20browser=20?= =?UTF-8?q?operations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Distinguish deadlines from invalidated capture contexts so real browser failures can be diagnosed without hiding races behind retries. Preserve cancellation behavior and redact the private browser connection. --- src/browser-control.js | 35 +++++++++++++++++++++++++---------- src/browser-session.js | 6 +++++- src/browser/shell.js | 10 +++++++--- tests/native/pool.test.js | 2 +- 4 files changed, 38 insertions(+), 15 deletions(-) diff --git a/src/browser-control.js b/src/browser-control.js index dcb94f8..73503b7 100644 --- a/src/browser-control.js +++ b/src/browser-control.js @@ -7,12 +7,9 @@ import { nativeCapabilities, normalizeNative } from './native.js'; import { createObserver } from './observer.js'; let problem = (status, message) => Object.assign(new Error(message), { status }); +let contextFields = Object.freeze(['executionId', 'documentId', 'revision', 'width', 'height']); let sameContext = (left, right) => - left && - right && - ['executionId', 'documentId', 'revision', 'width', 'height'].every( - (key) => left[key] === right[key], - ); + left && right && contextFields.every((key) => left[key] === right[key]); let commands = [ 'pause', 'continue', @@ -207,16 +204,20 @@ export function createBrowserControl({ artifactsDir, workerId = '0', onReplaced throw problem(409, 'This managed browser has moved to another run.'); return connection; }; - let discard = async (record, includeCommitted = false) => { + let discard = async ( + record, + includeCommitted = false, + reason = problem(409, 'Screenshot context ended.'), + ) => { if (record.status === 'accepted') return; - record.controller.abort(problem(409, 'Screenshot context ended.')); + record.controller.abort(reason); if (record.status !== 'committed' || includeCommitted) { records.delete(record.id); await rm(record.partial, { force: true }); await rm(record.file, { force: true }); } }; - let invalidate = (runId, endingRun = false) => { + let invalidate = (runId, endingRun = false, reason) => { contexts.delete(runId); for (let request of nativeRequests.values()) if (request.runId === runId && (endingRun || request.kind !== 'key')) @@ -224,7 +225,7 @@ export function createBrowserControl({ artifactsDir, workerId = '0', onReplaced for (let record of records.values()) { if (record.runId !== runId || record.status === 'accepted') continue; if (record.status === 'committed' && !endingRun) continue; - void track(discard(record, endingRun)).catch(() => {}); + void track(discard(record, endingRun, reason)).catch(() => {}); } }; let active = (record) => @@ -468,7 +469,21 @@ export function createBrowserControl({ artifactsDir, workerId = '0', onReplaced assignment.id === input.context.executionId && assignment.status !== 'released' && !run.tests[assignment.index].skipReason))) throw problem(409, 'This execution does not own an active test assignment.'); let previous = contexts.get(input.runId); - if (!sameContext(previous, input.context)) invalidate(input.runId); + if (!sameContext(previous, input.context)) { + let changes = contextFields + .filter((key) => previous?.[key] !== input.context?.[key]) + .map((key) => `${key}: ${previous?.[key] ?? 'none'} → ${input.context?.[key] ?? 'none'}`); + let describe = (viewport) => + `${viewport?.width} × ${viewport?.height} at DPR ${viewport?.pixelRatio}`; + let viewportChange = input.viewportChange + ? ` Browser viewport: ${describe(input.viewportChange.previous)} → ${describe(input.viewportChange.measured)}.` + : ''; + invalidate( + input.runId, + false, + problem(409, `Screenshot context changed (${changes.join(', ')}).${viewportChange}`), + ); + } if (input.context) { if ( !validViewport(input.context, Number.MAX_SAFE_INTEGER) || diff --git a/src/browser-session.js b/src/browser-session.js index 3e62274..5a0aa91 100644 --- a/src/browser-session.js +++ b/src/browser-session.js @@ -131,7 +131,11 @@ export async function createBrowserPool(plan, { size = 1, signal, onExit = () => catch { /* Channel closure still rejects and removes this request below. */ } } requests.delete(id); - request.reject(new Error(`Browser ${method} cancelled or exceeded its deadline.`, { cause: deadline.reason })); + let reason = deadline.reason; + let message = reason?.name === 'TimeoutError' + ? `Browser ${method} exceeded its deadline.` + : `Browser ${method} cancelled: ${reason?.message ?? String(reason)}`; + request.reject(new Error(message.replaceAll(server.wsEndpoint(), '[private browser connection]'), { cause: reason })); }; deadline.addEventListener('abort', abort, { once: true }); try { diff --git a/src/browser/shell.js b/src/browser/shell.js index c2d0321..c95c09a 100644 --- a/src/browser/shell.js +++ b/src/browser/shell.js @@ -146,9 +146,11 @@ let context = (current) => ...state.viewport, } : null; -let syncContext = (current) => { +let syncContext = (current, viewportChange) => { let value = context(current); - contextSync = contextSync.catch(() => {}).then(() => post('context', { context: value })); + contextSync = contextSync.catch(() => {}).then(() => + post('context', { context: value, ...(viewportChange ? { viewportChange } : {}) }), + ); return contextSync; }; let assertCurrent = (current, documentId, revision, metrics, operation = 'screenshot') => { @@ -418,9 +420,11 @@ window.addEventListener('message', (event) => { if (message.kind === 'resize' && message.executionId === resource?.id) { let measured = { ...appSize(), pixelRatio: appWindow.devicePixelRatio }; if (Object.keys(measured).every((key) => measured[key] === observedViewport[key])) return; + let previous = observedViewport; observedViewport = measured; update({ type: 'browser.resized', viewport: appSize() }); - if (runId) void syncContext(resource).catch((error) => console.error(error)); + if (runId) + void syncContext(resource, { previous, measured }).catch((error) => console.error(error)); } }); window.addEventListener('pagehide', () => { diff --git a/tests/native/pool.test.js b/tests/native/pool.test.js index 88d9d08..cec934e 100644 --- a/tests/native/pool.test.js +++ b/tests/native/pool.test.js @@ -252,7 +252,7 @@ test('cancelling queued viewport work closes the pool before it can be reused', let cancel = new AbortController(); let resizing = workers[0].resize({ width: 400, height: 300 }, cancel.signal); cancel.abort(new Error('Viewport request ended')); - await assert.rejects(resizing, /cancelled/); + await assert.rejects(resizing, /cancelled: Viewport request ended/); await assert.rejects(opening); await assert.rejects(workers[1].open(origin), /closed/); await assert.rejects(pool.createWorker('c'), /closed/);