mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-10-30 17:27:09 +00:00
fix(engine): name the data-no-timeline opt-out in the timeline-timeout warning (#4410)
* fix(engine): name the data-no-timeline opt-out in the timeline-timeout warning When the sub-composition timeline wait times out, two things are emitted: the stderr line inside pollSubCompositionTimelines, which names the pending composition ids and the data-no-timeline opt-out, and the structured warning on the session, which said only "Sub-composition timelines did not become ready within 45000ms". The structured one is what a programmatic caller reads: render re-prints it in the completed_with_warnings summary and returns it in the result's `warnings` array, so a CI consumer or an SDK caller sees the symptom with no remedy attached. It also reads as a fault to diagnose, when the commonest cause is a composition that legitimately has no GSAP timeline -- CSS animations or rAF -- and is paying the full playerReadyTimeout on every render because nobody told the author the wait is theirs to switch off. The message now says the wait can be intentional, names data-no-timeline as the opt-out, and keeps the async-setup case as the other branch. The pending composition ids were already computed for the stderr line and thrown away; an optional onPending callback carries them to the session so the structured warning and its details can name them too, matching the script-failure branch, which already names its failed sources. The stderr text is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(engine): normalise the pending-id result instead of casting it The enumerate step cast `page.evaluate`'s result with `as string[]`, which is a claim the call site cannot enforce: `evaluate` is loosely typed, and anything that returns a non-array turned the cast into a TypeError thrown on the very path that exists to REPORT a problem. Normalise the result instead. `pendingCompositionIds` was also never declared on `CaptureWarning.details`, whose type is closed, so the engine build failed and every downstream package failed with it. Adds coverage for the poll's own `onPending` handoff, which the existing tests in this PR could not see: they exercised `recordSubTimelineWarning` (the message text) and never `pollSubCompositionTimelines` (where the throw was). The non-array case is the one that discriminates this fix from the tempting alternative of leaving the cast and adjusting the stubs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
WaterrrForever
co-authored by
Claude Opus 5
Claude Opus 5
parent
250c9bd509
commit
9d4836902b
@@ -0,0 +1,152 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { Page } from "puppeteer-core";
|
||||
import type { CaptureSession } from "./frameCapture.js";
|
||||
import { pollSubCompositionTimelines, recordSubTimelineWarning } from "./frameCapture.js";
|
||||
|
||||
function makeSession(overrides: Partial<CaptureSession> = {}): CaptureSession {
|
||||
return {
|
||||
scriptLoadFailures: [],
|
||||
warnings: [],
|
||||
...overrides,
|
||||
} as unknown as CaptureSession;
|
||||
}
|
||||
|
||||
describe("recordSubTimelineWarning", () => {
|
||||
beforeEach(() => {
|
||||
vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
});
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("records nothing when the wait succeeded or never ran", () => {
|
||||
for (const outcome of ["ready", undefined] as const) {
|
||||
const session = makeSession({ subTimelineWaitOutcome: outcome });
|
||||
recordSubTimelineWarning(session, 45_000);
|
||||
expect(session.warnings).toEqual([]);
|
||||
}
|
||||
});
|
||||
|
||||
// The remedy used to live only in the stderr line inside the poll. A caller
|
||||
// reading the structured `warnings` (the render summary, the JSON result)
|
||||
// got the symptom with no way to act on it.
|
||||
it("names the data-no-timeline remedy on a timeout", () => {
|
||||
const session = makeSession({ subTimelineWaitOutcome: "timeout" });
|
||||
recordSubTimelineWarning(session, 45_000);
|
||||
|
||||
expect(session.warnings).toHaveLength(1);
|
||||
const [warning] = session.warnings;
|
||||
expect(warning.code).toBe("sub_timeline_readiness_timeout");
|
||||
expect(warning.message).toContain("45000ms");
|
||||
expect(warning.message).toContain("data-no-timeline");
|
||||
expect(warning.message).toContain("window.__timelines[id]");
|
||||
// Miao's ask: the author must learn the wait can be theirs to switch off,
|
||||
// not only that something timed out.
|
||||
expect(warning.message).toContain("can be intentional");
|
||||
expect(warning.details).toMatchObject({ timeoutMs: 45_000, pendingCompositionIds: [] });
|
||||
});
|
||||
|
||||
it("names the still-unregistered composition ids when the poll reported them", () => {
|
||||
const session = makeSession({
|
||||
subTimelineWaitOutcome: "timeout",
|
||||
pendingTimelineIds: ["scene-2", "scene-5"],
|
||||
});
|
||||
recordSubTimelineWarning(session, 45_000);
|
||||
|
||||
const [warning] = session.warnings;
|
||||
expect(warning.message).toContain("still unregistered: scene-2, scene-5");
|
||||
expect(warning.details).toMatchObject({ pendingCompositionIds: ["scene-2", "scene-5"] });
|
||||
});
|
||||
|
||||
// An empty list must not print an empty parenthetical.
|
||||
it("omits the id clause when no ids were reported", () => {
|
||||
const session = makeSession({ subTimelineWaitOutcome: "timeout", pendingTimelineIds: [] });
|
||||
recordSubTimelineWarning(session, 45_000);
|
||||
|
||||
expect(session.warnings[0].message).not.toContain("still unregistered");
|
||||
});
|
||||
|
||||
// The script-failure branch is a different diagnosis: the timeline can never
|
||||
// arrive, so data-no-timeline is the wrong advice there.
|
||||
it("leaves the script-failure branch alone", () => {
|
||||
const session = makeSession({
|
||||
subTimelineWaitOutcome: "script_failure",
|
||||
scriptLoadFailures: ["https://example.test/scene.js"],
|
||||
pendingTimelineIds: ["scene-2"],
|
||||
});
|
||||
recordSubTimelineWarning(session, 45_000);
|
||||
|
||||
const [warning] = session.warnings;
|
||||
expect(warning.code).toBe("sub_timeline_script_failure");
|
||||
expect(warning.message).toContain("https://example.test/scene.js");
|
||||
expect(warning.message).not.toContain("data-no-timeline");
|
||||
});
|
||||
});
|
||||
|
||||
// The warning text is only as good as the ids handed to it, and that handoff
|
||||
// lives in the poll, not in recordSubTimelineWarning. CI caught a TypeError
|
||||
// here that this file could not see: the enumerate step used to cast
|
||||
// page.evaluate's result with `as string[]`, so any caller whose evaluate did
|
||||
// not return an array crashed the DIAGNOSTIC path.
|
||||
describe("pollSubCompositionTimelines pending-id reporting", () => {
|
||||
beforeEach(() => {
|
||||
vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
});
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
// `data-composition-id` appears in the READINESS expression too, so a stub
|
||||
// keyed on it answers the readiness poll with the enumerate payload and the
|
||||
// poll returns "ready" before it ever enumerates. `m.push(` is unique to the
|
||||
// enumerate step. `matched` then asserts the stub was really reached, so a
|
||||
// later edit to that expression fails loudly instead of testing nothing.
|
||||
function pageReturning(enumerateResult: unknown) {
|
||||
const matched = { count: 0 };
|
||||
const page = {
|
||||
evaluate: vi.fn(async (expr: string) => {
|
||||
if (!expr.includes("m.push(")) return false;
|
||||
matched.count++;
|
||||
return enumerateResult;
|
||||
}),
|
||||
} as unknown as Page;
|
||||
return { page, matched };
|
||||
}
|
||||
|
||||
it("reports the still-unregistered ids to onPending", async () => {
|
||||
const seen: string[][] = [];
|
||||
const probe = pageReturning(["scene-2", "scene-5"]);
|
||||
const outcome = await pollSubCompositionTimelines(
|
||||
probe.page,
|
||||
60,
|
||||
10,
|
||||
() => [],
|
||||
undefined,
|
||||
(ids) => seen.push([...ids]),
|
||||
);
|
||||
|
||||
expect(outcome).toBe("timeout");
|
||||
expect(seen).toEqual([["scene-2", "scene-5"]]);
|
||||
expect(probe.matched.count).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
// `false` is what a blanket evaluate stub returns, and it is what CI hit: the
|
||||
// enumerate result used to be cast with `as string[]`, so a non-array threw a
|
||||
// TypeError on the very path that exists to report a problem.
|
||||
it("degrades to no ids instead of throwing when the page returns a non-array", async () => {
|
||||
const seen: string[][] = [];
|
||||
const probe = pageReturning(false);
|
||||
const outcome = await pollSubCompositionTimelines(
|
||||
probe.page,
|
||||
60,
|
||||
10,
|
||||
() => [],
|
||||
undefined,
|
||||
(ids) => seen.push([...ids]),
|
||||
);
|
||||
|
||||
expect(outcome).toBe("timeout");
|
||||
expect(seen).toEqual([[]]);
|
||||
expect(probe.matched.count).toBeGreaterThan(0);
|
||||
});
|
||||
});
|
||||
@@ -167,6 +167,13 @@ export interface CaptureSession {
|
||||
scriptLoadFailures: string[];
|
||||
/** Outcome of the sub-composition timeline wait: ready | timeout | script_failure. */
|
||||
subTimelineWaitOutcome?: SubTimelineWaitOutcome;
|
||||
/**
|
||||
* Composition ids still unregistered when the timeline wait gave up. Already
|
||||
* computed for the stderr warning; kept on the session so the STRUCTURED
|
||||
* warning can name them too — a programmatic caller reads `warnings`, not
|
||||
* our console output.
|
||||
*/
|
||||
pendingTimelineIds?: string[];
|
||||
/** Structured readiness warnings surfaced to the producer's render policy. */
|
||||
warnings: CaptureWarning[];
|
||||
initTelemetry?: {
|
||||
@@ -1730,6 +1737,9 @@ export async function pollSubCompositionTimelines(
|
||||
// is cut to `scriptFailureGraceMs` from its start.
|
||||
getScriptLoadFailures?: () => readonly string[],
|
||||
scriptFailureGraceMs: number = 2_000,
|
||||
// Reports the composition ids still unregistered at bail time, so the caller
|
||||
// can put them in the structured warning as well as in stderr.
|
||||
onPending?: (ids: readonly string[]) => void,
|
||||
): Promise<SubTimelineWaitOutcome> {
|
||||
// Hosts may opt out of the timeline wait with `data-no-timeline` —
|
||||
// compositions driven purely by CSS animations / rAF (the render-compat
|
||||
@@ -1788,7 +1798,7 @@ export async function pollSubCompositionTimelines(
|
||||
// reason — a script-failure bail used to skip this entirely, so a render
|
||||
// with multiple sub-compositions only named the failed script URL(s), not
|
||||
// which composition(s) it was still waiting on (review).
|
||||
const missing = await page.evaluate(`(function() {
|
||||
const evaluated = await page.evaluate(`(function() {
|
||||
var hosts = document.querySelectorAll("[data-composition-id]");
|
||||
var timelines = window.__timelines || {};
|
||||
var m = [];
|
||||
@@ -1797,8 +1807,15 @@ export async function pollSubCompositionTimelines(
|
||||
var id = hosts[i].getAttribute("data-composition-id");
|
||||
if (id && !timelines[id]) m.push(id);
|
||||
}
|
||||
return m.join(", ");
|
||||
return m;
|
||||
})()`);
|
||||
// This block exists to BUILD A WARNING, so it must never be the thing that
|
||||
// throws. `page.evaluate` is loosely typed, and a caller that stubs it (or a
|
||||
// runtime that returns nothing here) would turn a blind `as string[]` cast
|
||||
// into a TypeError on the diagnostic path. Normalise instead of asserting.
|
||||
const pendingIds = Array.isArray(evaluated) ? evaluated.map((id) => String(id)) : [];
|
||||
onPending?.(pendingIds);
|
||||
const missing = pendingIds.join(", ");
|
||||
if (scriptFailureBail) {
|
||||
console.warn(`[FrameCapture] Composition(s) still waiting on the failed script: ${missing}.`);
|
||||
} else {
|
||||
@@ -1975,10 +1992,12 @@ function recordCaptureWarnings(session: CaptureSession, warnings: readonly Captu
|
||||
}
|
||||
}
|
||||
|
||||
function recordSubTimelineWarning(session: CaptureSession, timeoutMs: number): void {
|
||||
export function recordSubTimelineWarning(session: CaptureSession, timeoutMs: number): void {
|
||||
if (session.subTimelineWaitOutcome === "ready" || !session.subTimelineWaitOutcome) return;
|
||||
const scriptFailure = session.subTimelineWaitOutcome === "script_failure";
|
||||
const hasRuntimeErrors = session.scriptLoadFailures.some((f) => f.startsWith("runtime-error:"));
|
||||
const pending = session.pendingTimelineIds ?? [];
|
||||
const pendingSuffix = pending.length > 0 ? ` (still unregistered: ${pending.join(", ")})` : "";
|
||||
recordCaptureWarnings(session, [
|
||||
{
|
||||
code: scriptFailure ? "sub_timeline_script_failure" : "sub_timeline_readiness_timeout",
|
||||
@@ -1986,8 +2005,16 @@ function recordSubTimelineWarning(session: CaptureSession, timeoutMs: number): v
|
||||
? hasRuntimeErrors
|
||||
? `A sub-composition script threw during execution — timeline registration never arrived (${session.scriptLoadFailures.join(", ")})`
|
||||
: `A sub-composition timeline script failed to load (${session.scriptLoadFailures.join(", ")})`
|
||||
: `Sub-composition timelines did not become ready within ${timeoutMs}ms`,
|
||||
details: { timeoutMs, sources: [...session.scriptLoadFailures] },
|
||||
: `Sub-composition timelines did not become ready within ${timeoutMs}ms${pendingSuffix}. ` +
|
||||
`This can be intentional: a composition driven by CSS animations or rAF never registers ` +
|
||||
`window.__timelines[id], and marking its host with data-no-timeline skips the wait entirely. ` +
|
||||
`Otherwise, a composition that sets up asynchronously must register window.__timelines[id] ` +
|
||||
`once setup completes.`,
|
||||
details: {
|
||||
timeoutMs,
|
||||
sources: [...session.scriptLoadFailures],
|
||||
pendingCompositionIds: [...pending],
|
||||
},
|
||||
},
|
||||
]);
|
||||
}
|
||||
@@ -2294,6 +2321,10 @@ export async function initializeSession(session: CaptureSession): Promise<void>
|
||||
pageReadyTimeout,
|
||||
undefined,
|
||||
() => session.scriptLoadFailures,
|
||||
undefined,
|
||||
(ids) => {
|
||||
session.pendingTimelineIds = [...ids];
|
||||
},
|
||||
);
|
||||
logInitPhase(`pollSubCompositionTimelines complete (${session.subTimelineWaitOutcome})`);
|
||||
recordSubTimelineWarning(session, pageReadyTimeout);
|
||||
@@ -2457,6 +2488,10 @@ export async function initializeSession(session: CaptureSession): Promise<void>
|
||||
pageReadyTimeout,
|
||||
undefined,
|
||||
() => session.scriptLoadFailures,
|
||||
undefined,
|
||||
(ids) => {
|
||||
session.pendingTimelineIds = [...ids];
|
||||
},
|
||||
);
|
||||
logInitPhase(`pollSubCompositionTimelines complete (${session.subTimelineWaitOutcome})`);
|
||||
recordSubTimelineWarning(session, pageReadyTimeout);
|
||||
|
||||
@@ -30,6 +30,7 @@ export interface CaptureWarning {
|
||||
details?: {
|
||||
mediaType?: "image" | "video" | "audio";
|
||||
sources?: string[];
|
||||
pendingCompositionIds?: string[];
|
||||
timeoutMs?: number;
|
||||
failureReasons?: string[];
|
||||
failureStages?: string[];
|
||||
|
||||
Reference in New Issue
Block a user