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 } 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

@@ -6,8 +6,8 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
-import execa from "execa"
 import { exec } from "../../../util/util"
+import { ChildProcessError } from "../../../exceptions"
 
 /**
  * Automatically set docker environment variables for minikube
@@ -19,7 +19,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)
@@ -122,6 +126,8 @@ export class CliWrapper {
    * Helper for using spawn with live log streaming. Waits for the command to finish before returning.
    *
    * If an error occurs and no output has been written to stderr, we use stdout for the error message instead.
+   *
+   * @throws RuntimeError
    */
   async spawnAndStreamLogs({
     args,
@@ -145,6 +151,10 @@ export class CliWrapper {
     })
   }
 
+  /**
+   * @throws RuntimeError on ENOENT (command not found)
+   * @throws ChildProcessError on any other error condition
+   */
   async spawnAndWait({ args, cwd, env, log, ignoreError, rawMode, stdout, stderr, timeoutSec, tty }: SpawnParams) {
     const path = await this.getPath(log)
 

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,
+            },
           })
         )
       }