fix: error handling issues

core/src/exceptions.ts

@@ -36,10 +36,10 @@ export interface GardenErrorParams<D extends object = any> {
 export type GardenErrorContext = {
   taskType?: string
 }
-export abstract class GardenError<D extends object = any> extends Error {
+export abstract class GardenError<D extends object = any | undefined> extends Error {
   abstract type: string
   public override message: string
-  public detail?: D
+  public detail: D
   public wrappedErrors?: GardenError<any>[]
   public context?: GardenErrorContext
 
@@ -166,6 +166,8 @@ interface ChildProcessErrorDetails {
   args: string[]
   code: number
   output: string
+  stderr: string
+  stdout: string
   opts?: SpawnOpts
 }
 export class ChildProcessError extends GardenError<ChildProcessErrorDetails> {

core/src/plugins/kubernetes/container/build/common.ts

@@ -18,7 +18,7 @@ import {
 } from "../../constants"
 import { KubeApi } from "../../api"
 import { KubernetesPluginContext, KubernetesProvider } from "../../config"
-import { PodRunner } from "../../run"
+import { PodRunner, PodRunnerError } from "../../run"
 import { PluginContext } from "../../../../plugin-context"
 import { hashString, sleep } from "../../../../util/util"
 import { InternalError, RuntimeError } from "../../../../exceptions"
@@ -230,20 +230,25 @@ export async function skopeoBuildStatus({
     })
     return { state: "ready", outputs, detail: {} }
   } catch (err) {
-    const res = err.detail?.result || {}
-
-    // Non-zero exit code can both mean the manifest is not found, and any other unexpected error
-    if (res.exitCode !== 0 && !skopeoManifestUnknown(res.stderr)) {
-      const output = res.allLogs || err.message
-
-      throw new RuntimeError({
-        message: `Unable to query registry for image status: ${output}`,
-        detail: {
-          command: skopeoCommand,
-          output,
-        },
-      })
+    if (err instanceof PodRunnerError) {
+      const res = err.detail.result
+
+      // Non-zero exit code can both mean the manifest is not found, and any other unexpected error
+      if (res?.exitCode !== 0 && !skopeoManifestUnknown(res?.stderr)) {
+        const output = res?.allLogs || err.message
+
+        throw new RuntimeError({
+          message: `Unable to query registry for image status: ${output}`,
+          detail: {
+            command: skopeoCommand,
+            output,
+          },
+        })
+      }
     }
+
+    // TODO: Do we really want to return state: "unknown" with no details on any error, even on TypeError etc?
+    // NOTE(steffen): I'd have expected us to throw here
     return { state: "unknown", outputs, detail: {} }
   }
 }

core/src/plugins/kubernetes/local/config.ts

@@ -24,6 +24,7 @@ import chalk from "chalk"
 import { isKindCluster } from "./kind"
 import { isK3sFamilyCluster } from "./k3s"
 import { getK8sClientServerVersions, K8sClientServerVersions } from "../util"
+import { ChildProcessError } from "../../../exceptions"
 
 // TODO: split this into separate plugins to handle Docker for Mac and Minikube
 // note: this is in order of preference, in case neither is set as the current kubectl context
@@ -154,7 +155,10 @@ export async function configureProvider(params: ConfigureProviderParams<LocalKub
       try {
         await exec("minikube", ["addons", "enable", "ingress"])
       } catch (err) {
-        providerLog.warn(chalk.yellow(`Unable to enable minikube ingress addon: ${err.all}`))
+        if (!(err instanceof ChildProcessError)) {
+          throw err
+        }
+        providerLog.warn(chalk.yellow(`Unable to enable minikube ingress addon: ${err.detail.output}`))
       }
       remove(_systemServices, (s) => nginxServices.includes(s))
     }

core/src/plugins/kubernetes/local/microk8s.ts

@@ -7,7 +7,7 @@
  */
 
 import tmp from "tmp-promise"
-import { RuntimeError } from "../../../exceptions"
+import { ChildProcessError, RuntimeError, toGardenError } from "../../../exceptions"
 import { Log } from "../../../logger/log-entry"
 import { exec } from "../../../util/util"
 import { containerHelpers } from "../../container/helpers"
@@ -20,15 +20,18 @@ import { parse as parsePath } from "path"
 
 // TODO: Pass the correct log context instead of creating it here.
 export async function configureMicrok8sAddons(log: Log, addons: string[]) {
-  let statusCommandResult: ExecaReturnValue | undefined = undefined
+  let statusCommandResult: ExecaReturnValue | ChildProcessError | undefined = undefined
   let status = ""
   const microK8sLog = log.createLog({ name: "microk8s" })
 
   try {
     statusCommandResult = await exec("microk8s", ["status"])
     status = statusCommandResult.stdout
   } catch (err) {
-    if (err.all?.includes("permission denied") || err.all?.includes("Insufficient permissions")) {
+    if (!(err instanceof ChildProcessError)) {
+      throw err
+    }
+    if (err.detail.output.includes("permission denied") || err.detail.output.includes("Insufficient permissions")) {
       microK8sLog.warn(
         chalk.yellow(
           deline`Unable to get microk8s status and manage addons. You may need to add the current user to the microk8s
@@ -147,9 +150,6 @@ export async function loadImageToMicrok8s({
   } catch (err) {
     throw new RuntimeError({
       message: `An attempt to load image ${imageId} into the microk8s cluster failed: ${err.message}`,
-      detail: {
-        err,
-      },
     })
   }
 }

core/src/plugins/kubernetes/local/minikube.ts

@@ -8,6 +8,7 @@
 
 import execa from "execa"
 import { exec } from "../../../util/util"
+import { ChildProcessError } from "../../../exceptions"
 
 /**
  * Automatically set docker environment variables for minikube
@@ -19,7 +20,10 @@ export async function setMinikubeDockerEnv() {
   try {
     minikubeEnv = (await exec("minikube", ["docker-env", "--shell=bash"])).stdout
   } catch (err) {
-    if ((<execa.ExecaError>err).stderr?.includes("driver does not support")) {
+    if (!(err instanceof ChildProcessError)) {
+      throw err
+    }
+    if (err.detail.output?.includes("driver does not support")) {
       return
     }
     throw err

core/src/plugins/kubernetes/run.ts

@@ -735,7 +735,7 @@ type RunParams = StartParams & {
   throwOnExitCode?: boolean
 }
 
-class PodRunnerError extends GardenError {
+export class PodRunnerError extends GardenError<PodErrorDetails> {
   type = "pod-runner"
 }
 

core/src/router/build.ts

@@ -54,20 +54,16 @@ export const buildRouter = (baseParams: BaseRouterParams) =>
         })
       })
 
-      try {
-        const output = await router.callHandler({
-          params,
-          handlerType: "build",
-          defaultHandler: async () => ({ state: "unknown" as const, outputs: {}, detail: {} }),
-        })
-        const { result } = output
+      const output = await router.callHandler({
+        params,
+        handlerType: "build",
+        defaultHandler: async () => ({ state: "unknown" as const, outputs: {}, detail: {} }),
+      })
+      const { result } = output
 
-        await router.validateActionOutputs(action, "runtime", result.outputs)
+      await router.validateActionOutputs(action, "runtime", result.outputs)
 
-        return output
-      } catch (err) {
-        throw err
-      }
+      return output
     },
 
     publish: async (params) => {

core/src/util/ext-tools.ts

@@ -63,6 +63,10 @@ export class CliWrapper {
     return this.toolPath
   }
 
+  /**
+   * @throws RuntimeError on EMFILE (Too many open files)
+   * @throws ChildProcessError on any other error condition
+   */
   async exec({ args, cwd, env, log, timeoutSec, input, ignoreError, stdout, stderr }: ExecParams) {
     const path = await this.getPath(log)
 
@@ -86,19 +90,19 @@ export class CliWrapper {
     })
   }
 
+  /**
+   * @throws RuntimeError on EMFILE (Too many open files)
+   * @throws ChildProcessError on any other error condition
+   */
   async stdout(params: ExecParams) {
-    try {
-      const res = await this.exec(params)
-      return res.stdout
-    } catch (err) {
-      // Add log output to error
-      if (err.all) {
-        err.message += "\n\n" + err.all
-      }
-      throw err
-    }
+    const res = await this.exec(params)
+    return res.stdout
   }
 
+  /**
+   * @throws RuntimeError on EMFILE (Too many open files)
+   * @throws ChildProcessError on any other error condition
+   */
   async json(params: ExecParams) {
     const out = await this.stdout(params)
     return JSON.parse(out)

core/src/util/util.ts

@@ -225,6 +225,9 @@ export interface ExecOpts extends execa.Options {
  * Enforces `buffer: true` (which is the default execa behavior).
  *
  * Also adds the ability to pipe stdout|stderr to an output stream.
+ *
+ * @throws RuntimeError on EMFILE (Too many open files)
+ * @throws ChildProcessError on any other error condition
  */
 export async function exec(cmd: string, args: string[], opts: ExecOpts = {}) {
   // Ensure buffer is always set to true so that we can read the error output
@@ -266,6 +269,8 @@ export async function exec(cmd: string, args: string[], opts: ExecOpts = {}) {
         args,
         code: error.exitCode || err.code || err.errno,
         output: error.all || error.stdout || error.stderr || "",
+        stderr: error.stderr || "",
+        stdout: error.stdout || "",
       },
     })
   }
@@ -297,6 +302,9 @@ export interface SpawnOutput {
 /**
  * Note: For line-by-line stdout/stderr streaming, both `opts.stdout` and `opts.stderr` must be defined. This is a
  * result of how Node's child process spawning works (which this function wraps).
+ *
+ * @throws RuntimeError on ENOENT (command not found)
+ * @throws ChildProcessError on any other error condition
  */
 export function spawn(cmd: string, args: string[], opts: SpawnOpts = {}) {
   const {
@@ -405,7 +413,15 @@ export function spawn(cmd: string, args: string[], opts: SpawnOpts = {}) {
         _reject(
           new ChildProcessError({
             message: msg,
-            detail: { cmd, args, opts, code: result.code, output: result.all },
+            detail: {
+              cmd,
+              args,
+              opts,
+              code: result.code,
+              output: result.all,
+              stderr: result.stderr,
+              stdout: result.stdout,
+            },
           })
         )
       }

core/src/vcs/git.ts

@@ -12,7 +12,7 @@ import { isString } from "lodash"
 import { createReadStream, ensureDir, lstat, pathExists, readlink, realpath, stat, Stats } from "fs-extra"
 import { PassThrough } from "stream"
 import { GetFilesParams, RemoteSourceParams, VcsFile, VcsHandler, VcsHandlerParams, VcsInfo } from "./vcs"
-import { ConfigurationError, RuntimeError } from "../exceptions"
+import { ChildProcessError, ConfigurationError, RuntimeError } from "../exceptions"
 import { getStatsType, joinWithPosix, matchPath } from "../util/fs"
 import { dedent, deline, splitLast } from "../util/string"
 import { defer, exec } from "../util/util"
@@ -88,6 +88,9 @@ export class GitHandler extends VcsHandler {
   }
 
   gitCli(log: Log, cwd: string, failOnPrompt = false): GitCli {
+    /**
+     * @throws ChildProcessError
+     */
     return async (...args: (string | undefined)[]) => {
       log.silly(`Calling git with args '${args.join(" ")}' in ${cwd}`)
       const { stdout } = await exec("git", args.filter(isString), {
@@ -103,7 +106,7 @@ export class GitHandler extends VcsHandler {
     try {
       return await git("diff-index", "--name-only", "HEAD", path)
     } catch (err) {
-      if (err.exitCode === 128) {
+      if (err instanceof ChildProcessError && err.detail.code === 128) {
         // no commit in repo
         return []
       } else {
@@ -165,8 +168,12 @@ export class GitHandler extends VcsHandler {
         await git("status")
         gitSafeDirs.add(path)
       } catch (err) {
+        if (!(err instanceof ChildProcessError)) {
+          throw err
+        }
+
         // Git has stricter repo ownerships checks since 2.36.0
-        if (err.exitCode === 128 && err.stderr?.toLowerCase().includes("fatal: unsafe repository")) {
+        if (err.detail.code === 128 && err.detail.stderr.toLowerCase().includes("fatal: unsafe repository")) {
           log.warn(
             chalk.yellow(
               `It looks like you're using Git 2.36.0 or newer and the directory "${path}" is owned by someone else. It will be added to safe.directory list in the .gitconfig.`
@@ -187,11 +194,11 @@ export class GitHandler extends VcsHandler {
           }
 
           return
-        } else if (err.exitCode === 128 && err.stderr?.toLowerCase().includes("fatal: not a git repository")) {
+        } else if (err.detail.code === 128 && err.detail.stderr.toLowerCase().includes("fatal: not a git repository")) {
           throw new RuntimeError({ message: notInRepoRootErrorMessage(path), detail: { path } })
         } else {
           log.error(
-            `Unexpected Git error occurred while running 'git status' from path "${path}". Exit code: ${err.exitCode}. Error message: ${err.stderr}`
+            `Unexpected Git error occurred while running 'git status' from path "${path}". Exit code: ${err.detail.code}. Error message: ${err.detail.stderr}`
           )
           throw err
         }
@@ -224,17 +231,23 @@ export class GitHandler extends VcsHandler {
         this.repoRoots.set(path, repoRoot)
         return repoRoot
       } catch (err) {
-        if (err.exitCode === 128 && err.stderr?.toLowerCase().includes("fatal: unsafe repository")) {
+        if (!(err instanceof ChildProcessError)) {
+          throw err
+        }
+        if (err.detail.code === 128 && err.detail.stderr.toLowerCase().includes("fatal: unsafe repository")) {
           // Throw nice error when we detect that we're not in a repo root
           throw new RuntimeError({
             message:
-              err.stderr +
+              err.detail.stderr +
               `\nIt looks like you're using Git 2.36.0 or newer and the repo directory containing "${path}" is owned by someone else. If this is intentional you can run "git config --global --add safe.directory '<repo root>'" and try again.`,
             detail: { path },
           })
-        } else if (err.exitCode === 128) {
+        } else if (err.detail.code === 128) {
           // Throw nice error when we detect that we're not in a repo root
-          throw new RuntimeError({ message: notInRepoRootErrorMessage(path), detail: { path, exitCode: err.exitCode } })
+          throw new RuntimeError({
+            message: notInRepoRootErrorMessage(path),
+            detail: { path, exitCode: err.detail.code },
+          })
         } else {
           throw err
         }
@@ -291,8 +304,7 @@ export class GitHandler extends VcsHandler {
         return []
       }
     } catch (err) {
-      // 128 = File no longer exists
-      if (err.exitCode === 128 || err.code === "ENOENT") {
+      if (err.code === "ENOENT") {
         gitLog.warn(`Attempted to scan directory at ${path}, but it does not exist.`)
         return []
       } else {
@@ -386,8 +398,7 @@ export class GitHandler extends VcsHandler {
             return []
           }
         } catch (err) {
-          // 128 = File no longer exists
-          if (err.exitCode === 128 || err.code === "ENOENT") {
+          if (err.code === "ENOENT") {
             gitLog.warn(
               `Found reference to submodule at ${submoduleRelPath}, but the path could not be found. ${submoduleErrorSuggestion}`
             )
@@ -783,7 +794,7 @@ export class GitHandler extends VcsHandler {
       output.branch = (await git("rev-parse", "--abbrev-ref", "HEAD"))[0]
       output.commitHash = (await git("rev-parse", "HEAD"))[0]
     } catch (err) {
-      if (err.exitCode !== 128) {
+      if (err instanceof ChildProcessError && err.detail.code !== 128) {
         throw err
       }
     }