From 13f6c9e6861471e3ccf7979f1eaa314f0074d328 Mon Sep 17 00:00:00 2001 From: vedanth Date: Tue, 27 Oct 2026 08:40:38 +0000 Subject: [PATCH 1/2] Fix video trimmer recording failures --- .github/workflows/ci.yml | 1 + .../trimming-tool-recording-failure.test.ts | 199 ++++++++++++++++++ apps/web/components/tools/TrimmingTool.tsx | 101 +++++++-- 3 files changed, 280 insertions(+), 21 deletions(-) create mode 100644 apps/web/__tests__/unit/trimming-tool-recording-failure.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 70bb14815be..f7c81875af0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -166,6 +166,7 @@ jobs: __tests__/unit/toggle-pro-seat.test.ts __tests__/unit/transcript-hours.test.ts __tests__/unit/transcript-numbers.test.ts + __tests__/unit/trimming-tool-recording-failure.test.ts __tests__/unit/video-analytics-access.test.ts __tests__/unit/video-analytics-bulk.test.ts __tests__/unit/video-create-atomicity.test.ts diff --git a/apps/web/__tests__/unit/trimming-tool-recording-failure.test.ts b/apps/web/__tests__/unit/trimming-tool-recording-failure.test.ts new file mode 100644 index 00000000000..954f244f281 --- /dev/null +++ b/apps/web/__tests__/unit/trimming-tool-recording-failure.test.ts @@ -0,0 +1,199 @@ +// @vitest-environment jsdom + +import { type ButtonHTMLAttributes, createElement } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import { TrimmingTool } from "@/components/tools/TrimmingTool"; + +vi.mock("@cap/ui", () => ({ + Button: (props: ButtonHTMLAttributes) => + createElement("button", { ...props, type: "button" }), +})); +vi.mock("@/app/utils/analytics", () => ({ trackEvent: vi.fn() })); + +class Recorder { + static instances: Recorder[] = []; + static isTypeSupported = (type: string) => type === "video/webm"; + state = "inactive"; + ondataavailable: ((event: { data: Blob }) => void) | null = null; + onstop: (() => void) | null = null; + onerror: (() => void) | null = null; + constructor() { + Recorder.instances.push(this); + } + start() { + this.state = "recording"; + } + stop() { + this.state = "inactive"; + queueMicrotask(() => { + this.ondataavailable?.({ + data: new Blob(["partial"], { type: "video/webm" }), + }); + this.onstop?.(); + }); + } +} +let root: Root; +let container: HTMLDivElement; +let originalCreate: typeof URL.createObjectURL; +let originalRevoke: typeof URL.revokeObjectURL; +let stopTrack: ReturnType; +let madeUrls: string[]; + +beforeEach(() => { + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", false); + vi.stubGlobal("MediaRecorder", Recorder); + vi.stubGlobal("requestAnimationFrame", () => 1); + vi.stubGlobal("cancelAnimationFrame", vi.fn()); + vi.spyOn(console, "error").mockImplementation(() => {}); + vi.spyOn(console, "warn").mockImplementation(() => {}); + vi.spyOn(HTMLMediaElement.prototype, "duration", "get").mockReturnValue(6); + vi.spyOn(HTMLMediaElement.prototype, "currentTime", "get").mockReturnValue(0); + vi.spyOn(HTMLMediaElement.prototype, "currentTime", "set").mockImplementation( + () => {}, + ); + vi.spyOn(HTMLMediaElement.prototype, "play").mockResolvedValue(); + vi.spyOn(HTMLMediaElement.prototype, "pause").mockImplementation(() => {}); + vi.spyOn(HTMLCanvasElement.prototype, "getContext").mockReturnValue({ + drawImage: vi.fn(), + } as unknown as CanvasRenderingContext2D); + stopTrack = vi.fn(); + Object.defineProperty(HTMLCanvasElement.prototype, "captureStream", { + configurable: true, + value: () => ({ + addTrack: vi.fn(), + getVideoTracks: () => [{ stop: stopTrack }], + }), + }); + originalCreate = URL.createObjectURL; + originalRevoke = URL.revokeObjectURL; + madeUrls = []; + URL.createObjectURL = (blob) => { + const url = blob instanceof File ? "blob:input" : "blob:output"; + madeUrls.push(url); + return url; + }; + URL.revokeObjectURL = vi.fn(); + Recorder.instances = []; + container = document.createElement("div"); + document.body.append(container); + root = createRoot(container); + root.render(createElement(TrimmingTool)); +}); +afterEach(() => { + root.unmount(); + container.remove(); + URL.createObjectURL = originalCreate; + URL.revokeObjectURL = originalRevoke; + Reflect.deleteProperty(HTMLCanvasElement.prototype, "captureStream"); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); +const button = (label: string) => + Array.from(container.querySelectorAll("button")).find( + (element) => element.textContent === label, + ); +async function startTrim() { + await vi.waitFor(() => + expect(container.querySelector("input[type=file]")).not.toBeNull(), + ); + const input = container.querySelector("input[type=file]") as HTMLInputElement; + Object.defineProperty(input, "files", { + value: [new File(["video"], "clip.mp4", { type: "video/mp4" })], + }); + input.dispatchEvent(new Event("change", { bubbles: true })); + const video = container.querySelector("video") as HTMLVideoElement; + await vi.waitFor(() => expect(video.getAttribute("src")).toBe("blob:input")); + video.dispatchEvent(new Event("loadedmetadata")); + await vi.waitFor(() => expect(button("Trim Video")).toBeDefined()); + button("Trim Video")?.click(); + await vi.waitFor(() => expect(Recorder.instances).toHaveLength(1)); + return video; +} +it("discards a failed playback recording and allows retry", async () => { + vi.mocked(HTMLMediaElement.prototype.play).mockRejectedValueOnce( + new Error("Playback blocked"), + ); + await startTrim(); + await vi.waitFor(() => + expect(container.textContent).toContain("Playback blocked"), + ); + expect(Recorder.instances[0]?.state).toBe("inactive"); + expect(stopTrack).toHaveBeenCalledOnce(); + expect(madeUrls).not.toContain("blob:output"); + button("Trim Video")?.click(); + await vi.waitFor(() => expect(Recorder.instances).toHaveLength(2)); + (container.querySelector("video") as HTMLVideoElement).dispatchEvent( + new Event("ended"), + ); + await vi.waitFor(() => + expect(button("Download Trimmed Video")).toBeDefined(), + ); +}); +it("discards recorder errors instead of publishing partial output", async () => { + await startTrim(); + Recorder.instances[0]?.onerror?.(); + await vi.waitFor(() => + expect(container.textContent).toContain("Recording failed"), + ); + expect(stopTrack).toHaveBeenCalledOnce(); + expect(madeUrls).not.toContain("blob:output"); +}); +it("releases canvas capture after a successful trim", async () => { + const video = await startTrim(); + video.dispatchEvent(new Event("ended")); + await vi.waitFor(() => + expect(button("Download Trimmed Video")).toBeDefined(), + ); + expect(stopTrack).toHaveBeenCalledOnce(); +}); +it("discards a recording when the input video reports an error", async () => { + const video = await startTrim(); + video.dispatchEvent(new Event("error")); + await vi.waitFor(() => + expect(container.textContent).toContain("Video playback failed"), + ); + expect(Recorder.instances[0]?.state).toBe("inactive"); + expect(stopTrack).toHaveBeenCalledOnce(); + expect(madeUrls).not.toContain("blob:output"); +}); +it("rejects a recorder that stops before the requested end", async () => { + await startTrim(); + Recorder.instances[0]?.stop(); + await vi.waitFor(() => + expect(container.textContent).toContain("Recording stopped unexpectedly"), + ); + expect(stopTrack).toHaveBeenCalledOnce(); + expect(madeUrls).not.toContain("blob:output"); +}); +it("cancels capture when choosing another video", async () => { + await startTrim(); + button("Choose Different Video")?.click(); + await vi.waitFor(() => + expect(container.querySelector("input[type=file]")).not.toBeNull(), + ); + expect(Recorder.instances[0]?.state).toBe("inactive"); + expect(stopTrack).toHaveBeenCalledOnce(); + expect(madeUrls).not.toContain("blob:output"); + expect(container.textContent).not.toContain("Trim cancelled"); +}); +it("cancels capture when the trimmer unmounts", async () => { + await startTrim(); + root.unmount(); + await Promise.resolve(); + expect(Recorder.instances[0]?.state).toBe("inactive"); + expect(stopTrack).toHaveBeenCalledOnce(); + expect(madeUrls).not.toContain("blob:output"); +}); +it("cleans up if starting the recorder throws", async () => { + vi.spyOn(Recorder.prototype, "start").mockImplementationOnce(() => { + throw new Error("Recorder unavailable"); + }); + await startTrim(); + await vi.waitFor(() => + expect(container.textContent).toContain("Recorder unavailable"), + ); + expect(stopTrack).toHaveBeenCalledOnce(); + expect(madeUrls).not.toContain("blob:output"); +}); diff --git a/apps/web/components/tools/TrimmingTool.tsx b/apps/web/components/tools/TrimmingTool.tsx index 98614e19be3..0413e85f2c6 100644 --- a/apps/web/components/tools/TrimmingTool.tsx +++ b/apps/web/components/tools/TrimmingTool.tsx @@ -56,6 +56,7 @@ export const TrimmingTool = () => { const outputVideoRef = useRef(null); const mediaRecorderRef = useRef(null); const recordedChunksRef = useRef([]); + const cancelTrimRef = useRef<(() => void) | null>(null); const isVideoFile = (file: File): boolean => { return ( @@ -74,6 +75,7 @@ export const TrimmingTool = () => { }; const cleanupResources = () => { + cancelTrimRef.current?.(); if (fileState.videoSrc) { URL.revokeObjectURL(fileState.videoSrc); } @@ -293,6 +295,7 @@ export const TrimmingTool = () => { try { await processTrim(); } catch (err: any) { + if (err instanceof DOMException && err.name === "AbortError") return; console.error("Detailed processing error:", err); let errorMessage = "Trimming failed: "; @@ -331,6 +334,8 @@ export const TrimmingTool = () => { return; } + let cleanupSetup: (() => void) | null = null; + let failSetup: ((error: unknown) => void) | null = null; try { const video = videoPreviewRef.current; const canvas = document.createElement("canvas"); @@ -406,6 +411,10 @@ export const TrimmingTool = () => { } const stream = canvas.captureStream(60); + const canvasTracks = stream.getVideoTracks(); + cleanupSetup = () => { + for (const track of canvasTracks) track.stop(); + }; video.muted = false; if (includeAudio) { @@ -447,6 +456,56 @@ export const TrimmingTool = () => { }); mediaRecorderRef.current = recorder; recordedChunksRef.current = []; + let settled = false; + let completed = false; + let frame: number | null = null; + let safetyTimeout: ReturnType | null = null; + + const cleanup = () => { + if (frame !== null) cancelAnimationFrame(frame); + if (safetyTimeout !== null) clearTimeout(safetyTimeout); + video.removeEventListener("ended", handleEnded); + video.removeEventListener("error", handleVideoFailure); + video.pause(); + cleanupSetup?.(); + cleanupSetup = null; + if (mediaRecorderRef.current === recorder) + mediaRecorderRef.current = null; + if (cancelTrimRef.current === cancel) cancelTrimRef.current = null; + }; + const fail = (error: unknown) => { + if (settled) return; + settled = true; + recorder.ondataavailable = null; + recorder.onstop = null; + recorder.onerror = null; + try { + if (recorder.state !== "inactive") recorder.stop(); + } catch { + } finally { + cleanup(); + reject(error); + } + }; + failSetup = fail; + const cancel = () => + fail(new DOMException("Trim cancelled", "AbortError")); + const stopRecording = () => { + if (settled || recorder.state === "inactive") return; + completed = true; + try { + recorder.stop(); + } catch (error) { + fail(error); + } + }; + const handleEnded = () => stopRecording(); + const handleVideoFailure = () => + fail(new Error("Video playback failed")); + cancelTrimRef.current = cancel; + video.addEventListener("ended", handleEnded); + video.addEventListener("error", handleVideoFailure); + recorder.onerror = () => fail(new Error("Recording failed")); recorder.ondataavailable = (e) => { if (e.data.size > 0) { @@ -455,6 +514,13 @@ export const TrimmingTool = () => { }; recorder.onstop = () => { + if (settled) return; + if (!completed) { + fail(new Error("Recording stopped unexpectedly")); + return; + } + settled = true; + cleanup(); try { const chunks = recordedChunksRef.current; const blob = new Blob(chunks, { @@ -490,11 +556,10 @@ export const TrimmingTool = () => { let recordingStartTime: number | null = null; const captureFrame = () => { + frame = null; + if (settled) return; if (video.currentTime >= trimState.endTime) { - if (recorder.state === "recording") { - recorder.stop(); - video.pause(); - } + stopRecording(); return; } @@ -512,36 +577,29 @@ export const TrimmingTool = () => { progress: Math.round(progress), })); - requestAnimationFrame(captureFrame); + frame = requestAnimationFrame(captureFrame); }; video .play() .then(() => { - requestAnimationFrame(captureFrame); + if (!settled) frame = requestAnimationFrame(captureFrame); }) .catch((err) => { console.error("Error playing video:", err); - reject(err); + fail(err); }); - const safetyTimeout = setTimeout( - () => { - if (recorder.state === "recording") { - recorder.stop(); - } - }, + safetyTimeout = setTimeout( + () => fail(new Error("Video processing timed out")), duration * 1000 + 10000, ); - - video.onended = () => { - clearTimeout(safetyTimeout); - if (recorder.state === "recording") { - recorder.stop(); - } - }; } catch (err) { - reject(err); + if (failSetup) failSetup(err); + else { + cleanupSetup?.(); + reject(err); + } } }); }; @@ -725,6 +783,7 @@ export const TrimmingTool = () => { useEffect(() => { return () => { + cancelTrimRef.current?.(); if (latestVideoSrcRef.current) { URL.revokeObjectURL(latestVideoSrcRef.current); } From e734feafda07bbe2acf997e459e9e860dbe8d314 Mon Sep 17 00:00:00 2001 From: vedanth Date: Tue, 27 Oct 2026 08:45:38 +0000 Subject: [PATCH 2/2] Test trimmer timeout cleanup --- .../trimming-tool-recording-failure.test.ts | 30 ++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/apps/web/__tests__/unit/trimming-tool-recording-failure.test.ts b/apps/web/__tests__/unit/trimming-tool-recording-failure.test.ts index 954f244f281..da2567cc740 100644 --- a/apps/web/__tests__/unit/trimming-tool-recording-failure.test.ts +++ b/apps/web/__tests__/unit/trimming-tool-recording-failure.test.ts @@ -1,6 +1,6 @@ // @vitest-environment jsdom -import { type ButtonHTMLAttributes, createElement } from "react"; +import { act, type ButtonHTMLAttributes, createElement } from "react"; import { createRoot, type Root } from "react-dom/client"; import { afterEach, beforeEach, expect, it, vi } from "vitest"; import { TrimmingTool } from "@/components/tools/TrimmingTool"; @@ -82,6 +82,7 @@ beforeEach(() => { root.render(createElement(TrimmingTool)); }); afterEach(() => { + vi.useRealTimers(); root.unmount(); container.remove(); URL.createObjectURL = originalCreate; @@ -94,7 +95,7 @@ const button = (label: string) => Array.from(container.querySelectorAll("button")).find( (element) => element.textContent === label, ); -async function startTrim() { +async function startTrim(useFakeClock = false) { await vi.waitFor(() => expect(container.querySelector("input[type=file]")).not.toBeNull(), ); @@ -107,8 +108,14 @@ async function startTrim() { await vi.waitFor(() => expect(video.getAttribute("src")).toBe("blob:input")); video.dispatchEvent(new Event("loadedmetadata")); await vi.waitFor(() => expect(button("Trim Video")).toBeDefined()); - button("Trim Video")?.click(); - await vi.waitFor(() => expect(Recorder.instances).toHaveLength(1)); + if (useFakeClock) { + vi.useFakeTimers(); + await act(async () => button("Trim Video")?.click()); + expect(Recorder.instances).toHaveLength(1); + } else { + button("Trim Video")?.click(); + await vi.waitFor(() => expect(Recorder.instances).toHaveLength(1)); + } return video; } it("discards a failed playback recording and allows retry", async () => { @@ -197,3 +204,18 @@ it("cleans up if starting the recorder throws", async () => { expect(stopTrack).toHaveBeenCalledOnce(); expect(madeUrls).not.toContain("blob:output"); }); +it("discards a timed-out recording even if its stop callback arrives late", async () => { + await startTrim(true); + const recorder = Recorder.instances[0]; + if (!recorder) throw new Error("Missing recorder"); + const lateStop = recorder.onstop; + await act(async () => { + await vi.advanceTimersByTimeAsync(16_000); + lateStop?.(); + }); + expect(container.textContent).toContain("Video processing timed out"); + expect(recorder.state).toBe("inactive"); + expect(stopTrack).toHaveBeenCalledOnce(); + expect(madeUrls).not.toContain("blob:output"); + expect(button("Download Trimmed Video")).toBeUndefined(); +});