Merge #4963

4963: Download artifacts into tmp dir r=matklad a=Veetaha

This should prevent partially downloaded files in cases when the user closes vsode before the download is complete.
There is also a new more descriptive error message when the user has multiple vscode windows open and tries to download the server.
Related: #4938 (comment)

Co-authored-by: Veetaha <veetaha2@gmail.com>

Author: bors[bot]

editors/code/src/main.ts

@@ -42,7 +42,16 @@ export async function activate(context: vscode.ExtensionContext) {
 
     const config = new Config(context);
     const state = new PersistentState(context.globalState);
-    const serverPath = await bootstrap(config, state);
+    const serverPath = await bootstrap(config, state).catch(err => {
+        let message = "Failed to bootstrap rust-analyzer.";
+        if (err.code === "EBUSY" || err.code === "ETXTBSY") {
+            message += " Other vscode windows might be using rust-analyzer, " +
+                "you should close them and reload this window to retry.";
+        }
+        message += " Open \"Help > Toggle Developer Tools > Console\" to see the logs";
+        log.error("Bootstrap error", err);
+        throw new Error(message);
+    });
 
     const workspaceFolder = vscode.workspace.workspaceFolders?.[0];
     if (workspaceFolder === undefined) {
@@ -285,6 +294,11 @@ async function getServer(config: Config, state: PersistentState): Promise<string
     const artifact = release.assets.find(artifact => artifact.name === binaryName);
     assert(!!artifact, `Bad release: ${JSON.stringify(release)}`);
 
+    // Unlinking the exe file before moving new one on its place should prevent ETXTBSY error.
+    await fs.unlink(dest).catch(err => {
+        if (err.code !== "ENOENT") throw err;
+    });
+
     await download(artifact.browser_download_url, dest, "Downloading rust-analyzer server", { mode: 0o755 });
 
     // Patching executable if that's NixOS.

editors/code/src/net.ts

@@ -1,7 +1,9 @@
 import fetch from "node-fetch";
 import * as vscode from "vscode";
-import * as fs from "fs";
 import * as stream from "stream";
+import * as fs from "fs";
+import * as os from "os";
+import * as path from "path";
 import * as util from "util";
 import { log, assert } from "./util";
 
@@ -87,7 +89,7 @@ export async function download(
 }
 
 /**
- * Downloads file from `url` and stores it at `destFilePath` with `destFilePermissions`.
+ * Downloads file from `url` and stores it at `destFilePath` with `mode` (unix permissions).
  * `onProgress` callback is called on recieveing each chunk of bytes
  * to track the progress of downloading, it gets the already read and total
  * amount of bytes to read as its parameters.
@@ -118,13 +120,46 @@ async function downloadFile(
         onProgress(readBytes, totalBytes);
     });
 
-    const destFileStream = fs.createWriteStream(destFilePath, { mode });
-
-    await pipeline(res.body, destFileStream);
-    return new Promise<void>(resolve => {
-        destFileStream.on("close", resolve);
-        destFileStream.destroy();
-        // This workaround is awaiting to be removed when vscode moves to newer nodejs version:
-        // https://github.com/rust-analyzer/rust-analyzer/issues/3167
+    // Put the artifact into a temporary folder to prevent partially downloaded files when user kills vscode
+    await withTempFile(async tempFilePath => {
+        const destFileStream = fs.createWriteStream(tempFilePath, { mode });
+        await pipeline(res.body, destFileStream);
+        await new Promise<void>(resolve => {
+            destFileStream.on("close", resolve);
+            destFileStream.destroy();
+            // This workaround is awaiting to be removed when vscode moves to newer nodejs version:
+            // https://github.com/rust-analyzer/rust-analyzer/issues/3167
+        });
+        await moveFile(tempFilePath, destFilePath);
     });
 }
+
+async function withTempFile(scope: (tempFilePath: string) => Promise<void>) {
+    // Based on the great article: https://advancedweb.hu/secure-tempfiles-in-nodejs-without-dependencies/
+
+    // `.realpath()` should handle the cases where os.tmpdir() contains symlinks
+    const osTempDir = await fs.promises.realpath(os.tmpdir());
+
+    const tempDir = await fs.promises.mkdtemp(path.join(osTempDir, "rust-analyzer"));
+
+    try {
+        return await scope(path.join(tempDir, "file"));
+    } finally {
+        // We are good citizens :D
+        void fs.promises.rmdir(tempDir, { recursive: true }).catch(log.error);
+    }
+};
+
+async function moveFile(src: fs.PathLike, dest: fs.PathLike) {
+    try {
+        await fs.promises.rename(src, dest);
+    } catch (err) {
+        if (err.code === 'EXDEV') {
+            // We are probably moving the file across partitions/devices
+            await fs.promises.copyFile(src, dest);
+            await fs.promises.unlink(src);
+        } else {
+            log.error(`Failed to rename the file ${src} -> ${dest}`, err);
+        }
+    }
+}