diff --git a/src/lib/screenshot/fixtures/renderer-crash-child.mjs b/src/lib/screenshot/fixtures/renderer-crash-child.mjs new file mode 100644 index 00000000..e4638c5b --- /dev/null +++ b/src/lib/screenshot/fixtures/renderer-crash-child.mjs @@ -0,0 +1,77 @@ +import assert from 'node:assert/strict'; +import { createServer } from 'node:http'; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { createRequire } from 'node:module'; +import { captureScreenshots } from '../screenshotter.ts'; + +const persistent = process.argv[2] === 'persistent'; +const server = createServer((request, response) => { + if (request.url === '/pending') { + response.writeHead(200, { 'Content-Type': 'application/octet-stream' }); + response.write('unfinished body'); + request.on('close', () => response.destroy()); + return; + } + response.end(`Neutral fixture
Fixture header

${request.url === '/healthy' ? 'Healthy later route' : 'Crash route'}

`); +}); +await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); +const origin = `http://127.0.0.1:${server.address().port}`; +mkdirSync('.tmp-test', { recursive: true }); +const outputDir = mkdtempSync(join(process.cwd(), '.tmp-test/renderer-crash-')); +let crashes = 0; +let attempts = 0; +let browser; +const pending = []; +const cleanup = []; +const logs = []; +try { + const result = await captureScreenshots({ + urls: [origin + '/', origin + '/healthy'], primaryUrl: origin + '/', outputDir, + concurrency: 1, settleMs: 0, evaluateTimeoutMs: 1000, captureImages: true, + viewports: [{ id: 'desktop', width: 800, height: 600 }], + server: { sendLoggingMessage: message => logs.push(message.data) }, + observeSource: async page => { + browser = page.context().browser(); + if (page.url() !== origin + '/') return; + attempts++; + if (!persistent && attempts > 1) return; + const cdp = await page.context().newCDPSession(page); + const response = page.waitForResponse(origin + '/pending'); + pending.push(page.evaluate(() => fetch('/pending').then(r => r.text())).then(() => 'resolved', e => e.message)); + pending.push((await response).body().then(() => 'resolved', e => e.message)); + pending.push(page.evaluate(() => { + window.pendingEvaluationStarted = true; + return new Promise(() => {}); + }).then(() => 'resolved', e => e.message)); + await page.waitForFunction(() => window.pendingEvaluationStarted); + const crash = new Promise(resolve => page.once('crash', resolve)); + pending.push(cdp.send('Page.crash').then(() => 'resolved', e => e.message)); + await crash; + crashes++; + assert.equal(browser.isConnected(), true, 'only the renderer must fail'); + }, + onProgress: () => cleanup.push(browser.contexts().length), + }); + const manifest = JSON.parse(readFileSync(result.manifestPath, 'utf8')); + const failurePath = join(outputDir, 'screenshots/failures.json'); + const failures = existsSync(failurePath) ? JSON.parse(readFileSync(failurePath, 'utf8')) : []; + const healthy = manifest.entries[origin + '/healthy']; + assert.match(readFileSync(join(outputDir, healthy.html), 'utf8'), /Healthy later route/); + assert.ok(healthy.desktop, 'later route screenshot must exist'); + const pendingOutcomes = await Promise.all(pending); + assert.ok(pendingOutcomes.every(message => /crashed|closed/i.test(message)), 'all pending work must reject and settle'); + assert.deepEqual(cleanup, [0, 0], 'every route must close its contexts'); + assert.equal(browser.isConnected(), false, 'capture must close its browser'); + const require = createRequire(import.meta.url); + console.log(JSON.stringify({ + node: process.version, platform: process.platform, playwright: require('playwright/package.json').version, + persistent, attempts, crashes, result, failures, cleanup, pendingOutcomes, logs, + entry: manifest.entries[origin + '/'], healthy, + })); +} finally { + await browser?.close().catch(() => {}); + server.closeAllConnections(); + await new Promise(resolve => server.close(resolve)); + rmSync(outputDir, { recursive: true, force: true }); +} diff --git a/src/lib/screenshot/screenshotter-interactions.test.ts b/src/lib/screenshot/screenshotter-interactions.test.ts index 5fcd943b..85a19220 100644 --- a/src/lib/screenshot/screenshotter-interactions.test.ts +++ b/src/lib/screenshot/screenshotter-interactions.test.ts @@ -36,6 +36,7 @@ function makeHarvestPage() { function makePage() { let currentUrl = ''; return { + once: vi.fn(), goto: vi.fn().mockImplementation( async ( url: string ) => { currentUrl = url; return { status: () => 200 }; diff --git a/src/lib/screenshot/screenshotter-renderer-crash.test.ts b/src/lib/screenshot/screenshotter-renderer-crash.test.ts new file mode 100644 index 00000000..813be463 --- /dev/null +++ b/src/lib/screenshot/screenshotter-renderer-crash.test.ts @@ -0,0 +1,40 @@ +import { execFile } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; +import { promisify } from 'node:util'; +import { describe, expect, it } from 'vitest'; + +const run = promisify( execFile ); +const fixture = fileURLToPath( new URL( './fixtures/renderer-crash-child.mjs', import.meta.url ) ); + +async function capture( mode: string ) { + // A fatal protocol assertion must fail the child, not disappear into Vitest's + // unhandled-error reporting. Exercise real Chromium and the capture pipeline. + const { stdout, stderr } = await run( process.execPath, [ '--import', 'tsx', fixture, mode ], { + timeout: 60_000, + maxBuffer: 1024 * 1024, + } ); + expect( stderr ).not.toMatch( /Assertion|uncaught|chrome-fidelity.*skipped/ ); + return JSON.parse( stdout.trim().split( '\n' ).at( -1 )! ); +} + +describe( 'renderer crash recovery in a child process', () => { + it( 'retries once in a fresh context without restarting a connected browser', async () => { + const evidence = await capture( 'recover' ); + expect( evidence.attempts ).toBe( 2 ); + expect( evidence.crashes ).toBe( 1 ); + expect( evidence.result ).toMatchObject( { captured: 2, failed: 0, browserRestarts: 0 } ); + expect( evidence.failures ).toEqual( [] ); + expect( evidence.entry ).toMatchObject( { html: 'html/homepage.html', desktop: 'screenshots/desktop/homepage.png' } ); + expect( evidence.logs.filter( ( log: string ) => log.startsWith( '[retry]' ) ) ).toHaveLength( 1 ); + }, 65_000 ); + + it( 'keeps a repeated crash failed with attempt two and captures the later route', async () => { + const evidence = await capture( 'persistent' ); + expect( evidence.attempts ).toBe( 2 ); + expect( evidence.crashes ).toBe( 2 ); + expect( evidence.result ).toMatchObject( { captured: 1, failed: 1, browserRestarts: 0 } ); + expect( evidence.failures ).toMatchObject( [ { stage: 'evaluate', error: 'source renderer crashed', attempt: 2 } ] ); + expect( evidence.entry.html ).toBeUndefined(); + expect( evidence.logs.filter( ( log: string ) => log.startsWith( '[retry]' ) ) ).toHaveLength( 1 ); + }, 65_000 ); +} ); diff --git a/src/lib/screenshot/screenshotter-resource-capture.test.ts b/src/lib/screenshot/screenshotter-resource-capture.test.ts index 2d18be15..9d4b6ef3 100644 --- a/src/lib/screenshot/screenshotter-resource-capture.test.ts +++ b/src/lib/screenshot/screenshotter-resource-capture.test.ts @@ -29,6 +29,7 @@ function makePage( mobile: boolean, routedRequest?: object ) { let routeHandler: ( route: object ) => Promise< void >; let currentUrl = ''; return { + once: vi.fn(), unroute: vi.fn().mockResolvedValue( undefined ), route: vi.fn().mockImplementation( async ( _pattern, handler ) => { routeHandler = handler; diff --git a/src/lib/screenshot/screenshotter.test.ts b/src/lib/screenshot/screenshotter.test.ts index 5e49e9be..acee0491 100644 --- a/src/lib/screenshot/screenshotter.test.ts +++ b/src/lib/screenshot/screenshotter.test.ts @@ -49,6 +49,7 @@ function makeGoodPage(gotoStatus: number | ((url: string) => number) = 200) { let currentUrl = ''; const statusOf = (url: string) => typeof gotoStatus === 'function' ? gotoStatus(url) : gotoStatus; return { + once: vi.fn(), goto: vi.fn().mockImplementation(async (url: string) => { currentUrl = url; return { status: () => statusOf(currentUrl) }; diff --git a/src/lib/screenshot/screenshotter.ts b/src/lib/screenshot/screenshotter.ts index 009edf31..45d5718a 100644 --- a/src/lib/screenshot/screenshotter.ts +++ b/src/lib/screenshot/screenshotter.ts @@ -171,6 +171,8 @@ interface DesignCaptureContext { interface CapturePerViewportArgs { page: Page; + /** Stop best-effort stages after a crash; recovery belongs to the viewport loop. */ + rendererCrashed: () => boolean; learnFluid?: boolean; fluidWidths?: number[]; collectResponsiveImages?: ( @@ -959,6 +961,7 @@ async function capturePerViewport( args: CapturePerViewportArgs ): Promise< void return; } await args.observeSource?.( page, url, isDesktop ? 'desktop' : 'mobile', sourceErrors, args.browserProfile ); + if ( args.rendererCrashed() ) return; if ( plan.captureHtml || plan.captureMobileHtml ) { try { const native = await captureNativeViewTimelines( page, viewport.id, evaluateTimeoutMs ); @@ -1006,6 +1009,7 @@ async function capturePerViewport( args: CapturePerViewportArgs ): Promise< void } ); } } + if ( args.rendererCrashed() ) return; await applyPagerSlideshowStates( page, pagerSlideshows ).catch( () => { /* best-effort — a picker that will not advance must not block capture */ @@ -1019,6 +1023,7 @@ async function capturePerViewport( args: CapturePerViewportArgs ): Promise< void /* best-effort — never block capture on a late platform widget */ } ); } + if ( args.rendererCrashed() ) return; // Hydrated panels belong to the serialization transaction. Browser probes // and width learning can rerender their source items, discarding injected @@ -1054,6 +1059,7 @@ async function capturePerViewport( args: CapturePerViewportArgs ): Promise< void } } + if ( args.rendererCrashed() ) return; if ( isDesktop && plan.captureHtml ) { try { // The cleanup observer may have exhausted its budget before the page @@ -1115,6 +1121,7 @@ async function capturePerViewport( args: CapturePerViewportArgs ): Promise< void } ); } } + if ( args.rendererCrashed() ) return; // --- mobile-DOM carry (mobile only) --------------------------------------- // On the mobile pass, the mobile UA + isMobile emulation make JS builders like @@ -1180,6 +1187,7 @@ async function capturePerViewport( args: CapturePerViewportArgs ): Promise< void } // --- scrolled screenshot -------------------------------------------------- + if ( args.rendererCrashed() ) return; if ( plan.captureScrolled ) { try { const docHeight = await page.evaluate( () => document.documentElement.scrollHeight ); @@ -1223,6 +1231,7 @@ async function capturePerViewport( args: CapturePerViewportArgs ): Promise< void } // --- desktop-only site analysis ------------------------------------------- + if ( args.rendererCrashed() ) return; if ( isDesktop && shouldAnalyze ) { try { const analysis = await analyzePage( page, evaluateTimeoutMs ); @@ -1238,6 +1247,7 @@ async function capturePerViewport( args: CapturePerViewportArgs ): Promise< void attempt: 1, } ); } + if ( args.rendererCrashed() ) return; // Best-effort: capture source chrome computed-style fingerprint for later // carry-vs-source fidelity audits. A failure here MUST NOT break the // screenshot run — the try/catch ensures this is never propagated. @@ -1261,6 +1271,7 @@ async function capturePerViewport( args: CapturePerViewportArgs ): Promise< void } // --- desktop-only design capture (page/post archetypes only) --------------- + if ( args.rendererCrashed() ) return; if ( isDesktop && designCtx ) { try { // The design sidecar slug MUST match the WXR item slug used by adapters @@ -1369,6 +1380,7 @@ async function capturePerViewport( args: CapturePerViewportArgs ): Promise< void // HTML. Each viewport needs its own probe: a desktop dialog must not suppress // a mobile-only trigger. Merge their bounded successful evidence rather than // replacing a desktop-only dialog with a mobile-only menu. + if ( args.rendererCrashed() ) return; const releaseNavigationLock = await lockMainFrameNavigation( page ); try { await probeInteractions(); @@ -1841,13 +1853,13 @@ export async function captureScreenshots( opts: ScreenshotOpts ): Promise< Scree const vpPlan = viewport.id === 'desktop' ? effectivePlan.desktop : effectivePlan.mobile; if ( ! vpPlan.needsLoad ) continue; - // A browser that died under this viewport is replaced and the viewport - // retried once, so a crash costs the viewports in flight a retry rather - // than failing every route the pool claims afterwards. + // Retry a crashed renderer in a fresh context once. A disconnected browser + // also needs the shared relaunch before the retry. for ( let crashRetry = false; ; crashRetry = true ) { const attemptBrowser = browser; const failuresBefore = urlFailures.length; let context: BrowserContext | undefined; + let rendererCrashed = false; try { // deviceScaleFactor < 1 reduces the OUTPUT pixel count of every // screenshot while keeping the rendered layout identical to a @@ -1889,8 +1901,10 @@ export async function captureScreenshots( opts: ScreenshotOpts ): Promise< Scree ` ); await context.addInitScript( observeViewportEntrances ); const page = await context.newPage(); + page.once( 'crash', () => { rendererCrashed = true; } ); await capturePerViewport( { page, + rendererCrashed: () => rendererCrashed, browserProfile: { isMobile: contextOptions.isMobile ?? false, hasTouch: contextOptions.hasTouch ?? false }, viewport, plan: vpPlan, @@ -1949,9 +1963,25 @@ export async function captureScreenshots( opts: ScreenshotOpts ): Promise< Scree } } } + if ( rendererCrashed && urlFailures.length === failuresBefore ) { + urlFailures.push( { + url, + viewport: viewport.id, + stage: 'evaluate', + error: 'source renderer crashed', + timestamp: new Date().toISOString(), + attempt: crashRetry ? 2 : 1, + } ); + } + for ( const failure of urlFailures.slice( failuresBefore ) ) { + failure.attempt = crashRetry ? 2 : failure.attempt; + } const failed = urlFailures.length > failuresBefore; - if ( ! failed || crashRetry || attemptBrowser.isConnected() ) break; - if ( ! ( await replaceCrashedBrowser( attemptBrowser ) ) ) break; + if ( ! failed || crashRetry ) break; + if ( attemptBrowser.isConnected() ) { + if ( ! rendererCrashed ) break; + sendLog( server, `[retry] renderer crashed for ${ url } (${ viewport.id }); using a fresh context` ); + } else if ( ! ( await replaceCrashedBrowser( attemptBrowser ) ) ) break; urlFailures.length = failuresBefore; } }