Improve fetching Coder binary

- Add more logging (the main goal of this commit).
- Break out parts specific to the binary and add tests for those parts.
- Check the version before making the request.
- Avoid deleting the old binary right away, mostly to make it possible
  to debug why the plugin thought it was invalid.  It will be cleaned up
  on the next download, so at most we have one.
- Clean up left-over temporary binaries too.
- Validate downloaded binary.
- Catch error on writing (for example if we are downloading the binary
  to a read-only location).

fixtures/bin.bash

@@ -0,0 +1,3 @@
+#!/usr/bin/env bash
+
+echo '$ECHO'

fixtures/bin.old.bash

@@ -0,0 +1,8 @@
+#!/usr/bin/env bash
+
+if [[ $* == *--output* ]] ; then
+  >&2 echo -n '$STDERR'
+  exit 1
+else
+  echo -n '$STDOUT'
+fi

src/cliManager.test.ts

@@ -0,0 +1,130 @@
+import fs from "fs/promises"
+import os from "os"
+import path from "path"
+import { beforeAll, describe, expect, it } from "vitest"
+import * as cli from "./cliManager"
+
+describe("cliManager", () => {
+  const tmp = path.join(os.tmpdir(), "vscode-coder-tests")
+
+  beforeAll(async () => {
+    // Clean up from previous tests, if any.
+    await fs.rm(tmp, { recursive: true, force: true })
+    await fs.mkdir(tmp, { recursive: true })
+  })
+
+  it("name", () => {
+    expect(cli.name().startsWith("coder-")).toBeTruthy()
+  })
+
+  it("stat", async () => {
+    const binPath = path.join(tmp, "stat")
+    expect(await cli.stat(binPath)).toBeUndefined()
+
+    await fs.writeFile(binPath, "test")
+    expect((await cli.stat(binPath))?.size).toBe(4)
+  })
+
+  it("rm", async () => {
+    const binPath = path.join(tmp, "rm")
+    await cli.rm(binPath)
+
+    await fs.writeFile(binPath, "test")
+    await cli.rm(binPath)
+  })
+
+  // TODO: CI only runs on Linux but we should run it on Windows too.
+  it("version", async () => {
+    const binPath = path.join(tmp, "version")
+    await expect(cli.version(binPath)).rejects.toThrow("ENOENT")
+
+    const binTmpl = await fs.readFile(path.join(__dirname, "../fixtures/bin.bash"), "utf8")
+    await fs.writeFile(binPath, binTmpl.replace("$ECHO", "hello"))
+    await expect(cli.version(binPath)).rejects.toThrow("EACCES")
+
+    await fs.chmod(binPath, "755")
+    await expect(cli.version(binPath)).rejects.toThrow("Unexpected token")
+
+    await fs.writeFile(binPath, binTmpl.replace("$ECHO", "{}"))
+    await expect(cli.version(binPath)).rejects.toThrow("No version found in output")
+
+    await fs.writeFile(
+      binPath,
+      binTmpl.replace(
+        "$ECHO",
+        JSON.stringify({
+          version: "v0.0.0",
+        }),
+      ),
+    )
+    expect(await cli.version(binPath)).toBe("v0.0.0")
+
+    const oldTmpl = await fs.readFile(path.join(__dirname, "../fixtures/bin.old.bash"), "utf8")
+    const old = (stderr: string, stdout: string): string => {
+      return oldTmpl.replace("$STDERR", stderr).replace("$STDOUT", stdout)
+    }
+
+    // Should fall back only if it says "unknown flag".
+    await fs.writeFile(binPath, old("foobar", "Coder v1.1.1"))
+    await expect(cli.version(binPath)).rejects.toThrow("foobar")
+
+    await fs.writeFile(binPath, old("unknown flag: --output", "Coder v1.1.1"))
+    expect(await cli.version(binPath)).toBe("v1.1.1")
+
+    // Should trim off the newline if necessary.
+    await fs.writeFile(binPath, old("unknown flag: --output\n", "Coder v1.1.1\n"))
+    expect(await cli.version(binPath)).toBe("v1.1.1")
+
+    // Error with original error if it does not begin with "Coder".
+    await fs.writeFile(binPath, old("unknown flag: --output", "Unrelated"))
+    await expect(cli.version(binPath)).rejects.toThrow("unknown flag")
+
+    // Error if no version.
+    await fs.writeFile(binPath, old("unknown flag: --output", "Coder"))
+    await expect(cli.version(binPath)).rejects.toThrow("No version found")
+  })
+
+  it("rmOld", async () => {
+    const binDir = path.join(tmp, "bins")
+    expect(await cli.rmOld(path.join(binDir, "bin1"))).toStrictEqual([])
+
+    await fs.mkdir(binDir, { recursive: true })
+    await fs.writeFile(path.join(binDir, "bin.old-1"), "echo hello")
+    await fs.writeFile(path.join(binDir, "bin.old-2"), "echo hello")
+    await fs.writeFile(path.join(binDir, "bin.temp-1"), "echo hello")
+    await fs.writeFile(path.join(binDir, "bin.temp-2"), "echo hello")
+    await fs.writeFile(path.join(binDir, "bin1"), "echo hello")
+    await fs.writeFile(path.join(binDir, "bin2"), "echo hello")
+
+    expect(await cli.rmOld(path.join(binDir, "bin1"))).toStrictEqual([
+      {
+        fileName: "bin.old-1",
+        error: undefined,
+      },
+      {
+        fileName: "bin.old-2",
+        error: undefined,
+      },
+      {
+        fileName: "bin.temp-1",
+        error: undefined,
+      },
+      {
+        fileName: "bin.temp-2",
+        error: undefined,
+      },
+    ])
+
+    expect(await fs.readdir(path.join(tmp, "bins"))).toStrictEqual(["bin1", "bin2"])
+  })
+
+  it("ETag", async () => {
+    const binPath = path.join(tmp, "hash")
+
+    await fs.writeFile(binPath, "foobar")
+    expect(await cli.eTag(binPath)).toBe("8843d7f92416211de9ebb963ff4ce28125932878")
+
+    await fs.writeFile(binPath, "test")
+    expect(await cli.eTag(binPath)).toBe("a94a8fe5ccb19ba61c4c0873d391e987982fbbd3")
+  })
+})

src/cliManager.ts

@@ -0,0 +1,167 @@
+import { execFile, type ExecFileException } from "child_process"
+import * as crypto from "crypto"
+import { createReadStream, type Stats } from "fs"
+import fs from "fs/promises"
+import os from "os"
+import path from "path"
+import { promisify } from "util"
+
+/**
+ * Stat the path or undefined if the path does not exist.  Throw if unable to
+ * stat for a reason other than the path not existing.
+ */
+export async function stat(binPath: string): Promise<Stats | undefined> {
+  try {
+    return await fs.stat(binPath)
+  } catch (error) {
+    if ((error as NodeJS.ErrnoException)?.code === "ENOENT") {
+      return undefined
+    }
+    throw error
+  }
+}
+
+/**
+ * Remove the path.  Throw if unable to remove.
+ */
+export async function rm(binPath: string): Promise<void> {
+  try {
+    await fs.rm(binPath, { force: true })
+  } catch (error) {
+    // Just in case; we should never get an ENOENT because of force: true.
+    if ((error as NodeJS.ErrnoException)?.code !== "ENOENT") {
+      throw error
+    }
+  }
+}
+
+// util.promisify types are dynamic so there is no concrete type we can import
+// and we have to make our own.
+type ExecException = ExecFileException & { stdout?: string; stderr?: string }
+
+/**
+ * Return the version from the binary.  Throw if unable to execute the binary or
+ * find the version for any reason.
+ */
+export async function version(binPath: string): Promise<string> {
+  let stdout: string
+  try {
+    const result = await promisify(execFile)(binPath, ["version", "--output", "json"])
+    stdout = result.stdout
+  } catch (error) {
+    // It could be an old version without support for --output.
+    if ((error as ExecException)?.stderr?.includes("unknown flag: --output")) {
+      const result = await promisify(execFile)(binPath, ["version"])
+      if (result.stdout?.startsWith("Coder")) {
+        const v = result.stdout.split(" ")[1]?.trim()
+        if (!v) {
+          throw new Error("No version found in output: ${result.stdout}")
+        }
+        return v
+      }
+    }
+    throw error
+  }
+
+  const json = JSON.parse(stdout)
+  if (!json.version) {
+    throw new Error("No version found in output: ${stdout}")
+  }
+  return json.version
+}
+
+export type RemovalResult = { fileName: string; error: unknown }
+
+/**
+ * Remove binaries in the same directory as the specified path that have a
+ * .old-* or .temp-* extension.  Return a list of files and the errors trying to
+ * remove them, when applicable.
+ */
+export async function rmOld(binPath: string): Promise<RemovalResult[]> {
+  const binDir = path.dirname(binPath)
+  try {
+    const files = await fs.readdir(binDir)
+    const results: RemovalResult[] = []
+    for (const file of files) {
+      const fileName = path.basename(file)
+      if (fileName.includes(".old-") || fileName.includes(".temp-")) {
+        try {
+          await fs.rm(path.join(binDir, file), { force: true })
+          results.push({ fileName, error: undefined })
+        } catch (error) {
+          results.push({ fileName, error })
+        }
+      }
+    }
+    return results
+  } catch (error) {
+    // If the directory does not exist, there is nothing to remove.
+    if ((error as NodeJS.ErrnoException)?.code === "ENOENT") {
+      return []
+    }
+    throw error
+  }
+}
+
+/**
+ * Return the etag (sha1) of the path.  Throw if unable to hash the file.
+ */
+export async function eTag(binPath: string): Promise<string> {
+  const hash = crypto.createHash("sha1")
+  const stream = createReadStream(binPath)
+  return new Promise((resolve, reject) => {
+    stream.on("end", () => {
+      hash.end()
+      resolve(hash.digest("hex"))
+    })
+    stream.on("error", (err) => {
+      reject(err)
+    })
+    stream.on("data", (chunk) => {
+      hash.update(chunk)
+    })
+  })
+}
+
+/**
+ * Return the binary name for the current platform.
+ */
+export function name(): string {
+  const os = goos()
+  const arch = goarch()
+  let binName = `coder-${os}-${arch}`
+  // Windows binaries have an exe suffix.
+  if (os === "windows") {
+    binName += ".exe"
+  }
+  return binName
+}
+
+/**
+ * Returns the Go format for the current platform.
+ * Coder binaries are created in Go, so we conform to that name structure.
+ */
+export function goos(): string {
+  const platform = os.platform()
+  switch (platform) {
+    case "win32":
+      return "windows"
+    default:
+      return platform
+  }
+}
+
+/**
+ * Return the Go format for the current architecture.
+ */
+export function goarch(): string {
+  const arch = os.arch()
+  switch (arch) {
+    case "arm":
+      return "armv7"
+    case "x64":
+      return "amd64"
+    default:
+      return arch
+  }
+}

src/remote.ts

@@ -465,7 +465,14 @@ export class Remote {
     //
     // If we didn't write to the SSH config file, connecting would fail with
     // "Host not found".
-    await this.updateSSHConfig(authorityParts[1], hasCoderLogs)
+    try {
+      await this.updateSSHConfig(authorityParts[1], hasCoderLogs)
+    } catch (error) {
+      // TODO: This is a bit weird, because even if we throw an error VS Code
+      // still tries to connect.  Can we stop it?
+      this.storage.writeToCoderOutputChannel(`Failed to configure SSH: ${error}`)
+      throw error
+    }
 
     this.findSSHProcessID().then((pid) => {
       // Once the SSH process has spawned we can reset the timeout.
@@ -587,9 +594,6 @@ export class Remote {
         binaryPath = await this.storage.fetchBinary()
       }
     }
-    if (!binaryPath) {
-      throw new Error("Failed to fetch the Coder binary!")
-    }
 
     const escape = (str: string): string => `"${str.replace(/"/g, '\\"')}"`