fix(producer): retry a dead browser tab once on the single-worker capture path (#4162)

* fix(producer): retry a dead browser tab once on the single-worker capture path * fix(producer): keep the caller-owned dedup perf list untouched on sequential retry * test(producer): extend the streaming-stage orchestrator mock with the retry exports
2026-11-01 19:26:52 +00:00 · 2026-09-19 04:27:34 -04:00
parent 0915affe97
commit 73f39862f2
4 changed files with 375 additions and 106 deletions
@@ -1,8 +1,38 @@
-import { describe, expect, it } from "vitest";
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { DEFAULT_CONFIG } from "@hyperframes/engine";
+import { createRenderJob } from "../../renderOrchestrator.js";
+import { formatCaptureFrameName } from "../../../utils/paths.js";
+
+// Mock only the engine session primitives; `classifyCaptureFailure` stays
+// real so the retry decision is exercised.
+const createCaptureSession = vi.fn();
+const initializeSession = vi.fn(async () => {});
+const captureFrame = vi.fn();
+const closeCaptureSession = vi.fn(async () => {});
+const verifyDiskDrawElementSamples = vi.fn(async () => {});
+const getCapturePerfSummary = vi.fn(() => ({}));
+
+vi.mock("@hyperframes/engine", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("@hyperframes/engine")>();
+  return {
+    ...actual,
+    createCaptureSession: (...args: unknown[]) => createCaptureSession(...args),
+    initializeSession: (...args: unknown[]) => initializeSession(...args),
+    captureFrame: (...args: unknown[]) => captureFrame(...args),
+    closeCaptureSession: (...args: unknown[]) => closeCaptureSession(...args),
+    verifyDiskDrawElementSamples: (...args: unknown[]) => verifyDiskDrawElementSamples(...args),
+    getCapturePerfSummary: (...args: unknown[]) => getCapturePerfSummary(...args),
+  };
+});
+
 import {
  assertDiskCaptureHeadroom,
  estimateDiskCaptureBytes,
  inspectDiskCaptureHeadroom,
+  runCaptureStage,
  shouldAllowAdaptiveCaptureRetry,
 } from "./captureStage.js";

@@ -96,3 +126,140 @@ describe("disk capture capacity", () => {
    expect(headroom.available).toBe(false);
  });
 });
+
+describe("runCaptureStage — sequential path transient Target-closed single retry (integration)", () => {
+  afterEach(() => {
+    createCaptureSession.mockReset();
+    initializeSession.mockReset().mockResolvedValue(undefined);
+    captureFrame.mockReset();
+    closeCaptureSession.mockReset().mockResolvedValue(undefined);
+    verifyDiskDrawElementSamples.mockReset().mockResolvedValue(undefined);
+    getCapturePerfSummary.mockReset().mockReturnValue({});
+  });
+
+  it("retries once with a fresh session on a Target-closed and renders the remaining frames", async () => {
+    const framesDir = mkdtempSync(join(tmpdir(), "hf-seq-transient-frames-"));
+    const totalFrames = 4;
+    let sessionCount = 0;
+    createCaptureSession.mockImplementation(async () => {
+      sessionCount++;
+      return {
+        isInitialized: false,
+        browserConsoleBuffer: [],
+        options: { captureBeyondViewport: false, format: "jpeg" },
+        workerEncodeEnabled: false,
+        outputDir: framesDir,
+      };
+    });
+    captureFrame.mockImplementation(async (_session: unknown, frameIndex: number) => {
+      // First session dies on frame 0 (zero forward progress).
+      if (sessionCount === 1) {
+        throw new Error("Protocol error (Page.captureScreenshot): Target closed");
+      }
+      writeFileSync(join(framesDir, formatCaptureFrameName(frameIndex, "jpg")), "captured-frame");
+    });
+    const log = { error: vi.fn(), warn: vi.fn(), info: vi.fn(), debug: vi.fn() };
+
+    try {
+      const job = createRenderJob({ fps: { num: 30, den: 1 }, quality: "draft" });
+      const result = await runCaptureStage({
+        fileServer: { url: "http://localhost:0" } as never,
+        workDir: framesDir,
+        framesDir,
+        job,
+        totalFrames,
+        cfg: DEFAULT_CONFIG,
+        plan: { workerCount: 1, forceScreenshot: false },
+        log: log as never,
+        probeSession: null,
+        captureAttempts: [],
+        dedupPerfs: [],
+        buildCaptureOptions: () => ({
+          width: 64,
+          height: 64,
+          fps: { num: 30, den: 1 },
+          format: "jpeg",
+        }),
+        createRenderVideoFrameInjector: () => null,
+        abortSignal: undefined,
+        assertNotAborted: () => {},
+      });
+
+      expect(createCaptureSession).toHaveBeenCalledTimes(2);
+      expect(captureFrame).toHaveBeenCalledTimes(1 + totalFrames);
+      expect(closeCaptureSession).toHaveBeenCalledTimes(2);
+      expect(result.workerCount).toBe(1);
+      expect(log.warn).toHaveBeenCalledWith(
+        expect.stringContaining("Transient browser failure during sequential capture"),
+        expect.objectContaining({ transientRetriesUsed: 1, resumeFrom: 0 }),
+      );
+    } finally {
+      rmSync(framesDir, { recursive: true, force: true });
+    }
+  });
+
+  it("resumes from the first missing frame inside a frameRange and fails on a second death", async () => {
+    const framesDir = mkdtempSync(join(tmpdir(), "hf-seq-transient-range-"));
+    let sessionCount = 0;
+    createCaptureSession.mockImplementation(async () => {
+      sessionCount++;
+      return {
+        isInitialized: false,
+        browserConsoleBuffer: [],
+        options: { captureBeyondViewport: false, format: "jpeg" },
+        workerEncodeEnabled: false,
+        outputDir: framesDir,
+      };
+    });
+    const times: number[] = [];
+    captureFrame.mockImplementation(async (_s: unknown, frameIndex: number, time: number) => {
+      times.push(time);
+      if (sessionCount === 1 && frameIndex === 2) {
+        throw new Error("Protocol error (Runtime.callFunctionOn): Target closed");
+      }
+      if (sessionCount === 2 && frameIndex === 3) {
+        throw new Error("Protocol error (Runtime.callFunctionOn): Target closed");
+      }
+      writeFileSync(join(framesDir, formatCaptureFrameName(frameIndex, "jpg")), "captured-frame");
+    });
+    const log = { error: vi.fn(), warn: vi.fn(), info: vi.fn(), debug: vi.fn() };
+    const job = createRenderJob({ fps: { num: 10, den: 1 }, quality: "draft" });
+
+    try {
+      await expect(
+        runCaptureStage({
+          fileServer: { url: "http://localhost:0" } as never,
+          workDir: framesDir,
+          framesDir,
+          job,
+          totalFrames: 4,
+          cfg: DEFAULT_CONFIG,
+          plan: { workerCount: 1, forceScreenshot: false },
+          log: log as never,
+          probeSession: null,
+          captureAttempts: [],
+          dedupPerfs: [],
+          buildCaptureOptions: () => ({
+            width: 64,
+            height: 64,
+            fps: { num: 10, den: 1 },
+            format: "jpeg",
+          }),
+          createRenderVideoFrameInjector: () => null,
+          abortSignal: undefined,
+          assertNotAborted: () => {},
+          frameRange: { startFrame: 10, endFrame: 14 },
+        }),
+      ).rejects.toThrow(/Target closed/);
+
+      // Session 1: frames 0,1 ok, 2 dies. Session 2 resumes at 2 (absolute 12 -> 1.2s),
+      // frame 2 ok, 3 dies; no third session.
+      expect(createCaptureSession).toHaveBeenCalledTimes(2);
+      expect(times).toEqual([1.0, 1.1, 1.2, 1.2, 1.3]);
+      expect(log.warn).toHaveBeenCalledTimes(1);
+      expect(closeCaptureSession).toHaveBeenCalledTimes(2);
+    } finally {
+      rmSync(framesDir, { recursive: true, force: true });
+    }
+  });
+});
@@ -5,7 +5,8 @@
 *   - `workerCount > 1`: parallel capture with adaptive retry via
 *     `executeDiskCaptureWithAdaptiveRetry`.
 *   - `workerCount === 1`: sequential capture in the orchestrator process,
- *     reusing `probeSession` when available.
+ *     reusing `probeSession` when available. A transient browser death retries
+ *     with a fresh session resuming from the first missing frame.
 *
 * The HDR layered branch (`useLayeredComposite === true`) and the streaming
 * encode fusion path (`useStreamingEncode === true` with successful encoder
@@ -38,6 +39,7 @@

 import { statfsSync } from "node:fs";
 import {
+  classifyCaptureFailure,
  frameFileExtension,
  type BeforeCaptureHook,
  type CaptureOptions,
@@ -59,6 +61,8 @@ import type { FileServerHandle } from "../../fileServer.js";
 import type { ProducerLogger } from "../../../logger.js";
 import {
  executeDiskCaptureWithAdaptiveRetry,
+  findMissingFrameRanges,
+  isTransientCaptureRetryEligible,
  type CaptureAttemptSummary,
  type ProgressCallback,
  type RenderJob,
@@ -221,7 +225,6 @@ export async function runCaptureStage(input: CaptureStageInput): Promise<Capture
    buildCaptureOptions,
    createRenderVideoFrameInjector,
    abortSignal,
-    assertNotAborted,
    onProgress,
    frameRange,
    dedupPerfs,
@@ -318,8 +321,51 @@ export async function runCaptureStage(input: CaptureStageInput): Promise<Capture
      probeSession = null;
    }
  } else {
-    // Sequential capture
+    const sequential = await runSequentialDiskCapture(input, captureOptions, captureCfg);
+    probeSession = null;
+    lastBrowserConsole = sequential.lastBrowserConsole;
+    captureBeyondViewport = sequential.captureBeyondViewport;
+  }

+  return { workerCount, probeSession, lastBrowserConsole, captureBeyondViewport };
+}
+
+interface SequentialCaptureResult {
+  lastBrowserConsole: string[];
+  captureBeyondViewport: boolean | undefined;
+}
+
+async function runSequentialDiskCapture(
+  input: CaptureStageInput,
+  captureOptions: CaptureOptions,
+  captureCfg: EngineConfig,
+): Promise<SequentialCaptureResult> {
+  const {
+    fileServer,
+    framesDir,
+    totalFrames,
+    log,
+    createRenderVideoFrameInjector,
+    abortSignal,
+    assertNotAborted,
+    frameRange,
+  } = input;
+  let { probeSession } = input;
+  let lastBrowserConsole: string[] = [];
+  let captureBeyondViewport: boolean | undefined;
+  // Sequential capture. Frame TIMES use the absolute composition index;
+  // file NAMES are relative (loop index `i`), so the encoder needs no
+  // `-start_number`.
+  const { rangeStart, rangeEnd } = resolveCaptureRange(frameRange, totalFrames);
+  const rangeFrames = rangeEnd - rangeStart;
+  const frameExt = frameFileExtension(captureOptions.format);
+  // Transient-browser retry: `resumeFrom` moves to the first frame missing
+  // from disk and a fresh session captures the remainder. Capped by
+  // `isTransientCaptureRetryEligible`.
+  let transientRetriesUsed = 0;
+  let resumeFrom = 0;
+
+  while (true) {
    const videoInjector = createRenderVideoFrameInjector();
    const session =
      probeSession ??
@@ -333,109 +379,44 @@ export async function runCaptureStage(input: CaptureStageInput): Promise<Capture
    captureBeyondViewport = session.options.captureBeyondViewport;

    try {
-      // Reuse preparation can fail while creating/resetting the output
-      // directory (for example EACCES, EROFS, or ENOSPC). Keep it inside the
-      // session-owning try/finally so the borrowed probe browser is closed
-      // even when preparation fails before capture starts.
+      // Reuse preparation can fail on the output dir (permissions, read-only, disk full);
+      // keep it inside try/finally so the borrowed probe browser is still closed.
      if (probeSession) {
        prepareCaptureSessionForReuse(session, framesDir, videoInjector);
        probeSession = null;
      }
-      if (!session.isInitialized) {
-        await initializeSession(session);
-      } else if (process.env.PRODUCER_EXPERIMENTAL_FAST_CAPTURE === "true") {
-        // Deferred drawElement init (probe-initialized video comps). The disk
-        // path has no drain-time self-verification, so only an explicit opt-in
-        // completes it here — mirroring the orchestrator clamp that routes
-        // default-on drawElement renders to the screenshot baseline on this
-        // path. No-op unless the session is deferred with an injector attached.
-        await completeDeferredDrawElementInit(session);
-      }
+      await ensureSessionReady(session);
      assertNotAborted();
      lastBrowserConsole = session.browserConsoleBuffer;

-      // `frameRange` captures only a sub-range of the timeline. Per-frame
-      // TIMES still use the absolute composition frame index so the page's
-      // virtual clock matches an in-process render at the same frame;
-      // file NAMES are normalized to 0 (via the relative loop index `i`)
-      // so the encoder can read frames without an `-start_number` override.
-      const rangeStart = frameRange?.startFrame ?? 0;
-      const rangeEnd = frameRange?.endFrame ?? totalFrames;
-      const rangeFrames = rangeEnd - rangeStart;
-
-      const reportFrame = (fileIndex: number): void => {
-        job.framesRendered = fileIndex + 1;
-        // Keep status cadence identical to the streaming sequential path; the
-        // capture error wrapper below must remain separate from finally so it
-        // can throw with the browser console before cleanup overwrites flow.
-        // fallow-ignore-next-line code-duplication
-        updateJobStatus(
-          job,
-          "rendering",
-          `Capturing frame ${fileIndex + 1}/${rangeFrames}`,
-          Math.round(25 + ((fileIndex + 1) / rangeFrames) * 45),
-          onProgress,
-        );
-      };
-
-      if (session.workerEncodeEnabled) {
-        // Worker-encode depth-2 pipeline on the DISK path (mirrors the streaming
-        // path): frame N's in-page Worker encodes while frame N+1's main thread
-        // does seek+paint+drawElement. Long comps (>streaming cap) land here, so
-        // without this they'd fall back to synchronous toDataURL and lose the
-        // ~1.5-2x worker-encode speedup entirely.
-        let prev: { fileIndex: number; encodeResult: Promise<Buffer> } | null = null;
-        const drainPrev = async (): Promise<void> => {
-          if (!prev) return;
-          assertNotAborted();
-          const buf = await prev.encodeResult;
-          writeCapturedFrame(session, prev.fileIndex, buf);
-          reportFrame(prev.fileIndex);
-        };
-        for (let i = 0; i < rangeFrames; i++) {
-          assertNotAborted();
-          const absoluteIdx = rangeStart + i;
-          const time = (absoluteIdx * job.config.fps.den) / job.config.fps.num;
-          const { encodeResult } = await captureFrameToBufferPipelined(session, i, time);
-          await drainPrev();
-          prev = { fileIndex: i, encodeResult };
-        }
-        await drainPrev();
-      } else {
-        for (let i = 0; i < rangeFrames; i++) {
-          assertNotAborted();
-          const absoluteIdx = rangeStart + i;
-          const time = (absoluteIdx * job.config.fps.den) / job.config.fps.num;
-          await captureFrame(session, i, time);
-          reportFrame(i);
-        }
-      }
-      // Sequential disk drawElement self-verification (PRINFRA-352 follow-up):
-      // the sequential disk path — reachable under the explicit fast-capture
-      // opt-in, including via probe-session reuse — armed ground-truth samples
-      // but never checked them, exactly like the parallel disk workers before
-      // #2749. Same synthetic-task shape the parallel verify uses; a breach
-      // throws DrawElementVerificationError and the orchestrator's disk-stage
-      // retry re-renders via screenshot.
-      await verifyDiskDrawElementSamples(
-        session,
-        {
-          workerId: 0,
-          startFrame: rangeStart,
-          endFrame: rangeEnd,
-          outputDir: framesDir,
-          outputFrameOffset: rangeStart,
-        },
-        false,
-      );
-      // Capture the sequential session's static-dedup perf before close (the
-      // counters are valid only while the session is live).
-      dedupPerfs.push(getCapturePerfSummary(session));
+      await captureSessionFrames(session, { input, rangeStart, rangeEnd, resumeFrom });
+      break;
      // This must mirror streaming capture: catch wraps the original failure with
      // browser diagnostics, finally only handles cleanup.
      // fallow-ignore-next-line code-duplication
    } catch (error) {
      lastBrowserConsole = session.browserConsoleBuffer;
+      const resume = planTransientResume(error, {
+        abortSignal,
+        rangeFrames,
+        framesDir,
+        frameExt,
+        retriesUsed: transientRetriesUsed,
+      });
+      if (resume !== null) {
+        transientRetriesUsed++;
+        resumeFrom = resume;
+        log.warn(
+          "[Render] Transient browser failure during sequential capture; retrying once with a fresh session.",
+          {
+            resumeFrom,
+            rangeFrames,
+            transientRetriesUsed,
+            error: error instanceof Error ? error.message : String(error),
+          },
+        );
+        continue;
+      }
      throw wrapCaptureStageError(error, lastBrowserConsole);
    } finally {
      // Keep the latest console buffer for success and cleanup-error summaries.
@@ -443,6 +424,118 @@ export async function runCaptureStage(input: CaptureStageInput): Promise<Capture
      await closeCaptureSession(session);
    }
  }
+  return { lastBrowserConsole, captureBeyondViewport };
+}

-  return { workerCount, probeSession, lastBrowserConsole, captureBeyondViewport };
+function resolveCaptureRange(
+  frameRange: CaptureStageInput["frameRange"],
+  totalFrames: number,
+): { rangeStart: number; rangeEnd: number } {
+  return { rangeStart: frameRange?.startFrame ?? 0, rangeEnd: frameRange?.endFrame ?? totalFrames };
+}
+
+/** First frame missing on disk when the failure is a retryable browser death, else null. */
+function planTransientResume(
+  error: unknown,
+  ctx: {
+    abortSignal: AbortSignal | undefined;
+    rangeFrames: number;
+    framesDir: string;
+    frameExt: "png" | "jpg";
+    retriesUsed: number;
+  },
+): number | null {
+  const failure = classifyCaptureFailure(error, { signal: ctx.abortSignal });
+  const missing = findMissingFrameRanges(ctx.rangeFrames, ctx.framesDir, ctx.frameExt);
+  const [firstMissing] = missing;
+  if (!firstMissing) return null;
+  return isTransientCaptureRetryEligible(failure.kind, missing, ctx.retriesUsed)
+    ? firstMissing.startFrame
+    : null;
+}
+
+async function ensureSessionReady(session: CaptureSession): Promise<void> {
+  if (!session.isInitialized) {
+    await initializeSession(session);
+    return;
+  }
+  // Deferred drawElement init (probe-initialized video comps). The disk path has no
+  // drain-time self-verification, so only the explicit fast-capture opt-in completes it.
+  if (process.env.PRODUCER_EXPERIMENTAL_FAST_CAPTURE === "true") {
+    await completeDeferredDrawElementInit(session);
+  }
+}
+
+interface SequentialFrameLoop {
+  input: CaptureStageInput;
+  rangeStart: number;
+  rangeEnd: number;
+  resumeFrom: number;
+}
+
+async function captureSessionFrames(
+  session: CaptureSession,
+  { input, rangeStart, rangeEnd, resumeFrom }: SequentialFrameLoop,
+): Promise<void> {
+  const { job, framesDir, dedupPerfs, assertNotAborted, onProgress } = input;
+  const rangeFrames = rangeEnd - rangeStart;
+  const reportFrame = (fileIndex: number): void => {
+    job.framesRendered = fileIndex + 1;
+    // Keep status cadence identical to the streaming sequential path; the
+    // capture error wrapper below must remain separate from finally so it
+    // can throw with the browser console before cleanup overwrites flow.
+    // fallow-ignore-next-line code-duplication
+    updateJobStatus(
+      job,
+      "rendering",
+      `Capturing frame ${fileIndex + 1}/${rangeFrames}`,
+      Math.round(25 + ((fileIndex + 1) / rangeFrames) * 45),
+      onProgress,
+    );
+  };
+
+  if (session.workerEncodeEnabled) {
+    // Depth-2 pipeline: frame N encodes in the page Worker while frame N+1 seeks and paints.
+    let prev: { fileIndex: number; encodeResult: Promise<Buffer> } | null = null;
+    const drainPrev = async (): Promise<void> => {
+      if (!prev) return;
+      assertNotAborted();
+      const buf = await prev.encodeResult;
+      writeCapturedFrame(session, prev.fileIndex, buf);
+      reportFrame(prev.fileIndex);
+    };
+    for (let i = resumeFrom; i < rangeFrames; i++) {
+      assertNotAborted();
+      const absoluteIdx = rangeStart + i;
+      const time = (absoluteIdx * job.config.fps.den) / job.config.fps.num;
+      const { encodeResult } = await captureFrameToBufferPipelined(session, i, time);
+      await drainPrev();
+      prev = { fileIndex: i, encodeResult };
+    }
+    await drainPrev();
+  } else {
+    for (let i = resumeFrom; i < rangeFrames; i++) {
+      assertNotAborted();
+      const absoluteIdx = rangeStart + i;
+      const time = (absoluteIdx * job.config.fps.den) / job.config.fps.num;
+      await captureFrame(session, i, time);
+      reportFrame(i);
+    }
+  }
+  // A breach throws DrawElementVerificationError; the orchestrator retries via screenshot.
+  // Only the resumed range is sampled after a transient retry.
+  await verifyDiskDrawElementSamples(
+    session,
+    {
+      workerId: 0,
+      startFrame: rangeStart + resumeFrom,
+      endFrame: rangeEnd,
+      outputDir: framesDir,
+      outputFrameOffset: rangeStart,
+    },
+    false,
+  );
+  // Capture the sequential session's static-dedup perf before close (the
+  // counters are valid only while the session is live).
+  dedupPerfs.push(getCapturePerfSummary(session));
 }
@@ -177,6 +177,8 @@ mock.module("../../renderOrchestrator.js", () => ({
  closeHdrVideoFrameSource: () => {},
  createHdrPerfCollector: () => ({}),
  executeDiskCaptureWithAdaptiveRetry: async () => [],
+  findMissingFrameRanges: () => [],
+  isTransientCaptureRetryEligible: () => false,
  resolveCompositeTransfer: () => "srgb",
 }));

@@ -90,6 +90,7 @@ import {
  explainDrawElementDisabled,
  scaleProtocolTimeoutForComposition,
  classifyCaptureFailure,
+  type CaptureFailureKind,
  cloneCaptureWarning,
  isMemoryExhaustionError,
  isTransientBrowserError,
@@ -1049,6 +1050,19 @@ export function resolveRenderWorkDirPrefix(
 */
 export const MAX_TRANSIENT_CAPTURE_RETRIES = 1;

+/** Single owner of the bounded transient-browser retry policy for both disk-capture paths. */
+export function isTransientCaptureRetryEligible(
+  failureKind: CaptureFailureKind,
+  missing: readonly FrameRange[],
+  retriesUsed: number,
+): boolean {
+  return (
+    missing.length > 0 &&
+    failureKind === "transient_browser" &&
+    retriesUsed < MAX_TRANSIENT_CAPTURE_RETRIES
+  );
+}
+
 /**
 * A retry only pays off if the attempt that just finished captured at least one
 * frame toward its target. When it captured nothing (frames still missing >=
@@ -1288,18 +1302,11 @@ export async function executeDiskCaptureWithAdaptiveRetry(options: {
      // composition. Unlike the worker-halving retry below, this keeps the same
      // worker count (parallelism isn't the problem) and does NOT require
      // forward progress — a tab that dies before frame 0 is the exact case we
-      // want to recover. Bounded by MAX_TRANSIENT_CAPTURE_RETRIES so a
-      // deterministically-dying tab still fails instead of looping.
-      //
-      // Scope: this covers the parallel disk-capture path (the multi-worker
-      // renders where a contended host most often drops a tab). The sequential
-      // and streaming capture paths run a single stateful session/encoder and
-      // don't route through here; probeStage already has its own transient
-      // retry for the session-init phase they share.
+      // want to recover. Eligibility is shared with the sequential branch in
+      // captureStage.ts; streaming capture doesn't route through here.
      if (
        options.allowRetry &&
-        failure.kind === "transient_browser" &&
-        transientRetriesUsed < MAX_TRANSIENT_CAPTURE_RETRIES
+        isTransientCaptureRetryEligible(failure.kind, remaining, transientRetriesUsed)
      ) {
        transientRetriesUsed++;
        options.log.warn(