fix(cli): await stalled download cleanup

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

@@ -8,7 +8,16 @@ import type { ClientRequest, IncomingMessage } from "node:http";
 import { afterEach, describe, expect, it, vi } from "vitest";
 import { downloadFile } from "./download.js";
 
+const { unlinkSyncMock } = vi.hoisted(() => ({
+  unlinkSyncMock: vi.fn(),
+}));
+
 vi.mock("node:https", () => ({ get: vi.fn() }));
+vi.mock("node:fs", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("node:fs")>();
+  unlinkSyncMock.mockImplementation(actual.unlinkSync);
+  return { ...actual, unlinkSync: unlinkSyncMock };
+});
 
 const mockGet = vi.mocked(httpsGet);
 const tempDirs: string[] = [];
@@ -20,6 +29,17 @@ afterEach(() => {
 
 describe("downloadFile", () => {
   it("rejects an idle response and removes the partial file", async () => {
+    const actualFs = await vi.importActual<typeof import("node:fs")>("node:fs");
+    let responseClosed = false;
+    unlinkSyncMock.mockImplementation((path) => {
+      if (!responseClosed) {
+        const error = new Error("file is locked");
+        Object.assign(error, { code: "EBUSY" });
+        throw error;
+      }
+      actualFs.unlinkSync(path);
+    });
+
     mockGet.mockImplementation(((_url: string, callback: (response: IncomingMessage) => void) => {
       const response = new PassThrough() as PassThrough & {
         statusCode: number;
@@ -27,6 +47,9 @@ describe("downloadFile", () => {
       };
       response.statusCode = 200;
       response.headers = {};
+      response.once("close", () => {
+        responseClosed = true;
+      });
 
       class FakeRequest extends EventEmitter {
         private timeout: ReturnType<typeof setTimeout> | undefined;

packages/cli/src/utils/download.ts

@@ -1,5 +1,6 @@
 import { createWriteStream, renameSync, unlinkSync } from "node:fs";
 import { get as httpsGet } from "node:https";
+import type { IncomingMessage } from "node:http";
 import { pipeline } from "node:stream/promises";
 
 const DEFAULT_DOWNLOAD_TIMEOUT_MS = 30_000;
@@ -31,7 +32,11 @@ export function downloadFile(
   const timeoutMs = options.timeoutMs ?? DEFAULT_DOWNLOAD_TIMEOUT_MS;
   return new Promise((resolve, reject) => {
     const follow = (u: string) => {
+      let activeResponse: IncomingMessage | undefined;
+      let responsePipelineStarted = false;
+      let requestError: Error | undefined;
       const request = httpsGet(u, (res) => {
+        activeResponse = res;
         if (res.statusCode === 301 || res.statusCode === 302) {
           const location = res.headers.location;
           if (location) {
@@ -47,6 +52,7 @@ export function downloadFile(
           return;
         }
         const file = createWriteStream(tmp);
+        responsePipelineStarted = true;
         pipeline(res, file)
           .then(() => {
             renameSync(tmp, dest);
@@ -54,13 +60,18 @@ export function downloadFile(
           })
           .catch((err) => {
             removePartialFile(tmp);
-            reject(err);
+            reject(requestError ?? err);
           });
       });
       request.setTimeout(timeoutMs, () => {
         request.destroy(new Error(`Download timed out after ${timeoutMs}ms`));
       });
       request.on("error", (err) => {
+        if (responsePipelineStarted) {
+          requestError = err;
+          activeResponse?.destroy(err);
+          return;
+        }
         removePartialFile(tmp);
         reject(err);
       });