Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
153 changes: 153 additions & 0 deletions packages/engine/src/services/frameCapture-pollImagesReady.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
/**
* 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);
});
});
125 changes: 125 additions & 0 deletions packages/engine/src/services/frameCapture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,89 @@ async function pollVideosReady(
return check();
}

// Wait for every `<img>` with a non-`data:` src to have settled — either
// successfully loaded (`complete && naturalWidth > 0`) or failed with a
// broken-image marker (`complete && naturalWidth === 0`, the HTMLImageElement
// equivalent of HTMLMediaElement.error). htmlCompiler localises remote `<img>`
// URLs to the local file server before this point, so in practice this polls
// for the local fetch to land — but the guard is a defensive net so that any
// future composition path that leaves a remote URL in place won't capture
// frames before the pixels arrive. Mirrors `pollVideosReady` for parity with
// the video-side readiness contract (videos exit-early on `ve.error`; images
// exit-early on `complete && naturalWidth === 0`).
/** @internal exported for unit testing only */
export async function pollImagesReady(
page: Page,
timeoutMs: number,
intervalMs: number = 100,
): Promise<boolean> {
const check = async (): Promise<boolean> => {
return Boolean(
await page.evaluate(() => {
const imgs = Array.from(document.querySelectorAll("img"));
return (
imgs.length === 0 ||
imgs.every((img) => {
const ie = img as HTMLImageElement;
const src = ie.getAttribute("src") || "";
if (!src || src.startsWith("data:")) return true;
// A `complete` image with zero naturalWidth has settled with an
// error (404 / decode failure / CORS rejection / blocked). Treat
// as done — waiting won't make it load — and let the render
// continue with the broken-image marker visible. Mirrors how
// pollVideosReady treats `ve.error`.
if (ie.complete && ie.naturalWidth === 0) return true;
if (ie.complete && ie.naturalWidth > 0) return true;
return false;
})
);
}),
);
};
const deadline = Date.now() + timeoutMs;
while (Date.now() < deadline) {
if (await check()) return true;
await new Promise((resolve) => setTimeout(resolve, intervalMs));
}
return check();
}

// Force every successfully-loaded `<img>` to be GPU-uploaded before the first
// frame capture. `naturalWidth > 0` means the bitmap has been decoded into
// CPU memory, but compositor-side GPU upload can still happen lazily on first
// paint. Calling `img.decode()` returns a Promise that resolves once the image
// is ready for synchronous painting — eliminating the small first-frame race
// between "image is technically loaded" and "the rasterized texture is on the
// GPU and ready to composite".
//
// Note this is purely an init-time guard; it doesn't prevent Chrome from
// evicting decoded pixels mid-render. The producer-side `localizeRemoteImageSources`
// is what bounds the eviction risk (a re-fetch hits the local file server's
// disk-backed paging, not S3 over the network).
//
// Critical: `decode()` on an in-flight image waits for the fetch to resolve.
// If `pollImagesReady` timed out with some images still loading (`!complete`),
// calling `decode()` on them would block here until the network finally
// completes — or until puppeteer's evaluate timeout fires and throws an
// uncaught error that aborts the render. Skip in-flight and broken images;
// only force GPU upload for images that successfully loaded.
async function decodeAllImages(page: Page): Promise<void> {
await page.evaluate(async () => {
const imgs = Array.from(document.querySelectorAll("img"));
await Promise.all(
imgs.map((img) => {
const ie = img as HTMLImageElement;
if (typeof ie.decode !== "function") return Promise.resolve();
// Skip still-loading images (in-flight decode() would hang) and
// broken images (decode() rejects, but pre-filtering is clearer
// than relying on the .catch).
if (!ie.complete || ie.naturalWidth === 0) return Promise.resolve();
return ie.decode().catch(() => undefined);
}),
);
});
}

async function applyVideoMetadataHints(
page: Page,
hints: readonly CaptureVideoMetadataHint[] | undefined,
Expand Down Expand Up @@ -707,6 +790,26 @@ export async function initializeSession(session: CaptureSession): Promise<void>
);
}

const imagesReady = await pollImagesReady(page, pageReadyTimeout);
if (!imagesReady) {
const failedImages = await page.evaluate(() => {
return Array.from(document.querySelectorAll("img"))
.filter((img) => {
const ie = img as HTMLImageElement;
const src = ie.getAttribute("src") || "";
if (!src || src.startsWith("data:")) return false;
return !(ie.complete && ie.naturalWidth > 0);
})
.map((img) => (img as HTMLImageElement).src || img.getAttribute("src") || "(no src)")
.join(", ");
});
console.warn(
`[FrameCapture] Some image elements did not load within ${pageReadyTimeout}ms: ${failedImages}. ` +
`Continuing render — affected images may appear blank/missing in early frames.`,
);
}
await decodeAllImages(page);

await page.evaluate(`document.fonts?.ready`);
await waitForOptionalTailwindReady(page, pageReadyTimeout);

Expand Down Expand Up @@ -812,6 +915,28 @@ export async function initializeSession(session: CaptureSession): Promise<void>
);
}

// Image readiness — parity with pollVideosReady. Defense against remote
// <img> URLs that bypass the htmlCompiler localize step.
const bfImagesReady = await pollImagesReady(page, pageReadyTimeout);
if (!bfImagesReady) {
const failedImages = await page.evaluate(() => {
return Array.from(document.querySelectorAll("img"))
.filter((img) => {
const ie = img as HTMLImageElement;
const src = ie.getAttribute("src") || "";
if (!src || src.startsWith("data:")) return false;
return !(ie.complete && ie.naturalWidth > 0);
})
.map((img) => (img as HTMLImageElement).src || img.getAttribute("src") || "(no src)")
.join(", ");
});
console.warn(
`[FrameCapture] Some image elements did not load within ${pageReadyTimeout}ms: ${failedImages}. ` +
`Continuing render — affected images may appear blank/missing in early frames.`,
);
}
await decodeAllImages(page);

// Font check (no rAF dependency — uses fonts.ready API directly)
await page.evaluate(`document.fonts?.ready`);
await waitForOptionalTailwindReady(page, pageReadyTimeout);
Expand Down
Loading
Loading