From 678ac17913084121b5da1a08bff5548ed67ffaac Mon Sep 17 00:00:00 2001 From: vedanth Date: Tue, 27 Oct 2026 02:55:06 +0000 Subject: [PATCH 1/3] fix(web): cancel Loom downloads when leaving the downloader --- .github/workflows/ci.yml | 1 + .../unit/loom-downloader-lifecycle.test.ts | 161 ++++++++++++++++++ apps/web/components/tools/LoomDownloader.tsx | 34 ++-- 3 files changed, 183 insertions(+), 13 deletions(-) create mode 100644 apps/web/__tests__/unit/loom-downloader-lifecycle.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 70bb14815be..457ca2b86cd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -141,6 +141,7 @@ jobs: __tests__/unit/invite-decline.test.ts __tests__/unit/linkified-text.test.ts __tests__/unit/loom-import-ui.test.ts + __tests__/unit/loom-downloader-lifecycle.test.ts __tests__/unit/media-comment-playback-cleanup.test.ts __tests__/unit/media-format-converter-selection.test.ts __tests__/unit/media-format-converter.test.ts diff --git a/apps/web/__tests__/unit/loom-downloader-lifecycle.test.ts b/apps/web/__tests__/unit/loom-downloader-lifecycle.test.ts new file mode 100644 index 00000000000..1e8dfd7c992 --- /dev/null +++ b/apps/web/__tests__/unit/loom-downloader-lifecycle.test.ts @@ -0,0 +1,161 @@ +// @vitest-environment jsdom + +import { act, type ComponentProps, createElement } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import { LoomDownloader } from "@/components/tools/LoomDownloader"; +import { saveLoomStreamAsMp4 } from "@/lib/loom-browser-conversion"; +import { resolveLoomBrowserDownload } from "@/lib/loom-browser-download"; + +vi.mock("@cap/ui", () => ({ + Button: ({ + spinner: _spinner, + ...props + }: ComponentProps<"button"> & { spinner?: boolean }) => + createElement("button", { ...props, type: "button" }), +})); +vi.mock("@/lib/loom-browser-download", () => ({ + resolveLoomBrowserDownload: vi.fn(), +})); +vi.mock("@/lib/loom-browser-conversion", () => ({ + saveLoomStreamAsMp4: vi.fn(), + getLoomBrowserConversionSupport: () => ({ supported: true }), + getLoomBrowserConversionErrorMessage: (error: Error) => error.message, + isLoomBrowserConversionAbort: (error: Error) => error.name === "AbortError", +})); + +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise((done) => { + resolve = done; + }); + return { promise, resolve }; +} +const downloadInfo = (direct = false) => ({ + success: true, + videoId: "fixture-video", + videoName: "Fixture video", + downloadUrl: `https://cdn.example/video.${direct ? "mp4" : "mpd"}`, + downloadMode: direct + ? ("direct-download" as const) + : ("browser-conversion" as const), +}); +let container: HTMLDivElement; +let root: Root; +let mounted: boolean; +let converter: ReturnType>; + +beforeEach(async () => { + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + vi.stubGlobal( + "URL", + class extends URL { + static createObjectURL = vi.fn(() => "blob:download"); + static revokeObjectURL = vi.fn(); + }, + ); + vi.spyOn(HTMLAnchorElement.prototype, "click").mockImplementation(() => {}); + vi.mocked(resolveLoomBrowserDownload) + .mockReset() + .mockResolvedValue(downloadInfo()); + converter = deferred(); + vi.mocked(saveLoomStreamAsMp4).mockReset().mockReturnValue(converter.promise); + container = document.createElement("div"); + document.body.append(container); + root = createRoot(container); + mounted = true; + await act(async () => root.render(createElement(LoomDownloader))); + const input = container.querySelector("input"); + if (!input) throw new Error("Missing Loom URL input"); + const setValue = Object.getOwnPropertyDescriptor( + HTMLInputElement.prototype, + "value", + )?.set; + setValue?.call(input, "https://www.loom.com/share/fixture-video"); + await act(async () => + input.dispatchEvent(new Event("input", { bubbles: true })), + ); +}); +afterEach(async () => { + if (mounted) await unmount(); + container.remove(); + vi.restoreAllMocks(); + vi.unstubAllGlobals(); +}); +async function unmount() { + await act(async () => root.unmount()); + mounted = false; +} +async function start() { + const button = Array.from(container.querySelectorAll("button")).find( + (button) => button.textContent === "Download Video", + ); + if (!button) throw new Error("Missing download action"); + await act(async () => button.click()); +} + +it("aborts the active conversion when the downloader unmounts", async () => { + await start(); + const signal = vi.mocked(saveLoomStreamAsMp4).mock.calls[0]?.[0].signal; + expect(signal?.aborted).toBe(false); + await unmount(); + expect(signal?.aborted).toBe(true); + await act(async () => converter.resolve(undefined)); +}); + +it("does not create a download or retain a Blob from late conversion completion", async () => { + await start(); + await unmount(); + await act(async () => + converter.resolve(new Blob(["MP4"], { type: "video/mp4" })), + ); + expect(URL.createObjectURL).not.toHaveBeenCalled(); + expect(HTMLAnchorElement.prototype.click).not.toHaveBeenCalled(); +}); + +it.each([false, true])( + "ignores late URL resolution after unmount (direct: %s)", + async (direct) => { + const resolver = + deferred>>(); + vi.mocked(resolveLoomBrowserDownload).mockReturnValueOnce(resolver.promise); + await start(); + await unmount(); + await act(async () => resolver.resolve(downloadInfo(direct))); + expect(saveLoomStreamAsMp4).not.toHaveBeenCalled(); + expect(HTMLAnchorElement.prototype.click).not.toHaveBeenCalled(); + await act(async () => converter.resolve(undefined)); + }, +); + +it("keeps a completed MP4 available until the downloader unmounts", async () => { + await start(); + await act(async () => + converter.resolve(new Blob(["MP4"], { type: "video/mp4" })), + ); + expect(HTMLAnchorElement.prototype.click).toHaveBeenCalledTimes(1); + expect(URL.createObjectURL).toHaveBeenCalledTimes(1); + expect(URL.revokeObjectURL).not.toHaveBeenCalled(); + await unmount(); + expect(URL.revokeObjectURL).toHaveBeenCalledWith("blob:download"); +}); + +it("still starts a direct MP4 download while mounted", async () => { + vi.mocked(resolveLoomBrowserDownload).mockResolvedValueOnce( + downloadInfo(true), + ); + await start(); + expect(HTMLAnchorElement.prototype.click).toHaveBeenCalledTimes(1); + expect(saveLoomStreamAsMp4).not.toHaveBeenCalled(); +}); + +it("allows another attempt after conversion fails", async () => { + vi.mocked(saveLoomStreamAsMp4).mockRejectedValueOnce( + new Error("Decoder failed"), + ); + await start(); + expect(container.textContent).toContain("Decoder failed"); + await start(); + expect(saveLoomStreamAsMp4).toHaveBeenCalledTimes(2); + await act(async () => converter.resolve(undefined)); +}); diff --git a/apps/web/components/tools/LoomDownloader.tsx b/apps/web/components/tools/LoomDownloader.tsx index bdbe7bc2fab..462016bb922 100644 --- a/apps/web/components/tools/LoomDownloader.tsx +++ b/apps/web/components/tools/LoomDownloader.tsx @@ -298,20 +298,21 @@ export function LoomDownloader() { useEffect( () => () => { + abortRef.current?.abort(); + abortRef.current = null; if (downloadObjectUrlRef.current) { URL.revokeObjectURL(downloadObjectUrlRef.current); + downloadObjectUrlRef.current = null; } }, [], ); const runBrowserConversion = useCallback( - async (conversion: BrowserConversion) => { + async (conversion: BrowserConversion, controller: AbortController) => { setStatus("converting"); setErrorMessage(""); setConvertProgress(0); - const controller = new AbortController(); - abortRef.current = controller; try { const convertedBlob = await saveLoomStreamAsMp4({ @@ -319,9 +320,10 @@ export function LoomDownloader() { filename: conversion.filename, signal: controller.signal, onProgress: ({ percent }) => { - setConvertProgress(percent); + if (!controller.signal.aborted) setConvertProgress(percent); }, }); + if (controller.signal.aborted) return; let objectUrl: string | null = null; if (convertedBlob) { objectUrl = triggerBlobDownload(convertedBlob, conversion.filename); @@ -332,6 +334,7 @@ export function LoomDownloader() { setLastDownloadedName(conversion.videoName); setStatus("success"); } catch (err) { + if (controller.signal.aborted) return; if ( (err instanceof DOMException && err.name === "AbortError") || isLoomBrowserConversionAbort(err) @@ -345,15 +348,15 @@ export function LoomDownloader() { getLoomBrowserConversionErrorMessage(err) ?? "An unexpected error occurred. Please try again.", ); - } finally { - abortRef.current = null; } }, [updateDownloadObjectUrl], ); const handleDownload = useCallback(async () => { - if (!url.trim()) return; + if (!url.trim() || abortRef.current) return; + const controller = new AbortController(); + abortRef.current = controller; setStatus("fetching"); setErrorMessage(""); @@ -361,6 +364,7 @@ export function LoomDownloader() { try { const result = await resolveLoomBrowserDownload(url.trim()); + if (controller.signal.aborted) return; if (!result.success || !result.videoId) { setStatus("error"); @@ -399,12 +403,16 @@ export function LoomDownloader() { return; } - await runBrowserConversion({ - url: result.downloadUrl, - filename, - videoName: result.videoName ?? "", - }); + await runBrowserConversion( + { + url: result.downloadUrl, + filename, + videoName: result.videoName ?? "", + }, + controller, + ); } catch (err) { + if (controller.signal.aborted) return; if (err instanceof DOMException && err.name === "AbortError") { setStatus("idle"); return; @@ -419,7 +427,7 @@ export function LoomDownloader() { "An unexpected error occurred. Please try again.", ); } finally { - abortRef.current = null; + if (abortRef.current === controller) abortRef.current = null; } }, [runBrowserConversion, updateDownloadObjectUrl, url]); From 6be5d5cc4ea78b96533c5ec540ae06f66a67ec8a Mon Sep 17 00:00:00 2001 From: vedanth Date: Tue, 27 Oct 2026 07:40:31 +0000 Subject: [PATCH 2/3] fix(loom): honor cancellation during conversion setup --- .github/workflows/ci.yml | 1 + .../unit/loom-conversion-abort.test.ts | 123 ++++++++++++++++++ apps/web/lib/loom-browser-conversion.ts | 6 + 3 files changed, 130 insertions(+) create mode 100644 apps/web/__tests__/unit/loom-conversion-abort.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 457ca2b86cd..54fe4ccd7a3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -142,6 +142,7 @@ jobs: __tests__/unit/linkified-text.test.ts __tests__/unit/loom-import-ui.test.ts __tests__/unit/loom-downloader-lifecycle.test.ts + __tests__/unit/loom-conversion-abort.test.ts __tests__/unit/media-comment-playback-cleanup.test.ts __tests__/unit/media-format-converter-selection.test.ts __tests__/unit/media-format-converter.test.ts diff --git a/apps/web/__tests__/unit/loom-conversion-abort.test.ts b/apps/web/__tests__/unit/loom-conversion-abort.test.ts new file mode 100644 index 00000000000..ced8b067631 --- /dev/null +++ b/apps/web/__tests__/unit/loom-conversion-abort.test.ts @@ -0,0 +1,123 @@ +// @vitest-environment jsdom +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import { saveLoomStreamAsMp4 } from "@/lib/loom-browser-conversion"; + +const mocks = vi.hoisted(() => ({ + video: vi.fn(), + audio: vi.fn(), + init: vi.fn(), + execute: vi.fn(), + cancel: vi.fn(), + dispose: vi.fn(), + abort: vi.fn(), +})); +vi.mock("mediabunny", () => { + class Empty {} + return { + ALL_FORMATS: [], + QUALITY_HIGH: 1, + QUALITY_MEDIUM: 1, + CustomPathedSource: Empty, + UrlSource: Empty, + Mp4OutputFormat: Empty, + BufferTarget: class { + buffer = new Uint8Array([1]).buffer; + }, + StreamTarget: Empty, + Input: class { + dispose = mocks.dispose; + }, + Output: class { + cancel = mocks.cancel; + }, + canEncodeVideo: mocks.video, + canEncodeAudio: mocks.audio, + Conversion: { init: mocks.init }, + }; +}); +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise((done) => { + resolve = done; + }); + return { promise, resolve }; +} +beforeEach(() => { + for (const key of [ + "VideoDecoder", + "VideoEncoder", + "AudioDecoder", + "AudioEncoder", + ]) + vi.stubGlobal(key, class {}); + vi.stubGlobal("showSaveFilePicker", undefined); + mocks.video.mockReset().mockResolvedValue(true); + mocks.audio.mockReset().mockResolvedValue(true); + mocks.execute.mockReset().mockResolvedValue(undefined); + mocks.cancel.mockReset().mockResolvedValue(undefined); + mocks.dispose.mockReset(); + mocks.abort.mockReset().mockResolvedValue(undefined); + mocks.init.mockReset().mockResolvedValue({ + isValid: true, + discardedTracks: [], + execute: mocks.execute, + cancel: mocks.cancel, + output: { cancel: mocks.cancel }, + }); +}); +afterEach(() => vi.unstubAllGlobals()); +const start = (signal: AbortSignal) => + saveLoomStreamAsMp4({ + url: "https://example.com/video.m3u8", + filename: "video.mp4", + signal, + }); +it("does not open the save picker when aborted during codec checks", async () => { + const gate = deferred(); + mocks.video.mockReturnValueOnce(gate.promise); + const picker = vi.fn(); + vi.stubGlobal("showSaveFilePicker", picker); + const controller = new AbortController(); + const result = start(controller.signal); + const rejected = expect(result).rejects.toMatchObject({ name: "AbortError" }); + await vi.waitFor(() => expect(mocks.video).toHaveBeenCalled()); + controller.abort(); + gate.resolve(true); + await rejected; + expect(picker).not.toHaveBeenCalled(); + expect(mocks.init).not.toHaveBeenCalled(); +}); +it("does not execute when aborted while conversion initializes", async () => { + const gate = deferred(); + mocks.init.mockReturnValueOnce(gate.promise); + const controller = new AbortController(); + const result = start(controller.signal); + const rejected = expect(result).rejects.toMatchObject({ name: "AbortError" }); + await vi.waitFor(() => expect(mocks.init).toHaveBeenCalled()); + controller.abort(); + gate.resolve({ + isValid: true, + discardedTracks: [], + execute: mocks.execute, + cancel: mocks.cancel, + output: { cancel: mocks.cancel }, + }); + await rejected; + expect(mocks.execute).not.toHaveBeenCalled(); + expect(mocks.cancel).toHaveBeenCalled(); + expect(mocks.dispose).toHaveBeenCalled(); +}); +it("aborts the writable when cancellation arrives while the picker is open", async () => { + const gate = deferred(); + const picker = vi.fn(() => gate.promise); + vi.stubGlobal("showSaveFilePicker", picker); + const controller = new AbortController(); + const result = start(controller.signal); + const rejected = expect(result).rejects.toMatchObject({ name: "AbortError" }); + await vi.waitFor(() => expect(picker).toHaveBeenCalled()); + controller.abort(); + gate.resolve({ createWritable: async () => ({ abort: mocks.abort }) }); + await rejected; + expect(mocks.abort).toHaveBeenCalled(); + expect(mocks.init).not.toHaveBeenCalled(); +}); diff --git a/apps/web/lib/loom-browser-conversion.ts b/apps/web/lib/loom-browser-conversion.ts index af22a2134db..df0f2b0325c 100644 --- a/apps/web/lib/loom-browser-conversion.ts +++ b/apps/web/lib/loom-browser-conversion.ts @@ -789,6 +789,7 @@ export async function saveLoomStreamAsMp4({ try { const mediabunny = await import("mediabunny"); + if (signal?.aborted) throw createAbortError(); const { ALL_FORMATS, Conversion, @@ -806,12 +807,14 @@ export async function saveLoomStreamAsMp4({ } = mediabunny; await assertMp4EncodingSupported(canEncodeVideo, canEncodeAudio); + if (signal?.aborted) throw createAbortError(); conversionTarget = await createConversionTarget({ filename, BufferTarget, StreamTarget, }); + if (signal?.aborted) throw createAbortError(); if (isDashManifestUrl(url)) { await convertDashWebmManifestToMp4({ @@ -821,6 +824,7 @@ export async function saveLoomStreamAsMp4({ onProgress, mediabunny, }); + if (signal?.aborted) throw createAbortError(); completed = true; return getBufferTargetBlob(conversionTarget); } @@ -874,6 +878,7 @@ export async function saveLoomStreamAsMp4({ }, showWarnings: false, }); + if (signal?.aborted) throw createAbortError(); if (!conversion.isValid) { const reasons = conversion.discardedTracks.map((track) => track.reason); @@ -894,6 +899,7 @@ export async function saveLoomStreamAsMp4({ signal?.addEventListener("abort", abortConversion, { once: true }); await conversion.execute(); + if (signal?.aborted) throw createAbortError(); completed = true; onProgress?.({ percent: 100, processedSeconds: 0 }); return getBufferTargetBlob(conversionTarget); From 042106caa281a4e59867f4af6ca5a0a5f3d6523c Mon Sep 17 00:00:00 2001 From: vedanth Date: Tue, 27 Oct 2026 07:41:52 +0000 Subject: [PATCH 3/3] test(loom): preserve cancellation across codec setup --- apps/web/__tests__/unit/loom-conversion-abort.test.ts | 3 +++ apps/web/lib/loom-browser-conversion.ts | 1 + 2 files changed, 4 insertions(+) diff --git a/apps/web/__tests__/unit/loom-conversion-abort.test.ts b/apps/web/__tests__/unit/loom-conversion-abort.test.ts index ced8b067631..c685effe68e 100644 --- a/apps/web/__tests__/unit/loom-conversion-abort.test.ts +++ b/apps/web/__tests__/unit/loom-conversion-abort.test.ts @@ -26,6 +26,9 @@ vi.mock("mediabunny", () => { StreamTarget: Empty, Input: class { dispose = mocks.dispose; + async getPrimaryAudioTrack() { + return null; + } }, Output: class { cancel = mocks.cancel; diff --git a/apps/web/lib/loom-browser-conversion.ts b/apps/web/lib/loom-browser-conversion.ts index df0f2b0325c..382f8163b8d 100644 --- a/apps/web/lib/loom-browser-conversion.ts +++ b/apps/web/lib/loom-browser-conversion.ts @@ -858,6 +858,7 @@ export async function saveLoomStreamAsMp4({ formats: ALL_FORMATS, }); + if (signal?.aborted) throw createAbortError(); const output = new Output({ format: new Mp4OutputFormat({ fastStart: false }), target: conversionTarget.target,