Skip to content

test(engine): the decode timeout warning reaches every caller - #7119

Open
miguel-heygen wants to merge 5 commits into
mainfrom
dscan/bg-decode-callers-tests
Open

miguel-heygen wants to merge 5 commits into
mainfrom
dscan/bg-decode-callers-tests

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

What changed

Adds packages/engine/src/services/frameCapture-decodeTimeoutCallers.test.ts, the connected tests left as a gap on #6972. Each drives the real caller and proves a non-empty background-decode timeout list reaches the session media_readiness_timeout warning with the url's credentials, query and fragment removed, and that an empty list adds none:

  • drawElement init: completeDeferredDrawElementInit with a page whose url() background never finishes decoding (real decodeDynamicCssBackgroundImages, fake timers).
  • per-frame seek: captureFrame through prepareFrameForCapture, same stuck page.
  • batch loop: captureFramesBatchPipelined with produceDrawElementFrameBatch returning lateBackgrounds.

Tests only, no production change.

What I measured

  • Run on a remote test host, one file, 3 runs: 6 passed, 0 failed each time.
  • Break the recording call at one caller, only that test goes red (1 failed, 5 passed): init, per-frame seek and batch loop each checked.

What I did NOT exercise

  • A real browser: the pages are in-process fakes, as in the neighbouring tests.
  • The per-frame path in drawElement mode: captureFrameToBuffer wraps the frame in a 15000ms deadline, equal to the decode timeout, so the test uses screenshot mode, which shares prepareFrameForCapture.

fails without the fix: packages/engine/src/services/frameCapture-decodeTimeoutCallers.test.ts "the batch loop: the late backgrounds of a batch are reported, sanitized"

heygengenesis[bot]
heygengenesis Bot previously approved these changes Nov 4, 2026

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved at d5b7ede7.

This PR only adds tests, and the three of them cover the three recordBackgroundDecodeTimeouts call sites in frameCapture.ts: init (line 1385), the per-frame seek (line 3405) and the batch loop (line 5268). I checked them locally with vitest:

  • On this head: 6 of 6 pass, on two separate runs.
  • Breaking one call site at a time: I changed each recordBackgroundDecodeTimeouts(...) to void (...). Each time exactly its matching test went red (1 failed, 5 passed), for init, the per-frame seek and the batch loop.
  • Turning off sanitizing: with urls.map(sanitizeDiagnosticUrl) changed to urls in recordBackgroundDecodeTimeouts, all 3 "reported, sanitized" tests went red. So the credentials, query and fragment check really is enforced.
  • The fails without the fix: line names the batch-loop case, and that case fails when its call site is removed. The other two cases fail the same way for their own call sites.

Nits, not blocking:

  • The backgroundPage docstring says "the any url() background"; one of those words is extra.
  • /SECRET|user|pw@|frag/ matches the bare word user. If a sanitized warning ever contains that word, the check will fail even though nothing leaked. user: or user:pw would be a tighter match.

Base automatically changed from dscan/bg-decode-bound to main November 4, 2026 20:58
@miguel-heygen
miguel-heygen dismissed heygengenesis[bot]’s stale review November 4, 2026 20:58

The base branch was changed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant