diff --git a/packages/backend-app-api/src/wiring/ServiceRegistry.test.ts b/packages/backend-app-api/src/wiring/ServiceRegistry.test.ts index d2f36339fa..25fcef3885 100644 --- a/packages/backend-app-api/src/wiring/ServiceRegistry.test.ts +++ b/packages/backend-app-api/src/wiring/ServiceRegistry.test.ts @@ -425,6 +425,66 @@ describe('ServiceRegistry', () => { ); }); + it('should throw if there are circular dependencies', async () => { + const refA = createServiceRef({ id: 'a' }); + const refB = createServiceRef({ id: 'b' }); + const refC = createServiceRef({ id: 'c' }); + + const factoryA = createServiceFactory({ + service: refA, + deps: { b: refB, c: refC }, + factory: async ({ b, c }) => b + c, + }); + + const factoryB = createServiceFactory({ + service: refB, + deps: {}, + factory: async () => 'b', + }); + + const factoryC = createServiceFactory({ + service: refC, + deps: { a: refA }, + factory: async ({ a }) => a, + }); + + const registry = new ServiceRegistry([factoryA(), factoryB(), factoryC()]); + + await expect(registry.get(refC, 'catalog')).rejects.toThrow( + "Failed to instantiate service 'c' for 'catalog' because of the following circular dependency: 'a' -> 'c' -> 'a'", + ); + }); + + it('should not infinitely loop if there are circular dependencies where not all nodes are in the cycle', async () => { + const refA = createServiceRef({ id: 'a' }); + const refB = createServiceRef({ id: 'b' }); + const refC = createServiceRef({ id: 'c' }); + + const factoryA = createServiceFactory({ + service: refA, + deps: { b: refB }, + factory: async ({ b }) => b, + }); + + const factoryB = createServiceFactory({ + service: refB, + deps: { c: refC }, + factory: async ({ c }) => c, + }); + + const factoryC = createServiceFactory({ + service: refC, + deps: { b: refB }, + factory: async ({ b }) => b, + }); + + const registry = new ServiceRegistry([factoryA(), factoryB(), factoryC()]); + + await expect(registry.get(refA, 'catalog')).rejects.toThrow( + "Failed to instantiate service 'a' for 'catalog' because of the following circular dependency: 'b' -> 'c' -> 'b'", + ); + }); + it('should decorate error messages thrown by the top-level factory function', async () => { const myFactory = createServiceFactory({ service: ref1, diff --git a/packages/backend-app-api/src/wiring/ServiceRegistry.ts b/packages/backend-app-api/src/wiring/ServiceRegistry.ts index d9d7120dbd..cdc6f0b711 100644 --- a/packages/backend-app-api/src/wiring/ServiceRegistry.ts +++ b/packages/backend-app-api/src/wiring/ServiceRegistry.ts @@ -25,6 +25,7 @@ import { EnumerableServiceHolder } from './types'; // Direct internal import to avoid duplication // eslint-disable-next-line @backstage/no-forbidden-package-imports import { InternalServiceFactory } from '@backstage/backend-plugin-api/src/services/system/types'; +import { DependencyGraph } from '../lib/DependencyGraph'; /** * Keep in sync with `@backstage/backend-plugin-api/src/services/system/types.ts` * @internal @@ -147,36 +148,21 @@ export class ServiceRegistry implements EnumerableServiceHolder { } #checkForCircularDeps(factory: InternalServiceFactory, pluginId: string) { - const head = factory; + const tree = DependencyGraph.fromIterable( + Array.from(this.#providedFactories).map( + ([serviceId, serviceFactory]) => ({ + value: { serviceId, serviceFactory }, + provides: [serviceId], + consumes: serviceFactory ? Object.keys(serviceFactory.deps) : [], + }), + ), + ); + const circular = tree.detectCircularDependency(); - const nodes = Object.values(factory.deps); - - const depChain: Array> = [ - head.service, - ]; - - let node = nodes.shift(); - - if (node) { - depChain.push(node); - } - - while (!!node && node.id !== head.service.id) { - const nodeFactory = this.#providedFactories.get(node.id); - const nodeDeps = nodeFactory?.deps; - if (nodeDeps) { - nodes.unshift(...Object.values(nodeDeps)); - } - node = nodes.shift(); - if (node) { - depChain.push(node); - } - } - - const isCircular = node?.id === head.service.id; - - if (isCircular) { - const circularDepChain = depChain.map(r => `'${r.id}'`).join(' -> '); + if (circular) { + const circularDepChain = circular + .map(({ serviceId }) => `'${serviceId}'`) + .join(' -> '); throw new Error( `Failed to instantiate service '${factory.service.id}' for '${pluginId}' because of the following circular dependency: ${circularDepChain}`, );