mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-12 15:20:13 +00:00
fix(cli): validate navigation timeout honors --timeout, hints on CDN scripts (#1929)
`validate` navigated the page with a hardcoded 10s timeout that ignored the --timeout option. A composition that loads GSAP (or any library) from a CDN <script> in <head> blocks `domcontentloaded` until that script finishes downloading; on a slow network that exceeds 10s and validate fails with an opaque "Navigation timeout of 10000ms exceeded" — even though the full render (much larger budget) rides it out fine, and even though --timeout (the documented "wait longer for slow loads" knob) had no effect on navigation. The only recourse was to change the composition (vendor the script locally). Reported precisely, with the exact error and the observation that render's 60s budget masks it while validate's 10s trips. Fix: - resolveNavigationTimeoutMs(optTimeout) = max(10s floor, --timeout), so --timeout now also extends the navigation budget. Default behavior is unchanged: the default --timeout (3000) stays clamped to the 10s floor. - navigationTimeoutHint() replaces Puppeteer's opaque timeout error with an actionable message naming the likely cause (a blocking CDN <script>) and the two fixes (vendor locally / raise --timeout). Any non-timeout error is rethrown unchanged. - --timeout help text updated to note it also governs navigation. Both helpers are pure and exported; validateInBrowser wires them around the single page.goto. No behavior change for compositions that navigate within 10s. Test: resolveNavigationTimeoutMs (floor kept for unset/small/zero, raised past the floor) and navigationTimeoutHint (rewrites a nav-timeout error with CDN + --timeout guidance; returns null for other errors so the caller rethrows as-is). validate suite 14 tests pass.
This commit is contained in:
@@ -1,7 +1,9 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
extractCompositionErrorsFromLint,
|
||||
navigationTimeoutHint,
|
||||
raceMediaReady,
|
||||
resolveNavigationTimeoutMs,
|
||||
shouldIgnoreRequestFailure,
|
||||
} from "./validate.js";
|
||||
import type { ProjectLintResult } from "../utils/lintProject.js";
|
||||
@@ -192,3 +194,39 @@ describe("extractCompositionErrorsFromLint", () => {
|
||||
expect(errors.map((e) => e.text)).toEqual(["scene-a is empty", "scene-b is empty"]);
|
||||
});
|
||||
});
|
||||
|
||||
// Regression: `validate` used a hardcoded 10s page-navigation timeout that
|
||||
// ignored --timeout, so a composition loading GSAP from a CDN <script> (which
|
||||
// blocks domcontentloaded) failed with an opaque "Navigation timeout of 10000ms"
|
||||
// even though the full render's larger budget rode it out — with no knob to
|
||||
// extend it. resolveNavigationTimeoutMs makes --timeout raise the nav budget
|
||||
// (never below the 10s floor); navigationTimeoutHint replaces the opaque error.
|
||||
describe("resolveNavigationTimeoutMs", () => {
|
||||
it("keeps the 10s floor when --timeout is unset or smaller", () => {
|
||||
expect(resolveNavigationTimeoutMs(undefined)).toBe(10000);
|
||||
expect(resolveNavigationTimeoutMs(3000)).toBe(10000); // the default --timeout
|
||||
expect(resolveNavigationTimeoutMs(0)).toBe(10000);
|
||||
});
|
||||
|
||||
it("raises the navigation budget to --timeout when it exceeds the floor", () => {
|
||||
expect(resolveNavigationTimeoutMs(30000)).toBe(30000);
|
||||
});
|
||||
});
|
||||
|
||||
describe("navigationTimeoutHint", () => {
|
||||
it("replaces a Puppeteer navigation-timeout error with an actionable CDN/--timeout hint", () => {
|
||||
const hinted = navigationTimeoutHint(
|
||||
new Error("Navigation timeout of 10000 ms exceeded"),
|
||||
10000,
|
||||
);
|
||||
expect(hinted).toBeInstanceOf(Error);
|
||||
expect(hinted?.message).toContain("10000ms");
|
||||
expect(hinted?.message).toContain("CDN");
|
||||
expect(hinted?.message).toContain("--timeout");
|
||||
});
|
||||
|
||||
it("returns null for any non-navigation-timeout error so the caller rethrows it as-is", () => {
|
||||
expect(navigationTimeoutHint(new Error("net::ERR_CONNECTION_REFUSED"), 10000)).toBeNull();
|
||||
expect(navigationTimeoutHint("some string failure", 10000)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -33,6 +33,33 @@ interface ContrastEntry {
|
||||
const CONTRAST_SAMPLES = 5;
|
||||
const SEEK_SETTLE_MS = 150;
|
||||
const MEDIA_EXTENSIONS = /\.(aac|flac|m4a|mov|mp3|mp4|oga|ogg|wav|webm)$/i;
|
||||
// Floor for the initial page navigation. A blocking external <script> (GSAP
|
||||
// from a CDN, etc.) delays `domcontentloaded`; the actual render (much larger
|
||||
// budget) rides it out, so validate's navigation must be at least as patient as
|
||||
// the user's --timeout, never stuck below this floor.
|
||||
const NAV_TIMEOUT_FLOOR_MS = 10000;
|
||||
|
||||
// Navigation budget = the larger of the floor and the user's --timeout, so
|
||||
// `--timeout` (already the "wait longer for slow loads" knob for media/settle)
|
||||
// also extends navigation instead of being ignored by a hardcoded 10s.
|
||||
export function resolveNavigationTimeoutMs(optTimeout?: number): number {
|
||||
return Math.max(NAV_TIMEOUT_FLOOR_MS, optTimeout ?? 0);
|
||||
}
|
||||
|
||||
// Turn Puppeteer's opaque "Navigation timeout of Nms exceeded" into an
|
||||
// actionable message: the usual cause is a blocking CDN <script> that render
|
||||
// tolerates but validate's tighter budget does not. Returns a replacement Error
|
||||
// for a navigation timeout, or null for any other error (caller rethrows as-is).
|
||||
export function navigationTimeoutHint(err: unknown, navTimeoutMs: number): Error | null {
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
if (!/navigation timeout/i.test(msg)) return null;
|
||||
return new Error(
|
||||
`Page navigation timed out after ${navTimeoutMs}ms. A blocking external <script> ` +
|
||||
`(e.g. GSAP loaded from a CDN) can delay page load past this budget even when the ` +
|
||||
`full render succeeds. Vendor the script locally (recommended for deterministic ` +
|
||||
`renders), or re-run with a longer --timeout.`,
|
||||
);
|
||||
}
|
||||
|
||||
export function shouldIgnoreRequestFailure(
|
||||
url: string,
|
||||
@@ -353,7 +380,14 @@ async function validateInBrowser(
|
||||
}
|
||||
});
|
||||
|
||||
await page.goto(server.url, { waitUntil: "domcontentloaded", timeout: 10000 });
|
||||
const navTimeoutMs = resolveNavigationTimeoutMs(opts.timeout);
|
||||
try {
|
||||
await page.goto(server.url, { waitUntil: "domcontentloaded", timeout: navTimeoutMs });
|
||||
} catch (err) {
|
||||
const hinted = navigationTimeoutHint(err, navTimeoutMs);
|
||||
if (hinted) throw hinted;
|
||||
throw err;
|
||||
}
|
||||
await new Promise((r) => setTimeout(r, opts.timeout ?? 3000));
|
||||
|
||||
for (const w of await auditClipDurations(page, analyzeClipMediaFit, opts.timeout ?? 3000)) {
|
||||
@@ -472,7 +506,9 @@ Examples:
|
||||
},
|
||||
timeout: {
|
||||
type: "string",
|
||||
description: "Ms to wait for scripts to settle (default: 3000)",
|
||||
description:
|
||||
"Ms to wait for scripts to settle and media to load (default: 3000). Also raises the " +
|
||||
"page-navigation budget above its 10s floor when a slow external <script> needs longer.",
|
||||
default: "3000",
|
||||
},
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user