fix(file-writer): only create file if content to write

garden-io · Jul 10, 2018 · 562daa8 · 562daa8
1 parent f372ab3
commit 562daa8
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 23 deletions.
diff --git a/src/cli/cli.ts b/src/cli/cli.ts
@@ -48,7 +48,7 @@ import {
   getArgSynopsis,
   getKeys,
   getOptionSynopsis,
-  filterByArray,
+  filterByKeys,
   prepareArgConfig,
   prepareOptionConfig,
   styleConfig,
@@ -206,8 +206,8 @@ export class GardenCli {
 
     const action = async (argv, cliContext) => {
       // Sywac returns positional args and options in a single object which we separate into args and opts
-      const parsedArgs = filterByArray(argv, argKeys)
-      const parsedOpts = filterByArray(argv, optKeys.concat(globalKeys))
+      const parsedArgs = filterByKeys(argv, argKeys)
+      const parsedOpts = filterByKeys(argv, optKeys.concat(globalKeys))
       const root = resolve(process.cwd(), parsedOpts.root)
       const { env, loglevel, silent, output } = parsedOpts
 
@@ -235,7 +235,7 @@ export class GardenCli {
           }),
           await FileWriter.factory({
             root,
-            logDirPath: ".",
+            path: ".",
             filename: ERROR_LOG_FILENAME,
             level: LogLevel.error,
             truncatePrevious: true,
@@ -292,6 +292,10 @@ export class GardenCli {
     let { code } = parseResult
     let logger
 
+    // Note: Circumvents an issue where the process exits before the output is fully flushed.
+    // Needed for output renderers and Winston (see: https://github.com/winstonjs/winston/issues/228)
+    const waitForOutputFlush = () => sleep(100)
+
     // Logger might not have been initialised if process exits early
     try {
       logger = getLogger()
@@ -327,8 +331,7 @@ export class GardenCli {
       } 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)
+      await waitForOutputFlush()
     }
 
     if (gardenErrors.length > 0) {
@@ -339,6 +342,7 @@ export class GardenCli {
 
       if (logger.writers.find(w => w instanceof FileWriter)) {
         logger.info(`\nSee ${ERROR_LOG_FILENAME} for detailed error message`)
+        await waitForOutputFlush()
       }
 
       code = 1

diff --git a/src/cli/helpers.ts b/src/cli/helpers.ts
@@ -49,8 +49,8 @@ ${chalk.bold(str.slice(0, 5).toUpperCase())}
 
 // Helper functions
 export const getKeys = (obj): string[] => Object.keys(obj || {})
-export const filterByArray = (obj: any, arr: string[]): any => {
-  return arr.reduce((memo, key) => {
+export const filterByKeys = (obj: any, keys: string[]): any => {
+  return keys.reduce((memo, key) => {
     if (obj[key]) {
       memo[key] = obj[key]
     }

diff --git a/src/logger/writers/file-writer.ts b/src/logger/writers/file-writer.ts
@@ -27,14 +27,16 @@ export interface FileWriterConfig {
   level: LogLevel
   root: string
   filename: string
-  logDirPath?: string
+  path?: string
   fileTransportOptions?: {}
   truncatePrevious?: boolean
 }
 
+type FileTransportOptions = winston.transports.FileTransportOptions
+
 const { combine: winstonCombine, timestamp, printf } = winston.format
 
-const DEFAULT_FILE_TRANSPORT_OPTIONS = {
+const DEFAULT_FILE_TRANSPORT_OPTIONS: FileTransportOptions = {
   format: winstonCombine(
     timestamp(),
     printf(info => `\n[${info.timestamp}] ${info.message}`),
@@ -46,7 +48,9 @@ const DEFAULT_FILE_TRANSPORT_OPTIONS = {
 const levelToStr = (lvl: LogLevel): string => LogLevel[lvl]
 
 export class FileWriter extends Writer {
-  private winston: any // Types are still missing from Winston 3.x.x.
+  private fileLogger: winston.Logger | null
+  private filePath: string
+  private fileTransportOptions: FileTransportOptions
 
   public level: LogLevel
 
@@ -58,25 +62,19 @@ export class FileWriter extends Writer {
 
     super({ level })
 
-    this.winston = winston.createLogger({
-      level: levelToStr(level),
-      transports: [
-        new winston.transports.File({
-          ...fileTransportOptions,
-          filename: filePath,
-        }),
-      ],
-    })
+    this.fileTransportOptions = fileTransportOptions
+    this.filePath = filePath
+    this.fileLogger = null
   }
 
   static async factory(config: FileWriterConfig) {
     const {
       filename,
       root,
       truncatePrevious,
-      logDirPath = LOGS_DIR,
+      path: logPath = LOGS_DIR,
     } = config
-    const fullPath = path.join(root, logDirPath)
+    const fullPath = path.join(root, logPath)
     await ensureDir(fullPath)
     const filePath = path.join(fullPath, filename)
     if (truncatePrevious) {
@@ -88,6 +86,19 @@ export class FileWriter extends Writer {
     return new FileWriter(filePath, config)
   }
 
+  // Only init if needed to prevent unnecessary file writes
+  initFileLogger() {
+    return winston.createLogger({
+      level: levelToStr(this.level),
+      transports: [
+        new winston.transports.File({
+          ...this.fileTransportOptions,
+          filename: this.filePath,
+        }),
+      ],
+    })
+  }
+
   render(entry: LogEntry): string | null {
     if (validate(this.level, entry)) {
       const renderFn = entry.level === LogLevel.error ? renderError : renderMsg
@@ -99,7 +110,11 @@ export class FileWriter extends Writer {
   onGraphChange(entry: LogEntry) {
     const out = this.render(entry)
     if (out) {
-      this.winston.log(levelToStr(entry.level), out)
+      if (!this.fileLogger) {
+
+        this.fileLogger = this.initFileLogger()
+      }
+      this.fileLogger.log(levelToStr(entry.level), out)
     }
   }