Repository navigation
fix(core): a runtime that fails to start names its real error instead of a half-built page - #5406
Conversation
… longer aborts the runtime
… or throwing entry
Edit accuracy: accurate 2061 (base branch 2061), smooth 1625 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (1)
|
…ad of a half-built page
…nd a later good post clears it
…d of reading it as stuck at 0
…ng cleared by a later post
… cannot be printed
…startup error recorder
terencecho
left a comment
There was a problem hiding this comment.
Reviewed b4c900785264bab7eb927167761eec69f1eea088 against API base 684c038483cdf9f6f25ea518b7fd446596fb4fab; the independently computed effective merge-base is the same SHA (24 changed files). No merge-blocking defect found in the reviewed diff.
Strengths
packages/core/src/runtime/timelineDuration.ts:5–17guards duration-property access, invocation and numeric conversion together. The migrated clip-tree, payload, start-resolver and binding consumers preserve their own positivity/clamping/fallback rules. Numeric, absent and throwing duration entries no longer abort those reads.- The synchronous bootstrap catch and timeline-post catch preserve the original exception and record a sticky named failure. CLI and engine waits prioritize that string even when readiness is otherwise true. The inspected
motionShotandvalidateowners close acquired browsers on opening/readiness failure; command failure propagation was source-audited, not exercised as a real process. - The seek-clock change rejects
nullrather than coercing it to zero; the producer browser probe retains authored-duration fallback when a duration getter/method throws.
Nonblocking follow-up — deferred selection failure is still unnamed
packages/core/src/runtime/init.ts:4067–4131 calls bindRootTimelineIfAvailable() before the new postTimeline() catch. On the external-composition first-readiness path (995–1013, 3431–3434), a child registered by a later authored script with a throwing play getter fails selection at 1597 outside both recorder boundaries. Registration itself does not read the member: the scoped registry uses Reflect.set (packages/core/src/compiler/compositionScoping.ts:423–433), and its script catch ends before this later selection.
Pinned base/head source-extraction controls both leave initial readiness false and the marker absent for this case. The actual extracted CLI waiter returns false on a modeled timeout rather than throwing the named error; the producer bridge holds duration at zero. No real served CLI/browser capture or pixel outcome was executed. A separate later-rebind control retains a previously true readiness flag.
This is a pre-existing diagnostic-coverage gap, not an introduced selection regression, and the throwing play accessor is outside the partial-duration shapes this fix makes safe. I am not making broader malformed/hostile timeline support a merge condition. As follow-up, put the entire readiness publisher/binding boundary under the recorder and add a deferred-first-readiness regression. The PR's known-limit statement that a Proxy failing in timeline selection “is now reported by name” should be narrowed: naming is established for synchronous selection, not every deferred selection path. The explicit timeline-post reporting guarantee does hold in the inspected paths; this caveat also is not the disclosed case where a wait already returned before an error was recorded.
Evidence and scope
- Two independent source audits were completed, followed by targeted first-readiness re-audits and a separate body-only critique. Full patch and all changed core runtime/CLI production files were read across the passes. Large engine/producer production files and giant pre-existing test files were read at changed hunks and relevant readiness, seek, cleanup and producer consumers, not end-to-end. The generated inline bundle was inspected, not rebuilt or parity-certified.
- I inspected and ran dependency-free witnesses: 62 assertions across duration/clip-tree, synchronous bootstrap, publication rollback/stickiness, CLI/engine error priority, numeric clock filtering and producer fallback; 58 additional assertions over pinned base/head deferred external/build-ready, synchronous-bad and healthy controls; and a later-rebind selection control. These execute actual extracted functions with explicit inert mocks, not full DOM/GSAP/browser initialization or real command shutdown.
- No local repository suite, typecheck, build, browser/native capture, dependency install or live vendor call ran. Historical successful runtime/producer CI used synthetic merge
71ff6f28997bb5ca5467e867f2147e05532f537f, not the standalone head. Pending/skipped checks were present at inspection; this is a code-merit verdict, not an all-CI-green claim. - No source fixes, CI reruns, deployment or direct merge performed.
Verdict: APPROVE
Reasoning: The supported partial-duration fix, named synchronous/post-failure reporting, consumer priority and acquired-resource cleanup are coherent. The deferred malformed-selection gap was source-audited and reproduced with extracted-function controls; it remains worth hardening, but neither the controls nor the source audit establishes a new regression or a blocker for the supported cases repaired here.
— tai
What changes for the user
hyperframes checkno longer dies withcheck_runtime_failure: Cannot access 'nS' before initializationon a project whose sub-composition registers a partial timeline, such aswindow.__timelines["extra"] = { duration: 4, seek() {} }or one whoseduration()throws. The browser audit runs and reports layout, motion and contrast again. Preview and render of the same page stop half-initialising too.Root cause
During runtime start-up the timeline is announced, which builds the clip tree and the timeline payload. Both read each registered sub-composition timeline's
duration:clipTree.tscalledregistry[id]?.duration?.(). Optional call only skipsnull/undefined, so a numericdurationwas called and threwTypeError: registry[compId]?.duration is not a function.timeline.tschecked thatdurationwas a function but did not guard the call, so aduration()that throws escaped too.Either throw aborted start-up partway, before the seek helpers further down were initialised. The next seek from
checkthen hit the temporal dead zone ofactivateSiblingTimelines(minifiednS), an error that names an internal variable instead of the real cause.The same guarded read existed in five places (
init.ts,timeline.tstwice,startResolver.ts,clipTree.ts) and none of them guarded a getter that throws (two did not guard the call either). They now share one reader,readTimelineDurationSeconds: a finiteduration()ornull, with the type check inside the guard so a throwing getter or a Proxy entry reads asnulltoo. Each caller keeps its own rule on top (> 0, or clamped at 0).A start-up that throws now says so
Removing the trigger leaves the class: any exception during runtime start-up left a half-built page that the next seek reported as an internal "before initialization" error. Now:
recordStartupError(diagnostics.ts), setswindow.__hfStartupError("HyperFrames runtime failed: : . Check the composition's scripts and window.__timelines entries; if they look right, report this as a HyperFrames bug."). A thrown value that cannot be printed is recorded as "unknown error" rather than making the recorder throw.entry.tscalls it when start-up throws, and the render-ready publish calls it when its timeline post throws, including publishes after start-up (GSAP batching finishing, the deferred rebind, adapter readiness). Both still rethrow. Other timeline posts (Studio's editor refresh among them) do not record: they keep their own retry.check,snapshot,keyframes,compare,grade-compare,layout,validate) and the engine's render wait fail at once with that message instead of timing out or seeking a half-built page.keyframes --shotandvalidateclose the browser they opened when opening the page throws, so a failed start-up ends the command instead of leaving Chrome running (it hung before; forvalidatethat was already true on main).duration()that throws (it runs in the page and cannot import the shared reader); its handling of a non-finite duration is unchanged.position-edits-render-inline.tsis regenerated because the bundle it holds importsdiagnostics.ts.frameCapture.tscomment no longer cites a doc file that was never committed.Also in this PR (same
checkaudit)check's seek-clock probe (collectSeekClock) read a custom timeline'stime()withNumber(...), sotime: () => nullbecame0and a working custom root looked like a stuck clock (exit 1). It now accepts only a finite number.Verification
init.test.tscases run real runtime start-up and a seek with a sub-composition timeline whosedurationis a number, a method that throws, or a getter that throws. With main's runtime files all three fail (registry[compId]?.duration is not a function,boom,getter); with this change they pass.clipTree.test.tscovers the same shapes at the unit level.entry.startupFailure.test.ts: a throwing start-up records the named error, a start-up whose timeline post throws is not left render-ready, a post that throws after start-up (the deferred rebind) is named and leaves the page not ready, and a start-up error survives a later successful post. Both fail on the previous code. The CLI and engine waits each have a test that fails without the change;motionShot.browserClose.test.tsandvalidate.browserClose.test.tscheck the browser is closed when the runtime fails to start (both fail on the previous code); and so does the producer probe; acheckBrowser.test.tscase pins the nulltime()clock (fails on the previous code).0,null,NaN,Infinityfromduration()give the same results).Known limits
validatereports only the start-up error on such a page, not the other console errors it collected.