refactor: remove error details

cli/src/cli.ts

@@ -13,7 +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 { InternalError, explainGardenError } from "@garden-io/core/build/src/exceptions"
+import { InternalError } from "@garden-io/core/build/src/exceptions"
 
 // These plugins are always registered
 export const getBundledPlugins = (): GardenPluginReference[] => [
@@ -77,8 +77,8 @@ export async function runCli({
 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 InternalError here, even if it is a GardenError already, because  if an error hits this code path, it's definitely a crash and we need to fix that bug.
-  const wrappedError = InternalError.wrapError(error, {}, context)
+  const wrappedError = InternalError.wrapError(error)
 
   // eslint-disable-next-line no-console
-  console.log(`${context}: ${explainGardenError(wrappedError, context)}`)
+  console.log(`${wrappedError.explain(context)}`)
 }

core/package.json

@@ -261,6 +261,7 @@
   "scripts": {
     "build": "gulp pegjs",
     "check-package-lock": "git diff-index --quiet HEAD -- yarn.lock || (echo 'yarn.lock is dirty!' && exit 1)",
+    "check-types": "tsc -p . --noEmit",
     "clean": "shx rm -rf build",
     "dev": "gulp pegjs-watch",
     "fix-format": "prettier --write \"{src,test}/**/*.ts\"",
@@ -281,4 +282,4 @@
     ]
   },
   "gitHead": "b0647221a4d2ff06952bae58000b104215aed922"
-}
+}

core/src/actions/base.ts

@@ -733,7 +733,6 @@ export abstract class ResolvedRuntimeAction<
       if (!buildAction) {
         throw new InternalError({
           message: `Could not find build dependency '${buildName}' specified on the build field on ${this.longDescription()}.`,
-          detail: { action: this.key(), buildName },
         })
       }
 

core/src/analytics/analytics.ts

@@ -23,7 +23,7 @@ import { Profile } from "../util/profiling"
 import { ModuleConfig } from "../config/module"
 import { UserResult } from "@garden-io/platform-api-types"
 import { uuidv4 } from "../util/random"
-import { GardenError, GardenErrorContext, StackTraceMetadata } from "../exceptions"
+import { GardenError, StackTraceMetadata } from "../exceptions"
 import { ActionConfigMap } from "../actions/types"
 import { actionKinds } from "../actions/types"
 import { getResultErrorProperties } from "./helpers"
@@ -146,8 +146,18 @@ interface ApiEvent extends EventBase {
 }
 
 export type AnalyticsGardenErrorDetail = {
+  /**
+   * The error type will be used for rendering the error to json, and also for analytics.
+   *
+   * Corresponds to GardenError.type
+   */
   errorType: string
-  context?: GardenErrorContext
+  /**
+   * The type of task, if the error was thrown as part of resolving or executing a node in the stack graph.
+   *
+   * Corresponds to GardenError.taskType
+   */
+  taskType?: string
   stackTrace?: StackTraceMetadata
 }
 

core/src/analytics/helpers.ts

@@ -15,7 +15,7 @@ function getErrorDetail(error: GardenError): AnalyticsGardenErrorDetail {
 
   return {
     errorType: error.type,
-    context: error.context,
+    taskType: error.taskType,
     stackTrace: firstEntry,
   }
 }

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

@@ -92,25 +92,18 @@ export class BuildStaging {
             message: `${action.longDescription()} specifies build '${
               copy.build
             }' in \`copyFrom\` which could not be found.`,
-            detail: { actionKey: action.key(), copy },
           })
         }
 
         if (isAbsolute(copy.sourcePath)) {
           throw new ConfigurationError({
-            message: `Source path in build dependency copy spec must be a relative path`,
-            detail: {
-              copySpec: copy,
-            },
+            message: `Source path in build dependency copy spec must be a relative path. Actually got '${copy.sourcePath}'`,
           })
         }
 
         if (isAbsolute(copy.targetPath)) {
           throw new ConfigurationError({
-            message: `Target path in build dependency copy spec must be a relative path`,
-            detail: {
-              copySpec: copy,
-            },
+            message: `Target path in build dependency copy spec must be a relative path. Actually got '${copy.targetPath}'`,
           })
         }
 
@@ -175,36 +168,18 @@ export class BuildStaging {
     if (targetRelPath && hasMagic(targetRelPath)) {
       throw new ConfigurationError({
         message: `Build staging: Target path (${targetRelPath}) must not contain wildcards`,
-        detail: {
-          sourceRoot,
-          targetRoot,
-          sourceRelPath,
-          targetRelPath,
-        },
       })
     }
 
     if (sourceRelPath && isAbsolute(sourceRelPath)) {
       throw new InternalError({
-        message: `Build staging: Got absolute path for sourceRelPath`,
-        detail: {
-          sourceRoot,
-          targetRoot,
-          sourceRelPath,
-          targetRelPath,
-        },
+        message: `Build staging: Got absolute path for sourceRelPath (${sourceRelPath})`,
       })
     }
 
     if (targetRelPath && isAbsolute(targetRelPath)) {
       throw new InternalError({
-        message: `Build staging: Got absolute path for targetRelPath`,
-        detail: {
-          sourceRoot,
-          targetRoot,
-          sourceRelPath,
-          targetRelPath,
-        },
+        message: `Build staging: Got absolute path for targetRelPath (${targetRelPath})`,
       })
     }
 
@@ -215,13 +190,6 @@ export class BuildStaging {
     if (!sourceStat || !(sourceStat.isDirectory() || sourceStat.target?.isDirectory())) {
       throw new InternalError({
         message: `Build staging: Source root ${sourceRoot} must exist and be a directory`,
-        detail: {
-          sourceRoot,
-          sourceStat,
-          targetRoot,
-          sourceRelPath,
-          targetRelPath,
-        },
       })
     }
 
@@ -275,9 +243,6 @@ export class BuildStaging {
     if (sourceShouldBeDirectory && !sourceIsDirectory) {
       throw new ConfigurationError({
         message: `Build staging: Expected source path ${sourceRoot + "/"} to be a directory`,
-        detail: {
-          sourcePath: sourceRoot + "/",
-        },
       })
     }
 
@@ -288,22 +253,13 @@ export class BuildStaging {
     if (targetShouldBeDirectory && targetStat && !targetStat.isDirectory()) {
       throw new ConfigurationError({
         message: `Build staging: Expected target path ${targetPath + "/"} to not exist or be a directory`,
-        detail: {
-          targetPath: targetPath + "/",
-        },
       })
     }
 
     // Throw if file list is specified and source+target are not both directories
     if (files && (!sourceStat?.isDirectory() || !targetStat?.isDirectory())) {
       throw new InternalError({
         message: `Build staging: Both source and target must be directories when specifying a file list`,
-        detail: {
-          sourceRoot,
-          sourceStat,
-          targetPath,
-          targetStat,
-        },
       })
     }
 
@@ -328,7 +284,6 @@ export class BuildStaging {
       if (sourceContainsWildcard) {
         throw new ConfigurationError({
           message: `Build staging: Attempting to copy multiple files from ${sourceRoot} to ${targetPath}, but a file exists at target path`,
-          detail: { sourcePath: sourceRoot, sourceStat, targetPath, targetStat },
         })
       } else if (withDelete) {
         // Source is a directory, delete file at target and create directory in its place before continuing
@@ -337,7 +292,6 @@ export class BuildStaging {
       } else {
         throw new ConfigurationError({
           message: `Build staging: Attempting to copy directory from ${sourceRoot} to ${targetPath}, but a file exists at target path`,
-          detail: { sourcePath: sourceRoot, sourceStat, targetPath, targetStat },
         })
       }
     }

core/src/build-staging/helpers.ts

@@ -67,7 +67,9 @@ export function cloneFile({ from, to, allowDelete, statsHelper }: CloneFileParam
 
       if (!sourceStats.isFile()) {
         return done(
-          new FilesystemError({ message: `Attempted to copy non-file ${from}`, detail: { from, to, sourceStats } })
+          new FilesystemError({
+            message: `Error while copying from '${from}' to '${to}': Source is neither a symbolic link, nor a file.`,
+          })
         )
       }
 
@@ -85,13 +87,7 @@ export function cloneFile({ from, to, allowDelete, statsHelper }: CloneFileParam
           } else {
             return done(
               new FilesystemError({
-                message: `Build staging: Failed copying file ${from} to ${to} because a directory exists at the target path`,
-                detail: {
-                  sourcePath: from,
-                  targetPath: to,
-                  sourceStats,
-                  targetStats,
-                },
+                message: `Build staging: Failed copying file from '${from}' to '${to}' because a directory exists at the target path`,
               })
             )
           }
@@ -370,7 +366,7 @@ export class FileStatsHelper {
         // Symlink target not found, so we ignore it
         return cb(null, null)
       } else if (readlinkErr) {
-        return cb(InternalError.wrapError(readlinkErr, { path, readlinkErr }, "Error reading symlink"), null)
+        return cb(InternalError.wrapError(readlinkErr, "Error reading symlink"), null)
       }
 
       // Ignore absolute symlinks unless specifically allowed
@@ -411,7 +407,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 InternalError({ message: `Must specify absolute path (got ${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 { ParameterError, NotFoundError } from "./exceptions"
 import { Log } from "./logger/log-entry"
 
 export type CacheKey = string[]
@@ -79,21 +79,13 @@ export class TreeCache {
   set(log: Log, key: CacheKey, value: CacheValue, ...contexts: CacheContext[]) {
     if (key.length === 0) {
       throw new ParameterError({
-        message: `Cache key must have at least one part`,
-        detail: {
-          key,
-          contexts,
-        },
+        message: `Cache key must have at least one part. Actually got empty list. Contexts: ${contexts.join(", ")}`,
       })
     }
 
     if (contexts.length === 0) {
       throw new ParameterError({
-        message: `Must specify at least one context`,
-        detail: {
-          key,
-          contexts,
-        },
+        message: `Could not set key '${key.join(".")}': Must specify at least one context. Got empty list.`,
       })
     }
 
@@ -118,11 +110,9 @@ export class TreeCache {
 
       if (context.length === 0) {
         throw new ParameterError({
-          message: `Context key must have at least one part`,
-          detail: {
-            key,
-            context,
-          },
+          message: `Could not set key '${key.join(
+            "."
+          )}': All context keys must have at least one part. At least one of them is an empty list.`,
         })
       }
 
@@ -158,7 +148,7 @@ export class TreeCache {
   getOrThrow(log: Log, key: CacheKey): CacheValue {
     const value = this.get(log, key)
     if (value === undefined) {
-      throw new NotFoundError({ message: `Could not find key ${key} in cache`, detail: { key } })
+      throw new NotFoundError({ message: `Could not find key ${key} in cache` })
     }
     return value
   }
@@ -172,9 +162,8 @@ export class TreeCache {
       pairs = Array.from(node.entries).map((stringKey) => {
         const entry = this.cache.get(stringKey)
         if (!entry) {
-          throw new InternalError({
+          throw new ParameterError({
             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, GardenError, explainGardenError } from "../exceptions"
+import { PluginError, toGardenError, GardenError } 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"
@@ -163,7 +163,7 @@ ${renderCommands(commands)}
 
     if (this.commands[fullName]) {
       // For now we don't allow multiple definitions of the same command. We may want to revisit this later.
-      throw new PluginError({ message: `Multiple definitions of command "${fullName}"`, detail: {} })
+      throw new PluginError({ message: `Multiple definitions of command "${fullName}"` })
     }
 
     this.commands[fullName] = command
@@ -175,7 +175,7 @@ ${renderCommands(commands)}
     const dupKeys: string[] = intersection(optKeys, globalKeys)
 
     if (dupKeys.length > 0) {
-      throw new PluginError({ message: `Global option(s) ${dupKeys.join(" ")} cannot be redefined`, detail: {} })
+      throw new PluginError({ message: `Global option(s) ${dupKeys.join(" ")} cannot be redefined` })
     }
   }
 
@@ -454,7 +454,7 @@ ${renderCommands(commands)}
         force: this.initLogger,
       })
     } catch (error) {
-      return done(1, explainGardenError(error, "Failed to initialize logger"))
+      return done(1, toGardenError(error).explain("Failed to initialize logger"))
     }
 
     const log = logger.createLog()
@@ -472,7 +472,7 @@ ${renderCommands(commands)}
           matchedPath = picked.matchedPath
         }
       } catch (error) {
-        return done(1, explainGardenError(error, "Failed to get custom commands"))
+        return done(1, toGardenError(error).explain("Failed to get custom commands"))
       }
     }
 
@@ -515,9 +515,9 @@ ${renderCommands(commands)}
         const parseResults = processCliArgs({ rawArgs: args, parsedArgs: argv, command, matchedPath, cli: true })
         parsedArgs = parseResults.args
         parsedOpts = parseResults.opts
-      } catch (err) {
-        errors.push(...(err.detail?.errors || []).map(toGardenError))
-        return done(1, err.message + "\n" + command.renderHelp())
+      } catch (err: unknown) {
+        errors.push(toGardenError(err))
+        return done(1, `${err}\n` + command.renderHelp())
       }
     }