fix(cli): time out stalled model downloads (#2415)

packages/cli/src/utils/download.test.ts

@@ -0,0 +1,66 @@
+import { EventEmitter } from "node:events";
+import { existsSync, mkdtempSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { PassThrough } from "node:stream";
+import { get as httpsGet } from "node:https";
+import type { ClientRequest, IncomingMessage } from "node:http";
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { downloadFile } from "./download.js";
+
+vi.mock("node:https", () => ({ get: vi.fn() }));
+
+const mockGet = vi.mocked(httpsGet);
+const tempDirs: string[] = [];
+
+afterEach(() => {
+  vi.clearAllMocks();
+  for (const dir of tempDirs.splice(0)) rmSync(dir, { recursive: true, force: true });
+});
+
+describe("downloadFile", () => {
+  it("rejects an idle response and removes the partial file", async () => {
+    mockGet.mockImplementation(((_url: string, callback: (response: IncomingMessage) => void) => {
+      const response = new PassThrough() as PassThrough & {
+        statusCode: number;
+        headers: Record<string, string>;
+      };
+      response.statusCode = 200;
+      response.headers = {};
+
+      class FakeRequest extends EventEmitter {
+        private timeout: ReturnType<typeof setTimeout> | undefined;
+        private fallback = setTimeout(() => this.destroy(new Error("late shutdown")), 50);
+        private destroyed = false;
+
+        setTimeout(ms: number, onTimeout: () => void): this {
+          this.timeout = setTimeout(onTimeout, ms);
+          return this;
+        }
+
+        destroy(error: Error): void {
+          if (this.destroyed) return;
+          this.destroyed = true;
+          clearTimeout(this.timeout);
+          clearTimeout(this.fallback);
+          response.destroy(error);
+          this.emit("error", error);
+        }
+      }
+
+      const request = new FakeRequest();
+      callback(response as unknown as IncomingMessage);
+      return request as unknown as ClientRequest;
+    }) as typeof httpsGet);
+
+    const dir = mkdtempSync(join(tmpdir(), "hyperframes-download-"));
+    tempDirs.push(dir);
+    const dest = join(dir, "model.onnx");
+
+    await expect(
+      downloadFile("https://example.test/model.onnx", dest, { timeoutMs: 10 }),
+    ).rejects.toThrow("Download timed out after 10ms");
+    expect(existsSync(`${dest}.tmp`)).toBe(false);
+    expect(existsSync(dest)).toBe(false);
+  });
+});

packages/cli/src/utils/download.ts

@@ -2,24 +2,47 @@ import { createWriteStream, renameSync, unlinkSync } from "node:fs";
 import { get as httpsGet } from "node:https";
 import { pipeline } from "node:stream/promises";
 
+const DEFAULT_DOWNLOAD_TIMEOUT_MS = 30_000;
+
+export interface DownloadOptions {
+  /** Abort after this many milliseconds without network activity. */
+  timeoutMs?: number;
+}
+
+function removePartialFile(path: string): void {
+  try {
+    unlinkSync(path);
+  } catch {
+    // Missing/locked partial files are handled by the next atomic download.
+  }
+}
+
 /**
  * Download a file from a URL, following redirects.
  * Uses atomic write (download to .tmp, rename on success) to prevent
  * corrupt partial files from persisting in the cache on interruption.
  */
-export function downloadFile(url: string, dest: string): Promise<void> {
+export function downloadFile(
+  url: string,
+  dest: string,
+  options: DownloadOptions = {},
+): Promise<void> {
   const tmp = `${dest}.tmp`;
+  const timeoutMs = options.timeoutMs ?? DEFAULT_DOWNLOAD_TIMEOUT_MS;
   return new Promise((resolve, reject) => {
     const follow = (u: string) => {
-      httpsGet(u, (res) => {
+      const request = httpsGet(u, (res) => {
         if (res.statusCode === 301 || res.statusCode === 302) {
           const location = res.headers.location;
           if (location) {
+            res.resume();
             follow(location);
             return;
           }
         }
         if (res.statusCode !== 200) {
+          res.resume();
+          removePartialFile(tmp);
           reject(new Error(`Download failed: HTTP ${res.statusCode}`));
           return;
         }
@@ -30,19 +53,15 @@ export function downloadFile(url: string, dest: string): Promise<void> {
             resolve();
           })
           .catch((err) => {
-            try {
-              unlinkSync(tmp);
-            } catch {
-              // ignore cleanup failure
-            }
+            removePartialFile(tmp);
             reject(err);
           });
-      }).on("error", (err) => {
-        try {
-          unlinkSync(tmp);
-        } catch {
-          // ignore cleanup failure
-        }
+      });
+      request.setTimeout(timeoutMs, () => {
+        request.destroy(new Error(`Download timed out after ${timeoutMs}ms`));
+      });
+      request.on("error", (err) => {
+        removePartialFile(tmp);
         reject(err);
       });
     };