diff --git a/packages/cli/src/utils/download.ts b/packages/cli/src/utils/download.ts index d669675869..4ccad12e98 100644 --- a/packages/cli/src/utils/download.ts +++ b/packages/cli/src/utils/download.ts @@ -31,6 +31,20 @@ export function downloadFile( const timeoutMs = options.timeoutMs ?? DEFAULT_DOWNLOAD_TIMEOUT_MS; return new Promise((resolve, reject) => { const follow = (u: string) => { + // Set once the 200 branch hands the response to pipeline(). From that + // point the pipeline's catch owns BOTH cleanup and rejection: it settles + // only after the write stream has closed, so the `.tmp` file provably + // exists (or never will) when it unlinks. The request "error" handler + // must NOT clean up or reject in that window — createWriteStream opens + // asynchronously, so an early unlink+reject races the open and can leave + // a stray `.tmp` created AFTER cleanup ran (flaked in CI as + // download.test.ts "removes the partial file"; in the wild it strands + // partials in the cache dir). + let pipelineOwnsCleanup = false; + // The request's error (e.g. the timeout message) is more precise than + // what the pipeline surfaces for the same failure (destroying the + // request can reject the pipeline with a generic premature-close). + let requestError: Error | undefined; const request = httpsGet(u, (res) => { if (res.statusCode === 301 || res.statusCode === 302) { const location = res.headers.location; @@ -46,6 +60,7 @@ export function downloadFile( reject(new Error(`Download failed: HTTP ${res.statusCode}`)); return; } + pipelineOwnsCleanup = true; const file = createWriteStream(tmp); pipeline(res, file) .then(() => { @@ -54,13 +69,17 @@ 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 (pipelineOwnsCleanup) { + requestError = err; + return; + } removePartialFile(tmp); reject(err); });