diff --git a/.changeset/capture-tolerates-dropped-frames.md b/.changeset/capture-tolerates-dropped-frames.md new file mode 100644 index 0000000..07dda5c --- /dev/null +++ b/.changeset/capture-tolerates-dropped-frames.md @@ -0,0 +1,9 @@ +--- +"@humanjs/playwright": patch +--- + +Keep recording when a single screenshot fails, instead of losing the whole take. + +The capture loop treated any `page.screenshot()` rejection as terminal: it stopped, captured nothing more, and the export then failed with "No frames were captured". A transient `Page.captureScreenshot` protocol error is ordinary on a page under load — a heavy animation or a font swap is enough to cause one — so recording a real site could fail outright with no useful explanation. + +Transient failures now drop a single frame and the loop continues, matching how a failed frame *write* was already handled a few lines away. A closed page still stops immediately, and ten consecutive failures still give up rather than spinning at the frame rate forever. The dropped-frame warning is emitted once per run of failures, not once per frame. diff --git a/packages/playwright/src/recording/capture.test.ts b/packages/playwright/src/recording/capture.test.ts new file mode 100644 index 0000000..208c907 --- /dev/null +++ b/packages/playwright/src/recording/capture.test.ts @@ -0,0 +1,114 @@ +import type { Page } from 'playwright'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { startCapture } from './capture'; + +/** + * A page whose screenshot behaviour is scripted per call, so the loop's + * failure handling can be driven deterministically. + */ +function makePage(behaviour: { + screenshot: (call: number) => Promise; + isClosed?: () => boolean; +}): { page: Page; calls: () => number } { + let call = 0; + const page = { + screenshot: () => { + const n = call++; + return behaviour.screenshot(n); + }, + isClosed: behaviour.isClosed ?? ((): boolean => false), + } as unknown as Page; + return { page, calls: () => call }; +} + +const frame = (): Promise => Promise.resolve(Buffer.from('x')); +const boom = (): Promise => Promise.reject(new Error('Unable to capture screenshot')); + +/** Waits for real time to pass so the capture loop can tick. */ +const tick = (ms: number): Promise => new Promise((r) => setTimeout(r, ms)); + +describe('startCapture failure handling', () => { + beforeEach(() => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('keeps recording after a transient screenshot failure', async () => { + // One bad frame in the middle of a healthy run. Losing the whole take + // over it is the regression this guards. + const { page } = makePage({ + screenshot: (n) => (n === 1 ? boom() : frame()), + }); + const session = await startCapture(page, { fps: 40 }); + await tick(300); + const result = await session.stop(); + + expect(result.frames.length).toBeGreaterThan(1); + }); + + it('reports the dropped frame once rather than at the frame rate', async () => { + const { page } = makePage({ screenshot: (n) => (n === 1 ? boom() : frame()) }); + const session = await startCapture(page, { fps: 40 }); + await tick(300); + await session.stop(); + + const dropped = vi + .mocked(console.warn) + .mock.calls.filter(([msg]) => String(msg).includes('dropping frame')); + expect(dropped).toHaveLength(1); + }); + + it('stops immediately once the page is closed, without warning', async () => { + let closed = false; + const { page } = makePage({ + screenshot: (n) => { + if (n >= 2) { + closed = true; + return boom(); + } + return frame(); + }, + isClosed: () => closed, + }); + const session = await startCapture(page, { fps: 40 }); + await tick(300); + const result = await session.stop(); + + expect(result.frames.length).toBeGreaterThan(0); + // A closed page is an expected end, not a fault worth reporting. + expect(console.warn).not.toHaveBeenCalled(); + }); + + it('gives up after a persistent run of failures instead of spinning forever', async () => { + const { page, calls } = makePage({ screenshot: () => boom() }); + const session = await startCapture(page, { fps: 60 }); + await tick(600); + await session.stop(); + + // Bounded by the consecutive-failure cap, not by how long we waited. + expect(calls()).toBeLessThanOrEqual(12); + expect( + vi + .mocked(console.warn) + .mock.calls.some(([msg]) => String(msg).includes('consecutive screenshot failures')), + ).toBe(true); + }); + + it('resets the failure count on success, so scattered misses never accumulate', async () => { + // Fails every other frame: 20+ failures overall, never 10 in a row. + const { page } = makePage({ screenshot: (n) => (n % 2 === 1 ? boom() : frame()) }); + const session = await startCapture(page, { fps: 60 }); + await tick(600); + const result = await session.stop(); + + expect(result.frames.length).toBeGreaterThan(5); + expect( + vi + .mocked(console.warn) + .mock.calls.some(([msg]) => String(msg).includes('consecutive screenshot failures')), + ).toBe(false); + }); +}); diff --git a/packages/playwright/src/recording/capture.ts b/packages/playwright/src/recording/capture.ts index 1b95cfd..e287db6 100644 --- a/packages/playwright/src/recording/capture.ts +++ b/packages/playwright/src/recording/capture.ts @@ -4,6 +4,14 @@ import { join } from 'node:path'; import { sleep } from '@humanjs/core'; import type { Page } from 'playwright'; +/** + * Consecutive screenshot failures tolerated before the capture loop gives + * up. Transient misses are survivable and common; persistent failure means + * something is actually broken, and spinning at the frame rate forever + * helps nobody. + */ +const MAX_CONSECUTIVE_SCREENSHOT_FAILURES = 10; + /** * A captured frame in the timer-based capture session. `tMs` is the * wall-clock offset (ms) from the capture start. @@ -90,6 +98,8 @@ export async function startCapture( let stopped = false; let frameIndex = 0; + // Reset by every successful screenshot; only an unbroken run counts. + let consecutiveFailures = 0; // Collects every in-flight write so stop()/abort() can wait for them to // settle. Using `allSettled` (in `finish`) means an individual writeFile // failure drops one frame but never blocks the rest of the queue or the @@ -107,6 +117,7 @@ export async function startCapture( quality: format === 'jpeg' ? quality : undefined, }); if (stopped) return; + consecutiveFailures = 0; const idx = frameIndex++; const path = join(dir, `frame_${String(idx).padStart(6, '0')}.${ext}`); const tMs = loopStart - startedAtMs; @@ -124,12 +135,28 @@ export async function startCapture( ), ); } catch (err) { - // Page closed mid-capture, or screenshot otherwise failed. Bail - // cleanly rather than crashing the loop. if (stopped) return; - console.warn('humanjs capture: screenshot failed, stopping loop:', err); - stopped = true; - return; + // The page going away is terminal — there is nothing left to shoot. + if (page.isClosed()) return; + // Anything else is treated the way a failed write is, a few lines + // up: drop the frame, keep the recording. A single + // `Page.captureScreenshot` protocol error is ordinary on a page + // under load — a heavy animation or a font swap is enough — and + // losing the entire take over one missed frame is the wrong trade. + consecutiveFailures += 1; + if (consecutiveFailures >= MAX_CONSECUTIVE_SCREENSHOT_FAILURES) { + console.warn( + `humanjs capture: ${consecutiveFailures} consecutive screenshot failures, stopping loop:`, + err, + ); + stopped = true; + return; + } + // Warned once per run of failures: at the frame rate, a page that + // keeps missing would otherwise bury everything else in the console. + if (consecutiveFailures === 1) { + console.warn('humanjs capture: screenshot failed, dropping frame:', err); + } } const elapsed = Date.now() - loopStart; const wait = intervalMs - elapsed;