From 713cd42a1b705699dee67cd4b478b664bf04ac69 Mon Sep 17 00:00:00 2001 From: Steffen Neubauer Date: Wed, 30 Aug 2023 23:00:17 +0200 Subject: [PATCH] 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 | 16 ++- core/src/actions/base.ts | 4 +- core/src/build-staging/build-staging.ts | 10 +- core/src/build-staging/helpers.ts | 12 +-- core/src/cache.ts | 7 +- core/src/cli/cli.ts | 33 +++++-- core/src/cli/helpers.ts | 5 +- core/src/commands/base.ts | 12 +-- core/src/commands/custom.ts | 4 +- core/src/commands/login.ts | 4 +- core/src/commands/version.ts | 9 +- core/src/config-store/base.ts | 6 +- core/src/config/common.ts | 4 +- core/src/docs/template-strings.ts | 4 +- core/src/exceptions.ts | 98 ++++++++++++++----- core/src/garden.ts | 11 ++- core/src/graph/actions.ts | 4 +- core/src/graph/config-graph.ts | 1 + core/src/graph/nodes.ts | 12 ++- core/src/graph/results.ts | 20 +++- core/src/graph/solver.ts | 21 ++-- core/src/logger/logger.ts | 17 ++-- core/src/logger/renderers.ts | 6 +- core/src/logger/writers/file-writer.ts | 4 +- .../kubernetes/container/build/common.ts | 4 +- core/src/plugins/kubernetes/kubernetes.ts | 2 +- core/src/plugins/kubernetes/util.ts | 11 ++- core/src/router/base.ts | 14 +-- core/src/router/module.ts | 8 +- core/src/tasks/base.ts | 6 +- core/src/tasks/helpers.ts | 4 +- core/src/tasks/resolve-action.ts | 7 +- core/src/util/module-overlap.ts | 4 +- core/src/util/streams.ts | 7 +- core/src/util/testing.ts | 4 +- core/src/util/util.ts | 18 +++- core/src/watch.ts | 9 +- core/test/unit/src/commands/version.ts | 2 +- 38 files changed, 274 insertions(+), 150 deletions(-) diff --git a/cli/src/cli.ts b/cli/src/cli.ts index c495da6faaa..c5ef919d900 100644 --- a/cli/src/cli.ts +++ b/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,8 +65,7 @@ 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) @@ -74,3 +73,12 @@ export async function runCli({ 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)}`) +} diff --git a/core/src/actions/base.ts b/core/src/actions/base.ts index 7ee8cc5274b..db16032115f 100644 --- a/core/src/actions/base.ts +++ b/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 }, }) diff --git a/core/src/build-staging/build-staging.ts b/core/src/build-staging/build-staging.ts index 34cc81f9ee4..878bad2bdc2 100644 --- a/core/src/build-staging/build-staging.ts +++ b/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, diff --git a/core/src/build-staging/helpers.ts b/core/src/build-staging/helpers.ts index 90600c5fa0f..9d44e2749e8 100644 --- a/core/src/build-staging/helpers.ts +++ b/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 } }) } } } diff --git a/core/src/cache.ts b/core/src/cache.ts index 33f213f8e17..282cba31601 100644 --- a/core/src/cache.ts +++ b/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] }) diff --git a/core/src/cli/cli.ts b/core/src/cli/cli.ts index 2c4ecfd8f98..dc1286fb132 100644 --- a/core/src/cli/cli.ts +++ b/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 { + // 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 { diff --git a/core/src/cli/helpers.ts b/core/src/cli/helpers.ts index 2b6074ecf61..8307e6011c6 100644 --- a/core/src/cli/helpers.ts +++ b/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, ` + diff --git a/core/src/commands/base.ts b/core/src/commands/base.ts index 523fe68c5d8..2e0c50696ad 100644 --- a/core/src/commands/base.ts +++ b/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 { - return f && f.error && isGardenError(f.error) ? [f.error as GardenError] : [] + return f && f.error ? [toGardenError(f.error)] : [] }) const error = new RuntimeError({ diff --git a/core/src/commands/custom.ts b/core/src/commands/custom.ts index 8f31f41e0c8..c42d7d81ef4 100644 --- a/core/src/commands/custom.ts +++ b/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. diff --git a/core/src/commands/login.ts b/core/src/commands/login.ts index c8d34a9a105..1d7a8914019 100644 --- a/core/src/commands/login.ts +++ b/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 diff --git a/core/src/commands/version.ts b/core/src/commands/version.ts index 6cc4ce55780..1263f553676 100644 --- a/core/src/commands/version.ts +++ b/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> { + override async action(params: CommandParams): Promise> { + 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 }, } diff --git a/core/src/config-store/base.ts b/core/src/config-store/base.ts index 94394372626..3c0b382de16 100644 --- a/core/src/config-store/base.ts +++ b/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> = z.infer @@ -95,7 +95,7 @@ export abstract class ConfigStore> { 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> { 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, diff --git a/core/src/config/common.ts b/core/src/config/common.ts index ea494b62ccc..315e79d5b32 100644 --- a/core/src/config/common.ts +++ b/core/src/config/common.ts @@ -15,7 +15,7 @@ import { isArray, isPlainObject, isString, mapValues, memoize } from "lodash" import { joiPathPlaceholder } from "./validation" import { DOCS_BASE_URL, GardenApiVersion } from "../constants" import { ActionKind, actionKinds, actionKindsLower } from "../actions/types" -import { ConfigurationError, InternalError } from "../exceptions" +import { ConfigurationError, UnexpectedCrashError } from "../exceptions" import type { ConfigContextType } from "./template-contexts/base" import { z } from "zod" import { @@ -562,7 +562,7 @@ export function createSchema(spec: CreateSchemaParams): CreateSchemaOutput { let { name } = spec if (schemaRegistry[name]) { - throw new InternalError({ message: `Object schema ${name} defined multiple times`, detail: { name } }) + throw new UnexpectedCrashError({ message: `Object schema ${name} defined multiple times`, detail: { name } }) } schemaRegistry[name] = { spec } diff --git a/core/src/docs/template-strings.ts b/core/src/docs/template-strings.ts index a6f1d899217..f25336c720c 100644 --- a/core/src/docs/template-strings.ts +++ b/core/src/docs/template-strings.ts @@ -21,10 +21,10 @@ import { ModuleConfigContext, OutputConfigContext } from "../config/template-con import { WorkflowStepConfigContext } from "../config/template-contexts/workflow" import { getHelperFunctions } from "../template-string/functions" import { isEqual, kebabCase, sortBy } from "lodash" -import { InternalError } from "../exceptions" import { CustomCommandContext } from "../config/template-contexts/custom-command" import Joi from "@hapi/joi" import { ActionConfigContext, ActionSpecContext } from "../config/template-contexts/actions" +import { UnexpectedCrashError } from "../exceptions" interface ContextSpec { schema: Joi.ObjectSchema @@ -123,7 +123,7 @@ export function writeTemplateStringReferenceDocs(docsRoot: string) { // This implicitly tests the helpers at documentation render time if (!example.skipTest && !isEqual(computedOutput, example.output)) { const renderedComputed = JSON.stringify(computedOutput) - throw new InternalError({ + throw new UnexpectedCrashError({ message: `Test failed for ${spec.name} helper. Expected input args ${example.input} to resolve to ${renderedResult}, got ${renderedComputed}`, detail: { spec }, }) diff --git a/core/src/exceptions.ts b/core/src/exceptions.ts index afa5c97f2b0..40178e3a1cc 100644 --- a/core/src/exceptions.ts +++ b/core/src/exceptions.ts @@ -9,7 +9,10 @@ import { isEmpty, isString } from "lodash" import { stringify } from "yaml" import { withoutInternalFields, sanitizeValue } from "./util/logging" -import { testFlags } from "./util/util" +import { getGitHubIssueLink, testFlags } from "./util/util" +import dedent from "dedent" +import chalk from "chalk" +import stripAnsi from "strip-ansi" export interface GardenError extends Error { type: string @@ -80,6 +83,16 @@ export abstract class GardenBaseError extends Error impl } } + toJSON() { + return { + type: this.type, + message: this.message, + stack: this.stack, + detail: this.detail, + wrappedErrors: this.wrappedErrors, + } + } + toSanitizedValue() { return { type: this.type, @@ -142,11 +155,37 @@ export class RuntimeError extends GardenBaseError { type = "runtime" } -export class InternalError extends GardenBaseError { - type = "internal" +/** + * Throw this error only when this error condition is definitely a Garden bug. + * + * Examples where throwing this error is appropriate: + * - A Javascript TypeError has occurred, e.g. reading property on undefined. + * - "This should not happen" kind of situations, e.g. internal data structures are in an invalid state. + * - An unhandled exception has been thrown by a library. If you don't know what to do with this exception and it is most likely not due to user error, wrap it with "UnexpectedCrashError". + * + * In case the network is involved, we should *not* use the "UnexpectedCrashError", because that's usually a situation that the user needs to resolve. + */ +export class UnexpectedCrashError extends GardenBaseError { + type = "unexpected-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): UnexpectedCrashError { + let message: string + let stack: string | undefined + + if (error instanceof Error) { + message = error.message + stack = error.stack + } else if (isString(error)) { + message = error + } else { + message = error["message"] + stack = error["stack"] + } - constructor({ message, detail }: { message: string; detail: any }) { - super({ message: message + "\nThis is a bug. Please report it!", detail }) + message = stripAnsi(message) + + return new UnexpectedCrashError({ message: prefix ? `${stripAnsi(prefix)}: ${message}` : message, stack, detail }) } } @@ -174,36 +213,47 @@ export class TemplateStringError extends GardenBaseError { type = "template-string" } -export class WrappedNativeError extends GardenBaseError { - type = "wrapped-native-error" - - constructor(error: Error) { - super({ message: error.message, stack: error.stack }) - } -} - -export function toGardenError(err: Error | GardenBaseError | string): GardenBaseError { +export function toGardenError(err: Error | GardenBaseError | string | any): GardenBaseError { if (err instanceof GardenBaseError) { return err - } else if (err instanceof Error) { - const wrappedError = new WrappedNativeError(err) - const out = new RuntimeError({ message: err.message, wrappedErrors: [wrappedError] }) - out.stack = err.stack - return out - } else if (isString(err)) { - return new RuntimeError({ message: err }) } else { - const msg = err["message"] - return new RuntimeError({ message: msg }) + return UnexpectedCrashError.wrapError(err) } } -function filterErrorDetail(detail: any) { +export function filterErrorDetail(detail: any) { return withoutInternalFields(sanitizeValue(detail)) } +export function explainGardenError(error: GardenError, context?: string) { + 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. + if (error instanceof UnexpectedCrashError) { + let bugReportInformation = formatGardenErrorWithDetail(error) + + if (context) { + bugReportInformation = `${stripAnsi(context)}\n${bugReportInformation}` + } + + return chalk.red(dedent` + ${chalk.bold("Encountered an unexpected Garden error. We are sorry for this. This is likely a bug šŸ‚")} + + You can help by reporting this on GitHub: ${getGitHubIssueLink(`Unexpected Crash: ${errorMessage}`, "bug")} + + Please attach the following information to the bug report after making sure that the error message does not contain sensitive information: + + ${chalk.gray(bugReportInformation)} + `) + } + + // 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) +} + export function formatGardenErrorWithDetail(error: GardenError) { const { detail, message, stack } = error + let out = stack || message || "" // We sanitize and recursively filter out internal fields (i.e. having names starting with _). diff --git a/core/src/garden.ts b/core/src/garden.ts index 595a9f21c80..d79aaddfac7 100644 --- a/core/src/garden.ts +++ b/core/src/garden.ts @@ -40,7 +40,14 @@ import { getCloudDistributionName, getCloudLogSectionName, } from "./util/util" -import { ConfigurationError, isGardenError, GardenError, InternalError, PluginError, RuntimeError } from "./exceptions" +import { + ConfigurationError, + isGardenError, + GardenError, + PluginError, + RuntimeError, + UnexpectedCrashError, +} from "./exceptions" import { VcsHandler, ModuleVersion, getModuleVersionString, VcsInfo } from "./vcs/vcs" import { GitHandler } from "./vcs/git" import { BuildStaging } from "./build-staging/build-staging" @@ -1576,7 +1583,7 @@ export class Garden { } } - throw new InternalError({ + throw new UnexpectedCrashError({ message: `Could not find environment config ${this.environmentName}`, detail: { environmentName: this.environmentName, diff --git a/core/src/graph/actions.ts b/core/src/graph/actions.ts index 7d8db47c36e..cadb3068322 100644 --- a/core/src/graph/actions.ts +++ b/core/src/graph/actions.ts @@ -38,7 +38,7 @@ import { ActionReference, describeSchema, JoiDescription, parseActionReference } import type { GroupConfig } from "../config/group" import { ActionConfigContext } from "../config/template-contexts/actions" import { validateWithPath } from "../config/validation" -import { ConfigurationError, InternalError, PluginError, ValidationError } from "../exceptions" +import { ConfigurationError, PluginError, UnexpectedCrashError, ValidationError } from "../exceptions" import { overrideVariables, type Garden } from "../garden" import type { Log } from "../logger/log-entry" import type { ActionTypeDefinition } from "../plugin/action-types" @@ -556,7 +556,7 @@ export const preprocessActionConfig = profileAsync(async function preprocessActi // Note: This shouldn't happen in normal user flows if (!template) { - throw new InternalError({ + throw new UnexpectedCrashError({ message: `${description} references template '${ config.internal.templateName }' which cannot be found. Available templates: ${naturalList(Object.keys(garden.configTemplates)) || "(none)"}`, diff --git a/core/src/graph/config-graph.ts b/core/src/graph/config-graph.ts index 4c66b90e8be..83435cfb83f 100644 --- a/core/src/graph/config-graph.ts +++ b/core/src/graph/config-graph.ts @@ -55,6 +55,7 @@ interface GetActionsParams extends GetActionOpts { excludeNames?: string[] // Glob patterns to exclude. An action is returned if its name matches none of these. } +// TODO: There are two different GraphError classes export class GraphError extends GardenBaseError { type = "graph" } diff --git a/core/src/graph/nodes.ts b/core/src/graph/nodes.ts index f9e2fd29702..bc8ac3de525 100644 --- a/core/src/graph/nodes.ts +++ b/core/src/graph/nodes.ts @@ -7,7 +7,7 @@ */ import { Task, ValidResultType } from "../tasks/base" -import { GardenBaseError, InternalError, isGardenError } from "../exceptions" +import { GardenBaseError, UnexpectedCrashError, toGardenError } from "../exceptions" import { GraphResult, GraphResultFromTask, GraphResults } from "./results" import type { GraphSolver } from "./solver" import { ValuesType } from "utility-types" @@ -260,7 +260,7 @@ export class ProcessTaskNode extends TaskNode { const statusResult = this.getDependencyResult(statusTask) as GraphResultFromTask if (statusResult === undefined) { - throw new InternalError({ + throw new UnexpectedCrashError({ message: `Attempted to execute ${this.describe()} before resolving status.`, detail: { nodeKey: this.getKey(), @@ -345,12 +345,14 @@ export interface GraphNodeErrorDetail extends GraphResult { export interface GraphNodeErrorParams extends GraphNodeErrorDetail {} export class GraphNodeError extends GardenBaseError { + // TODO: type graph is already taken by the (two) GraphError(s) type = "graph" constructor(params: GraphNodeErrorParams) { const { node, failedDependency, error } = params let message = "" + let stack: string | undefined if (failedDependency) { message = `${node.describe()} aborted because a dependency could not be completed:` @@ -371,15 +373,17 @@ export class GraphNodeError extends GardenBaseError { } } else if (result?.error) { message += chalk.red.bold(`\nā†³ ${nextDep.describe()} [FAILED] - ${result.error.message}`) + stack = result.error.stack nextDep = null } } } else { message = `${node.describe()} failed: ${error}` + stack = error?.stack } - const wrappedErrors = isGardenError(error) ? [error] : [] - super({ message, detail: params, wrappedErrors, context: { taskType: node.task.type } }) + const wrappedErrors = [toGardenError(error)] + super({ message, stack, detail: params, wrappedErrors, context: { taskType: node.task.type } }) } aborted() { diff --git a/core/src/graph/results.ts b/core/src/graph/results.ts index 70c2f5ff178..2ff1502742c 100644 --- a/core/src/graph/results.ts +++ b/core/src/graph/results.ts @@ -7,10 +7,10 @@ */ import { BaseTask, Task, ValidResultType } from "../tasks/base" -import { InternalError } from "../exceptions" import { fromPairs, omit, pick } from "lodash" import { toGraphResultEventPayload } from "../events/events" import CircularJSON from "circular-json" +import { GardenBaseError, GardenErrorParams, UnexpectedCrashError } from "../exceptions" export interface TaskEventBase { type: string @@ -109,7 +109,7 @@ export class GraphResults { private checkKey(key: string) { if (!this.tasks.has(key)) { const taskKeys = Array.from(this.tasks.keys()) - throw new InternalError({ + throw new UnexpectedCrashError({ message: `GraphResults object does not have task ${key}. Available keys: [${taskKeys.join(", ")}]`, detail: { key, @@ -258,9 +258,23 @@ function filterErrorForExport(error: any) { } }) - return { + // we can't just return a plain object here, because then it wouldn't be + // recognizable anymore as a GardenError further down the line and handled as an unexpected crash + return new FilteredError({ ...pick(error, "message", "type", "stack"), detail: filteredDetail, wrappedErrors: filteredWrappedErrors, + }) +} + +interface FilteredErrorParams extends GardenErrorParams { + readonly type: string +} +class FilteredError extends GardenBaseError { + type: string + + constructor(params: FilteredErrorParams) { + super(params) + this.type = params.type } } diff --git a/core/src/graph/solver.ts b/core/src/graph/solver.ts index ac030d773a2..787d0a1dcad 100644 --- a/core/src/graph/solver.ts +++ b/core/src/graph/solver.ts @@ -8,7 +8,7 @@ import type { BaseTask, Task, ValidResultType } from "../tasks/base" import type { Log } from "../logger/log-entry" -import { GardenBaseError, GardenError, isGardenError, toGardenError } from "../exceptions" +import { GardenBaseError, GardenError, explainGardenError, isGardenError, toGardenError } from "../exceptions" import { uuidv4 } from "../util/random" import { DependencyGraph, metadataForLog } from "./common" import { Profile } from "../util/profiling" @@ -142,6 +142,7 @@ export class GraphSolver extends TypedEventEmitter { error: new GraphError({ message: `Failed to ${result.description}: ${result.error}`, detail: { results }, + wrappedErrors: [toGardenError(result.error)], }), }) return @@ -354,7 +355,7 @@ export class GraphSolver extends TypedEventEmitter { }) .catch((error) => { this.garden.events.emit("internalError", { error, timestamp: new Date() }) - this.logInternalError(node, error) + this.logUnexpectedCrashError(node, error) node.complete({ startedAt, error, aborted: true, result: null }) // Abort execution on internal error this.emit("abort", { error }) @@ -492,26 +493,29 @@ export class GraphSolver extends TypedEventEmitter { private logTaskError(node: TaskNode, err: Error) { const log = node.task.log - const prefix = `Failed ${node.describe()} ${renderDuration(log.getDuration())}. Here is the output:` + const prefix = `Failed ${node.describe()} ${renderDuration(log.getDuration())}. This is what happened:` this.logError(log, err, prefix) } - private logInternalError(node: TaskNode, err: Error) { - const prefix = `An internal error occurred while ${node.describe()}. Here is the output:` + private logUnexpectedCrashError(node: TaskNode, err: Error) { + const prefix = `An internal error occurred while ${node.describe()}. This is what happened:` this.logError(node.task.log, err, prefix) } private logError(log: Log, err: Error, errMessagePrefix: string) { const error = toGardenError(err) - const errorMessage = error.message.trim() const msg = renderMessageWithDivider({ prefix: errMessagePrefix, - msg: errorMessage, + msg: explainGardenError(error, errMessagePrefix), isError: true, }) log.error({ msg, error, showDuration: false }) const divider = renderDivider() - log.silly(chalk.gray(`Full error with stack trace:\n${divider}\n${formatGardenErrorWithDetail(error)}\n${divider}`)) + log.silly( + chalk.gray( + `Full error with stack trace and details:\n${divider}\n${formatGardenErrorWithDetail(error)}\n${divider}` + ) + ) } } @@ -548,6 +552,7 @@ interface GraphErrorDetail { results: GraphResults } +// TODO: There are two different GraphError classes class GraphError extends GardenBaseError { type = "graph" } diff --git a/core/src/logger/logger.ts b/core/src/logger/logger.ts index d8fb0ef2475..21b99c31384 100644 --- a/core/src/logger/logger.ts +++ b/core/src/logger/logger.ts @@ -8,7 +8,7 @@ import { LogMetadata, LogEntry, CoreLog, CoreLogContext } from "./log-entry" import { Writer } from "./writers/base" -import { CommandError, InternalError, ParameterError } from "../exceptions" +import { CommandError, ParameterError, UnexpectedCrashError } from "../exceptions" import { TerminalWriter } from "./writers/terminal-writer" import { JsonTerminalWriter } from "./writers/json-terminal-writer" import { EventBus } from "../events/events" @@ -51,7 +51,8 @@ export function parseLogLevel(level: string): LogLevel { lvl = LogLevel[level] } if (!getNumericLogLevels().includes(lvl)) { - throw new InternalError({ + // This should be validated on a different level + throw new ParameterError({ message: `Unexpected log level, expected one of ${getLogLevelChoices().join(", ")}, got ${level}`, detail: {}, }) @@ -289,11 +290,11 @@ export abstract class LoggerBase implements Logger { /** * Returns all log entries. Throws if storeEntries=false * - * @throws(InternalError) + * @throws(UnexpectedCrashError) */ getLogEntries(): LogEntry[] { if (!this.storeEntries) { - throw new InternalError({ message: `Cannot get entries when storeEntries=false`, detail: {} }) + throw new UnexpectedCrashError({ message: `Cannot get entries when storeEntries=false`, detail: {} }) } return this.entries } @@ -301,11 +302,11 @@ export abstract class LoggerBase implements Logger { /** * Returns latest log entry. Throws if storeEntries=false * - * @throws(InternalError) + * @throws(UnexpectedCrashError) */ getLatestEntry() { if (!this.storeEntries) { - throw new InternalError({ message: `Cannot get entries when storeEntries=false`, detail: {} }) + throw new UnexpectedCrashError({ message: `Cannot get entries when storeEntries=false`, detail: {} }) } return this.entries.slice(-1)[0] } @@ -337,11 +338,11 @@ export class RootLogger extends LoggerBase { * * Throws and error if called before logger is initialized. * - * @throws(InternalError) + * @throws(UnexpectedCrashError) */ static getInstance() { if (!RootLogger.instance) { - throw new InternalError({ message: "Logger not initialized", detail: {} }) + throw new UnexpectedCrashError({ message: "Logger not initialized", detail: {} }) } return RootLogger.instance } diff --git a/core/src/logger/renderers.ts b/core/src/logger/renderers.ts index de5d696c7e6..ce09ba4ce7a 100644 --- a/core/src/logger/renderers.ts +++ b/core/src/logger/renderers.ts @@ -17,7 +17,7 @@ import { LogEntry } from "./log-entry" import { JsonLogEntry } from "./writers/json-terminal-writer" import { highlightYaml, safeDumpYaml } from "../util/serialization" import { Logger, logLevelMap, LogLevel } from "./logger" -import { toGardenError, formatGardenErrorWithDetail } from "../exceptions" +import { toGardenError, formatGardenErrorWithDetail, explainGardenError } from "../exceptions" type RenderFn = (entry: LogEntry, logger: Logger) => string @@ -48,7 +48,9 @@ export function renderError(entry: LogEntry): string { let out = "" - if (error) { + if (!msg && error) { + out = explainGardenError(error) + } else if (error) { const noAnsiErr = stripAnsi(error.message || "") const noAnsiMsg = stripAnsi(msg || "") // render error only if message doesn't already contain it diff --git a/core/src/logger/writers/file-writer.ts b/core/src/logger/writers/file-writer.ts index 0e3954849b9..78c5a7a7c6a 100644 --- a/core/src/logger/writers/file-writer.ts +++ b/core/src/logger/writers/file-writer.ts @@ -15,7 +15,7 @@ import { LogLevel } from "../logger" import { LogEntry } from "../log-entry" import { BaseWriterParams, Writer } from "./base" import { renderError, renderMsg } from "../renderers" -import { InternalError } from "../../exceptions" +import { UnexpectedCrashError } from "../../exceptions" export interface FileWriterConfig extends BaseWriterParams { level: LogLevel @@ -67,7 +67,7 @@ export class FileWriter extends Writer { static async factory(config: FileWriterConfig) { const { logFilePath, truncatePrevious } = config if (!isAbsolute(logFilePath)) { - throw new InternalError({ + throw new UnexpectedCrashError({ message: `FileWriter expected absolute log file path, got ${logFilePath}`, detail: { logFilePath }, }) diff --git a/core/src/plugins/kubernetes/container/build/common.ts b/core/src/plugins/kubernetes/container/build/common.ts index d55ec1cb4db..eb0334063ec 100644 --- a/core/src/plugins/kubernetes/container/build/common.ts +++ b/core/src/plugins/kubernetes/container/build/common.ts @@ -21,7 +21,7 @@ import { KubernetesPluginContext, KubernetesProvider } from "../../config" import { PodRunner } from "../../run" import { PluginContext } from "../../../../plugin-context" import { hashString, sleep } from "../../../../util/util" -import { InternalError, RuntimeError } from "../../../../exceptions" +import { UnexpectedCrashError, RuntimeError } from "../../../../exceptions" import { Log } from "../../../../logger/log-entry" import { prepareDockerAuth } from "../../init" import { prepareSecrets } from "../../secrets" @@ -187,7 +187,7 @@ export async function skopeoBuildStatus({ if (!deploymentRegistry) { // This is validated in the provider configure handler, so this is an internal error if it happens - throw new InternalError({ + throw new UnexpectedCrashError({ message: `Expected configured deploymentRegistry for remote build`, detail: { config: provider.config }, }) diff --git a/core/src/plugins/kubernetes/kubernetes.ts b/core/src/plugins/kubernetes/kubernetes.ts index 41301b5fda0..d49dc5e171e 100644 --- a/core/src/plugins/kubernetes/kubernetes.ts +++ b/core/src/plugins/kubernetes/kubernetes.ts @@ -77,7 +77,7 @@ export async function configureProvider({ if (config.name !== "local-kubernetes" && !config.deploymentRegistry) { throw new ConfigurationError({ - message: `kubernetes: must specify deploymentRegistry in config`, + message: `Must specify deploymentRegistry in provider configuration`, detail: { config, }, diff --git a/core/src/plugins/kubernetes/util.ts b/core/src/plugins/kubernetes/util.ts index 01f7e2eccb3..81c8f9bdc35 100644 --- a/core/src/plugins/kubernetes/util.ts +++ b/core/src/plugins/kubernetes/util.ts @@ -25,7 +25,7 @@ import { KubeApi, KubernetesError } from "./api" import { gardenAnnotationKey, base64, deline, stableStringify, splitLast, truncate } from "../../util/string" import { MAX_CONFIGMAP_DATA_SIZE } from "./constants" import { ContainerEnvVars } from "../container/moduleConfig" -import { ConfigurationError, DeploymentError, InternalError, PluginError } from "../../exceptions" +import { ConfigurationError, DeploymentError, PluginError, UnexpectedCrashError } from "../../exceptions" import { KubernetesProvider, KubernetesPluginContext, KubernetesTargetResourceSpec } from "./config" import { Log } from "../../logger/log-entry" import { PluginContext } from "../../plugin-context" @@ -620,7 +620,7 @@ export async function getTargetResource({ if (!targetKind) { // This should be caught in config/schema validation - throw new InternalError({ message: `Neither kind nor podSelector set in resource query`, detail: { query } }) + throw new UnexpectedCrashError({ message: `Neither kind nor podSelector set in resource query`, detail: { query } }) } // Look in the specified manifests, if provided @@ -715,7 +715,7 @@ export async function readTargetResource({ if (!targetName) { // This should be caught in config/schema validation - throw new InternalError({ message: `Must specify name in resource/target query`, detail: { query } }) + throw new UnexpectedCrashError({ message: `Must specify name in resource/target query`, detail: { query } }) } if (targetKind === "Deployment") { @@ -726,7 +726,10 @@ export async function readTargetResource({ return api.apps.readNamespacedStatefulSet(targetName, namespace) } else { // This should be caught in config/schema validation - throw new InternalError({ message: `Unsupported kind specified in resource/target query`, detail: { query } }) + throw new UnexpectedCrashError({ + message: `Unsupported kind specified in resource/target query`, + detail: { query }, + }) } } diff --git a/core/src/router/base.ts b/core/src/router/base.ts index 47661b6a449..bb714f49bb4 100644 --- a/core/src/router/base.ts +++ b/core/src/router/base.ts @@ -31,7 +31,7 @@ import { ActionTypeClasses, GetActionTypeParams, } from "../plugin/action-types" -import { InternalError, ParameterError, PluginError } from "../exceptions" +import { ParameterError, PluginError, UnexpectedCrashError } from "../exceptions" import { validateSchema } from "../config/validation" import { getActionTypeBases, getPluginBases, getPluginDependencies } from "../plugins" import { getNames, MaybeUndefined } from "../util/util" @@ -231,7 +231,7 @@ export abstract class BaseActionRouter extends BaseRouter async configure({ config, log }: { config: BaseActionConfig; log: Log }) { if (config.kind !== this.kind) { - throw new InternalError({ + throw new UnexpectedCrashError({ message: `Attempted to call ${this.kind} handler for ${config.kind} action`, detail: {}, }) @@ -279,7 +279,7 @@ export abstract class BaseActionRouter extends BaseRouter log.silly(`Getting '${String(handlerType)}' handler for ${action.longDescription()}`) if (action.kind !== this.kind) { - throw new InternalError({ + throw new UnexpectedCrashError({ message: `Attempted to call ${this.kind} handler for ${action.kind} action`, detail: {}, }) @@ -466,10 +466,10 @@ export abstract class BaseActionRouter extends BaseRouter } // This should never happen - throw new InternalError({ - message: - `Unable to find any matching configuration when selecting ${actionType}/${String(handlerType)} handler ` + - `(please report this as a bug).`, + throw new UnexpectedCrashError({ + message: `Unable to find any matching configuration when selecting ${actionType}/${String( + handlerType + )} handler.`, detail: { handlers, configs }, }) } else { diff --git a/core/src/router/module.ts b/core/src/router/module.ts index 76940b0387f..a0140f92ce7 100644 --- a/core/src/router/module.ts +++ b/core/src/router/module.ts @@ -9,7 +9,7 @@ import { fromPairs, uniqBy } from "lodash" import { validateSchema } from "../config/validation" import { defaultProvider } from "../config/provider" -import { ParameterError, PluginError, InternalError } from "../exceptions" +import { ParameterError, PluginError, UnexpectedCrashError } from "../exceptions" import { Log } from "../logger/log-entry" import { GardenModule } from "../types/module" import { @@ -296,10 +296,8 @@ export class ModuleRouter extends BaseRouter { } // This should never happen - throw new InternalError({ - message: - `Unable to find any matching configuration when selecting ${moduleType}/${handlerType} handler ` + - `(please report this as a bug).`, + throw new UnexpectedCrashError({ + message: `Unable to find any matching configuration when selecting ${moduleType}/${handlerType} handler.`, detail: { handlers, configs }, }) } else { diff --git a/core/src/tasks/base.ts b/core/src/tasks/base.ts index 5b6ba304191..3df0cae7d42 100644 --- a/core/src/tasks/base.ts +++ b/core/src/tasks/base.ts @@ -14,7 +14,7 @@ import { Profile } from "../util/profiling" import { type Action, type ActionState, type Executed, type Resolved } from "../actions/types" import { ConfigGraph, GraphError } from "../graph/config-graph" import type { ActionReference } from "../config/common" -import { InternalError, RuntimeError } from "../exceptions" +import { UnexpectedCrashError, RuntimeError } from "../exceptions" import type { DeleteDeployTask } from "./delete-deploy" import type { BuildTask } from "./build" import type { DeployTask } from "./deploy" @@ -322,7 +322,7 @@ export abstract class BaseActionTask( return new test.TestTask({ ...baseParams, action }) } else { // Shouldn't happen - throw new InternalError({ message: `Unexpected action kind`, detail: {} }) + throw new UnexpectedCrashError({ message: `Unexpected action kind`, detail: {} }) } } diff --git a/core/src/tasks/resolve-action.ts b/core/src/tasks/resolve-action.ts index 9cc83194e01..10bd923f718 100644 --- a/core/src/tasks/resolve-action.ts +++ b/core/src/tasks/resolve-action.ts @@ -11,7 +11,7 @@ import { Profile } from "../util/profiling" import { Action, ActionState, ExecutedAction, Resolved, ResolvedAction } from "../actions/types" import { ActionSpecContext } from "../config/template-contexts/actions" import { resolveTemplateStrings } from "../template-string/template-string" -import { InternalError } from "../exceptions" +import { UnexpectedCrashError } from "../exceptions" import { validateWithPath } from "../config/validation" import { DeepPrimitiveMap } from "../config/common" import { merge } from "lodash" @@ -243,7 +243,10 @@ export class ResolveActionTask extends BaseActionTask extends Readable { this.lastValues[i] = value if (lastValue !== undefined && this.comparisonFn(lastValue, value) > 0) { - this.emit("error", new InternalError({ message: `Received unordered stream`, detail: { streamIndex: i } })) + this.emit( + "error", + new UnexpectedCrashError({ message: `Received unordered stream`, detail: { streamIndex: i } }) + ) return } diff --git a/core/src/util/testing.ts b/core/src/util/testing.ts index 0a7307c75c6..a8f44c007e2 100644 --- a/core/src/util/testing.ts +++ b/core/src/util/testing.ts @@ -16,7 +16,7 @@ import { WorkflowConfig } from "../config/workflow" import { Log, LogEntry } from "../logger/log-entry" import { GardenModule } from "../types/module" import { findByName, getNames } from "./util" -import { GardenBaseError, GardenError, InternalError } from "../exceptions" +import { GardenBaseError, GardenError, UnexpectedCrashError } from "../exceptions" import { EventBus, EventName, Events } from "../events/events" import { dedent } from "./string" import pathIsInside from "path-is-inside" @@ -401,7 +401,7 @@ export class TestGarden extends Garden { if (result.result && command.outputsSchema) { await validateSchema(result.result, command.outputsSchema(), { context: `outputs from '${command.name}' command`, - ErrorClass: InternalError, + ErrorClass: UnexpectedCrashError, }) } diff --git a/core/src/util/util.ts b/core/src/util/util.ts index 0ae8c288071..d40fbfd2e07 100644 --- a/core/src/util/util.ts +++ b/core/src/util/util.ts @@ -21,6 +21,7 @@ import { range, some, trimEnd, + truncate, uniqBy, } from "lodash" import { asyncExitHook, gracefulExit } from "@scg82/exit-hook" @@ -840,10 +841,23 @@ export function userPrompt(params: { } export function getGitHubIssueLink(title: string, type: "bug" | "feature-request") { + try { + title = encodeURIComponent( + truncate(title, { + length: 80, + omission: encodeURIComponent("..."), + }) + ).replaceAll("'", "%27") + } catch (e) { + // encodeURIComponent might throw URIError with malformed unicode strings. + // The title is not that important, we can also leave it empty in that case. + title = "" + } + if (type === "feature-request") { - return `https://github.com/garden-io/garden/issues/new?assignees=&labels=feature+request&template=FEATURE_REQUEST.md&title=%5BFEATURE%5D%3A+${title}` + return `https://github.com/garden-io/garden/issues/new?labels=feature+request&template=FEATURE_REQUEST.md&title=%5BFEATURE%5D%3A+${title}` } else { - return `https://github.com/garden-io/garden/issues/new?assignees=&labels=&template=BUG_REPORT.md&title=${title}` + return `https://github.com/garden-io/garden/issues/new?template=BUG_REPORT.md&title=${title}` } } diff --git a/core/src/watch.ts b/core/src/watch.ts index 59662858409..23f19b92627 100644 --- a/core/src/watch.ts +++ b/core/src/watch.ts @@ -8,7 +8,7 @@ import { watch, FSWatcher } from "chokidar" import { Log } from "./logger/log-entry" -import { InternalError } from "./exceptions" +import { UnexpectedCrashError } from "./exceptions" import EventEmitter2 from "eventemitter2" import { EventBus } from "./events/events" import { Stats } from "fs" @@ -56,12 +56,7 @@ export class Watcher extends EventEmitter2 { try { require("fsevents") } catch (error) { - throw new InternalError({ - message: `Unable to load fsevents module: ${error}`, - detail: { - error, - }, - }) + throw UnexpectedCrashError.wrapError(error, {}, "Unable to load fsevents module") } } diff --git a/core/test/unit/src/commands/version.ts b/core/test/unit/src/commands/version.ts index 9c9297783b9..e81be36d693 100644 --- a/core/test/unit/src/commands/version.ts +++ b/core/test/unit/src/commands/version.ts @@ -59,7 +59,7 @@ describe("VersionCommand", () => { const garden = await makeDummyGarden(tmpDir.path, { commandInfo: { name: "version", args: {}, opts: {} } }) const { result } = await command.action({ log: garden.log, - }) + } as any) expect(result).to.eql({ version: getPackageVersion(), })