fix(render): bound and recover capture stalls - #3700
Open
miguel-heygen wants to merge 4 commits into
Open
Conversation
… instead of failing (#3172) * fix(engine): bound drawElement frames so a wedged renderer falls back instead of failing A single drawElement frame could kill an entire render. On one comp (caption-editorial-emphasis) drawElementImage returns normally and the renderer then stops draining its task queue: the setTimeout(…, 0) that schedules toDataURL never fires, the capture page.evaluate never settles, and 60s later the stage watchdog fails the whole render. Deterministic on 152.0.7977.30, always the same frame. Root-cause detail in PRINFRA-488. Two gaps, both closed here. 1. Nothing below the 60s stage watchdog bounded a frame. Adds a per-frame deadline (HF_DE_FRAME_TIMEOUT_MS, default 15000, 0 disables) around the WHOLE frame operation, not just the drawElementImage call — the stall surfaces at whichever page round-trip comes next, and was observed at both the capture and the seek's background-image decode. Bounding one call missed it. Deliberately NO per-frame screenshot fallback for this error: once the renderer stops scheduling it is wedged for every subsequent round-trip on that page. Measured — the screenshot fallback blew the same deadline. The frame fails fast instead, so the recovery that actually works can run. 2. That recovery was ineligible. shouldRetryViaPinnedFallback only retried a generic capture failure when the worker count was PINNED by the inversion or the router; a comp that engaged drawElement on the ordinary single-worker path had no whole-render fallback at all. A renderer stall is now retryable on any routing — the failure is a property of drawElement itself, and the retry re-renders on a fresh page via screenshot. Reported as de_fallback_reason "de_renderer_stall" so it is distinguishable from capture_error in telemetry, and counted per-session as deFrameTimeouts. Verified end to end on the repro: previously exit 1 with "stalled: no frame progress for 60000ms"; now the deadline fires, the render retries via screenshot, and RENDER_OK with a valid 1920x1080 / 240-frame / 8.0s MP4. Tests cover the predicate both ways plus the cross-package error match. Engine 1,481 and producer 587 green (audioPadTrim.integration flakes only under full-suite parallel load — passes in isolation, and on clean HEAD it is green too, so it is not from this change). Refs PRINFRA-488 * fix(engine,producer,cli): count drawElement frame timeouts where they happen `session.deFrameTimeouts` was incremented inside `captureFrameCore`'s catch, but the deadline races that function from OUTSIDE it — `withFrameDeadline` is applied by `captureFrameToBuffer`, and `DeFrameTimeoutError` is constructed nowhere else. So the branch was unreachable: on a wedged renderer the counter stayed 0, the engine-side stall log never printed, and `CapturePerfSummary.deFrameTimeouts` reported 0 on every affected render. The end-to-end recovery worked throughout — `isDeRendererStallError` matches by name at the producer — only the observability was dead. Moved the increment and the log into an `onTimeout` hook that fires from the deadline itself, deleted the unreachable branch and the now-unused `isDeFrameTimeoutError`, and threaded the count out to `render_completed.de_frame_timeouts` so the rate is graphable apart from the other `capture_error` fallbacks. A stall always costs a whole-render re-run, which is why it deserves its own series. Also corrected the `fallbackReason` docstring, which still listed only `psnr | blank | oom | capture_error` after this branch added `de_renderer_stall`. `withFrameDeadline` is exported for a two-case fake-timer test: the deadline fires and calls `onTimeout` once, and a resolving inner promise passes its value through with no late timer. The first case fails if the counter moves back inside the work promise. (cherry picked from commit ec2b2b5)
miguel-heygen
changed the title
miguel-heygen
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Wedged capture operations now fail on bounded worker phases and retry through the existing fresh screenshot plan. Sequential watchdogs name their resolved mode, while parallel drawElement workers report whether they stopped during browser launch/probing, session initialization, first-frame capture, or pipelined frame encoding.
Why
The original 60-second parallel watchdog measured only completed frames. A stall at
0/Ncould therefore mean browser launch, Chrome probing, drawElement initialization, first capture, or the in-page encoder, and logs did not identify the executable/build, CanvasDrawElement result, GPU backend, or worker.Router-pinned parallel fallback was already correct: it aborts peers, closes every worker session, and retries once via screenshot. This change reduces the default drawElement worker-operation bound to 30 seconds and makes that recovery attributable; it does not add another retry layer.
How
HF_DE_PARALLEL_PHASE_TIMEOUT_MS(30 seconds by default;0disables).System Chrome causality remains unproven because the report did not include an exact browser build, CanvasDrawElement capability, or GPU backend comparison.
Test plan
Unit tests added/updated
Manual testing performed
Documentation updated (not applicable)
Engine parallel deadline/coordinator/peer-abort suites: 53 tests passed.
Producer streaming-stage lifecycle/watchdog suite: 12 tests passed.
Producer sequential-stall retry policy: 4 tests passed.
Engine and producer package typechecks passed.
Changed-file oxlint, oxfmt, and diff checks passed.
The full orchestrator suite previously passed 203/204 tests. The remaining
alpha + outputResolutioncontract-drift assertion is unrelated to capture-stall recovery.