fix(framework): several error handling improvements

- Renamed InternalError to make it more clear that it's indicating a crash.
  This error type is used to wrap unhandled errors that originate from libraries
  or the NodeJS standard library, and also denotes errors that "should not happen".

- Stop masking leaf errors (leaf means basically the root cause).
  One of the most occuring errors on Amplitude is "graph", but that's a
  wrapping error. This will give us better insights in what error types
  actually causing command failures.

- Consistently handle unexpected errors and format them equally on all
  levels.

- Format errors that should not happen ("crashes") with an explanation
  and a call to action to report the bug on all levels

- Treat uncaught errors on the CLI level (cli.ts) as crashes, even if
  they are GardenError instances, because the GardenCli class should not
  throw but return an exit code.

cli/src/cli.ts

@@ -13,6 +13,7 @@ import { GlobalConfigStore } from "@garden-io/core/build/src/config-store/global
 import { getOtelSDK } from "@garden-io/core/build/src/util/open-telemetry/tracing"
 import { withContextFromEnv } from "@garden-io/core/build/src/util/open-telemetry/propagation"
 import { wrapActiveSpan } from "@garden-io/core/build/src/util/open-telemetry/spans"
+import { UnexpectedCrashError, explainGardenError } from "@garden-io/core/build/src/exceptions"
 
 // These plugins are always registered
 export const getBundledPlugins = (): GardenPluginReference[] => [
@@ -53,8 +54,7 @@ export async function runCli({
     )
     code = result.code
   } catch (err) {
-    // eslint-disable-next-line no-console
-    console.log(`Warning: Exiting with unhandled error\n${err.message}`)
+    logUnexpectedError(err, "Unhandled error")
     code = 1
   } finally {
     if (cli?.processRecord) {
@@ -65,12 +65,20 @@ export async function runCli({
     try {
       await Promise.race([getOtelSDK().shutdown(), new Promise((resolve) => setTimeout(resolve, 3000))])
     } catch (err) {
-      // eslint-disable-next-line no-console
-      console.log(`Debug: OTEL shutdown failed with error ${err.toString()}`)
+      logUnexpectedError(err, "OTEL shutdown failed")
     }
 
     await shutdown(code)
   }
 
   return { cli, result }
 }
+
+function logUnexpectedError(error: Error, context: string) {
+  // NOTE: If this function is called, this is always a bug, because GardenCli.run is designed to return an error code. If it throws an error, something is wrong with our code and we need to fix it.
+  // This is why we wrap the error with UnexpectedCrashError here, even if it is a GardenBaseError.
+  // If an error hits this code path, it's definitely a crash and we need to fix that bug.
+  const wrappedError = UnexpectedCrashError.wrapError(error, {}, context)
+  // eslint-disable-next-line no-console
+  console.log(`${context}: ${explainGardenError(wrappedError, context)}`)
+}

core/src/actions/base.ts

@@ -37,7 +37,7 @@ import cloneDeep from "fast-copy"
 import { flatten, fromPairs, isString, memoize, omit, sortBy } from "lodash"
 import { ActionConfigContext, ActionSpecContext } from "../config/template-contexts/actions"
 import { relative } from "path"
-import { InternalError } from "../exceptions"
+import { ConfigurationError } from "../exceptions"
 import {
   Action,
   ActionConfig,
@@ -731,7 +731,7 @@ export abstract class ResolvedRuntimeAction<
       const buildAction = this.getResolvedDependencies().find((a) => a.kind === "Build" && a.name === buildName)
 
       if (!buildAction) {
-        throw new InternalError({
+        throw new ConfigurationError({
           message: `Could not find build dependency '${buildName}' specified on the build field on ${this.longDescription()}.`,
           detail: { action: this.key(), buildName },
         })

core/src/build-staging/build-staging.ts

@@ -8,7 +8,7 @@
 
 import { isAbsolute, join, resolve, relative, parse, basename } from "path"
 import { emptyDir, ensureDir, mkdirp, pathExists, remove } from "fs-extra"
-import { ConfigurationError, InternalError } from "../exceptions"
+import { ConfigurationError, UnexpectedCrashError } from "../exceptions"
 import { normalizeRelativePath, joinWithPosix } from "../util/fs"
 import { Log } from "../logger/log-entry"
 import { Profile } from "../util/profiling"
@@ -185,7 +185,7 @@ export class BuildStaging {
     }
 
     if (sourceRelPath && isAbsolute(sourceRelPath)) {
-      throw new InternalError({
+      throw new UnexpectedCrashError({
         message: `Build staging: Got absolute path for sourceRelPath`,
         detail: {
           sourceRoot,
@@ -197,7 +197,7 @@ export class BuildStaging {
     }
 
     if (targetRelPath && isAbsolute(targetRelPath)) {
-      throw new InternalError({
+      throw new UnexpectedCrashError({
         message: `Build staging: Got absolute path for targetRelPath`,
         detail: {
           sourceRoot,
@@ -213,7 +213,7 @@ export class BuildStaging {
     // Source root must exist and be a directory
     let sourceStat = await statsHelper.extendedStat({ path: sourceRoot })
     if (!sourceStat || !(sourceStat.isDirectory() || sourceStat.target?.isDirectory())) {
-      throw new InternalError({
+      throw new UnexpectedCrashError({
         message: `Build staging: Source root ${sourceRoot} must exist and be a directory`,
         detail: {
           sourceRoot,
@@ -296,7 +296,7 @@ export class BuildStaging {
 
     // Throw if file list is specified and source+target are not both directories
     if (files && (!sourceStat?.isDirectory() || !targetStat?.isDirectory())) {
-      throw new InternalError({
+      throw new UnexpectedCrashError({
         message: `Build staging: Both source and target must be directories when specifying a file list`,
         detail: {
           sourceRoot,

core/src/build-staging/helpers.ts

@@ -12,7 +12,7 @@ import { splitLast } from "../util/string"
 import { Minimatch } from "minimatch"
 import { isAbsolute, parse, basename, resolve } from "path"
 import { ensureDir, Stats, lstat, remove } from "fs-extra"
-import { FilesystemError, InternalError } from "../exceptions"
+import { FilesystemError, UnexpectedCrashError } from "../exceptions"
 import async, { AsyncResultCallback } from "async"
 import { round } from "lodash"
 import { promisify } from "util"
@@ -370,13 +370,7 @@ export class FileStatsHelper {
         // Symlink target not found, so we ignore it
         return cb(null, null)
       } else if (readlinkErr) {
-        return cb(
-          new InternalError({
-            message: `Error reading symlink: ${readlinkErr.message}`,
-            detail: { path, readlinkErr },
-          }),
-          null
-        )
+        return cb(UnexpectedCrashError.wrapError(readlinkErr, { path, readlinkErr }, "Error reading symlink"), null)
       }
 
       // Ignore absolute symlinks unless specifically allowed
@@ -417,7 +411,7 @@ export class FileStatsHelper {
 
   private assertAbsolute(path: string) {
     if (!isAbsolute(path)) {
-      throw new InternalError({ message: `Must specify absolute path (got ${path})`, detail: { path } })
+      throw new UnexpectedCrashError({ message: `Must specify absolute path (got ${path})`, detail: { path } })
     }
   }
 }

core/src/cache.ts

@@ -8,7 +8,7 @@
 
 import { isEqual } from "lodash"
 import { normalize, parse, sep } from "path"
-import { InternalError, NotFoundError, ParameterError } from "./exceptions"
+import { UnexpectedCrashError, NotFoundError, ParameterError } from "./exceptions"
 import { Log } from "./logger/log-entry"
 
 export type CacheKey = string[]
@@ -172,7 +172,10 @@ export class TreeCache {
       pairs = Array.from(node.entries).map((stringKey) => {
         const entry = this.cache.get(stringKey)
         if (!entry) {
-          throw new InternalError({ message: `Invalid reference found in cache: ${stringKey}`, detail: { stringKey } })
+          throw new UnexpectedCrashError({
+            message: `Invalid reference found in cache: ${stringKey}`,
+            detail: { stringKey },
+          })
         }
         return <[CacheKey, CacheValue]>[entry.key, entry.value]
       })

core/src/cli/cli.ts

@@ -13,7 +13,7 @@ import { pathExists } from "fs-extra"
 import { getBuiltinCommands } from "../commands/commands"
 import { shutdown, getPackageVersion, getCloudDistributionName } from "../util/util"
 import { Command, CommandResult, CommandGroup, BuiltinArgs } from "../commands/base"
-import { PluginError, toGardenError, GardenBaseError } from "../exceptions"
+import { PluginError, toGardenError, GardenBaseError, explainGardenError } from "../exceptions"
 import { Garden, GardenOpts, makeDummyGarden } from "../garden"
 import { getRootLogger, getTerminalWriterType, LogLevel, parseLogLevel, RootLogger } from "../logger/logger"
 import { FileWriter, FileWriterConfig } from "../logger/writers/file-writer"
@@ -402,6 +402,11 @@ ${renderCommands(commands)}
     processRecord?: GardenProcess
     cwd?: string
   }): Promise<RunOutput> {
+    // for testing
+    if (args.includes("--test-cli-error")) {
+      throw new Error("Test")
+    }
+
     let argv = parseCliArgs({ stringArgs: args, cli: true })
 
     const errors: (GardenBaseError | Error)[] = []
@@ -443,14 +448,19 @@ ${renderCommands(commands)}
       "logger-type": loggerTypeOpt,
       "log-level": logLevelStr,
     } = argv
-    const logger = RootLogger.initialize({
-      level: parseLogLevel(logLevelStr),
-      storeEntries: false,
-      displayWriterType: getTerminalWriterType({ silent, output, loggerTypeOpt, commandLoggerType: null }),
-      useEmoji: emoji,
-      showTimestamps,
-      force: this.initLogger,
-    })
+    let logger: RootLogger
+    try {
+      logger = RootLogger.initialize({
+        level: parseLogLevel(logLevelStr),
+        storeEntries: false,
+        displayWriterType: getTerminalWriterType({ silent, output, loggerTypeOpt, commandLoggerType: null }),
+        useEmoji: emoji,
+        showTimestamps,
+        force: this.initLogger,
+      })
+    } catch (error) {
+      return done(1, explainGardenError(error, "Failed to initialize logger"))
+    }
 
     const log = logger.createLog()
     log.verbose(`garden version: ${getPackageVersion()}`)
@@ -562,7 +572,10 @@ ${renderCommands(commands)}
       if (gardenErrors.length > 0 || (commandResult.exitCode && commandResult.exitCode !== 0)) {
         return done(
           commandResult.exitCode || 1,
-          renderer({ success: false, errors: gardenErrors }),
+          renderer({
+            success: false,
+            errors: gardenErrors,
+          }),
           commandResult?.result
         )
       } else {

core/src/cli/helpers.ts

@@ -19,7 +19,7 @@ import { maxBy, zip } from "lodash"
 import { Logger } from "../logger/logger"
 
 import { ParameterValues, Parameter, Parameters, globalDisplayOptions } from "./params"
-import { GardenBaseError, InternalError, ParameterError, toGardenError } from "../exceptions"
+import { GardenBaseError, ParameterError, RuntimeError, toGardenError } from "../exceptions"
 import { getPackageVersion, removeSlice } from "../util/util"
 import { Log } from "../logger/log-entry"
 import { STATIC_DIR, VERSION_CHECK_URL, gardenEnv, ERROR_LOG_FILENAME } from "../constants"
@@ -58,7 +58,8 @@ export function helpTextMaxWidth() {
 
 export async function checkForStaticDir() {
   if (!(await pathExists(STATIC_DIR))) {
-    throw new InternalError({
+    // if this happens, this is most likely not a Garden bug but some kind of corrupted installation, and the user needs to do something about it.
+    throw new RuntimeError({
       message:
         `Could not find the static data directory. Garden is packaged with a data directory ` +
         `called 'static', which should be located next to your garden binary. Please try reinstalling, ` +

core/src/commands/base.ts

@@ -20,7 +20,7 @@ import {
   joiStringMap,
   joiVariables,
 } from "../config/common"
-import { InternalError, RuntimeError, GardenBaseError, GardenError, isGardenError } from "../exceptions"
+import { RuntimeError, GardenBaseError, GardenError, UnexpectedCrashError, toGardenError } from "../exceptions"
 import { Garden } from "../garden"
 import { Log } from "../logger/log-entry"
 import { LoggerType, LoggerBase, LoggerConfigBase, eventLogLevel, LogLevel } from "../logger/logger"
@@ -210,7 +210,7 @@ export abstract class Command<A extends Parameters = {}, O extends Parameters =
     if (this.arguments && this.options) {
       for (const key of Object.keys(this.options)) {
         if (key in this.arguments) {
-          throw new InternalError({
+          throw new UnexpectedCrashError({
             message: `Key ${key} is defined in both options and arguments for command ${commandName}`,
             detail: {
               commandName,
@@ -229,7 +229,7 @@ export abstract class Command<A extends Parameters = {}, O extends Parameters =
 
       // Make sure arguments don't have default values
       if (arg.defaultValue) {
-        throw new InternalError({
+        throw new UnexpectedCrashError({
           message: `A positional argument cannot have a default value`,
           detail: {
             commandName,
@@ -241,7 +241,7 @@ export abstract class Command<A extends Parameters = {}, O extends Parameters =
       if (arg.required) {
         // Make sure required arguments don't follow optional ones
         if (foundOptional) {
-          throw new InternalError({
+          throw new UnexpectedCrashError({
             message: `A required argument cannot follow an optional one`,
             detail: {
               commandName,
@@ -255,7 +255,7 @@ export abstract class Command<A extends Parameters = {}, O extends Parameters =
 
       // Make sure only last argument is spread
       if (arg.spread && i < args.length - 1) {
-        throw new InternalError({
+        throw new UnexpectedCrashError({
           message: `Only the last command argument can set spread to true`,
           detail: {
             commandName,
@@ -1079,7 +1079,7 @@ export async function handleProcessResults(
 
   if (!success) {
     const wrappedErrors: GardenError[] = flatMap(failed, (f) => {
-      return f && f.error && isGardenError(f.error) ? [f.error as GardenError] : []
+      return f && f.error ? [toGardenError(f.error)] : []
     })
 
     const error = new RuntimeError({

core/src/commands/custom.ts

@@ -24,7 +24,7 @@ import {
 import { joi } from "../config/common"
 import { CustomCommandContext } from "../config/template-contexts/custom-command"
 import { validateWithPath } from "../config/validation"
-import { ConfigurationError, GardenBaseError, InternalError, RuntimeError, toGardenError } from "../exceptions"
+import { ConfigurationError, GardenBaseError, RuntimeError, UnexpectedCrashError, toGardenError } from "../exceptions"
 import { resolveTemplateStrings } from "../template-string/template-string"
 import { listDirectory, isConfigFilename } from "../util/fs"
 import { Command, CommandParams, CommandResult, PrintHeaderParams } from "./base"
@@ -185,7 +185,7 @@ export class CustomCommandWrapper extends Command {
 
       // Doing runtime check to avoid updating hundreds of test invocations with a new required param, sorry. - JE
       if (!cli) {
-        throw new InternalError({ message: `Missing cli argument in custom command wrapperd.`, detail: {} })
+        throw new UnexpectedCrashError({ message: `Missing cli argument in custom command wrapperd.`, detail: {} })
       }
 
       // Pass explicitly set global opts with the command, if they're not set in the command itself.

core/src/commands/login.ts

@@ -11,7 +11,7 @@ import { printHeader } from "../logger/util"
 import dedent = require("dedent")
 import { AuthTokenResponse, CloudApi, getGardenCloudDomain } from "../cloud/api"
 import { Log } from "../logger/log-entry"
-import { ConfigurationError, InternalError, TimeoutError } from "../exceptions"
+import { ConfigurationError, TimeoutError, UnexpectedCrashError } from "../exceptions"
 import { AuthRedirectServer } from "../cloud/auth"
 import { EventBus } from "../events/events"
 import { getCloudDistributionName } from "../util/util"
@@ -150,7 +150,7 @@ export async function login(log: Log, cloudDomain: string, events: EventBus) {
   })
   await server.close()
   if (!response) {
-    throw new InternalError({ message: `Error: Did not receive an auth token after logging in.`, detail: {} })
+    throw new UnexpectedCrashError({ message: `Error: Did not receive an auth token after logging in.`, detail: {} })
   }
 
   return response

core/src/commands/version.ts

@@ -7,7 +7,7 @@
  */
 
 import { getPackageVersion } from "../util/util"
-import { Command, CommandResult } from "./base"
+import { Command, CommandParams, CommandResult } from "./base"
 
 interface VersionCommandResult {
   version: string
@@ -19,10 +19,15 @@ export class VersionCommand extends Command {
   help = "Shows the current garden version."
   override noProject = true
 
-  async action({ log }): Promise<CommandResult<VersionCommandResult>> {
+  override async action(params: CommandParams): Promise<CommandResult<VersionCommandResult>> {
+    const { log, opts } = params
     const version = getPackageVersion()
     log.info(`garden version: ${version}`)
 
+    if (opts.env === "test-command-error") {
+      throw new Error("Test")
+    }
+
     return {
       result: { version },
     }

core/src/config-store/base.ts

@@ -9,9 +9,9 @@
 import { ensureFile, readFile } from "fs-extra"
 import { z, ZodType } from "zod"
 import { lock } from "proper-lockfile"
-import { InternalError } from "../exceptions"
 import { dump } from "js-yaml"
 import writeFileAtomic from "write-file-atomic"
+import { UnexpectedCrashError } from "../exceptions"
 
 // Just a shorthand to make the code below a little more compact
 type I<T extends ZodType<any>> = z.infer<T>
@@ -95,7 +95,7 @@ export abstract class ConfigStore<T extends z.ZodObject<any>> {
       const record = config[section]?.[key]
 
       if (!record) {
-        throw new InternalError({
+        throw new UnexpectedCrashError({
           message: `The config store does not contain a record for key '${String(section)}.${String(
             key
           )}. Cannot update.'`,
@@ -144,7 +144,7 @@ export abstract class ConfigStore<T extends z.ZodObject<any>> {
       return this.schema.parse(data)
     } catch (error) {
       const configPath = this.getConfigPath()
-      throw new InternalError({
+      throw new UnexpectedCrashError({
         message: `Validation error(s) when ${context} configuration file at ${configPath}:\n${dump(error.message)}`,
         detail: {
           error,