Skip to content

Commit

Permalink
improvement(template): show available keys when key is not found
Browse files Browse the repository at this point in the history
This had been bothering me for ages. We now show the available keys
when a key is not found.
  • Loading branch information
edvald committed Jun 25, 2020
1 parent 578e555 commit e6bb2cb
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 15 deletions.
14 changes: 12 additions & 2 deletions garden-service/src/config/config-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { Garden } from "../garden"
import { joi } from "../config/common"
import { KeyedSet } from "../util/keyed-set"
import { RuntimeContext } from "../runtime-context"
import { deline, dedent } from "../util/string"
import { deline, dedent, naturalList } from "../util/string"
import { getProviderUrl, getModuleTypeUrl } from "../docs/common"
import { Module } from "../types/module"
import { ModuleConfig } from "./module"
Expand Down Expand Up @@ -96,6 +96,7 @@ export abstract class ConfigContext {
}

// keep track of which resolvers have been called, in order to detect circular references
let available: any[] | null = null
let value: any = this
let partial = false
let nextKey = key[0]
Expand All @@ -109,6 +110,7 @@ export abstract class ConfigContext {
const remainder = key.slice(p + 1)
nestedNodePath = nodePath.concat(lookupPath)
const stackEntry = nestedNodePath.join(".")
available = null

if (typeof nextKey === "string" && nextKey.startsWith("_")) {
value = undefined
Expand All @@ -119,8 +121,12 @@ export abstract class ConfigContext {
fullPath,
opts,
})
} else if (value instanceof Map) {
available = [...value.keys()]
value = value.get(nextKey)
} else {
value = value instanceof Map ? value.get(nextKey) : value[nextKey]
available = Object.keys(value).filter((k) => !k.startsWith("_"))
value = value[nextKey]
}

if (typeof value === "function") {
Expand Down Expand Up @@ -170,6 +176,10 @@ export abstract class ConfigContext {
message += chalk.red(" under ") + chalk.white(nestedNodePath.slice(0, -1).join("."))
}
message += chalk.red(".")

if (available && available.length) {
message += chalk.red(" Available keys: " + naturalList(available.sort().map((k) => chalk.white(k))) + ".")
}
}

if (opts.allowUndefined) {
Expand Down
28 changes: 21 additions & 7 deletions garden-service/test/unit/src/config/config-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,8 @@ describe("ProjectConfigContext", () => {
expectError(
() => c.resolve({ key: ["local", "env", key], nodePath: [], opts: {} }),
(err) =>
expect(stripAnsi(err.message)).to.equal(
"Could not find key fiaogsyecgbsjyawecygaewbxrbxajyrgew under local.env."
expect(stripAnsi(err.message)).to.match(
/Could not find key fiaogsyecgbsjyawecygaewbxrbxajyrgew under local.env. Available keys: /
)
)
})
Expand Down Expand Up @@ -424,7 +424,10 @@ describe("ModuleConfigContext", () => {
it("should throw when resolving a secret with a missing key", async () => {
await expectError(
() => c.resolve({ key: ["secrets", "missingSecret"], nodePath: [], opts: {} }),
(err) => expect(stripAnsi(err.message)).to.equal("Could not find key missingSecret under secrets.")
(err) =>
expect(stripAnsi(err.message)).to.equal(
"Could not find key missingSecret under secrets. Available keys: someSecret."
)
)
})
})
Expand All @@ -439,7 +442,10 @@ describe("ModuleConfigContext", () => {
it("should throw if resolving missing runtime key with allowPartial=false", async () => {
await expectError(
() => c.resolve({ key: ["runtime", "some", "key"], nodePath: [], opts: {} }),
(err) => expect(stripAnsi(err.message)).to.equal("Could not find key some under runtime.")
(err) =>
expect(stripAnsi(err.message)).to.equal(
"Could not find key some under runtime. Available keys: services and tasks."
)
)
})

Expand Down Expand Up @@ -550,7 +556,9 @@ describe("ModuleConfigContext", () => {
opts: {},
}),
(err) =>
expect(stripAnsi(err.message)).to.equal("Could not find key boo under runtime.services.service-b.outputs.")
expect(stripAnsi(err.message)).to.equal(
"Could not find key boo under runtime.services.service-b.outputs. Available keys: foo."
)
)
})
})
Expand Down Expand Up @@ -610,7 +618,10 @@ describe("WorkflowConfigContext", () => {
it("should throw when resolving a secret with a missing key", async () => {
await expectError(
() => c.resolve({ key: ["secrets", "missingSecret"], nodePath: [], opts: {} }),
(err) => expect(stripAnsi(err.message)).to.equal("Could not find key missingSecret under secrets.")
(err) =>
expect(stripAnsi(err.message)).to.equal(
"Could not find key missingSecret under secrets. Available keys: someSecret."
)
)
})
})
Expand Down Expand Up @@ -682,7 +693,10 @@ describe("WorkflowStepConfigContext", () => {
})
expectError(
() => c.resolve({ key: ["steps", "step-foo", "log"], nodePath: [], opts: {} }),
(err) => expect(stripAnsi(err.message)).to.equal("Could not find key step-foo under steps.")
(err) =>
expect(stripAnsi(err.message)).to.equal(
"Could not find key step-foo under steps. Available keys: step-1 and step-2."
)
)
})

Expand Down
2 changes: 1 addition & 1 deletion garden-service/test/unit/src/garden.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1459,7 +1459,7 @@ describe("Garden", () => {
(err) => {
expect(err.message).to.equal("Failed resolving one or more providers:\n" + "- test")
expect(stripAnsi(err.detail.messages[0])).to.equal(
"- test: Invalid template string ${bla.ble}: Could not find key bla."
"- test: Invalid template string ${bla.ble}: Could not find key bla. Available keys: environment, local, project, providers, secrets, steps, var and variables."
)
}
)
Expand Down
17 changes: 12 additions & 5 deletions garden-service/test/unit/src/template-string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,10 @@ describe("resolveTemplateString", async () => {
it("should throw when using comparison operators on missing keys", async () => {
return expectError(
() => resolveTemplateString("${a >= b}", new TestContext({ a: 123 })),
(err) => expect(stripAnsi(err.message)).to.equal("Invalid template string ${a >= b}: Could not find key b.")
(err) =>
expect(stripAnsi(err.message)).to.equal(
"Invalid template string ${a >= b}: Could not find key b. Available keys: a."
)
)
})

Expand Down Expand Up @@ -568,20 +571,24 @@ describe("resolveTemplateString", async () => {

it("should correctly propagate errors from nested contexts", async () => {
await expectError(
() => resolveTemplateString("${nested.missing}", new TestContext({ nested: new TestContext({}) })),
() =>
resolveTemplateString(
"${nested.missing}",
new TestContext({ nested: new TestContext({ foo: 123, bar: 456, baz: 789 }) })
),
(err) =>
expect(stripAnsi(err.message)).to.equal(
"Invalid template string ${nested.missing}: Could not find key missing under nested."
"Invalid template string ${nested.missing}: Could not find key missing under nested. Available keys: bar, baz and foo."
)
)
})

it("should correctly propagate errors from nested objects", async () => {
await expectError(
() => resolveTemplateString("${nested.missing}", new TestContext({ nested: {} })),
() => resolveTemplateString("${nested.missing}", new TestContext({ nested: { foo: 123, bar: 456 } })),
(err) =>
expect(stripAnsi(err.message)).to.equal(
"Invalid template string ${nested.missing}: Could not find key missing under nested."
"Invalid template string ${nested.missing}: Could not find key missing under nested. Available keys: bar and foo."
)
)
})
Expand Down

0 comments on commit e6bb2cb

Please sign in to comment.