Merge pull request #957 from bbassie/feature/download-file-cleanup

Discard the temporary file when a download fails

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") {

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.