diff --git a/integration/injector/e2e/circular-custom-providers.spec.ts b/integration/injector/e2e/circular-custom-providers.spec.ts new file mode 100644 index 00000000000..b05a95ea365 --- /dev/null +++ b/integration/injector/e2e/circular-custom-providers.spec.ts @@ -0,0 +1,63 @@ +import { Test } from '@nestjs/testing'; +import { expect } from 'chai'; + +import { Controller, Injectable, Module } from '@nestjs/common'; + +class B {} + +@Injectable() +class A { + constructor(b: B) {} +} + +@Injectable() +class BImpl { + constructor(a: A) {} +} + +@Controller() +class AppController { + constructor(a: A) {} +} + +@Module({ + imports: [], + controllers: [AppController], + providers: [A, { provide: B, useClass: BImpl }], +}) +export class AppModule {} + +describe('Circular custom providers', () => { + it('should throw an exception (useClass + regular provider)', async () => { + try { + const builder = Test.createTestingModule({ + imports: [AppModule], + }); + await builder.compile(); + + expect(true).to.be.eql(false); + } catch (err) { + expect(err.message).to.be.eql( + 'A circular dependency has been detected inside "A". Please, make sure that each side of a bidirectional relationships are decorated with "forwardRef()". Note that circular relationships between custom providers (e.g., factories) are not supported since functions cannot be called more than once.', + ); + } + }); + + it('should throw an exception (2 factories)', async () => { + try { + const builder = Test.createTestingModule({ + providers: [ + { provide: 'ABC', useFactory: () => ({}), inject: ['DEF'] }, + { provide: 'DEF', useFactory: () => ({}), inject: ['ABC'] }, + ], + }); + await builder.compile(); + + expect(true).to.be.eql(false); + } catch (err) { + expect(err.message).to.be.eql( + 'A circular dependency has been detected inside "ABC". Please, make sure that each side of a bidirectional relationships are decorated with "forwardRef()". Note that circular relationships between custom providers (e.g., factories) are not supported since functions cannot be called more than once.', + ); + } + }); +}); diff --git a/packages/core/errors/exceptions/circular-dependency.exception.ts b/packages/core/errors/exceptions/circular-dependency.exception.ts index 83f3e68bf3d..2ca8726ab9d 100644 --- a/packages/core/errors/exceptions/circular-dependency.exception.ts +++ b/packages/core/errors/exceptions/circular-dependency.exception.ts @@ -4,7 +4,7 @@ export class CircularDependencyException extends RuntimeException { constructor(context?: string) { const ctx = context ? ` inside ${context}` : ``; super( - `A circular dependency has been detected${ctx}. Please, make sure that each side of a bidirectional relationships are decorated with "forwardRef()".`, + `A circular dependency has been detected${ctx}. Please, make sure that each side of a bidirectional relationships are decorated with "forwardRef()". Note that circular relationships between custom providers (e.g., factories) are not supported since functions cannot be called more than once.`, ); } } diff --git a/packages/core/injector/injector.ts b/packages/core/injector/injector.ts index 07361bfd990..a731fb444f7 100644 --- a/packages/core/injector/injector.ts +++ b/packages/core/injector/injector.ts @@ -23,6 +23,7 @@ import { } from '@nestjs/common/utils/shared.utils'; import { iterate } from 'iterare'; import { performance } from 'perf_hooks'; +import { CircularDependencyException } from '../errors/exceptions'; import { RuntimeException } from '../errors/exceptions/runtime.exception'; import { UndefinedDependencyException } from '../errors/exceptions/undefined-dependency.exception'; import { UnknownDependenciesException } from '../errors/exceptions/unknown-dependencies.exception'; @@ -35,6 +36,7 @@ import { PropertyMetadata, } from './instance-wrapper'; import { Module } from './module'; +import { SettlementSignal } from './settlement-signal'; /** * The type of an injectable dependency @@ -111,14 +113,21 @@ export class Injector { this.getContextId(contextId, wrapper), inquirerId, ); + if (instanceHost.isPending) { + const settlementSignal = wrapper.settlementSignal; + if (inquirer && settlementSignal?.isCycle(inquirer.id)) { + throw new CircularDependencyException(`"${wrapper.name}"`); + } + return instanceHost.donePromise.then((err?: unknown) => { if (err) { throw err; } }); } - const done = this.applyDoneHook(instanceHost); + + const settlementSignal = this.applySettlementSignal(instanceHost, wrapper); const token = wrapper.token || wrapper.name; const { inject } = wrapper; @@ -127,7 +136,7 @@ export class Injector { throw new RuntimeException(); } if (instanceHost.isResolved) { - return done(); + return settlementSignal.complete(); } try { const t0 = this.getNowTimestamp(); @@ -149,7 +158,7 @@ export class Injector { ); this.applyProperties(instance, properties); wrapper.initTime = this.getNowTimestamp() - t0; - done(); + settlementSignal.complete(); }; await this.resolveConstructorParams( wrapper, @@ -161,7 +170,7 @@ export class Injector { inquirer, ); } catch (err) { - done(err); + settlementSignal.error(err); throw err; } } @@ -237,15 +246,16 @@ export class Injector { await this.loadEnhancersPerContext(wrapper, contextId, wrapper); } - public applyDoneHook( - wrapper: InstancePerContext, - ): (err?: unknown) => void { - let done: (err?: unknown) => void; - wrapper.donePromise = new Promise((resolve, reject) => { - done = resolve; - }); - wrapper.isPending = true; - return done; + public applySettlementSignal( + instancePerContext: InstancePerContext, + host: InstanceWrapper, + ) { + const settlementSignal = new SettlementSignal(); + instancePerContext.donePromise = settlementSignal.asPromise(); + instancePerContext.isPending = true; + host.settlementSignal = settlementSignal; + + return settlementSignal; } public async resolveConstructorParams( @@ -457,6 +467,8 @@ export class Injector { inquirerId, ); if (!instanceHost.isResolved && !instanceWrapper.forwardRef) { + inquirer?.settlementSignal?.insertRef(instanceWrapper.id); + await this.loadProvider( instanceWrapper, instanceWrapper.host ?? moduleRef, @@ -581,6 +593,7 @@ export class Injector { } this.printLookingForProviderLog(name, relatedModule); moduleRegistry.push(relatedModule.id); + const { providers, exports } = relatedModule; if (!exports.has(name) || !providers.has(name)) { const instanceRef = await this.lookupComponentInImports( @@ -609,6 +622,8 @@ export class Injector { inquirerId, ); if (!instanceHost.isResolved && !instanceWrapperRef.forwardRef) { + wrapper.settlementSignal?.insertRef(instanceWrapperRef.id); + await this.loadProvider( instanceWrapperRef, relatedModule, diff --git a/packages/core/injector/instance-wrapper.ts b/packages/core/injector/instance-wrapper.ts index d29ae711dff..5c5f729f985 100644 --- a/packages/core/injector/instance-wrapper.ts +++ b/packages/core/injector/instance-wrapper.ts @@ -17,6 +17,7 @@ import { isValueProvider, } from './helpers/provider-classifier'; import { Module } from './module'; +import { SettlementSignal } from './settlement-signal'; export const INSTANCE_METADATA_SYMBOL = Symbol.for('instance_metadata:cache'); export const INSTANCE_ID_SYMBOL = Symbol.for('instance_metadata:id'); @@ -63,13 +64,13 @@ export class InstanceWrapper { public readonly host?: Module; public readonly isAlias: boolean = false; public readonly subtype?: EnhancerSubtype; - public scope?: Scope = Scope.DEFAULT; public metatype: Type | Function; public inject?: FactoryProvider['inject']; public forwardRef?: boolean; public durable?: boolean; public initTime?: number; + public settlementSignal?: SettlementSignal; private static logger: LoggerService = new Logger(InstanceWrapper.name); @@ -104,8 +105,11 @@ export class InstanceWrapper { } get isNotMetatype(): boolean { - const isFactory = this.metatype && !isNil(this.inject); - return !this.metatype || isFactory; + return !this.metatype || this.isFactory; + } + + get isFactory(): boolean { + return this.metatype && !isNil(this.inject); } get isTransient(): boolean { diff --git a/packages/core/injector/settlement-signal.ts b/packages/core/injector/settlement-signal.ts new file mode 100644 index 00000000000..fe21b54187b --- /dev/null +++ b/packages/core/injector/settlement-signal.ts @@ -0,0 +1,56 @@ +/** + * SettlementSignal is used to signal the resolution of a provider/instance. + * Calling `complete` or `error` will resolve the promise returned by `asPromise`. + * Can be used to detect circular dependencies. + */ +export class SettlementSignal { + private readonly _refs = new Set(); + private readonly settledPromise: Promise; + private settleFn!: (err?: unknown) => void; + + constructor() { + this.settledPromise = new Promise(resolve => { + this.settleFn = resolve; + }); + } + + /** + * Resolves the promise returned by `asPromise`. + */ + public complete() { + this.settleFn(); + } + + /** + * Rejects the promise returned by `asPromise` with the given error. + * @param err Error to reject the promise returned by `asPromise` with. + */ + public error(err: unknown) { + this.settleFn(err); + } + + /** + * Returns a promise that will be resolved when `complete` or `error` is called. + * @returns Promise that will be resolved when `complete` or `error` is called. + */ + public asPromise() { + return this.settledPromise; + } + + /** + * Inserts a wrapper id that the host of this signal depends on. + * @param wrapperId Wrapper id to insert. + */ + public insertRef(wrapperId: string) { + this._refs.add(wrapperId); + } + + /** + * Check if relationship is circular. + * @param wrapperId Wrapper id to check. + * @returns True if relationship is circular, false otherwise. + */ + public isCycle(wrapperId: string) { + return this._refs.has(wrapperId); + } +}