Merge pull request #122 from garden-io/cli-fixes

Cli fixes (fixes #88)

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,
@@ -33,6 +33,7 @@ import {
   GardenError,
   InternalError,
   PluginError,
+  toGardenError,
 } from "./exceptions"
 import { Garden } from "./garden"
 import { FileWriter } from "./logger/writers/file-writer"
@@ -194,6 +195,11 @@ function makeOptConfig(param: Parameter<any>): OptConfig {
   return config
 }
 
+function getAliases(params: ParameterValues<any>) {
+  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
  */
@@ -239,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))
       })
@@ -262,6 +268,7 @@ export class GardenCli {
       new TestCommand(),
       new ValidateCommand(),
     ]
+
     const globalOptions = Object.entries(GLOBAL_OPTIONS)
 
     commands.forEach(command => this.addCommand(command, this.program))
@@ -285,8 +292,8 @@ export class GardenCli {
 
     this.commands[fullName] = command
 
-    const args = command.arguments as Parameter<any>
-    const options = command.options as Parameter<any>
+    const args = <ParameterValues<any>>command.arguments || {}
+    const options = <ParameterValues<any>>command.options || {}
     const subCommands = command.subCommands || []
     const argKeys = getKeys(args)
     const optKeys = getKeys(options)
@@ -304,15 +311,43 @@ export class GardenCli {
       const root = resolve(process.cwd(), optsForAction.root)
       const env = optsForAction.env
 
-      // Update logger
+      // 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
       const level = LogLevel[<string>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 = []
@@ -346,37 +381,40 @@ export class GardenCli {
 
   async parse(): Promise<ParseResults> {
     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 }))
       }
       // Note: this circumvents an issue where the process exits before the output is fully flushed
       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`)

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"

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 {

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 = (<any>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 {

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
 }

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