fix: handle all promises and add no-floating-promises linting rule

This addresses a few bugs, including error handling issues in the
TaskGraph.

garden-cli/package.json

@@ -58,6 +58,7 @@
     "node-emoji": "^1.8.1",
     "node-pty": "^0.7.6",
     "normalize-url": "^3.2.0",
+    "p-queue": "^3.0.0",
     "path-is-inside": "^1.0.2",
     "shx": "^0.3.2",
     "snyk": "^1.90.2",
@@ -100,6 +101,7 @@
     "@types/node": "^10.7.0",
     "@types/node-emoji": "^1.8.0",
     "@types/normalize-url": "^1.9.1",
+    "@types/p-queue": "^2.3.1",
     "@types/path-is-inside": "^1.0.0",
     "@types/prettyjson": "0.0.28",
     "@types/string-width": "^2.0.0",

garden-cli/src/commands/logs.ts

@@ -63,7 +63,7 @@ export class LogsCommand extends Command<Args, Opts> {
     const stream = new Stream<ServiceLogEntry>()
 
     // TODO: use basic logger (no need for fancy stuff here, just causes flickering)
-    stream.forEach((entry) => {
+    void stream.forEach((entry) => {
       // TODO: color each service differently for easier visual parsing
       let timestamp = "                        "
 

garden-cli/src/logger/writers/fancy-terminal-writer.ts

@@ -20,7 +20,6 @@ import {
 import { LogEntry } from "../log-entry"
 import { Logger } from "../logger"
 import { LogLevel } from "../log-node"
-import { sleep } from "../../util/util"
 import { getChildEntries, getTerminalWidth, interceptStream, validate } from "../util"
 import { Writer, WriterConfig } from "./base"
 
@@ -149,13 +148,11 @@ export class FancyTerminalWriter extends Writer {
         this.updatePending = true
 
         // Resume processing if idle and original update is still pending
-        const maybeResume = async () => {
-          await sleep(FANCY_LOGGER_THROTTLE_MS)
+        setTimeout(() => {
           if (this.updatePending) {
             this.handleGraphChange(logEntry, logger, true)
           }
-        }
-        maybeResume()
+        }, FANCY_LOGGER_THROTTLE_MS)
         return
       }
     }

garden-cli/src/plugins/kubernetes/actions.ts

@@ -290,7 +290,7 @@ export async function getServiceLogs(
       try {
         timestamp = moment(timestampStr).toDate()
       } catch { }
-      stream.write({ serviceName: service.name, timestamp, msg })
+      void stream.write({ serviceName: service.name, timestamp, msg })
     })
 
   proc.stderr.pipe(process.stderr)

garden-cli/src/plugins/kubernetes/helm.ts

@@ -208,7 +208,7 @@ async function build({ ctx, module, logEntry }: BuildModuleParams<HelmModule>):
     .map(([k, v]) => set(values, k, v))
 
   const valuesPath = getValuesPath(chartPath)
-  dumpYaml(valuesPath, values)
+  await dumpYaml(valuesPath, values)
 
   // keep track of which version has been built
   const buildVersionFilePath = join(buildPath, GARDEN_BUILD_VERSION_FILENAME)

garden-cli/src/process.ts

@@ -95,9 +95,7 @@ export async function processModules(
         if (changedModule) {
           garden.log.debug({ msg: `Files changed for module ${changedModule.name}` })
 
-          await Bluebird.map(changeHandler!(changedModule), task => {
-            garden.addTask(task)
-          })
+          await Bluebird.map(changeHandler!(changedModule), (task) => garden.addTask(task))
         }
 
         await garden.processTasks()

garden-cli/src/task-graph.ts

@@ -7,6 +7,7 @@
  */
 
 import * as Bluebird from "bluebird"
+import * as PQueue from "p-queue"
 import chalk from "chalk"
 import { merge, padEnd, pick } from "lodash"
 import { Task, TaskDefinitionError } from "./tasks/base"
@@ -33,21 +34,8 @@ export interface TaskResults {
   [baseKey: string]: TaskResult
 }
 
-interface LogEntryMap { [key: string]: LogEntry }
-
 export const DEFAULT_CONCURRENCY = 4
 
-const taskStyle = chalk.cyan.bold
-
-function inProgressToStr(nodes) {
-  return `Currently in progress [${nodes.map(n => taskStyle(n.getKey())).join(", ")}]`
-}
-
-function remainingTasksToStr(num) {
-  const style = num === 0 ? chalk.green : chalk.yellow
-  return `Remaining tasks ${style.bold(String(num))}`
-}
-
 export class TaskGraph {
   private roots: TaskNodeMap
   private index: TaskNodeMap
@@ -56,22 +44,26 @@ export class TaskGraph {
   private logEntryMap: LogEntryMap
 
   private resultCache: ResultCache
-  private opQueue: OperationQueue
+  private opQueue: PQueue
 
   constructor(private garden: Garden, private concurrency: number = DEFAULT_CONCURRENCY) {
     this.roots = new TaskNodeMap()
     this.index = new TaskNodeMap()
     this.inProgress = new TaskNodeMap()
     this.resultCache = new ResultCache()
-    this.opQueue = new OperationQueue(this)
+    this.opQueue = new PQueue({ concurrency: 1 })
     this.logEntryMap = {}
   }
 
-  addTask(task: Task): Promise<any> {
-    return this.opQueue.request({ type: "addTask", task })
+  async addTask(task: Task): Promise<void> {
+    return this.opQueue.add(() => this.addTaskInternal(task))
   }
 
-  async addTaskInternal(task: Task) {
+  async processTasks(): Promise<TaskResults> {
+    return this.opQueue.add(() => this.processTasksInternal())
+  }
+
+  private async addTaskInternal(task: Task) {
     const predecessor = this.getPredecessor(task)
     let node = this.getNode(task)
 
@@ -109,16 +101,10 @@ export class TaskGraph {
     const existing = this.index.getNode(task)
     return existing || new TaskNode(task)
   }
-
-  processTasks(): Promise<TaskResults> {
-    return this.opQueue.request({ type: "processTasks" })
-  }
-
   /*
     Process the graph until it's complete
    */
-  async processTasksInternal(): Promise<TaskResults> {
-
+  private async processTasksInternal(): Promise<TaskResults> {
     const _this = this
     const results: TaskResults = {}
 
@@ -480,80 +466,15 @@ class ResultCache {
 
 }
 
-// TODO: Add more typing to this class.
-
-/*
-  Used by TaskGraph to prevent race conditions e.g. when calling addTask or
-  processTasks.
-*/
-class OperationQueue {
-  queue: object[]
-  draining: boolean
-
-  constructor(private taskGraph: TaskGraph) {
-    this.queue = []
-    this.draining = false
-  }
-
-  request(opRequest): Promise<any> {
-    let findFn
-
-    switch (opRequest.type) {
-
-      case "addTask":
-        findFn = (o) => o.type === "addTask" && o.task.getBaseKey() === opRequest.task.getBaseKey()
-        break
-
-      case "processTasks":
-        findFn = (o) => o.type === "processTasks"
-        break
-    }
-
-    const existingOp = this.queue.find(findFn)
-
-    const prom = new Promise((resolver) => {
-      if (existingOp) {
-        existingOp["resolvers"].push(resolver)
-      } else {
-        this.queue.push({ ...opRequest, resolvers: [resolver] })
-      }
-    })
-
-    if (!this.draining) {
-      this.process()
-    }
-
-    return prom
-  }
-
-  async process() {
-    this.draining = true
-    const op = this.queue.shift()
-
-    if (!op) {
-      this.draining = false
-      return
-    }
-
-    switch (op["type"]) {
-
-      case "addTask":
-        const task = op["task"]
-        await this.taskGraph.addTaskInternal(task)
-        for (const resolver of op["resolvers"]) {
-          resolver()
-        }
-        break
+interface LogEntryMap { [key: string]: LogEntry }
 
-      case "processTasks":
-        const results = await this.taskGraph.processTasksInternal()
-        for (const resolver of op["resolvers"]) {
-          resolver(results)
-        }
-        break
-    }
+const taskStyle = chalk.cyan.bold
 
-    this.process()
-  }
+function inProgressToStr(nodes) {
+  return `Currently in progress [${nodes.map(n => taskStyle(n.getKey())).join(", ")}]`
+}
 
+function remainingTasksToStr(num) {
+  const style = num === 0 ? chalk.green : chalk.yellow
+  return `Remaining tasks ${style.bold(String(num))}`
 }

garden-cli/src/vcs/git.ts

@@ -9,7 +9,6 @@
 import execa = require("execa")
 import { join } from "path"
 import { ensureDir, pathExists, stat } from "fs-extra"
-import { argv } from "process"
 import Bluebird = require("bluebird")
 
 import { NEW_MODULE_VERSION, VcsHandler, RemoteSourceParams } from "./base"
@@ -140,14 +139,3 @@ export class GitHandler extends VcsHandler {
   }
 
 }
-
-// used by the build process to resolve and store the tree version for plugin modules
-if (require.main === module) {
-  const path = argv[2]
-  const handler = new GitHandler(path)
-
-  handler.getTreeVersion(path)
-    .then((treeVersion) => {
-      console.log(JSON.stringify(treeVersion, null, 4))
-    })
-}

garden-cli/src/watch.ts

@@ -149,7 +149,7 @@ export class FSWatcher {
 
       if (configChanged) {
         // The added/removed dir contains one or more garden.yml files
-        this.invalidateCachedForAll()
+        await this.invalidateCachedForAll()
         return changeHandler(null, true)
       }
 

tslint.json

@@ -36,6 +36,7 @@
     "no-console": false,
     "no-debugger": true,
     "no-eval": true,
+    "no-floating-promises": true,
     "no-invalid-template-strings": true,
     "no-invalid-this": true,
     "no-null-keyword": false,