Ensure exec/spawn failures include stderr

Without this it would be difficult for users to debug.

Author: code-asher

fixtures/coder

@@ -2,39 +2,46 @@
 # Coder CLI mock
 
 need_login() {
-  echo "fatal: failed to read session credentials"
-  echo "  | hint: did you run \"coder login [https://coder.domain.com]\"?"
+  >&2 echo "fatal: failed to read session credentials"
+  >&2 echo "  | hint: did you run \"coder login [https://coder.domain.com]\"?"
   exit 1
 }
 
-fail() {
-  echo fail
+invalid_token() {
+  >&2 echo "fatal: Get \"https://coder.domain.com/api\": net/http: invalid header field value \"some value\" for key Session-Token"
   exit 1
 }
 
-success () {
-  echo success
-  exit 0
+fail() {
+  >&2 echo stderr message from fail state
+  exit 1
 }
 
 workspaces() {
   cat ./workspaces.json
   exit 0
 }
 
+help() {
+  echo help
+  exit 0
+}
+
 main() {
   cd "$(dirname "${0}")"
 
   # Use CODER_MOCK_STATE to force certain states.
   case "${CODER_MOCK_STATE-}" in
     "need_login") need_login ;;
+    "invalid_token") invalid_token ;;
+    "fail_stderr") fail_stderr ;;
+    "fail") fail ;;
   esac
 
   # Mock the output based on the passed arguments.
   case "$*" in
     "envs ls --output json") workspaces ;;
-    "test fail") fail ;;
-    "test success") success ;;
+    "--help") help ;;
     *) echo "$* is not mocked" ; exit 1 ;;
   esac
 }

src/download.test.ts

@@ -5,16 +5,23 @@ import * as download from "./download"
 suite("Download", () => {
   vscode.window.showInformationMessage("Start download tests.")
 
+  teardown(() => {
+    delete process.env.CODER_MOCK_STATE
+  })
+
   test("binaryExists", async () => {
     assert.strictEqual(await download.binaryExists("sh"), true)
     assert.strictEqual(await download.binaryExists("surely-no-binary-named-like-this-exists"), false)
   })
 
   test("execCoder", async () => {
-    assert.strictEqual(await download.execCoder("test success"), "success\n")
-    await assert.rejects(download.execCoder("test fail"), {
+    assert.strictEqual(await download.execCoder("--help"), "help\n")
+
+    // This will attempt to authenticate first, which will fail.
+    process.env.CODER_MOCK_STATE = "fail"
+    await assert.rejects(download.execCoder("--help"), {
       name: "Error",
-      message: /Command failed: .+ test fail/,
+      message: /Command failed: .+ --help\nstderr message from fail state\n/,
     })
   })
 
@@ -23,6 +30,7 @@ suite("Download", () => {
       name: "Error",
       message: `Command "false" failed with code 1`,
     })
+    // TODO: Test successful download.
   })
 
   // TODO: Implement.

src/download.ts

@@ -17,8 +17,9 @@ export const binaryExists = async (bin: string): Promise<boolean> => {
 }
 
 /**
- * Run a command with the Coder CLI after making sure it is installed. Stderr is
- * ignored; only stdout is returned.
+ * Run a command with the Coder CLI after making sure it is installed.  On
+ * success stdout is returned.  On failure the error will include stderr in the
+ * message.
  */
 export const execCoder = async (command: string): Promise<string> => {
   debug(`Run command: ${command}`)

src/utils.test.ts

@@ -53,6 +53,10 @@ suite("Utils", () => {
       name: "Error",
       message: `Command "false" failed with code 1`,
     })
+    await assert.rejects(utils.wrapExit(cp.spawn("bash", ["-c", ">&2 printf stderr && exit 42"])), {
+      name: "Error",
+      message: `Command "bash" failed with code 42: stderr`,
+    })
     await assert.rejects(utils.wrapExit(cp.spawn("surely-no-executable-named-like-this-exists")), {
       name: "Error",
       message: `spawn surely-no-executable-named-like-this-exists ENOENT`,

src/utils.ts

@@ -50,6 +50,8 @@ export const split = (str: string, delimiter: string): [string, string] => {
  *
  * The callback will always fire at least once (even with just a blank string)
  * even if the process has no output.
+ *
+ * This will set the encoding on the stream to utf8.
  */
 export const onLine = (stream: stream.Readable, callback: (line: string) => void): void => {
   let buffer = ""
@@ -73,18 +75,23 @@ export const onLine = (stream: stream.Readable, callback: (line: string) => void
 }
 
 /**
- * Wrap a promise around a spawned process's exit.
+ * Wrap a promise around a spawned process's exit.  Rejects if the code is
+ * non-zero.  The error will include the code and the stderr if any in the
+ * message.
  *
  * Use in conjunction with `child_process.spawn()`.
  */
 export function wrapExit(proc: cp.ChildProcess): Promise<void> {
   return new Promise<void>((resolve, reject) => {
+    const stderr: string[] = []
+    proc.stderr?.on("data", (d) => stderr.push(d.toString()))
     proc.on("error", reject) // Catches ENOENT for example.
     proc.on("exit", (code) => {
       if (code === 0) {
         resolve()
       } else {
-        reject(new Error(`Command "${proc.spawnfile}" failed with code ${code}`))
+        const details = stderr.length > 0 ? `: ${stderr.join()}` : ""
+        reject(new Error(`Command "${proc.spawnfile}" failed with code ${code}${details}`))
       }
     })
   })