From 8a362e605cf34cd48896418b2ba5116506675c7e Mon Sep 17 00:00:00 2001 From: AHMED Date: Mon, 14 Sep 2026 20:21:34 +0500 Subject: [PATCH 1/3] fix(captions): guard webContents and abort Whisper model download on window close --- electron/ipc/captions/whisper.test.ts | 116 ++++++++++++++++++++++++ electron/ipc/captions/whisper.ts | 121 ++++++++++++++++++++++---- 2 files changed, 220 insertions(+), 17 deletions(-) create mode 100644 electron/ipc/captions/whisper.test.ts diff --git a/electron/ipc/captions/whisper.test.ts b/electron/ipc/captions/whisper.test.ts new file mode 100644 index 000000000..1b639e841 --- /dev/null +++ b/electron/ipc/captions/whisper.test.ts @@ -0,0 +1,116 @@ +import type Electron from "electron"; +import { describe, expect, it, vi } from "vitest"; + +vi.mock("electron", () => ({ + app: { + getPath: () => "/mock-user-data", + }, +})); + +import { + downloadFileWithProgress, + downloadWhisperSmallModel, + sendWhisperModelDownloadProgress, +} from "./whisper"; + +describe("sendWhisperModelDownloadProgress", () => { + it("no-ops safely when webContents is null or undefined", () => { + expect(() => + sendWhisperModelDownloadProgress(null, { + status: "downloading", + progress: 50, + }), + ).not.toThrow(); + + expect(() => + sendWhisperModelDownloadProgress(undefined, { + status: "downloading", + progress: 50, + }), + ).not.toThrow(); + }); + + it("no-ops safely when webContents is destroyed", () => { + const sendMock = vi.fn(); + const mockWebContents = { + isDestroyed: () => true, + send: sendMock, + } as unknown as Electron.WebContents; + + expect(() => + sendWhisperModelDownloadProgress(mockWebContents, { + status: "downloading", + progress: 50, + }), + ).not.toThrow(); + + expect(sendMock).not.toHaveBeenCalled(); + }); + + it("safely catches error if webContents is destroyed during send", () => { + const mockWebContents = { + isDestroyed: () => false, + send: vi.fn(() => { + throw new Error("Object has been destroyed"); + }), + } as unknown as Electron.WebContents; + + expect(() => + sendWhisperModelDownloadProgress(mockWebContents, { + status: "downloading", + progress: 50, + }), + ).not.toThrow(); + }); + + it("sends IPC progress when webContents is alive", () => { + const sendMock = vi.fn(); + const mockWebContents = { + isDestroyed: () => false, + send: sendMock, + } as unknown as Electron.WebContents; + + sendWhisperModelDownloadProgress(mockWebContents, { + status: "downloading", + progress: 75, + path: null, + }); + + expect(sendMock).toHaveBeenCalledWith("whisper-small-model-download-progress", { + status: "downloading", + progress: 75, + path: null, + }); + }); +}); + +describe("downloadWhisperSmallModel", () => { + it("rejects immediately if webContents is already destroyed", async () => { + const mockWebContents = { + isDestroyed: () => true, + once: vi.fn(), + removeListener: vi.fn(), + send: vi.fn(), + } as unknown as Electron.WebContents; + + await expect(downloadWhisperSmallModel(mockWebContents)).rejects.toThrow( + "Window was closed before Whisper model download could start.", + ); + }); +}); + +describe("downloadFileWithProgress", () => { + it("rejects immediately when passed an already-aborted signal", async () => { + const abortController = new AbortController(); + abortController.abort(); + + await expect( + downloadFileWithProgress( + "https://example.com/test.bin", + "/tmp/test.bin", + vi.fn(), + abortController.signal, + ), + ).rejects.toThrow("Download aborted"); + }); +}); diff --git a/electron/ipc/captions/whisper.ts b/electron/ipc/captions/whisper.ts index 67b04cd58..c19f97500 100644 --- a/electron/ipc/captions/whisper.ts +++ b/electron/ipc/captions/whisper.ts @@ -10,7 +10,7 @@ import { } from "../constants"; export function sendWhisperModelDownloadProgress( - webContents: Electron.WebContents, + webContents: Electron.WebContents | null | undefined, payload: { status: "idle" | "downloading" | "downloaded" | "error"; progress: number; @@ -18,7 +18,15 @@ export function sendWhisperModelDownloadProgress( error?: string; }, ) { - webContents.send("whisper-small-model-download-progress", payload); + if (!webContents || webContents.isDestroyed()) { + return; + } + + try { + webContents.send("whisper-small-model-download-progress", payload); + } catch { + // webContents may have been destroyed concurrently + } } export async function getWhisperSmallModelStatus() { @@ -42,16 +50,48 @@ export function downloadFileWithProgress( url: string, destinationPath: string, onProgress: (progress: number) => void, + signal?: AbortSignal, ): Promise { const request = (currentUrl: string, redirectCount = 0): Promise => { return new Promise((resolve, reject) => { + if (signal?.aborted) { + reject(new Error("Download aborted")); + return; + } + + let fileStream: ReturnType | null = null; + let settled = false; + + const onAbort = () => { + if (settled) return; + settled = true; + cleanupSignal(); + if (fileStream) { + fileStream.destroy(); + } + req.destroy(new Error("Download aborted")); + reject(new Error("Download aborted")); + }; + + const cleanupSignal = () => { + if (signal) { + signal.removeEventListener("abort", onAbort); + } + }; + + if (signal) { + signal.addEventListener("abort", onAbort, { once: true }); + } + const req = httpsGet(currentUrl, { timeout: 30_000 }, (response) => { const statusCode = response.statusCode ?? 0; const location = response.headers.location; if (statusCode >= 300 && statusCode < 400 && location) { response.resume(); + cleanupSignal(); if (redirectCount >= 5) { + settled = true; reject(new Error("Too many redirects while downloading Whisper model.")); return; } @@ -65,6 +105,8 @@ export function downloadFileWithProgress( if (statusCode < 200 || statusCode >= 300) { response.resume(); + cleanupSignal(); + settled = true; reject(new Error(`Whisper model download failed with status ${statusCode}.`)); return; } @@ -74,7 +116,7 @@ export function downloadFileWithProgress( 10, ); let downloadedBytes = 0; - const fileStream = createWriteStream(destinationPath); + fileStream = createWriteStream(destinationPath); response.on("data", (chunk: Buffer) => { downloadedBytes += chunk.length; @@ -84,25 +126,50 @@ export function downloadFileWithProgress( }); response.on("error", (error) => { - fileStream.destroy(error); + cleanupSignal(); + if (!settled) { + settled = true; + fileStream?.destroy(error); + reject(error); + } }); fileStream.on("error", (error) => { - response.destroy(error); - reject(error); + cleanupSignal(); + if (!settled) { + settled = true; + response.destroy(error); + reject(error); + } }); fileStream.on("finish", () => { - onProgress(100); - resolve(); + cleanupSignal(); + if (!settled) { + settled = true; + onProgress(100); + resolve(); + } }); response.pipe(fileStream); }); - req.on("error", reject); + req.on("error", (error) => { + cleanupSignal(); + if (!settled) { + settled = true; + reject(error); + } + }); + req.on("timeout", () => { - req.destroy(new Error("Whisper model download timed out.")); + cleanupSignal(); + if (!settled) { + settled = true; + req.destroy(new Error("Whisper model download timed out.")); + reject(new Error("Whisper model download timed out.")); + } }); }); }; @@ -116,6 +183,17 @@ export async function downloadWhisperSmallModel( await fs.mkdir(WHISPER_MODEL_DIR, { recursive: true }); const tempPath = `${WHISPER_SMALL_MODEL_PATH}.download`; + if (webContents.isDestroyed()) { + throw new Error("Window was closed before Whisper model download could start."); + } + + const abortController = new AbortController(); + const handleDestroyed = () => { + abortController.abort(); + }; + + webContents.once("destroyed", handleDestroyed); + sendWhisperModelDownloadProgress(webContents, { status: "downloading", progress: 0, @@ -124,13 +202,18 @@ export async function downloadWhisperSmallModel( try { await fs.rm(tempPath, { force: true }); - await downloadFileWithProgress(WHISPER_MODEL_DOWNLOAD_URL, tempPath, (progress) => { - sendWhisperModelDownloadProgress(webContents, { - status: "downloading", - progress, - path: null, - }); - }); + await downloadFileWithProgress( + WHISPER_MODEL_DOWNLOAD_URL, + tempPath, + (progress) => { + sendWhisperModelDownloadProgress(webContents, { + status: "downloading", + progress, + path: null, + }); + }, + abortController.signal, + ); await fs.rename(tempPath, WHISPER_SMALL_MODEL_PATH); sendWhisperModelDownloadProgress(webContents, { status: "downloaded", @@ -147,6 +230,10 @@ export async function downloadWhisperSmallModel( error: String(error), }); throw error; + } finally { + if (!webContents.isDestroyed()) { + webContents.removeListener("destroyed", handleDestroyed); + } } } From 13a19f3a237f1f2c92016e9b9203b8afcfb5f111 Mon Sep 17 00:00:00 2001 From: AHMED Date: Thu, 17 Sep 2026 10:58:37 +0500 Subject: [PATCH 2/3] fix(captions): address review feedback on redirect lifecycle, stream cleanup, and docs --- electron/ipc/captions/whisper.test.ts | 124 +++++++++++++++++++++++++- electron/ipc/captions/whisper.ts | 39 +++++++- 2 files changed, 160 insertions(+), 3 deletions(-) diff --git a/electron/ipc/captions/whisper.test.ts b/electron/ipc/captions/whisper.test.ts index 1b639e841..18ad423f8 100644 --- a/electron/ipc/captions/whisper.test.ts +++ b/electron/ipc/captions/whisper.test.ts @@ -1,5 +1,11 @@ +import { EventEmitter } from "node:events"; +import fs from "node:fs/promises"; +import { get as httpsGet } from "node:https"; +import os from "node:os"; +import path from "node:path"; +import { PassThrough } from "node:stream"; import type Electron from "electron"; -import { describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; vi.mock("electron", () => ({ app: { @@ -7,12 +13,23 @@ vi.mock("electron", () => ({ }, })); +vi.mock("node:https", () => ({ + get: vi.fn(), +})); + import { downloadFileWithProgress, downloadWhisperSmallModel, sendWhisperModelDownloadProgress, } from "./whisper"; +const tempFiles: string[] = []; + +afterEach(async () => { + vi.clearAllMocks(); + await Promise.all(tempFiles.splice(0).map((file) => fs.rm(file, { force: true }))); +}); + describe("sendWhisperModelDownloadProgress", () => { it("no-ops safely when webContents is null or undefined", () => { expect(() => @@ -113,4 +130,109 @@ describe("downloadFileWithProgress", () => { ), ).rejects.toThrow("Download aborted"); }); + + it("completes redirected download and ignores late events from original request", async () => { + const tempPath = path.join(os.tmpdir(), `whisper-test-${Date.now()}.bin`); + tempFiles.push(tempPath); + + const req1 = Object.assign(new EventEmitter(), { + destroy: vi.fn(), + }); + const res1 = Object.assign(new EventEmitter(), { + statusCode: 302, + headers: { location: "https://example.com/redirected.bin" }, + resume: vi.fn(), + destroy: vi.fn(), + }); + + const req2 = Object.assign(new EventEmitter(), { + destroy: vi.fn(), + }); + const res2 = Object.assign(new PassThrough(), { + statusCode: 200, + headers: { "content-length": "4" }, + }); + + const mockedGet = vi.mocked(httpsGet); + mockedGet.mockImplementationOnce((_url, _options, callback) => { + setImmediate(() => { + if (typeof callback === "function") { + callback(res1 as unknown as Parameters[0]); + } + }); + return req1 as unknown as ReturnType; + }); + + mockedGet.mockImplementationOnce((_url, _options, callback) => { + setImmediate(() => { + if (typeof callback === "function") { + callback(res2 as unknown as Parameters[0]); + res2.end(Buffer.from("test")); + } + }); + return req2 as unknown as ReturnType; + }); + + const progressSpy = vi.fn(); + const downloadPromise = downloadFileWithProgress( + "https://example.com/initial.bin", + tempPath, + progressSpy, + ); + + // Wait for redirect response to be processed + await new Promise((resolve) => setTimeout(resolve, 50)); + + // Emit late timeout and error events on original request after redirect handoff + req1.emit("timeout"); + req1.emit("error", new Error("Late socket error")); + + await expect(downloadPromise).resolves.toBeUndefined(); + expect(res1.resume).toHaveBeenCalled(); + expect(res1.destroy).toHaveBeenCalled(); + expect(progressSpy).toHaveBeenCalledWith(100); + }); + + it("destroys output fileStream and rejects on request error", async () => { + const tempPath = path.join(os.tmpdir(), `whisper-test-err-${Date.now()}.bin`); + tempFiles.push(tempPath); + + const req = Object.assign(new EventEmitter(), { + destroy: vi.fn(), + }); + + const mockedGet = vi.mocked(httpsGet); + mockedGet.mockImplementationOnce(() => { + setImmediate(() => { + req.emit("error", new Error("Network unreachable")); + }); + return req as unknown as ReturnType; + }); + + await expect( + downloadFileWithProgress("https://example.com/file.bin", tempPath, vi.fn()), + ).rejects.toThrow("Network unreachable"); + }); + + it("destroys output fileStream and rejects on request timeout", async () => { + const tempPath = path.join(os.tmpdir(), `whisper-test-timeout-${Date.now()}.bin`); + tempFiles.push(tempPath); + + const req = Object.assign(new EventEmitter(), { + destroy: vi.fn(), + }); + + const mockedGet = vi.mocked(httpsGet); + mockedGet.mockImplementationOnce(() => { + setImmediate(() => { + req.emit("timeout"); + }); + return req as unknown as ReturnType; + }); + + await expect( + downloadFileWithProgress("https://example.com/file.bin", tempPath, vi.fn()), + ).rejects.toThrow("Whisper model download timed out."); + expect(req.destroy).toHaveBeenCalled(); + }); }); diff --git a/electron/ipc/captions/whisper.ts b/electron/ipc/captions/whisper.ts index c19f97500..ac35c4af2 100644 --- a/electron/ipc/captions/whisper.ts +++ b/electron/ipc/captions/whisper.ts @@ -9,6 +9,13 @@ import { WHISPER_SMALL_MODEL_PATH, } from "../constants"; +/** + * Sends Whisper speech-to-text model download progress to the renderer process. + * Safely ignores missing, uninitialized, or destroyed WebContents instances. + * + * @param webContents The target Electron WebContents to dispatch IPC events to. + * @param payload The progress payload containing status, progress percentage, path, and error info. + */ export function sendWhisperModelDownloadProgress( webContents: Electron.WebContents | null | undefined, payload: { @@ -29,6 +36,11 @@ export function sendWhisperModelDownloadProgress( } } +/** + * Checks whether the Whisper small model file exists on disk and is readable. + * + * @returns An object indicating existence status and file path. + */ export async function getWhisperSmallModelStatus() { try { await fs.access(WHISPER_SMALL_MODEL_PATH, fsConstants.R_OK); @@ -46,6 +58,14 @@ export async function getWhisperSmallModelStatus() { } } +/** + * Downloads a file over HTTPS with incremental progress tracking, redirect handling, and abort support. + * + * @param url The source URL to download from. + * @param destinationPath The local file destination path. + * @param onProgress Callback receiving progress percentage (0-100). + * @param signal Optional AbortSignal to cancel the download in flight. + */ export function downloadFileWithProgress( url: string, destinationPath: string, @@ -89,6 +109,7 @@ export function downloadFileWithProgress( if (statusCode >= 300 && statusCode < 400 && location) { response.resume(); + response.destroy(); cleanupSignal(); if (redirectCount >= 5) { settled = true; @@ -96,6 +117,7 @@ export function downloadFileWithProgress( return; } + settled = true; const nextUrl = new URL(location, currentUrl).toString(); void request(nextUrl, redirectCount + 1) .then(resolve) @@ -159,6 +181,7 @@ export function downloadFileWithProgress( cleanupSignal(); if (!settled) { settled = true; + fileStream?.destroy(error); reject(error); } }); @@ -166,9 +189,11 @@ export function downloadFileWithProgress( req.on("timeout", () => { cleanupSignal(); if (!settled) { + const error = new Error("Whisper model download timed out."); settled = true; - req.destroy(new Error("Whisper model download timed out.")); - reject(new Error("Whisper model download timed out.")); + fileStream?.destroy(error); + req.destroy(error); + reject(error); } }); }); @@ -177,6 +202,13 @@ export function downloadFileWithProgress( return request(url); } +/** + * Downloads the Whisper small model into the local application directory. + * Dispatches progress events via IPC and aborts if the caller WebContents is destroyed. + * + * @param webContents The initiating Electron WebContents. + * @returns The file path of the downloaded Whisper model. + */ export async function downloadWhisperSmallModel( webContents: Electron.WebContents, ): Promise { @@ -237,6 +269,9 @@ export async function downloadWhisperSmallModel( } } +/** + * Deletes the downloaded Whisper small model from disk if present. + */ export async function deleteWhisperSmallModel(): Promise { await fs.rm(WHISPER_SMALL_MODEL_PATH, { force: true }); } From 67fc16b0fc2af35bdffb3d4a8c1d773a5101da98 Mon Sep 17 00:00:00 2001 From: AHMED Date: Thu, 17 Sep 2026 12:13:52 +0500 Subject: [PATCH 3/3] test(captions): cover window destruction during in-flight model download --- electron/ipc/captions/whisper.test.ts | 28 +++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/electron/ipc/captions/whisper.test.ts b/electron/ipc/captions/whisper.test.ts index 18ad423f8..874177412 100644 --- a/electron/ipc/captions/whisper.test.ts +++ b/electron/ipc/captions/whisper.test.ts @@ -114,6 +114,34 @@ describe("downloadWhisperSmallModel", () => { "Window was closed before Whisper model download could start.", ); }); + + it("aborts and cleans up when webContents is destroyed during an in-flight download", async () => { + const webContentsEmitter = new EventEmitter(); + let isDestroyed = false; + + const mockWebContents = Object.assign(webContentsEmitter, { + isDestroyed: () => isDestroyed, + send: vi.fn(), + }) as unknown as Electron.WebContents; + + const req = Object.assign(new EventEmitter(), { + destroy: vi.fn(), + }); + + const mockedGet = vi.mocked(httpsGet); + mockedGet.mockImplementationOnce(() => { + setImmediate(() => { + isDestroyed = true; + webContentsEmitter.emit("destroyed"); + }); + return req as unknown as ReturnType; + }); + + await expect(downloadWhisperSmallModel(mockWebContents)).rejects.toThrow( + "Download aborted", + ); + expect(req.destroy).toHaveBeenCalled(); + }); }); describe("downloadFileWithProgress", () => {