Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(framework): several error handling improvements #5001

Merged
merged 18 commits into from
Sep 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions .github/ISSUE_TEMPLATE/CRASH.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
---
name: Crash report
about: Report crashes
title: ''
labels: 'bug,crash'
assignees: ''

---

## Crash report

<!-- First of all: Thank you for helping us improve Garden! 💚 -->

### Error message

<!-- Please post the full output of the Garden command (after you made sure that it does not contain sensitive information) -->
<!-- If possible, please also attach additional logs, e.g. using the --log-level=silly option -->

### What did you do?

<!-- Which Garden command did you run, including flags? Did you do anything unusual before? -->

### Your environment

<!-- Without this information, we are likely not able to reproduce the issue -->
* **OS:** <!-- which macOS, linux distro, Windows, etc. -->
* **How I'm running Kubernetes:** <!-- e.g. minikube, GKE, EKS, other -->
* **Garden version:** <!-- Please run `garden version` and copy and paste the results -->

### Frequency

<!-- Is this crash reproducable, or does it only happen sporadically? -->

### Workaround

<!-- If applicable, a way to work around the issue until it has been resolved. -->

### Additional context

<!-- Add any other context about the problem here. -->

16 changes: 12 additions & 4 deletions cli/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 { InternalError, explainGardenError } from "@garden-io/core/build/src/exceptions"

// These plugins are always registered
export const getBundledPlugins = (): GardenPluginReference[] => [
Expand Down Expand Up @@ -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) {
Expand All @@ -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 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)}`)
}
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
stefreak marked this conversation as resolved.
Show resolved Hide resolved
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: 1 addition & 7 deletions core/src/build-staging/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(InternalError.wrapError(readlinkErr, { path, readlinkErr }, "Error reading symlink"), null)
}

// Ignore absolute symlinks unless specifically allowed
Expand Down
5 changes: 4 additions & 1 deletion core/src/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 InternalError({
message: `Invalid reference found in cache: ${stringKey}`,
detail: { stringKey },
})
}
return <[CacheKey, CacheValue]>[entry.key, entry.value]
})
Expand Down
50 changes: 31 additions & 19 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 } 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 @@ -404,7 +404,7 @@ ${renderCommands(commands)}
}): Promise<RunOutput> {
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 @@ -443,27 +443,36 @@ ${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"))
stefreak marked this conversation as resolved.
Show resolved Hide resolved
}

const log = logger.createLog()
log.verbose(`garden version: ${getPackageVersion()}`)

// Load custom commands from current project (if applicable) and see if any match the arguments
if (!command) {
projectConfig = await this.getProjectConfig(log, workingDir)
try {
projectConfig = await this.getProjectConfig(log, workingDir)

if (projectConfig) {
const customCommands = await this.getCustomCommands(log, workingDir)
const picked = pickCommand(customCommands, argv._)
command = picked.command
matchedPath = picked.matchedPath
if (projectConfig) {
const customCommands = await this.getCustomCommands(log, workingDir)
const picked = pickCommand(customCommands, argv._)
command = picked.command
matchedPath = picked.matchedPath
}
} catch (error) {
return done(1, explainGardenError(error, "Failed to get custom commands"))
}
}

Expand Down Expand Up @@ -550,7 +559,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 All @@ -562,7 +571,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 {
Expand Down
7 changes: 4 additions & 3 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, InternalError, ParameterError, 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 @@ -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, ` +
Expand Down Expand Up @@ -549,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
6 changes: 3 additions & 3 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 { InternalError, RuntimeError, GardenBaseError, GardenError, isGardenError } 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 Expand Up @@ -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({
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, InternalError, RuntimeError, 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
2 changes: 1 addition & 1 deletion core/src/commands/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, InternalError } from "../exceptions"
import { AuthRedirectServer } from "../cloud/auth"
import { EventBus } from "../events/events"
import { getCloudDistributionName } from "../util/util"
Expand Down
5 changes: 3 additions & 2 deletions core/src/commands/version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import { getPackageVersion } from "../util/util"
import { Command, CommandResult } from "./base"
import { Command, CommandParams, CommandResult } from "./base"

interface VersionCommandResult {
version: string
Expand All @@ -19,7 +19,8 @@ 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 } = params
const version = getPackageVersion()
log.info(`garden version: ${version}`)

Expand Down
Loading