Repository navigation
fix(producer): a composition with no timeline-registering script no longer waits 45 s at setup - #6436
Conversation
…onger waits 45 s at setup The render waited the full timeout for every composition that never registers a timeline, such as one driven by canvas or requestAnimationFrame.
…gister a timeline
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 2 blocker · 0 important · 1 nit · head cdc5d082
- blocker — CI
Producer: integration testsfails:scriptFailureAttribution.test.tshas 18 of 22 failing (expected [] to deeply equal [ 'sub_timeline_script_failure' ]/[ 'sub_timeline_readiness_timeout' ]). The scan short-circuits readiness before the fail-fast and timeout paths run, so a composition whose scripts throw now gives no warning at all. Either keep those paths reachable when a page error or script-load failure is present, or re-baseline the tests with a reason. ↓ 404 inline - nit — the scan runs before the cheap readiness expression on every 150 ms poll and re-fetches every same-origin script each time. Evaluate the expression first, or cache the scan once
readyStateiscomplete.
Scope
Read all 7 files and both pollSubCompositionTimelines call sites (the __name shim is installed on both pages). Locally: core compiler 635/635, including goldens; engine frameCapture 326/326. Comments (not required) fails: comment share rose in compositionScoping.ts. 6f3543e was superseded by cdc5d08 before this review.
| const url = new URL(script.src, location.href); | ||
| if (url.origin !== location.origin) return true; | ||
| try { | ||
| if (mayRegister(await (await fetch(url)).text())) return true; |
There was a problem hiding this comment.
blocker — fetch resolves on a 404, so a missing project script reads as "cannot register" and the composition is ready at once. Before this change, <script src="scene.js"> with a typo'd path failed fast with sub_timeline_script_failure. Now it renders a frozen scene with no warning (this is the a script from the project is missing case in scriptFailureAttribution.test.ts). The unit test stub throws on a missing file, but a real fetch doesn't.
| if (mayRegister(await (await fetch(url)).text())) return true; | |
| const response = await fetch(url); | |
| if (!response.ok || mayRegister(await response.text())) return true; |
There was a problem hiding this comment.
✅ Fixed in e138bd1: a recorded script-load failure, a 404 included, now keeps the wait. I checked it against the original fixtures (without the added registrar), and a script from the project is missing still ends in sub_timeline_script_failure.
Edit accuracy: accurate 271 (base branch 271), smooth 122 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (1)
|
There was a problem hiding this comment.
Verdict: APPROVE — 2 resolved, 0 open, 0 new · cdc5d08 → e138bd1. A failed script or page error now keeps the wait, the scan runs after the readiness check and only once per page, and all CI is green.
Note
I re-ran scriptFailureAttribution.test.ts without the new registrar line. All 8 sub_timeline_script_failure cases pass, the 404 one included. The 10 cross-origin widget cases end with no warning instead of sub_timeline_readiness_timeout. That's the intended trade: no script can register a timeline, so the frames match and only the 45 s wait and its warning are gone. The fixture comment says so.
Merge 2 more upstream commits (7d6c02f..acd0bc3: heygen-com#5366, heygen-com#3943). Regenerate skills-manifest.json for the media-use hash conflict. Record the 10 open PRs heygen-com#6430-heygen-com#6439 in docs/DECISIONS.md (none adopted; heygen-com#6436 cannot be copied because the fork wraps authored scripts in Function()) and advance the commit watermark to acd0bc3 and the PR watermark to heygen-com#6439. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Brings in 10 upstream commits (ad4d3fa..15367a9): heygen-com#6425, heygen-com#6427, heygen-com#6429, heygen-com#6431, heygen-com#6436, heygen-com#6432, heygen-com#6453, heygen-com#6457, heygen-com#6450, heygen-com#6451. One conflict, packages/core/src/compiler/compositionScoping.ts (heygen-com#6436). This fork passes authored composition code to Function() as a JSON string literal, so upstream's markers around the raw source would sit outside the text its page-side scan reads and the fix would silently not apply: the wrapper's own __timelines use would make every composition look able to register a timeline. The markers go inside the literal instead, begin marker on the first source line so error line numbers do not shift. The pageBuild goldens change only by those two marker comments. A new engine test runs upstream's scan on this fork's real wrapper output and fails without the markers. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…d PR watermark to heygen-com#6459 Commit watermark 8f618c5; PR watermark heygen-com#6459 only, because heygen-com#6460 onward and issue heygen-com#6465 were not reviewed this round (listed in DECISIONS). Issue watermark stays heygen-com#6443. Records the heygen-com#6436 Function() wrapper port, the chromeOrphans failure that is not caused by heygen-com#6431, and the environment-only test failures. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
What
A render no longer waits 45 s at setup for a composition that has no script able to register a timeline, for example a scene drawn on a canvas or driven by
requestAnimationFrame.Why
The readiness poll waited for
window.__timelines[id]on everydata-composition-idhost withoutdata-no-timeline. A composition that never registers one (the lint rule already warns about this) sat out the wholeplayerReadyTimeout, on every render, in every capture worker, then finished with asub_timeline_readiness_timeoutwarning. The wait is only worth paying when something might still register a timeline.How
pollSubCompositionTimelinesnow first asks the page whether any author script can register a timeline (authorScriptsMayRegisterTimelines): every inline, template and same-origin script is read and none mentions__timelines. If none can, the composition is ready at once. A module script, any code that imports another file (importorimport()), a cross-origin or unreadable script, or a page that is still loading answers "may register", so those still wait as before.A page that already has a script error or a script that failed to load keeps the old wait, so the failure is still reported as
sub_timeline_script_failurerather than the page passing as ready. The readiness check runs first and the script scan runs once per page.The compiler's scope wrapper mentions
__timelinesitself, sowrapScopedCompositionScriptand the module prelude now delimit the authored source (AUTHORED_SOURCE_BEGIN/AUTHORED_SOURCE_END) and the scan reads only that part.Design call: the options were (a) a shorter timeout (rejected: it still taxes every such render and cuts off slow registrations), (b) treat every cross-origin script as non-registering (rejected: a late registration would silently render without its animation; for the same reason a registration inside an imported file counts as possible), (c) read what the author scripts say and wait only when one can register (chosen). Remaining limit: a composition whose scripts come from another origin keeps waiting unless its host says
data-no-timeline.Tests
frameCapture-subTimelinePoll.test.ts: a page with no registering script is ready at once instead of waiting out a 60 s timeout; the scan is false for scripts that never mention a registration (inline, template, same-origin file), ignores the runtime, reads only the authored part of a scoped wrapper, and answers true for a registering, module, importing, unreadable or cross-origin script. Fail on main (the function and the early exit do not exist).scriptFailureAttribution.test.ts: its compositions now carry a registrar that is defined and never called, which is what a registration that never arrives looks like.compositionScoping.test.ts: the wrapper puts exactly the authored source between the markers.Measured
Fixture: a root audio clip plus a sub-composition drawing on a canvas with
requestAnimationFrame, no timeline, 6 s at 1280x720, software GL, same box and same render.The audio mix is unchanged (the clip window reads -23.0 dB before and after).