From 0bf33cb117b111e89ee8d031d565c291aabf9160 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Mon, 27 Jul 2026 22:58:17 +0000 Subject: [PATCH] fix(cli): await stalled download cleanup --- packages/cli/src/utils/download.test.ts | 23 +++++++++++++++++++++++ packages/cli/src/utils/download.ts | 13 ++++++++++++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/utils/download.test.ts b/packages/cli/src/utils/download.test.ts index c0de5ec0c..84f0f9305 100644 --- a/packages/cli/src/utils/download.test.ts +++ b/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(); + 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("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 | undefined; diff --git a/packages/cli/src/utils/download.ts b/packages/cli/src/utils/download.ts index d66967586..6e5a06b9e 100644 --- a/packages/cli/src/utils/download.ts +++ b/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); });