Skip to content

Commit

Permalink
improvement: make GardenError recognition less error-prone
Browse files Browse the repository at this point in the history
Previously we had an exported interface called GardenError together with
an exported abstract base class called GardenBaseError.

We had a function called isGardenError that checked for interface
conformance, and used it in some parts of the code base, and then we
also had parts of the code base that checked for `instanceof
GardenBaseError`.

This commit eliminates the GardenError interface to eliminate a class of
errors, where we fail to recognize the GardenError and treat it as a
crash.
  • Loading branch information
stefreak committed Aug 31, 2023
1 parent 241c652 commit d2ca019
Show file tree
Hide file tree
Showing 28 changed files with 129 additions and 165 deletions.
4 changes: 2 additions & 2 deletions cli/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ 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 GardenBaseError.
// If an error hits this code path, it's definitely a crash and we need to fix that bug.
// 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)

// eslint-disable-next-line no-console
console.log(`${context}: ${explainGardenError(wrappedError, context)}`)
}
4 changes: 2 additions & 2 deletions core/src/actions/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 { ConfigurationError } from "../exceptions"
import { InternalError } from "../exceptions"
import {
Action,
ActionConfig,
Expand Down Expand Up @@ -731,7 +731,7 @@ export abstract class ResolvedRuntimeAction<
const buildAction = this.getResolvedDependencies().find((a) => a.kind === "Build" && a.name === buildName)

if (!buildAction) {
throw new ConfigurationError({
throw new InternalError({
message: `Could not find build dependency '${buildName}' specified on the build field on ${this.longDescription()}.`,
detail: { action: this.key(), buildName },
})
Expand Down
6 changes: 3 additions & 3 deletions core/src/analytics/analytics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 { GardenBaseError, GardenErrorContext, StackTraceMetadata } from "../exceptions"
import { GardenError, GardenErrorContext, StackTraceMetadata } from "../exceptions"
import { ActionConfigMap } from "../actions/types"
import { actionKinds } from "../actions/types"
import { getResultErrorProperties } from "./helpers"
Expand Down Expand Up @@ -163,7 +163,7 @@ export interface CommandResultEvent extends EventBase {
name: string
durationMsec: number
result: AnalyticsCommandResult
errors: string[] // list of GardenBaseError types
errors: string[] // list of GardenError types
lastError?: AnalyticsGardenError
exitCode?: number
}
Expand Down Expand Up @@ -613,7 +613,7 @@ export class AnalyticsHandler {
*/
trackCommandResult(
commandName: string,
errors: GardenBaseError[],
errors: GardenError[],
startTime: Date,
exitCode?: number,
parentSessionId?: string
Expand Down
4 changes: 2 additions & 2 deletions core/src/analytics/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

import { GardenBaseError, GardenError, getStackTraceMetadata } from "../exceptions"
import { GardenError, getStackTraceMetadata } from "../exceptions"
import { AnalyticsGardenError, AnalyticsGardenErrorDetail } from "./analytics"

function getErrorDetail(error: GardenError): AnalyticsGardenErrorDetail {
Expand Down Expand Up @@ -39,7 +39,7 @@ function getAnalyticsError(error: GardenError): AnalyticsGardenError {
}
}

export function getResultErrorProperties(errors: GardenBaseError[]): {
export function getResultErrorProperties(errors: GardenError[]): {
errors: string[]
lastError?: AnalyticsGardenError
} {
Expand Down
8 changes: 4 additions & 4 deletions core/src/cli/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, explainGardenError } from "../exceptions"
import { PluginError, toGardenError, GardenError, 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"
Expand Down Expand Up @@ -55,7 +55,7 @@ import { JsonFileWriter } from "../logger/writers/json-file-writer"
export interface RunOutput {
argv: any
code: number
errors: (GardenBaseError | Error)[]
errors: (GardenError | Error)[]
result: any
// Mainly used for testing
consoleOutput?: string
Expand Down Expand Up @@ -409,7 +409,7 @@ ${renderCommands(commands)}

let argv = parseCliArgs({ stringArgs: args, cli: true })

const errors: (GardenBaseError | Error)[] = []
const errors: (GardenError | Error)[] = []
const _this = this

async function done(abortCode: number, consoleOutput: string, result: any = {}) {
Expand Down Expand Up @@ -560,7 +560,7 @@ ${renderCommands(commands)}

errors.push(...(commandResult.errors || []))

const gardenErrors: GardenBaseError[] = errors.map(toGardenError)
const gardenErrors: GardenError[] = errors.map(toGardenError)

// Flushes the Analytics events queue in case there are some remaining events.
await analytics?.flush()
Expand Down
4 changes: 2 additions & 2 deletions core/src/cli/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { maxBy, zip } from "lodash"
import { Logger } from "../logger/logger"

import { ParameterValues, Parameter, Parameters, globalDisplayOptions } from "./params"
import { GardenBaseError, ParameterError, RuntimeError, toGardenError } from "../exceptions"
import { GardenError, 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"
Expand Down Expand Up @@ -550,7 +550,7 @@ function renderParameters(params: Parameters, formatName: (name: string, param:
}

export function renderCommandErrors(logger: Logger, errors: Error[], log?: Log) {
const gardenErrors: GardenBaseError[] = errors.map(toGardenError)
const gardenErrors: GardenError[] = errors.map(toGardenError)

const errorLog = log || logger.createLog()

Expand Down
4 changes: 2 additions & 2 deletions core/src/commands/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
joiStringMap,
joiVariables,
} from "../config/common"
import { RuntimeError, GardenBaseError, GardenError, InternalError, toGardenError } from "../exceptions"
import { RuntimeError, GardenError, InternalError, toGardenError } from "../exceptions"
import { Garden } from "../garden"
import { Log } from "../logger/log-entry"
import { LoggerType, LoggerBase, LoggerConfigBase, eventLogLevel, LogLevel } from "../logger/logger"
Expand Down Expand Up @@ -64,7 +64,7 @@ export interface CommandConstructor {

export interface CommandResult<T = any> {
result?: T
errors?: GardenBaseError[]
errors?: GardenError[]
exitCode?: number
}

Expand Down
4 changes: 2 additions & 2 deletions core/src/commands/create/create-project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { printHeader } from "../../logger/util"
import { isDirectory } from "../../util/fs"
import { loadConfigResources } from "../../config/base"
import { resolve, basename, relative, join } from "path"
import { GardenBaseError, ParameterError } from "../../exceptions"
import { GardenError, ParameterError } from "../../exceptions"
import { addConfig } from "./helpers"
import { wordWrap } from "../../util/string"
import { PathParameter, StringParameter, BooleanParameter, StringOption } from "../../cli/params"
Expand Down Expand Up @@ -60,7 +60,7 @@ interface CreateProjectResult {
name: string
}

class CreateError extends GardenBaseError {
class CreateError extends GardenError {
type: "create"
}

Expand Down
6 changes: 3 additions & 3 deletions core/src/commands/custom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, RuntimeError, InternalError, toGardenError } from "../exceptions"
import { ConfigurationError, GardenError, RuntimeError, InternalError, toGardenError } from "../exceptions"
import { resolveTemplateStrings } from "../template-string/template-string"
import { listDirectory, isConfigFilename } from "../util/fs"
import { Command, CommandParams, CommandResult, PrintHeaderParams } from "./base"
Expand Down Expand Up @@ -66,7 +66,7 @@ interface CustomCommandResult {
completedAt: Date
command: string[]
result: any
errors: (Error | GardenBaseError)[]
errors: (Error | GardenError)[]
}
}

Expand Down Expand Up @@ -117,7 +117,7 @@ export class CustomCommandWrapper extends Command {
const commandContext = new CustomCommandContext({ ...garden, args, opts, variables, rest })

const result: CustomCommandResult = {}
const errors: GardenBaseError[] = []
const errors: GardenError[] = []

// Run exec command
if (this.spec.exec) {
Expand Down
118 changes: 61 additions & 57 deletions core/src/exceptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,6 @@ import dedent from "dedent"
import chalk from "chalk"
import stripAnsi from "strip-ansi"

export interface GardenError<D extends object = any> extends Error {
type: string
message: string
detail?: D
stack?: string
wrappedErrors?: GardenError[]
context?: GardenErrorContext
}

export function isGardenError(err: any): err is GardenError {
return "type" in err && "message" in err
}

export type StackTraceMetadata = {
functionName: string
relativeFileName?: string
Expand All @@ -49,8 +36,7 @@ export interface GardenErrorParams<D extends object = any> {
export type GardenErrorContext = {
taskType?: string
}

export abstract class GardenBaseError<D extends object = any> extends Error implements GardenError<D> {
export abstract class GardenError<D extends object = any> extends Error {
abstract type: string
public override message: string
public detail?: D
Expand Down Expand Up @@ -107,54 +93,94 @@ export abstract class GardenBaseError<D extends object = any> extends Error impl
}
}

export class AuthenticationError extends GardenBaseError {
export class AuthenticationError extends GardenError {
type = "authentication"
}

export class BuildError extends GardenBaseError {
export class BuildError extends GardenError {
type = "build"
}

export class ConfigurationError extends GardenBaseError {
export class ConfigurationError extends GardenError {
type = "configuration"
}

export class CommandError extends GardenBaseError {
export class CommandError extends GardenError {
type = "command"
}

export class FilesystemError extends GardenBaseError {
export class FilesystemError extends GardenError {
type = "filesystem"
}

export class LocalConfigError extends GardenBaseError {
export class LocalConfigError extends GardenError {
type = "local-config"
}

export class ValidationError extends GardenBaseError {
export class ValidationError extends GardenError {
type = "validation"
}

export class PluginError extends GardenBaseError {
export class PluginError extends GardenError {
type = "plugin"
}

export class ParameterError extends GardenBaseError {
export class ParameterError extends GardenError {
type = "parameter"
}

export class NotImplementedError extends GardenBaseError {
export class NotImplementedError extends GardenError {
type = "not-implemented"
}

export class DeploymentError extends GardenBaseError {
export class DeploymentError extends GardenError {
type = "deployment"
}

export class RuntimeError extends GardenBaseError {
export class RuntimeError extends GardenError {
type = "runtime"
}

export class GraphError<D extends object> extends GardenError<D> {
type = "graph"
}

export class TimeoutError extends GardenError {
type = "timeout"
}

export class OutOfMemoryError extends GardenError {
type = "out-of-memory"
}

export class NotFoundError extends GardenError {
type = "not-found"
}

export class WorkflowScriptError extends GardenError {
type = "workflow-script"
}

export class CloudApiError extends GardenError {
type = "cloud-api"
}

export class TemplateStringError extends GardenError {
type = "template-string"
}

interface GenericGardenErrorParams extends GardenErrorParams {
type: string
}
export class GenericGardenError extends GardenError {
type: string

constructor(params: GenericGardenErrorParams) {
super(params)
this.type = params.type
}
}

/**
* Throw this error only when this error condition is definitely a Garden bug.
*
Expand All @@ -165,12 +191,12 @@ export class RuntimeError extends GardenBaseError {
*
* In case the network is involved, we should *not* use the "InternalError", because that's usually a situation that the user needs to resolve.
*/
export class InternalError extends GardenBaseError {
export class InternalError extends GardenError {
// we want it to be obvious in amplitude data that this is not a normal error condition
type = "crash"

// not using object destructuring here on purpose, because errors are of type any and then the error might be passed as the params object accidentally.
static wrapError(error: Error, detail?: unknown, prefix?: string): InternalError {
static wrapError(error: Error | string | any, detail?: unknown, prefix?: string): InternalError {
let message: string
let stack: string | undefined

Expand All @@ -190,32 +216,8 @@ export class InternalError extends GardenBaseError {
}
}

export class TimeoutError extends GardenBaseError {
type = "timeout"
}

export class OutOfMemoryError extends GardenBaseError {
type = "out-of-memory"
}

export class NotFoundError extends GardenBaseError {
type = "not-found"
}

export class WorkflowScriptError extends GardenBaseError {
type = "workflow-script"
}

export class CloudApiError extends GardenBaseError {
type = "cloud-api"
}

export class TemplateStringError extends GardenBaseError {
type = "template-string"
}

export function toGardenError(err: Error | GardenBaseError | string | any): GardenBaseError {
if (err instanceof GardenBaseError) {
export function toGardenError(err: Error | GardenError | string | any): GardenError {
if (err instanceof GardenError) {
return err
} else {
return InternalError.wrapError(err)
Expand All @@ -226,7 +228,9 @@ export function filterErrorDetail(detail: any) {
return withoutInternalFields(sanitizeValue(detail))
}

export function explainGardenError(error: GardenError, context?: string) {
export function explainGardenError(rawError: GardenError | Error | string, context?: string) {
const error = toGardenError(rawError)

let errorMessage = error.message.trim()

// If this is an unexpected error, we want to output more details by default and provide some guidance for the user.
Expand All @@ -249,7 +253,7 @@ export function explainGardenError(error: GardenError, context?: string) {
}

// In case this is another Garden error, the error message is already designed to be digestable as-is for the user.
return chalk.red(error.message)
return chalk.red(errorMessage)
}

export function formatGardenErrorWithDetail(error: GardenError) {
Expand Down
Loading

0 comments on commit d2ca019

Please sign in to comment.