diff --git a/packages/lib/src/file_ops.ts b/packages/lib/src/file_ops.ts index 6b54548d..55d2bda7 100644 --- a/packages/lib/src/file_ops.ts +++ b/packages/lib/src/file_ops.ts @@ -281,6 +281,32 @@ export async function createTempWriteStream(filePath: string) { return [tempFilePath, writeStream] as const; } +/** + * Discard a partially written download + * + * Destroys the write stream and removes the file it was writing to, so a + * download which failed part way through does not leave a stray temporary + * file and an open file handle behind. Safe to call whether or not the + * stream has already closed. + * + * @param writeStream - stream the download was written to. + * @param tempFilePath - path the stream was writing to. + */ +async function discardPartialDownload(writeStream: stream.Writable, tempFilePath: string) { + if (!writeStream.closed) { + writeStream.destroy(); + await events.once(writeStream, "close"); + } + try { + await fs.unlink(tempFilePath); + } catch (err: any) { + // Nothing to discard if the file never made it to disk. + if (err.code !== "ENOENT") { + throw err; + } + } +} + /** * Download a file by fetching the given url and streaming the body content to it. * @@ -307,16 +333,20 @@ export async function downloadFile( [tempFilePath, writeStream] = await createTempWriteStream(filePath); } - // Fetch stream bytes - const response = await fetch(url); - if (!response.ok || !response.body) { - writeStream.end(); - await events.once(writeStream, "close"); - await fs.unlink(tempFilePath); - throw new Error(`Failed to download ${redactUrl(url)}: ${response.status} ${response.statusText}`); + // Fetch stream bytes. Anything that goes wrong from here until the + // body has been written leaves a temporary file that needs to be removed + try { + const response = await fetch(url); + if (!response.ok || !response.body) { + throw new Error(`Failed to download ${redactUrl(url)}: ${response.status} ${response.statusText}`); + } + const writer = stream.Writable.toWeb(writeStream); + await response.body.pipeTo(writer); + + } catch (err) { + await discardPartialDownload(writeStream, tempFilePath); + throw err; } - const writer = stream.Writable.toWeb(writeStream); - await response.body.pipeTo(writer); let targetFilePath = filePath; if (overwriteMode === "overwrite") { diff --git a/test/lib/file_ops.js b/test/lib/file_ops.js index 428fd493..581cff94 100644 --- a/test/lib/file_ops.js +++ b/test/lib/file_ops.js @@ -383,6 +383,38 @@ describe("lib/file_ops", function() { assert.equal(writtenContent, "original content"); }); + it("should not leave a temporary file behind if the request fails", async function () { + // Bind a server only to get a port nothing is listening on, so + // that the fetch is refused before there is a response at all. + const deadServer = http.createServer(); + await util.promisify(deadServer.listen.bind(deadServer))(); + const deadUrl = new URL("/simple-file", baseUrl); + deadUrl.port = deadServer.address().port; + await util.promisify(deadServer.close.bind(deadServer))(); + + const downloadPath = path.join(downloadDir, "refused.txt"); + await assert.rejects(lib.downloadFile(deadUrl, downloadPath, "overwrite")); + await assert.rejects( + fs.readFile(path.join(downloadDir, "refused.tmp.txt")), + { code: "ENOENT" }, + ); + await assert.rejects(fs.readFile(downloadPath), { code: "ENOENT" }); + }); + + it("should not leave a temporary file behind if the download is interrupted", async function () { + const downloadPath = path.join(downloadDir, "interrupted.txt"); + onStream = (res) => { + // Cut the connection with the body half written. + res.destroy(); + }; + await assert.rejects(lib.downloadFile(new URL("/stream", baseUrl), downloadPath, "overwrite")); + await assert.rejects( + fs.readFile(path.join(downloadDir, "interrupted.tmp.txt")), + { code: "ENOENT" }, + ); + await assert.rejects(fs.readFile(downloadPath), { code: "ENOENT" }); + }); + it("should throw if overwriteMode is rename and directory disappeared", async function () { if (process.platform === "win32") { // Can't remove the directory while the stream is running on Windows.