mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-09 12:00:26 +00:00
* fix(producer): localize remote <img> sources + await image readiness Producer's frame-capture has `pollVideosReady` (waits readyState >= 2 for every <video>) but no equivalent for <img>. Combined with htmlCompiler's `collectExternalAssets` explicitly skipping http(s) URLs (line 805-806), agent-pipeline-generated compositions (astral / daphne / hyperion multi-v2 outputs with raw S3 <img src>) reach Chrome with a network dependency that races the readiness gate AND can be evicted mid-render. Either path produces blank-frame flicker. Reproduction (02_kobe agent output, 42s render @ 30fps): scene_02's remote S3 background-image painted from t=7.0s, vanished at t=10.5s (frame size 139KB vs 700-940KB neighbors), back at t=11.0s. GSAP timeline said opacity:1 throughout — Chrome simply didn't have the pixels. Two-layer fix: 1. **Producer** — `localizeRemoteImageSources` in `htmlCompiler.ts` mirrors the existing `localizeRemoteMediaSources` (video/audio) + `localizeRemoteFontFaces` pattern, reusing `downloadAndRewriteUrls` and the `_remote_media/` subdir. Wired into `compileForRender` between the media and font localize steps. Once the file is local, Chrome's image cache is bounded by disk reads, not S3 latency. 2. **Engine** — `pollImagesReady` + `decodeAllImages` helpers in `frameCapture.ts` parallel to `pollVideosReady`. Waits for every `<img>` (skipping data: URIs) to have `complete && naturalWidth > 0`, then forces GPU upload via `img.decode()`. Called from both the classic-xvfb path and the BeginFrame path after their respective video readiness checks. Defense-in-depth — Layer 1 closes the symptom for current+future agent-pipeline outputs; Layer 2 protects any future code path that leaves a remote URL in place. Tests: 7 new cases in `htmlCompiler.test.ts` covering happy-path rewrite, 404 fallback, dedup of duplicate URLs, non-HTTP and data: URI passthrough, both quote styles, and the agent-pipeline shape where `src` is not the first attribute. All pass alongside the existing 56 htmlCompiler tests. * fix(producer): scope remote-img regex to real src; correct stale comments Review follow-ups on the remote-<img> localization fix: - Tighten REMOTE_IMG_TAG_RE with a (?<![\w-]) lookbehind so it matches a real `src` attribute only. The previous `\bsrc` also matched `data-src` (and `data-*-src`) lazy-loader placeholders, which would download/rewrite a URL the render never paints. Added a regression test; `srcset` stays excluded by the `\s*=` requirement. - Fix comments that claimed frameCapture has "no pollImagesReady analog" — this PR adds exactly that, so the docstrings were self-contradictory. Reframed localization as the primary fix and pollImagesReady as the defense-in-depth layer, and documented the <img src>-only scope (srcset / <picture> / SVG <image> / CSS background-image are follow-ups). Verified locally end-to-end on the 02_kobe repro: all 4 remote S3 <img> URLs localize to _remote_media/, the render completes, and the frame at t~10.5s that was a 139KB blank in the broken render now paints the trophy background in every native-fps frame. htmlCompiler.test.ts 64 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(engine): pollImagesReady broken-image escape + skip decode on in-flight Addresses two real bugs Magi caught in review on hf#1197: 1. pollImagesReady would spin the full pageReadyTimeout (45s default) for any <img> that settled with an error — Chrome marks 404 / decode failure / CORS rejection with (complete=true, naturalWidth=0), and the previous predicate `complete && naturalWidth > 0` returned false for those, so the poll ran to timeout. This is the HTMLImageElement equivalent of pollVideosReady's `ve.error` early-exit. Add a `complete && naturalWidth === 0` branch that treats settled-with- error as done — waiting won't make it load. Particularly relevant because localizeRemoteImageSources falls back to the original URL on download failure; that failed URL is now hit by a 45s stall instead of the broken-image marker rendering immediately. 2. decodeAllImages called img.decode() on every image, including those still in flight after pollImagesReady timed out. Per the WHATWG spec, decode() on a loading image awaits the fetch — never resolving until the network completes or puppeteer's evaluate timeout fires and throws an uncaught error that aborts the render. Pre-filter to only call decode() on images that successfully loaded. Test coverage: new frameCapture-pollImagesReady.test.ts with 8 cases covering empty docs, all-loaded, broken (complete + naturalWidth=0), data: URI, empty src, in-flight → resolves, in-flight → timeout, and the mixed batch. The broken-image test explicitly asserts elapsed < 500ms on a 1000ms timeout — guards against the regression Magi flagged. * docs(engine): clarify decodeAllImages prevents init race, not eviction Vai correctly noted that decode() forces initial GPU upload but does not prevent Chrome from evicting decoded pixels mid-render. The producer-side localizeRemoteImageSources is what bounds the eviction risk (local file-server paging vs S3 re-fetch). Comment updated to reflect that split of responsibilities. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
154 lines
5.9 KiB
TypeScript
154 lines
5.9 KiB
TypeScript
/**
|
|
* Tests for `pollImagesReady` — the image-side analog of `pollVideosReady`.
|
|
*
|
|
* Critical contract:
|
|
* - Successfully loaded image (complete=true, naturalWidth>0) → settled.
|
|
* - Broken / 404 image (complete=true, naturalWidth=0) → settled.
|
|
* This mirrors `pollVideosReady`'s `ve.error` early-exit. Without it,
|
|
* the htmlCompiler 404-fallback path (where a remote <img> URL failed
|
|
* to download and the original URL is preserved) would silently spin
|
|
* the full `pageReadyTimeout` budget waiting for an image that will
|
|
* never load — a 45 s regression vs the pre-PR behavior.
|
|
* - In-flight image (complete=false) → still waiting.
|
|
* - data: URI src → settled (no network fetch).
|
|
* - Empty src → settled (nothing to load).
|
|
*/
|
|
|
|
import { describe, expect, it } from "vitest";
|
|
import type { Page } from "puppeteer-core";
|
|
import { pollImagesReady } from "./frameCapture.js";
|
|
|
|
interface ImageSpec {
|
|
src: string;
|
|
complete: boolean;
|
|
naturalWidth: number;
|
|
}
|
|
|
|
// Mock `page` whose `evaluate(fn)` invokes `fn` with a Node-side `document`
|
|
// mock that returns synthetic image objects matching the spec. Snapshots the
|
|
// image state at evaluate-time, so callers can mutate `imgs` between polls
|
|
// to simulate progressive load completion.
|
|
function makeMockPage(imgs: () => ImageSpec[]): Page {
|
|
return {
|
|
evaluate: async (fn: () => unknown) => {
|
|
const snapshot = imgs();
|
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
|
const prevDoc = (globalThis as any).document;
|
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
|
(globalThis as any).document = {
|
|
querySelectorAll: () =>
|
|
snapshot.map((spec) => ({
|
|
getAttribute: (attr: string) => (attr === "src" ? spec.src : null),
|
|
complete: spec.complete,
|
|
naturalWidth: spec.naturalWidth,
|
|
})),
|
|
};
|
|
try {
|
|
return await fn();
|
|
} finally {
|
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
|
(globalThis as any).document = prevDoc;
|
|
}
|
|
},
|
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
|
} as any as Page;
|
|
}
|
|
|
|
describe("pollImagesReady", () => {
|
|
it("resolves immediately when there are no <img> elements", async () => {
|
|
const page = makeMockPage(() => []);
|
|
const result = await pollImagesReady(page, 1000, 10);
|
|
expect(result).toBe(true);
|
|
});
|
|
|
|
it("resolves immediately when every image has loaded successfully", async () => {
|
|
const page = makeMockPage(() => [
|
|
{ src: "/a.png", complete: true, naturalWidth: 100 },
|
|
{ src: "/b.png", complete: true, naturalWidth: 200 },
|
|
]);
|
|
const result = await pollImagesReady(page, 1000, 10);
|
|
expect(result).toBe(true);
|
|
});
|
|
|
|
it("treats a broken image (complete=true, naturalWidth=0) as settled — does NOT wait for timeout", async () => {
|
|
// This is the bug Magi flagged. Without the broken-image escape, this
|
|
// test would block the full 1000ms timeout and return false.
|
|
const page = makeMockPage(() => [
|
|
{ src: "/a.png", complete: true, naturalWidth: 100 },
|
|
{ src: "https://broken.example.com/404.png", complete: true, naturalWidth: 0 },
|
|
]);
|
|
const t0 = Date.now();
|
|
const result = await pollImagesReady(page, 1000, 10);
|
|
const elapsed = Date.now() - t0;
|
|
expect(result).toBe(true);
|
|
// Must resolve fast — well under the 1000ms timeout.
|
|
expect(elapsed).toBeLessThan(500);
|
|
});
|
|
|
|
it("treats a data: URI src as settled regardless of complete/naturalWidth", async () => {
|
|
const page = makeMockPage(() => [
|
|
{ src: "data:image/svg+xml,%3Csvg/%3E", complete: false, naturalWidth: 0 },
|
|
]);
|
|
const result = await pollImagesReady(page, 1000, 10);
|
|
expect(result).toBe(true);
|
|
});
|
|
|
|
it("treats an empty src as settled (nothing to load)", async () => {
|
|
const page = makeMockPage(() => [{ src: "", complete: false, naturalWidth: 0 }]);
|
|
const result = await pollImagesReady(page, 1000, 10);
|
|
expect(result).toBe(true);
|
|
});
|
|
|
|
it("waits for an in-flight image and resolves once it completes", async () => {
|
|
// Image starts in-flight, then completes after ~50ms.
|
|
let started = false;
|
|
const startTime = { value: 0 };
|
|
const page = makeMockPage(() => {
|
|
if (!started) {
|
|
started = true;
|
|
startTime.value = Date.now();
|
|
}
|
|
const elapsed = Date.now() - startTime.value;
|
|
const loaded = elapsed >= 50;
|
|
return [{ src: "/slow.png", complete: loaded, naturalWidth: loaded ? 100 : 0 }];
|
|
});
|
|
const result = await pollImagesReady(page, 1000, 10);
|
|
expect(result).toBe(true);
|
|
});
|
|
|
|
it("times out and returns false when an in-flight image never resolves", async () => {
|
|
// Image stays in-flight (complete=false) for the full timeout.
|
|
const page = makeMockPage(() => [
|
|
{ src: "/never-loads.png", complete: false, naturalWidth: 0 },
|
|
]);
|
|
const result = await pollImagesReady(page, 100, 10);
|
|
expect(result).toBe(false);
|
|
});
|
|
|
|
it("mixed batch: loaded + broken + data: + in-flight → waits only on the in-flight image", async () => {
|
|
let resolved = false;
|
|
const start = Date.now();
|
|
const page = makeMockPage(() => {
|
|
const elapsed = Date.now() - start;
|
|
if (elapsed >= 30) resolved = true;
|
|
return [
|
|
{ src: "/loaded.png", complete: true, naturalWidth: 800 },
|
|
{ src: "https://broken.example.com/404.jpg", complete: true, naturalWidth: 0 },
|
|
{ src: "data:image/svg+xml,abc", complete: false, naturalWidth: 0 },
|
|
{
|
|
src: "/in-flight.png",
|
|
complete: resolved,
|
|
naturalWidth: resolved ? 200 : 0,
|
|
},
|
|
];
|
|
});
|
|
const t0 = Date.now();
|
|
const result = await pollImagesReady(page, 1000, 10);
|
|
const elapsed = Date.now() - t0;
|
|
expect(result).toBe(true);
|
|
// Should wait roughly for the in-flight image to settle (~30ms) — not the
|
|
// full timeout. Allow generous slack for CI scheduler jitter.
|
|
expect(elapsed).toBeLessThan(500);
|
|
});
|
|
});
|