fix(studio): stop VideoFrameThumbnail error→cleanup infinite loop (#4037)

The error handler called cleanup(), which sets video.src = "" and calls
load(). Clearing src itself fires another error event, re-entering the
handler at event-loop speed — every pass allocating a React update via
setFailed(true). The seeked path triggers the same loop: its cleanup
provokes the synthetic error right after a successful frame extract.

Detach the error listener inside cleanup() and ignore the synthetic
empty-src error. Instrumentation on a real session measured ~475k src
assignments/second and ~8.6 MB/s of update allocations before the fix.

Fixes #4036

Co-authored-by: CasbaL <26603379+CasbaL@users.noreply.github.com>
This commit is contained in:
CasbaL
2026-09-23 01:00:15 -04:00
committed by GitHub
co-authored by CasbaL
parent 1b8f8a43d4
commit e5eeeef972
2 changed files with 180 additions and 5 deletions
@@ -0,0 +1,170 @@
// @vitest-environment happy-dom
import { act } from "react";
import { createRoot, type Root } from "react-dom/client";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { VideoFrameThumbnail } from "./VideoFrameThumbnail";
Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true });
// The component builds its thumbnail from document.createElement("video"/"canvas"),
// so both are faked here. The video fake records load() calls and keeps listener
// sets, letting a test re-fire an event handler after cleanup removed it — the
// exact shape of the error→cleanup→error loop this suite guards against.
interface FakeVideo {
crossOrigin: string;
muted: boolean;
preload: string;
duration: number;
videoWidth: number;
videoHeight: number;
currentTime: number;
loadCalls: number;
_src: string;
src: string;
addEventListener(type: string, fn: () => void): void;
removeEventListener(type: string, fn: () => void): void;
getAttribute(name: string): string | null;
load(): void;
dispatch(type: string): void;
}
function makeVideo(): FakeVideo {
const listeners = new Map<string, Set<() => void>>();
const video: FakeVideo = {
crossOrigin: "",
muted: false,
preload: "",
duration: 10,
videoWidth: 640,
videoHeight: 360,
currentTime: 0,
loadCalls: 0,
_src: "",
src: "",
addEventListener(type, fn) {
if (!listeners.has(type)) listeners.set(type, new Set());
listeners.get(type)!.add(fn);
},
removeEventListener(type, fn) {
listeners.get(type)?.delete(fn);
},
getAttribute(name) {
return name === "src" ? video.src : null;
},
load() {
video.loadCalls++;
},
dispatch(type) {
for (const fn of [...(listeners.get(type) ?? [])]) fn();
},
};
Object.defineProperty(video, "src", {
get: () => video._src,
set: (v: string) => {
video._src = v;
},
});
return video;
}
const canvas = {
width: 0,
height: 0,
getContext: () => ({ drawImage: () => {} }),
toDataURL: () => "data:image/jpeg;base64,AAAA",
};
describe("VideoFrameThumbnail", () => {
let videos: FakeVideo[];
let root: Root;
let container: HTMLDivElement;
beforeEach(() => {
videos = [];
const original = document.createElement.bind(document);
vi.spyOn(document, "createElement").mockImplementation(((tag: string) => {
if (tag === "video") {
const v = makeVideo();
videos.push(v);
return v as unknown as HTMLVideoElement;
}
if (tag === "canvas") return canvas as unknown as HTMLCanvasElement;
return original(tag);
}) as typeof document.createElement);
container = document.createElement("div");
document.body.append(container);
root = createRoot(container);
});
afterEach(() => {
act(() => root.unmount());
container.remove();
vi.restoreAllMocks();
});
const render = (props: { src: string; fallbackLabel?: string }) => {
act(() => root.render(<VideoFrameThumbnail {...props} />));
return videos[videos.length - 1];
};
it("renders the fallback label when the video errors", () => {
render({ src: "missing.mp4", fallbackLabel: "VIDEO" });
const video = videos[0];
expect(video.src).toBe("missing.mp4");
expect(video.loadCalls).toBe(1);
act(() => video.dispatch("error"));
expect(container.textContent).toContain("VIDEO");
// cleanup ran once: detached the error listener and reset the media element
expect(video.src).toBe("");
expect(video.loadCalls).toBe(2);
});
it("does not loop when the cleared src fires a synthetic error", () => {
render({ src: "missing.mp4", fallbackLabel: "VIDEO" });
const video = videos[0];
act(() => video.dispatch("error"));
expect(video.loadCalls).toBe(2);
// The empty src makes the browser fire `error` again; the detached handler
// must stay detached — repeated dispatches must not touch load() anymore.
act(() => {
video.dispatch("error");
video.dispatch("error");
video.dispatch("error");
});
expect(video.loadCalls).toBe(2);
expect(container.textContent).toContain("VIDEO");
});
it("keeps the extracted frame and stays inert after a post-seek synthetic error", () => {
render({ src: "clip.mp4" });
const video = videos[0];
act(() => video.dispatch("loadedmetadata"));
expect(video.currentTime).toBe(1); // 10% of a 10s clip
act(() => video.dispatch("seeked"));
const img = container.querySelector("img");
expect(img?.getAttribute("src")).toBe("data:image/jpeg;base64,AAAA");
expect(video.src).toBe("");
expect(video.loadCalls).toBe(2);
act(() => {
video.dispatch("error");
video.dispatch("error");
});
expect(container.querySelector("img")?.getAttribute("src")).toBe("data:image/jpeg;base64,AAAA");
expect(video.loadCalls).toBe(2);
});
it("retries with a fresh video element when src changes", () => {
render({ src: "a.mp4" });
act(() => root.render(<VideoFrameThumbnail src="b.mp4" />));
expect(videos.length).toBe(2);
expect(videos[1].src).toBe("b.mp4");
});
});
@@ -26,10 +26,19 @@ export function VideoFrameThumbnail({
const canvas = document.createElement("canvas");
const ctx = canvas.getContext("2d");
// Clearing src fires one more `error` on the video, so the error handler
// must be detached first — otherwise error → cleanup → error spins forever.
const cleanup = () => {
video.removeEventListener("error", onError);
video.src = "";
video.load();
};
const onError = () => {
// Ignore the synthetic error cleanup itself just triggered.
if (!video.getAttribute("src")) return;
setFailed(true);
cleanup();
};
video.addEventListener("loadedmetadata", () => {
video.currentTime = Math.min(2, video.duration * 0.1 || 2);
@@ -44,11 +53,7 @@ export function VideoFrameThumbnail({
cleanup();
});
video.addEventListener("error", () => {
// Resolve the loading state — a permanent shimmer reads as "still loading".
setFailed(true);
cleanup();
});
video.addEventListener("error", onError);
video.src = src;
video.load();