fix(core): better error messages for invalid YAML

When YAML parsing fails, emit more useful error messages by passing the
failed file contents through a YAML validator.

This will e.g. identify duplicate keys and other syntax errors.

garden-service/package.json

@@ -131,7 +131,8 @@
     "winston": "^3.2.1",
     "wrap-ansi": "^6.2.0",
     "ws": "^7.2.1",
-    "xml-js": "^1.6.11"
+    "xml-js": "^1.6.11",
+    "yaml-lint": "^1.2.4"
   },
   "devDependencies": {
     "@commitlint/cli": "^8.3.5",

garden-service/src/commands/migrate.ts

@@ -7,18 +7,19 @@
  */
 
 import { Command, CommandParams, CommandResult, BooleanParameter, StringsParameter } from "./base"
-import yaml, { safeDump } from "js-yaml"
+import { safeDump } from "js-yaml"
 import { dedent } from "../util/string"
 import { readFile, writeFile } from "fs-extra"
 import { cloneDeep, isEqual } from "lodash"
 import { ConfigurationError, RuntimeError } from "../exceptions"
-import { basename, resolve, parse } from "path"
+import { resolve, parse } from "path"
 import { findConfigPathsInPath, getConfigFilePath } from "../util/fs"
 import { GitHandler } from "../vcs/git"
 import { DEFAULT_GARDEN_DIR_NAME } from "../constants"
 import { exec } from "../util/util"
 import { LoggerType } from "../logger/logger"
 import Bluebird from "bluebird"
+import { loadAndValidateYaml } from "../config/base"
 
 const migrateOptions = {
   write: new BooleanParameter({ help: "Update the `garden.yml` in place." }),
@@ -196,17 +197,9 @@ async function findRoot(path: string): Promise<string | null> {
  * Read the contents of a YAML file and dump to JSON
  */
 async function readYaml(path: string) {
-  let rawSpecs: any[]
   const fileData = await readFile(path)
-
-  try {
-    rawSpecs = yaml.safeLoadAll(fileData.toString()) || []
-  } catch (err) {
-    throw new ConfigurationError(`Could not parse ${basename(path)} in directory ${path} as valid YAML`, err)
-  }
-
-  // Ignore empty resources
-  return rawSpecs.filter(Boolean)
+  const rawSpecs = await loadAndValidateYaml(fileData.toString(), path)
+  return rawSpecs.filter(Boolean) // Ignore empty resources
 }
 
 /**

garden-service/src/config/base.ts

@@ -8,6 +8,7 @@
 
 import { sep, resolve, relative, basename, dirname } from "path"
 import yaml from "js-yaml"
+import yamlLint from "yaml-lint"
 import { readFile } from "fs-extra"
 import { omit, isPlainObject, find, isArray } from "lodash"
 import { ModuleResource, coreModuleSpecSchema, baseModuleSchemaKeys, BuildDependencyConfig } from "./module"
@@ -24,11 +25,34 @@ export interface GardenResource {
   path: string
 }
 
+/**
+ * Attempts to parse content as YAML, and applies a linter to produce more informative error messages when
+ * content is not valid YAML.
+ *
+ * @param content - The contents of the file as a string.
+ * @param path - The path to the file.
+ */
+export async function loadAndValidateYaml(content: string, path: string): Promise<any[]> {
+  try {
+    return yaml.safeLoadAll(content) || []
+  } catch (err) {
+    // We try to find the error using a YAML linter
+    try {
+      await yamlLint(content)
+    } catch (linterErr) {
+      throw new ConfigurationError(
+        `Could not parse ${basename(path)} in directory ${path} as valid YAML: ${err.message}`,
+        linterErr
+      )
+    }
+    // ... but default to throwing a generic error, in case the error wasn't caught by yaml-lint.
+    throw new ConfigurationError(`Could not parse ${basename(path)} in directory ${path} as valid YAML.`, err)
+  }
+}
+
 export async function loadConfig(projectRoot: string, path: string): Promise<GardenResource[]> {
-  // TODO: nicer error messages when load/validation fails
   const configPath = await getConfigFilePath(path)
   let fileData: Buffer
-  let rawSpecs: any[]
 
   // loadConfig returns undefined if config file is not found in the given directory
   try {
@@ -37,11 +61,7 @@ export async function loadConfig(projectRoot: string, path: string): Promise<Gar
     return []
   }
 
-  try {
-    rawSpecs = yaml.safeLoadAll(fileData.toString()) || []
-  } catch (err) {
-    throw new ConfigurationError(`Could not parse ${basename(configPath)} in directory ${path} as valid YAML`, err)
-  }
+  let rawSpecs = await loadAndValidateYaml(fileData.toString(), path)
 
   // Ignore empty resources
   rawSpecs = rawSpecs.filter(Boolean)

garden-service/test/data/test-project-invalid-config/invalid-syntax-module/garden.yml

@@ -1,3 +1,4 @@
 kind: Module
 name: invalid-syntax
+name: invalid-syntax-again
 foo

garden-service/test/unit/src/config/base.ts

@@ -34,6 +34,7 @@ describe("loadConfig", () => {
       async () => await loadConfig(projectPath, resolve(projectPath, "invalid-syntax-module")),
       (err) => {
         expect(err.message).to.match(/Could not parse/)
+        expect(err.message).to.match(/duplicated mapping key/) // include syntax erorrs in the output
       }
     )
   })