diff --git a/packages/core/src/effect/layer-node.ts b/packages/core/src/effect/layer-node.ts index 9dbc3d51607b..e307618d6387 100644 --- a/packages/core/src/effect/layer-node.ts +++ b/packages/core/src/effect/layer-node.ts @@ -78,6 +78,23 @@ type MakeInput< readonly tag?: T } +/** + * Dependency arrays are built at module-evaluation time, so a circular import can leave an entry + * `undefined` instead of a node. That stays invisible until the graph is walked, where it surfaces + * as a `TypeError` reading `.name` off `undefined` with no indication of which module is at fault. + * Failing here instead names the node and the offending position. + */ +function checkDependencies(name: string, dependencies: readonly AnyNode[] | undefined) { + if (dependencies === undefined) return + const index = dependencies.findIndex((dependency) => dependency === undefined || dependency === null) + if (index === -1) return + throw new Error( + `Layer node "${name}" has an undefined dependency at index ${index}. ` + + `This usually means a circular import: the module that provides it has not finished ` + + `initializing. Break the cycle, for example by making the import type-only.`, + ) +} + export function make< const Implementation extends Layer.Any, const Items extends NodeList, @@ -85,9 +102,11 @@ export function make< >( input: MakeInput, ): Node, Layer.Error | Error, T> { + const name = input.service !== undefined ? input.service.key : input.name + checkDependencies(name, input.deps) return { kind: "layer", - name: input.service !== undefined ? input.service.key : input.name, + name, service: input.service, implementation: input.layer, dependencies: input.deps, @@ -108,6 +127,7 @@ export function unbound(service: Context.Key( dependencies: Items, ): Node, Error, NodeTag> { + checkDependencies("group", dependencies) return { kind: "group", name: "group", dependencies } } @@ -193,6 +213,9 @@ function walk( ) } + // validate before any visitor dereferences an entry + checkDependencies(target.name, target.dependencies) + visiting.add(target) stack.push(target) try { @@ -303,6 +326,8 @@ function rewriteReplacementDependencies(root: AnyNode, replacements: ReadonlyMap visiting.add(target) stack.push(target) try { + // this recursion is separate from `walk`, and hoisting a tagged subtree reaches it directly + checkDependencies(target.name, target.dependencies) const dependencies = target.dependencies.map((dependency) => recur(dependency)) const result = dependencies.every((dependency, index) => dependency === target.dependencies[index]) ? target @@ -327,7 +352,10 @@ export function hasUnbound(root: Node, source: AnyNode): } function flatten(node: AnyNode): readonly AnyNode[] { - return node.kind === "group" ? node.dependencies.flatMap(flatten) : [node] + if (node.kind !== "group") return [node] + // groups are expanded before the compile-time check runs, so validate here too + checkDependencies(node.name, node.dependencies) + return node.dependencies.flatMap(flatten) } export * as LayerNode from "./layer-node" diff --git a/packages/core/test/effect/layer-node/layer-node.test.ts b/packages/core/test/effect/layer-node/layer-node.test.ts index b671792c59af..a5b38416475a 100644 --- a/packages/core/test/effect/layer-node/layer-node.test.ts +++ b/packages/core/test/effect/layer-node/layer-node.test.ts @@ -63,6 +63,73 @@ describe("layer node", () => { expect(await Effect.runPromise(program)).toEqual(["first", "second"]) }) + test("rejects an undefined dependency", () => { + // a circular import leaves the dependency undefined at module-evaluation time + // @ts-expect-error A dependency must be a node + expect(() => make({ service: Greeting, layer: greetingLayer, deps: [undefined] })).toThrow( + 'Layer node "test/LayerNodeGreeting" has an undefined dependency at index 0', + ) + }) + + test("reports the position of an undefined dependency", () => { + // @ts-expect-error A dependency must be a node + expect(() => make({ service: Greeting, layer: greetingLayer, deps: [value, undefined] })).toThrow( + "undefined dependency at index 1", + ) + }) + + test("rejects an undefined dependency in a group", () => { + // @ts-expect-error A dependency must be a node + expect(() => LayerNode.group([value, undefined])).toThrow( + 'Layer node "group" has an undefined dependency at index 1', + ) + }) + + test("rejects an undefined dependency on a node that bypassed make", () => { + // `Node` is structural, so a node built by hand never passes through `make` + // @ts-expect-error A dependency must be a node + const handBuilt: LayerNode.Node = { ...greeting, dependencies: [undefined] } + expect(() => LayerNode.compile(handBuilt)).toThrow( + 'Layer node "test/LayerNodeGreeting" has an undefined dependency at index 0', + ) + }) + + // groups are expanded by `flatten` before a node is visited, so they need their own coverage + const handBuiltGroup = (): LayerNode.Node => ({ + kind: "group", + name: "external-group", + // @ts-expect-error A dependency must be a node + dependencies: [undefined], + }) + + test("rejects an undefined dependency in a hand-built group at the root", () => { + expect(() => LayerNode.compile(handBuiltGroup())).toThrow( + 'Layer node "external-group" has an undefined dependency at index 0', + ) + }) + + test("rejects an undefined dependency in a nested hand-built group", () => { + const parent = make({ service: Greeting, layer: greetingLayer, deps: [handBuiltGroup()] }) + expect(() => LayerNode.compile(parent)).toThrow( + 'Layer node "external-group" has an undefined dependency at index 0', + ) + }) + + test("rejects an undefined dependency while hoisting", () => { + expect(() => LayerNode.hoist(handBuiltGroup(), tags.values.app)).toThrow( + 'Layer node "external-group" has an undefined dependency at index 0', + ) + }) + + test("rejects an undefined dependency while rewriting replacements", () => { + // a hoisted tagged node is rewritten rather than visited, so it reaches a separate recursion + const parent = make({ service: Greeting, layer: greetingLayer, deps: [handBuiltGroup()] }) + const replacement = Layer.succeed(Value, Value.of({ value: "simulation" })) + expect(() => LayerNode.hoist(parent, tags.values.app, [[value, replacement]])).toThrow( + 'Layer node "external-group" has an undefined dependency at index 0', + ) + }) + test("requires unbound nodes to be replaced before compilation", async () => { const unbound = LayerNode.unbound(Value, tags.values.app) const greeting = make({ service: Greeting, layer: greetingLayer, deps: [unbound] })