Skip to content

test(player): a Safari perf run that fails says which wait timed out and why - #7347

Merged
miguel-heygen merged 2 commits into
mainfrom
test/player-safari-bench-errors
Nov 7, 2026
Merged

miguel-heygen merged 2 commits into
mainfrom
test/player-safari-bench-errors

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Nov 7, 2026 •

Copy link
Copy Markdown
Collaborator

What

A Safari perf run that fails now says why.

  • The error names the wait that timed out, not just that a WebDriver script errored.
  • If the film never started because a video stayed at "loading" with no data, the error names that video: the browser never started loading h264-1080.mp4.

Why

In some Safari runs of the heavy alpha WebM film, the H.264 video reports networkState LOADING and readyState HAVE_NOTHING and stays there.

  • The perf server never receives a request for it, and the page has no resource-timing entry for it, so Safari never started that load.
  • The runtime waits for every due video to have data, so the film holds at its first frame and the run times out.
  • Safari puts no message in e.stack, so before this change the failure read as a bare script error.

Tried and dropped

A one-time load() on such a video, 5 s after it was first asked to play, does not recover it. Across 24 CI runs, the restart fired in each of the 4 stalled runs. 15 s later the video was still at HAVE_NOTHING and LOADING. The runtime is left unchanged, and this PR only makes the failure name its cause.

Test plan

  • Fails without the fix: without this change, a stalled Safari run reports only POST /session/.../execute/sync with an empty error message.
  • Safari CI runs of this film: 4 stalled runs in 12 each carried restarted fetches 1 and the stuck video's state, which confirmed that the restart does not help.
  • oxfmt and oxlint clean on the changed file.

Size

Small on purpose: two error messages in the perf bench. No runtime change.

@miguel-heygen
miguel-heygen force-pushed the test/player-safari-bench-errors branch from ac00789 to 969950c Compare November 7, 2026 10:49
@miguel-heygen miguel-heygen changed the title test(player): a Safari perf run that fails says which wait timed out fix(core): restart a video fetch Safari never started instead of freezing the film Nov 7, 2026
@github-actions

github-actions Bot commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 267 (base branch 267), smooth 107 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (8)

Unstable (1)

  • seqplainkeys-none-pct-r0-nested-z100: tracking 0.05, pressJump 0, drop 0, reload 0, render 0.02, renderKey -, undo false, teleport true / tracking 0.05, pressJump 0, drop 0, reload 0, render 0.02, renderKey -, undo true, teleport true / tracking 0.05, pressJump 0, drop 0, reload 0, render 0.02, renderKey -, undo true, teleport true

@miguel-heygen
miguel-heygen force-pushed the test/player-safari-bench-errors branch from 846b295 to 157d6e4 Compare November 7, 2026 11:40
@miguel-heygen miguel-heygen changed the title fix(core): restart a video fetch Safari never started instead of freezing the film test(player): a Safari perf run that fails says which wait timed out and why Nov 7, 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.

Approving at 157d6e4a. No blockers.

  • bench-page.js: the isPlaying timeout now names the videos stuck at readyState === 0 and networkState === 2 (HAVE_NOTHING, LOADING), using currentSrc basenames. If no video matches, it rethrows the original timed out after 5000 ms waiting for isPlaying, so nothing else changes. videos() and until are the existing helpers.
  • run.ts: the error is now String(e) plus e.stack. Before, e.stack || e dropped the message on Safari, whose stack has no message line. The "\\n" inside the template literal reaches the page as a real \n, which is correct.
  • Fails-without-the-fix line: this PR only changes test code, adding diagnostics to the perf bench. No product behavior changes. The line names the bench run that gets the vaguer error without the change. That fits the change, so I'm accepting it as the before/after evidence.
  • CI on this head: green, including Player Safari perf ("Heavy films: Safari vs Chrome"), CI, CodeQL, regression, preview-regression and Player perf. Windows render verification was still running and doesn't touch this code.

Nit, non-blocking: on Chrome/V8, e.stack already starts with Error: <message>, so the first line now shows up twice there. It's harmless.

@miguel-heygen
miguel-heygen added this pull request to the merge queue Nov 7, 2026
@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

On the nit: yes, Chrome shows the first line twice. I'm leaving it as is, so the bench doesn't branch on browser just for its error text.

Merged via the queue into main with commit 7b1d9cc Nov 7, 2026
121 checks passed
@miguel-heygen
miguel-heygen deleted the test/player-safari-bench-errors branch November 7, 2026 12:21
SanHsien added a commit to SanHsien/hyperframes that referenced this pull request Nov 7, 2026
…, heygen-com#7360, heygen-com#7347); review to PR heygen-com#7362

Adopt heygen-com#7357 and heygen-com#7361 (upload completion waits and retries), heygen-com#7360 (Chrome
start failure names its cause in check) and heygen-com#7347 (Safari perf tooling);
release v0.9.20 not applicable. Pin open heygen-com#7362 (needs its golden LFS pointer
when adopted); record issue heygen-com#7363. Monitored 126 -> 127.

Co-authored-by: Miguel Ángel <miguel.sierra@heygen.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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