From 02b05b398e2726406d994ff760289fda6d8c1dbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ey=C3=BE=C3=B3r=20Magn=C3=BAsson?= Date: Sun, 27 May 2018 15:25:51 +0200 Subject: [PATCH 1/3] fix(cli): map all Errors to GardenErrors and log accordingly --- src/cli.ts | 20 ++++++++++++-------- src/exceptions.ts | 2 +- src/logger/renderers.ts | 8 ++++---- src/logger/types.ts | 2 +- 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index 9d6832033a..99d1cc71b4 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -33,6 +33,7 @@ import { GardenError, InternalError, PluginError, + toGardenError, } from "./exceptions" import { Garden } from "./garden" import { FileWriter } from "./logger/writers/file-writer" @@ -346,28 +347,28 @@ export class GardenCli { async parse(): Promise { const parseResult: SywacParseResults = await this.program.parse() - const { argv, details, errors: parseErrors, output: cliOutput } = parseResult + const { argv, details, errors, output: cliOutput } = parseResult const commandResult = details.result const { output } = argv let code = parseResult.code // --help or --version options were called so we log the cli output and exit - if (cliOutput && parseErrors.length < 1) { + if (cliOutput && errors.length < 1) { this.logger.stop() console.log(cliOutput) process.exit(parseResult.code) } - const errors: GardenError[] = parseErrors - .map(e => ({ type: "parameter", message: e.toString() })) + const gardenErrors: GardenError[] = errors + .map(toGardenError) .concat((commandResult && commandResult.errors) || []) // --output option set if (output) { const renderer = OUTPUT_RENDERERS[output] - if (errors.length > 0) { - console.error(renderer({ success: false, errors })) + if (gardenErrors.length > 0) { + console.error(renderer({ success: false, errors: gardenErrors })) } else { console.log(renderer({ success: true, ...commandResult })) } @@ -375,8 +376,11 @@ export class GardenCli { await sleep(100) } - if (errors.length > 0) { - errors.forEach(err => this.logger.error({ msg: err.message, error: err })) + if (gardenErrors.length > 0) { + gardenErrors.forEach(error => this.logger.error({ + msg: error.message, + error, + })) if (this.logger.writers.find(w => w instanceof FileWriter)) { this.logger.info(`\nSee ${ERROR_LOG_FILENAME} for detailed error message`) diff --git a/src/exceptions.ts b/src/exceptions.ts index 2d18967885..8cdd861f0a 100644 --- a/src/exceptions.ts +++ b/src/exceptions.ts @@ -23,7 +23,7 @@ export abstract class GardenBaseError extends Error implements GardenError { } } -export function toGardenError(err: Error): GardenError { +export function toGardenError(err: Error | GardenError): GardenError { if (err instanceof GardenBaseError) { return err } else { diff --git a/src/logger/renderers.ts b/src/logger/renderers.ts index a8249410f3..fff163beb9 100644 --- a/src/logger/renderers.ts +++ b/src/logger/renderers.ts @@ -70,17 +70,17 @@ export function renderEmoji(entry: LogEntry): string { } export function renderError(entry: LogEntry) { - const { error } = entry.opts + const { msg, error } = entry.opts if (error) { - let out = `${error.stack || error.message}` - const detail = (error).detail + const { detail, message, stack } = error + let out = `${stack || message}` if (detail) { const yamlDetail = yaml.safeDump(detail, { noRefs: true, skipInvalid: true }) out += `\nError Details:\n${yamlDetail}` } return out } - return "" + return msg || "" } export function renderSymbol(entry: LogEntry): string { diff --git a/src/logger/types.ts b/src/logger/types.ts index 3c3f66948c..0b682ef928 100644 --- a/src/logger/types.ts +++ b/src/logger/types.ts @@ -59,7 +59,7 @@ export interface LogEntryOpts { append?: boolean fromStdStream?: boolean showDuration?: boolean - error?: GardenError | Error + error?: GardenError id?: string unindentChildren?: boolean } From a64fbb0c84b8e82329dc012a639c9ce32b521965 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ey=C3=BE=C3=B3r=20Magn=C3=BAsson?= Date: Sun, 27 May 2018 15:35:25 +0200 Subject: [PATCH 2/3] feat: add truncatePrevious option to file-writer Root error.log is now truncated on each cli run. Moved development.log to the now default .garden/logs directory and added a persistent error.log there as well --- src/cli.ts | 21 ++++++++++++++++--- src/constants.ts | 1 + src/logger/writers/file-writer.ts | 34 ++++++++++++++++++++++++------- 3 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index 99d1cc71b4..2950e91e19 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -305,15 +305,30 @@ export class GardenCli { const root = resolve(process.cwd(), optsForAction.root) const env = optsForAction.env - // Update logger + // Configure logger const logger = this.logger const { loglevel, silent, output } = optsForAction const level = LogLevel[loglevel] logger.level = level if (!silent && !output) { logger.writers.push( - new FileWriter({ level, root }), - new FileWriter({ level: LogLevel.error, filename: ERROR_LOG_FILENAME, root }), + await FileWriter.factory({ + root, + level, + filename: "development.log", + }), + await FileWriter.factory({ + root, + filename: ERROR_LOG_FILENAME, + level: LogLevel.error, + }), + await FileWriter.factory({ + root, + logDirPath: ".", + filename: ERROR_LOG_FILENAME, + level: LogLevel.error, + truncatePrevious: true, + }), ) } else { logger.writers = [] diff --git a/src/constants.ts b/src/constants.ts index eeaa17b984..1ee9a4c982 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -11,6 +11,7 @@ import { resolve } from "path" export const MODULE_CONFIG_FILENAME = "garden.yml" export const STATIC_DIR = resolve(__dirname, "..", "static") export const GARDEN_DIR_NAME = ".garden" +export const LOGS_DIR = `${GARDEN_DIR_NAME}/logs` export const GARDEN_VERSIONFILE_NAME = ".garden-version" export const DEFAULT_NAMESPACE = "default" export const DEFAULT_PORT_PROTOCOL = "TCP" diff --git a/src/logger/writers/file-writer.ts b/src/logger/writers/file-writer.ts index 5a04e92e57..1c6ae2a390 100644 --- a/src/logger/writers/file-writer.ts +++ b/src/logger/writers/file-writer.ts @@ -9,6 +9,7 @@ import * as winston from "winston" import * as path from "path" import * as stripAnsi from "strip-ansi" +import { ensureDir, truncate } from "fs-extra" import { LogLevel, @@ -20,17 +21,19 @@ import { renderError, renderMsg, } from "../renderers" +import { LOGS_DIR } from "../../constants" export interface FileWriterConfig { level: LogLevel root: string - filename?: string + filename: string + logDirPath?: string fileTransportOptions?: {} + truncatePrevious?: boolean } const { combine: winstonCombine, timestamp, printf } = winston.format -const DEFAULT_LOG_FILENAME = "development.log" const DEFAULT_FILE_TRANSPORT_OPTIONS = { format: winstonCombine( timestamp(), @@ -47,12 +50,10 @@ export class FileWriter extends Writer { public level: LogLevel - constructor(config: FileWriterConfig) { + constructor(filePath: string, config: FileWriterConfig) { const { fileTransportOptions = DEFAULT_FILE_TRANSPORT_OPTIONS, - filename = DEFAULT_LOG_FILENAME, level, - root, } = config super({ level }) @@ -62,15 +63,34 @@ export class FileWriter extends Writer { transports: [ new winston.transports.File({ ...fileTransportOptions, - filename: path.join(root, filename), + filename: filePath, }), ], }) } + static async factory(config: FileWriterConfig) { + const { + filename, + root, + truncatePrevious, + logDirPath = LOGS_DIR, + } = config + const fullPath = path.join(root, logDirPath) + await ensureDir(fullPath) + const filePath = path.join(fullPath, filename) + if (truncatePrevious) { + try { + await truncate(filePath) + } catch (_) { + } + } + return new FileWriter(filePath, config) + } + render(entry: LogEntry): string | null { - const renderFn = entry.level === LogLevel.error ? renderError : renderMsg if (validate(this.level, entry)) { + const renderFn = entry.level === LogLevel.error ? renderError : renderMsg return stripAnsi(renderFn(entry)) } return null From 8c249bdd1c0c855c71b6ab955bddc7eae774ff1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ey=C3=BE=C3=B3r=20Magn=C3=BAsson?= Date: Sun, 27 May 2018 23:09:25 +0200 Subject: [PATCH 3/3] feat(cli): validate option flags --- src/cli.ts | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index 2950e91e19..ac3b23042d 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -17,7 +17,7 @@ import { shutdown, sleep, } from "./util" -import { merge, intersection, reduce } from "lodash" +import { difference, flatten, merge, intersection, reduce } from "lodash" import { BooleanParameter, Command, @@ -195,6 +195,11 @@ function makeOptConfig(param: Parameter): OptConfig { return config } +function getAliases(params: ParameterValues) { + return flatten(Object.entries(params) + .map(([key, param]) => param.alias ? [key, param.alias] : [key])) +} + /** * Returns the params that need to be overridden set to false */ @@ -240,7 +245,7 @@ export class GardenCli { implicitCommand: false, }) .showHelpByDefault() - .check((argv, _context) => { + .check((argv, _ctx) => { // NOTE: Need to mutate argv! merge(argv, falsifyConflictingParams(argv, GLOBAL_OPTIONS)) }) @@ -263,6 +268,7 @@ export class GardenCli { new TestCommand(), new ValidateCommand(), ] + const globalOptions = Object.entries(GLOBAL_OPTIONS) commands.forEach(command => this.addCommand(command, this.program)) @@ -286,8 +292,8 @@ export class GardenCli { this.commands[fullName] = command - const args = command.arguments as Parameter - const options = command.options as Parameter + const args = >command.arguments || {} + const options = >command.options || {} const subCommands = command.subCommands || [] const argKeys = getKeys(args) const optKeys = getKeys(options) @@ -305,6 +311,19 @@ export class GardenCli { const root = resolve(process.cwd(), optsForAction.root) const env = optsForAction.env + // Validate options (feels like the parser should handle this) + const builtinOptions = ["help", "h", "version", "v"] + const availableOptions = [...getAliases(options), ...getAliases(GLOBAL_OPTIONS), ...builtinOptions] + const passedOptions = cliContext.args + .filter(str => str.startsWith("-") || str.startsWith("--")) + .map(str => str.startsWith("--") ? str.slice(2) : str.slice(1)) + .map(str => str.split("=")[0]) + const invalid = difference(passedOptions, availableOptions) + if (invalid.length > 0) { + cliContext.cliMessage(`Received invalid flag(s): ${invalid.join(" ")}`) + return + } + // Configure logger const logger = this.logger const { loglevel, silent, output } = optsForAction