feat: improve template string error messages

Include a snippet to the template string in a YAML file, and point to
YAML file if available.

core/src/config/base.ts

@@ -54,6 +54,7 @@ export type ObjectPath = (string | number)[]
 
 export interface YamlDocumentWithSource extends Document {
   source: string
+  filename: string | undefined
 }
 
 export function getEffectiveConfigFileLocation(actionConfig: BaseActionConfig): string {
@@ -128,6 +129,7 @@ export const allConfigKinds = ["Module", "Workflow", "Project", configTemplateKi
 export async function loadAndValidateYaml(
   content: string,
   sourceDescription: string,
+  filename: string | undefined,
   version: DocumentOptions["version"] = "1.2"
 ): Promise<YamlDocumentWithSource[]> {
   try {
@@ -142,6 +144,7 @@ export async function loadAndValidateYaml(
 
       const docWithSource = doc as YamlDocumentWithSource
       docWithSource.source = content
+      docWithSource.filename = filename
 
       return docWithSource
     })
@@ -197,7 +200,11 @@ export async function validateRawConfig({
   projectRoot: string
   allowInvalid?: boolean
 }) {
-  let rawSpecs = await loadAndValidateYaml(rawConfig, `${basename(configPath)} in directory ${dirname(configPath)}`)
+  let rawSpecs = await loadAndValidateYaml(
+    rawConfig,
+    `${basename(configPath)} in directory ${dirname(configPath)}`,
+    configPath
+  )
 
   // Ignore empty resources
   rawSpecs = rawSpecs.filter(Boolean)

core/src/config/validation.ts

@@ -11,7 +11,7 @@ import { ConfigurationError } from "../exceptions.js"
 import { relative } from "path"
 import { uuidv4 } from "../util/random.js"
 import type { BaseGardenResource, ObjectPath, YamlDocumentWithSource } from "./base.js"
-import type { ParsedNode, Range } from "yaml"
+import type { ParsedNode } from "yaml"
 import { padEnd } from "lodash-es"
 import { styles } from "../logger/styles.js"
 
@@ -130,8 +130,6 @@ export function validateSchema<T>(
     return result.value
   }
 
-  const yamlDoc = source?.yamlDoc
-  const rawYaml = yamlDoc?.source
   const yamlBasePath = source?.path || []
 
   const errorDetails = error.details.map((e) => {
@@ -140,15 +138,16 @@ export function validateSchema<T>(
         ? improveZodValidationErrorMessage(e, yamlBasePath)
         : improveJoiValidationErrorMessage(e, schema, yamlBasePath)
 
-    const node = yamlDoc?.getIn([...yamlBasePath, ...e.path], true) as ParsedNode | undefined
-    const range = node?.range
-
-    if (rawYaml && yamlDoc?.contents && range) {
-      try {
-        e.message = addYamlContext({ rawYaml, range, message: e.message })
-      } catch {
-        // ignore
-      }
+    try {
+      e.message = addYamlContext({
+        source: {
+          ...source,
+          path: [...yamlBasePath, ...e.path],
+        },
+        message: e.message,
+      })
+    } catch {
+      // ignore
     }
 
     return e
@@ -218,7 +217,25 @@ function improveZodValidationErrorMessage(item: Joi.ValidationErrorItem, yamlBas
   }
 }
 
-function addYamlContext({ rawYaml, range, message }: { rawYaml: string; range: Range; message: string }): string {
+export function addYamlContext({
+  source: { yamlDoc, path },
+  message,
+}: {
+  source: ConfigSource
+  message: string
+}): string {
+  if (!yamlDoc) {
+    return message
+  }
+
+  const node = yamlDoc.getIn(path, true) as ParsedNode | undefined
+  const range = node?.range
+  const rawYaml = yamlDoc.source
+
+  if (!node || !range || !rawYaml) {
+    return message
+  }
+
   // Get one line before the error location start, and the line including the error location end
   const toStart = rawYaml.slice(0, range[0])
   const lineNumber = toStart.split("\n").length + 1
@@ -256,5 +273,5 @@ function addYamlContext({ rawYaml, range, message }: { rawYaml: string; range: R
   const errorLineOffset = range[0] - errorLineStart + linePrefixLength + 2
   const marker = styles.error("-".repeat(errorLineOffset)) + styles.error.bold("^")
 
-  return `${snippet}\n${marker}\n${styles.error.bold(message)}`
+  return `${yamlDoc.filename ? `${styles.secondary(`${yamlDoc.filename}:${lineNumber}`)}\n` : ""}${snippet}\n${marker}\n${styles.error.bold(message)}`
 }

core/src/exceptions.ts

@@ -18,7 +18,8 @@ import dns from "node:dns"
 import { styles } from "./logger/styles.js"
 import type { ExecaError } from "execa"
 import type { Location } from "./template-string/ast.js"
-import type { ConfigSource } from "./config/validation.js"
+import { addYamlContext, type ConfigSource } from "./config/validation.js"
+import { ParsedNode } from "yaml"
 
 // Unfortunately, NodeJS does not provide a list of all error codes, so we have to maintain this list manually.
 // See https://nodejs.org/docs/latest-v18.x/api/dns.html#error-codes
@@ -314,14 +315,25 @@ export class TemplateStringError extends GardenError {
   originalMessage: string
 
   constructor(params: GardenErrorParams & { loc: Location; yamlSource: ConfigSource }) {
-    const path = params.yamlSource.path
-    const pathDescription = path.length > 0 ? ` at path ${styles.accent(path.join("."))}` : ""
-    const prefix = `Invalid template string (${styles.accent(
-      truncate(params.loc.source.rawTemplateString, { length: 200 }).replace(/\n/g, "\\n")
-    )})${pathDescription}: `
-    const message = params.message.startsWith(prefix) ? params.message : prefix + params.message
-
-    super({ ...params, message })
+    let enriched: string = params.message
+    try {
+      // TODO: Use Location information from parser to point to the specific part
+      enriched = addYamlContext({ source: params.yamlSource, message: params.message })
+    } catch {
+      // ignore
+    }
+
+    if (enriched === params.message) {
+      const { path } = params.yamlSource
+
+      const pathDescription = path.length > 0 ? ` at path ${styles.accent(path.join("."))}` : ""
+      const prefix = `Invalid template string (${styles.accent(
+        truncate(params.loc.source.rawTemplateString, { length: 200 }).replace(/\n/g, "\\n")
+      )})${pathDescription}: `
+      enriched = params.message.startsWith(prefix) ? params.message : prefix + params.message
+    }
+
+    super({ ...params, message: enriched })
     this.loc = params.loc
     this.originalMessage = params.message
   }

core/src/plugins/kubernetes/kubernetes-type/common.ts

@@ -368,7 +368,11 @@ async function readKustomizeManifests(
       log,
       args: ["build", kustomizePath, ...extraArgs],
     })
-    const manifests = await parseKubernetesManifests(kustomizeOutput, `kustomize output of ${action.longDescription()}`)
+    const manifests = await parseKubernetesManifests(
+      kustomizeOutput,
+      `kustomize output of ${action.longDescription()}`,
+      undefined
+    )
     return manifests.map((manifest, index) => ({
       declaration: {
         type: "kustomize",
@@ -427,7 +431,8 @@ async function readFileManifests(
         const resolved = ctx.resolveTemplateStrings(str, { allowPartial: true, unescape: true })
         const manifests = await parseKubernetesManifests(
           resolved,
-          `${basename(absPath)} in directory ${dirname(absPath)} (specified in ${action.longDescription()})`
+          `${basename(absPath)} in directory ${dirname(absPath)} (specified in ${action.longDescription()})`,
+          absPath
         )
         return manifests.map((manifest, index) => ({
           declaration: {
@@ -449,7 +454,11 @@ async function readFileManifests(
  * @param str raw string containing Kubernetes manifests in YAML format
  * @param sourceDescription description of where the YAML string comes from, e.g. "foo.yaml in directory /bar"
  */
-async function parseKubernetesManifests(str: string, sourceDescription: string): Promise<KubernetesResource[]> {
+async function parseKubernetesManifests(
+  str: string,
+  sourceDescription: string,
+  filename: string | undefined
+): Promise<KubernetesResource[]> {
   // parse yaml with version 1.1 by default, as Kubernetes still uses this version.
   // See also https://github.com/kubernetes/kubernetes/issues/34146
   const docs = await loadAndValidateYaml(str, sourceDescription, "1.1")

core/test/unit/src/config/base.ts

@@ -610,7 +610,7 @@ describe("loadAndValidateYaml", () => {
       name: foo
     `
 
-    const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar")
+    const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar", "bar/foo.yaml")
 
     expect(yamlDocs).to.have.length(1)
     expect(yamlDocs[0].source).to.equal(yaml)
@@ -633,7 +633,7 @@ describe("loadAndValidateYaml", () => {
       name: doc3
     `
 
-    const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar")
+    const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar", "bar/foo.yaml")
 
     expect(yamlDocs).to.have.length(3)
 
@@ -662,7 +662,7 @@ describe("loadAndValidateYaml", () => {
       newYamlOctalNumber: 0o777
     `
 
-    const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar")
+    const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar", "bar/foo.yaml")
 
     expect(yamlDocs).to.have.length(1)
     expect(yamlDocs[0].source).to.equal(yaml)
@@ -684,7 +684,7 @@ describe("loadAndValidateYaml", () => {
       newYamlOctalNumber: 0o777
     `
 
-    const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar")
+    const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar", "bar/foo.yaml")
 
     expect(yamlDocs).to.have.length(1)
     expect(yamlDocs[0].source).to.equal(yaml)
@@ -704,7 +704,7 @@ describe("loadAndValidateYaml", () => {
     `
 
     // we use the version parameter to force the yaml 1.1 standard
-    const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar", "1.1")
+    const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar", "bar/foo.yaml", "1.1")
 
     expect(yamlDocs).to.have.length(1)
     expect(yamlDocs[0].source).to.equal(yaml)
@@ -720,7 +720,7 @@ describe("loadAndValidateYaml", () => {
     `
 
     await expectError(
-      () => loadAndValidateYaml(yaml, "foo.yaml in directory bar"),
+      () => loadAndValidateYaml(yaml, "foo.yaml in directory bar", "bar/foo.yaml"),
       (err) => {
         expect(err.message).to.eql(dedent`
           Could not parse foo.yaml in directory bar as valid YAML: YAMLException: unidentified alias "bar" (1:10)

core/test/unit/src/config/validation.ts

@@ -306,7 +306,7 @@ describe("validateSchema", () => {
       name: bar
     `
 
-    const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar")
+    const yamlDocs = await loadAndValidateYaml(yaml, "foo.yaml in directory bar", "bar/foo.yaml")
     const yamlDoc = yamlDocs[1]
 
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -324,6 +324,7 @@ describe("validateSchema", () => {
         expect(stripAnsi(err.message)).to.equal(dedent`
         Validation error:
 
+        bar/foo.yaml:11
         ...
         9   | spec:
         10  |   foo: 456
@@ -347,7 +348,8 @@ describe("validateSchema", () => {
     `
 
     const yamlDoc = parseDocument(yaml) as YamlDocumentWithSource
-    yamlDoc["source"] = yaml
+    yamlDoc.source = yaml
+    yamlDoc.filename = "foo/bar.yaml"
 
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     const config: any = {
@@ -364,6 +366,7 @@ describe("validateSchema", () => {
         expect(stripAnsi(err.message)).to.equal(dedent`
         Validation error:
 
+        foo/bar.yaml:5
         ...
         3  | spec:
         4  |   foo: 123

core/test/unit/src/template-string.ts

@@ -26,6 +26,8 @@ import {
   UnresolvableValue,
   visitAll,
 } from "../../../src/template-string/static-analysis.js"
+import { Document } from "yaml"
+import { loadAndValidateYaml, YamlDocumentWithSource } from "../../../src/config/base.js"
 
 describe("resolveTemplateString", () => {
   it("should return a non-templated string unchanged", () => {
@@ -314,6 +316,41 @@ describe("resolveTemplateString", () => {
     })
   })
 
+  it("if available, should include yaml context in error message", async () => {
+    const command = "${resol${part}ed}"
+    const yamlDoc = await loadAndValidateYaml(
+      dedent`
+      name: test,
+      kind: Build
+      spec:
+        command: '${command}'
+    `,
+      "test",
+      "bar/foo.yaml"
+    )
+    void expectError(
+      () =>
+        resolveTemplateString({
+          string: command,
+          context: new GenericContext({}),
+          source: {
+            yamlDoc: yamlDoc[0],
+            path: ["spec", "command"],
+          },
+        }),
+      {
+        contains: dedent`
+          bar/foo.yaml:5
+          ...
+          3  | spec:
+          4  |   command: '\${resol\${part}ed}
+          ----------------^
+          unable to parse as valid template string.
+        `,
+      }
+    )
+  })
+
   it("should handle a single-quoted string", () => {
     const res = resolveTemplateString({ string: "${'foo'}", context: new GenericContext({}) })
     expect(res).to.equal("foo")

plugins/pulumi/src/helpers.ts

@@ -264,7 +264,8 @@ export async function applyConfig(params: PulumiParams & { previewDirPath?: stri
     const fileData = await readFile(stackConfigPath)
     const stackConfigDocs = await loadAndValidateYaml(
       fileData.toString(),
-      `${basename(stackConfigPath)} in directory ${dirname(stackConfigPath)}`
+      `${basename(stackConfigPath)} in directory ${dirname(stackConfigPath)}`,
+      stackConfigPath
     )
     stackConfig = stackConfigDocs[0].toJS()
     stackConfigFileExists = true