feat: don't restart command when config is invalid

Before restarting a watch-mode command when a config file is
changed/added/removed, attempt creating a new Garden instance using the
project's (and its modules') updated config before performing the
restart.

If a configuration error was introduced by the change, log an error
message and keep the existing Garden instance instead of restarting.

garden-service/src/events.ts

@@ -56,9 +56,13 @@ export interface Events {
   configAdded: {
     path: string,
   },
+  configRemoved: {
+    path: string,
+  },
   projectConfigChanged: {},
   moduleConfigChanged: {
     name: string,
+    path: string,
   },
   moduleSourcesChanged: {
     name: string,

garden-service/src/process.ts

@@ -19,6 +19,8 @@ import { Garden } from "./garden"
 import { LogEntry } from "./logger/log-entry"
 import { startServer } from "./server/server"
 import { ConfigGraph } from "./config-graph"
+import { dedent } from "./util/string"
+import { ConfigurationError } from "./exceptions"
 
 export type ProcessHandler = (graph: ConfigGraph, module: Module) => Promise<BaseTask[]>
 
@@ -121,19 +123,32 @@ export async function processModules(
       resolve()
     })
 
-    garden.events.on("projectConfigChanged", () => {
-      log.info({ symbol: "info", msg: `Project configuration changed, reloading...` })
-      resolve()
+    garden.events.on("projectConfigChanged", async () => {
+      if (await validateConfigChange(garden, log, garden.projectRoot, "changed")) {
+        log.info({ symbol: "info", msg: `Project configuration changed, reloading...` })
+        resolve()
+      }
     })
 
-    garden.events.on("configAdded", (event) => {
-      log.info({ symbol: "info", msg: `Garden config added at ${event.path}, reloading...` })
-      resolve()
+    garden.events.on("configAdded", async (event) => {
+      if (await validateConfigChange(garden, log, event.path, "added")) {
+        log.info({ symbol: "info", msg: `Garden config added at ${event.path}, reloading...` })
+        resolve()
+      }
     })
 
-    garden.events.on("moduleConfigChanged", (event) => {
-      log.info({ symbol: "info", section: event.name, msg: `Module configuration changed, reloading...` })
-      resolve()
+    garden.events.on("configRemoved", async (event) => {
+      if (await validateConfigChange(garden, log, event.path, "removed")) {
+        log.info({ symbol: "info", msg: `Garden config at ${event.path} removed, reloading...` })
+        resolve()
+      }
+    })
+
+    garden.events.on("moduleConfigChanged", async (event) => {
+      if (await validateConfigChange(garden, log, event.path, "changed")) {
+        log.info({ symbol: "info", section: event.name, msg: `Module configuration changed, reloading...` })
+        resolve()
+      }
     })
 
     garden.events.on("moduleSourcesChanged", async (event) => {
@@ -164,3 +179,35 @@ export async function processModules(
   }
 
 }
+
+/**
+ * When config files change / are added / are removed, we try initializing a new Garden instance
+ * with the changed config files and performing a bit of validation on it before proceeding with
+ * a restart. If a config error was encountered, we simply log the error and keep the existing
+ * Garden instance.
+ *
+ * Returns true if no configuration errors occurred.
+ */
+async function validateConfigChange(
+  garden: Garden, log: LogEntry, changedPath: string, operationType: "added" | "changed" | "removed",
+): Promise<boolean> {
+
+  try {
+    const nextGarden = await Garden.factory(garden.projectRoot, garden.opts)
+    await nextGarden.getConfigGraph()
+  } catch (error) {
+    if (error instanceof ConfigurationError) {
+      const msg = dedent`
+        Encountered configuration error after ${changedPath} was ${operationType}:
+
+        ${error.message}
+
+        Keeping existing configuration and skipping restart.`
+      log.error({ symbol: "error", msg, error })
+      return false
+    } else {
+      throw error
+    }
+  }
+  return true
+}

garden-service/src/watch.ts

@@ -86,12 +86,16 @@ export class Watcher {
         this.invalidateCached(modules)
 
         if (changedModule) {
-          this.garden.events.emit("moduleConfigChanged", { name: changedModule.name })
+          this.garden.events.emit("moduleConfigChanged", { name: changedModule.name, path })
         } else if (filename === MODULE_CONFIG_FILENAME) {
           if (parsed.dir === this.garden.projectRoot) {
             this.garden.events.emit("projectConfigChanged", {})
           } else {
-            this.garden.events.emit("configAdded", { path })
+            if (type === "added") {
+              this.garden.events.emit("configAdded", { path })
+            } else {
+              this.garden.events.emit("configRemoved", { path })
+            }
           }
         }
 

garden-service/test/src/watch.ts

@@ -33,9 +33,10 @@ describe("Watcher", () => {
   }
 
   it("should emit a moduleConfigChanged changed event when module config is changed", async () => {
-    emitEvent("change", resolve(modulePath, "garden.yml"))
+    const path = resolve(modulePath, "garden.yml")
+    emitEvent("change", path)
     expect(garden.events.log).to.eql([
-      { name: "moduleConfigChanged", payload: { name: "module-a" } },
+      { name: "moduleConfigChanged", payload: { name: "module-a", path } },
     ])
   })
 
@@ -71,6 +72,14 @@ describe("Watcher", () => {
     ])
   })
 
+  it("should emit a configRemoved event when removing a garden.yml file", async () => {
+    const path = resolve(garden.projectRoot, "module-b", "garden.yml")
+    emitEvent("unlink", path)
+    expect(garden.events.log).to.eql([
+      { name: "configRemoved", payload: { path } },
+    ])
+  })
+
   it("should emit a moduleSourcesChanged event when a module file is changed", async () => {
     const pathChanged = resolve(modulePath, "foo.txt")
     emitEvent("change", pathChanged)