From 2af9b0bbfd7194cfbe5f44ca81e758163f31ee18 Mon Sep 17 00:00:00 2001 From: Jon Edvald Date: Fri, 17 Apr 2020 19:35:58 +0200 Subject: [PATCH] fix(template): fix imprecise error when key on nested context is missing Fixes #1611 --- garden-service/src/config/config-context.ts | 12 ++-- garden-service/test/unit/src/actions.ts | 4 +- .../test/unit/src/config/config-context.ts | 63 ++++++++++++++++++- .../test/unit/src/template-string.ts | 44 +++++++++++++ 4 files changed, 116 insertions(+), 7 deletions(-) diff --git a/garden-service/src/config/config-context.ts b/garden-service/src/config/config-context.ts index e0103c99c5..e6d525c7a9 100644 --- a/garden-service/src/config/config-context.ts +++ b/garden-service/src/config/config-context.ts @@ -100,6 +100,7 @@ export abstract class ConfigContext { let nextKey = key[0] let lookupPath: string[] = [] let nestedNodePath = nodePath + let message: string | undefined = undefined for (let p = 0; p < key.length; p++) { nextKey = key[p] @@ -137,6 +138,7 @@ export abstract class ConfigContext { opts.stack.push(stackEntry) const res = value.resolve({ key: remainder, nodePath: nestedNodePath, opts }) value = res.resolved + message = res.message partial = !!res.partial } break @@ -154,11 +156,13 @@ export abstract class ConfigContext { } if (value === undefined) { - let message = chalk.red(`Could not find key ${chalk.white(nextKey)}`) - if (nestedNodePath.length > 1) { - message += chalk.red(" under ") + chalk.white(nestedNodePath.slice(0, -1).join(".")) + if (message === undefined) { + message = chalk.red(`Could not find key ${chalk.white(nextKey)}`) + if (nestedNodePath.length > 1) { + message += chalk.red(" under ") + chalk.white(nestedNodePath.slice(0, -1).join(".")) + } + message += chalk.red(".") } - message += chalk.red(".") if (opts.allowUndefined) { return { resolved: undefined, message } diff --git a/garden-service/test/unit/src/actions.ts b/garden-service/test/unit/src/actions.ts index 5901316cc2..1af2025945 100644 --- a/garden-service/test/unit/src/actions.ts +++ b/garden-service/test/unit/src/actions.ts @@ -1489,7 +1489,7 @@ describe("ActionRouter", () => { }), (err) => expect(stripAnsi(err.message)).to.equal( - "Invalid template string ${runtime.services.service-b.outputs.foo}: Could not find key runtime." + "Invalid template string ${runtime.services.service-b.outputs.foo}: Could not find key service-b under runtime.services." ) ) }) @@ -1664,7 +1664,7 @@ describe("ActionRouter", () => { }), (err) => expect(stripAnsi(err.message)).to.equal( - "Invalid template string ${runtime.services.service-b.outputs.foo}: Could not find key runtime." + "Invalid template string ${runtime.services.service-b.outputs.foo}: Could not find key service-b under runtime.services." ) ) }) diff --git a/garden-service/test/unit/src/config/config-context.ts b/garden-service/test/unit/src/config/config-context.ts index 6224d1abf4..fe3d9981dc 100644 --- a/garden-service/test/unit/src/config/config-context.ts +++ b/garden-service/test/unit/src/config/config-context.ts @@ -129,7 +129,55 @@ describe("ConfigContext", () => { await expectError(() => resolveKey(c, ["nested", "bla"]), "configuration") }) - it("should show full template string in error when unable to resolve in nested context", async () => { + it("should show helpful error when unable to resolve nested key in map", async () => { + class Context extends ConfigContext { + nested: Map + + constructor(parent?: ConfigContext) { + super(parent) + this.nested = new Map() + } + } + const c = new Context() + await expectError( + () => resolveKey(c, ["nested", "bla"]), + (err) => expect(stripAnsi(err.message)).to.equal("Could not find key bla under nested.") + ) + }) + + it("should show helpful error when unable to resolve nested key in object", async () => { + class Context extends ConfigContext { + nested: any + + constructor(parent?: ConfigContext) { + super(parent) + this.nested = {} + } + } + const c = new Context() + await expectError( + () => resolveKey(c, ["nested", "bla"]), + (err) => expect(stripAnsi(err.message)).to.equal("Could not find key bla under nested.") + ) + }) + + it("should show helpful error when unable to resolve two-level nested key in object", async () => { + class Context extends ConfigContext { + nested: any + + constructor(parent?: ConfigContext) { + super(parent) + this.nested = { deeper: {} } + } + } + const c = new Context() + await expectError( + () => resolveKey(c, ["nested", "deeper", "bla"]), + (err) => expect(stripAnsi(err.message)).to.equal("Could not find key bla under nested.deeper.") + ) + }) + + it("should show helpful error when unable to resolve in nested context", async () => { class Nested extends ConfigContext {} class Context extends ConfigContext { nested: ConfigContext @@ -259,6 +307,19 @@ describe("ProjectConfigContext", () => { delete process.env.TEST_VARIABLE }) + it("should throw helpful error when resolving missing env variable", async () => { + const c = new ProjectConfigContext("/tmp", "some-user") + const key = "fiaogsyecgbsjyawecygaewbxrbxajyrgew" + + await expectError( + () => c.resolve({ key: ["local", "env", key], nodePath: [], opts: {} }), + (err) => + expect(stripAnsi(err.message)).to.equal( + "Could not find key fiaogsyecgbsjyawecygaewbxrbxajyrgew under local.env." + ) + ) + }) + it("should should resolve the local platform", async () => { const c = new ProjectConfigContext("/tmp", "some-user") expect(await c.resolve({ key: ["local", "platform"], nodePath: [], opts: {} })).to.eql({ diff --git a/garden-service/test/unit/src/template-string.ts b/garden-service/test/unit/src/template-string.ts index 131cca705e..8f7fce9d05 100644 --- a/garden-service/test/unit/src/template-string.ts +++ b/garden-service/test/unit/src/template-string.ts @@ -480,6 +480,50 @@ describe("resolveTemplateString", async () => { expect(res).to.equal(true) }) + it("should correctly propagate errors from nested contexts", async () => { + await expectError( + () => resolveTemplateString("${nested.missing}", new TestContext({ nested: new TestContext({}) })), + (err) => + expect(stripAnsi(err.message)).to.equal( + "Invalid template string ${nested.missing}: Could not find key missing under nested." + ) + ) + }) + + it("should correctly propagate errors from nested objects", async () => { + await expectError( + () => resolveTemplateString("${nested.missing}", new TestContext({ nested: {} })), + (err) => + expect(stripAnsi(err.message)).to.equal( + "Invalid template string ${nested.missing}: Could not find key missing under nested." + ) + ) + }) + + it("should correctly propagate errors when resolving key on object in nested context", async () => { + const c = new TestContext({ nested: new TestContext({ deeper: {} }) }) + + await expectError( + () => resolveTemplateString("${nested.deeper.missing}", c), + (err) => + expect(stripAnsi(err.message)).to.equal( + "Invalid template string ${nested.deeper.missing}: Could not find key missing under nested.deeper." + ) + ) + }) + + it("should correctly propagate errors from deeply nested contexts", async () => { + const c = new TestContext({ nested: new TestContext({ deeper: new TestContext({}) }) }) + + await expectError( + () => resolveTemplateString("${nested.deeper.missing}", c), + (err) => + expect(stripAnsi(err.message)).to.equal( + "Invalid template string ${nested.deeper.missing}: Could not find key missing under nested.deeper." + ) + ) + }) + context("when the template string is the full input string", () => { it("should return a resolved number directly", async () => { const res = resolveTemplateString("${a}", new TestContext({ a: 100 }))