diff --git a/docs/docs/03_features-and-use-cases/04_usage-outside-of-web-request.md b/docs/docs/03_features-and-use-cases/04_usage-outside-of-web-request.md index 5d339bb4..e361a93c 100644 --- a/docs/docs/03_features-and-use-cases/04_usage-outside-of-web-request.md +++ b/docs/docs/03_features-and-use-cases/04_usage-outside-of-web-request.md @@ -43,3 +43,9 @@ export class CronController { Special care must be taken in case you're using [Proxy Providers](../03_features-and-use-cases/06_proxy-providers.md#outside-web-request). ::: + +:::warning + +If you are using Proxy Providers in a background worker (e.g. BullMQ), make sure the worker does not start consuming jobs before `app.init()` / `app.listen()` resolves, as the Proxy Provider resolver may not yet be initialized. See the [relevant caveat](./06_proxy-providers.md#proxy-providers-require-full-application-bootstrap) in the Proxy Providers documentation. + +::: diff --git a/docs/docs/03_features-and-use-cases/06_proxy-providers.md b/docs/docs/03_features-and-use-cases/06_proxy-providers.md index 1a6ffeb4..1b44423e 100644 --- a/docs/docs/03_features-and-use-cases/06_proxy-providers.md +++ b/docs/docs/03_features-and-use-cases/06_proxy-providers.md @@ -276,7 +276,7 @@ await this.cls.proxy.resolve(); since `v4.4.0` -By default, accessing an unresolved Proxy Provider behaves as if it was an _empty object_. In order to prevent silent failures, you can set the `strict` option to `true` in the proxy provider registration. In this case, any attempt to access a property or a method on an unresolved Proxy Provider will throw an error. +By default, accessing an unresolved Proxy Provider behaves as if it was an _empty object_. In order to prevent silent failures, you can set the `strict` option to `true` in the proxy provider registration. In this case, any attempt to access a property or a method on an unresolved Proxy Provider will throw a `ProxyProviderNotResolvedException`. For Class Proxy Providers, you can use the according option on the `@InjectableProxy()` decorator. @@ -310,9 +310,72 @@ ClsModule.forFeatureAsync({ ## Caveats +### Proxy Providers require full application bootstrap + +The Proxy Provider resolver is initialized in `ClsRootModule.onModuleInit()`. Any code that runs before `app.init()` (or `app.listen()`) resolves may encounter an uninitialized resolver and will not be able to use Proxy Providers. + +It goes without saying that any access to any property on a Proxy provider **in the constructor** will always evaluate to `undefined` (or throw in strict mode). + +:::warning + +This is a common pitfall with background job processors such as **BullMQ workers**, which start consuming messages in `onModuleInit`. If the worker's `onModuleInit` runs before `ClsRootModule.onModuleInit()` (which depends on module initialization order), Proxy Providers will not yet be available. + +To avoid this, ensure the application is fully bootstrapped before your worker begins consuming. Either start consuming in `onApplicationBootstrap` (which is guaranteed to run after all `onModuleInit` hooks) or delay consumption until after `app.listen()` / `app.init()` resolves: + +```ts +@Injectable() +export class WorkerService implements OnApplicationBootstrap { + constructor(private readonly worker: Worker) {} + + // Use onApplicationBootstrap instead of onModuleInit + // to ensure Proxy Providers are available. + onApplicationBootstrap() { + this.worker.run(); + } +} +``` + +::: + +### Do not mix REQUEST-scoped providers with Proxy Providers + +:::danger + +Never inject a real NestJS `Scope.REQUEST` (or `durable: true`) provider as a dependency of a Proxy Provider or any `ClsModule` plugin. + +::: + +Proxy Providers are **singletons** from NestJS's DI perspective. When NestJS detects that a singleton depends on a REQUEST-scoped provider, it changes the scope of the singleton to REQUEST as well. This means the Proxy wrapper itself is re-created on every request, which defeats the purpose of Proxy Providers and can cause **cross-request contamination** (e.g. tenant connections leaking between requests). + +```ts +// ❌ Wrong: TENANT_CONNECTION depends on a real Scope.REQUEST provider +{ + provide: TENANT_CONNECTION, + scope: Scope.REQUEST, + durable: true, + inject: [REQUEST, TenantRegistry], + useFactory: (req: Request, registry: TenantRegistry) => + registry.getConnection(req.headers['tenant-id']), +} +``` + +Convert it to a `ClsModule.forFeatureAsync` Proxy Provider using `CLS_REQ` or `ClsService` instead: + +```ts +// ✅ Correct: factory is a singleton; request data comes from CLS context +ClsModule.forFeatureAsync({ + provide: TENANT_CONNECTION, + inject: [CLS_REQ, TenantRegistry], + useFactory: (req: Request, registry: TenantRegistry) => + registry.getConnection(req.headers['tenant-id']), +}); +``` + +`CLS_REQ` is itself a Proxy Provider (a singleton that delegates to the per-request value stored in CLS), so the factory above remains a singleton from NestJS's point of view while still resolving the correct request on each access. + ### No primitive values -Proxy Factory providers _cannot_ return a _primitive value_. This is because the provider itself is the Proxy and it only delegates access once a property or a method is called on it (or if it itself is called in case the factory returns a function). +Proxy Factory providers _cannot_ return a _primitive value_ (`string`, `number`, `boolean`, `null`, or `undefined`). Doing so throws a `ProxyProviderInvalidReturnTypeException` at resolution time. This is because the provider itself is the Proxy and it only delegates access once a property or a method is called on it (or if it itself is called in case the factory returns a function). ### `function` Proxies must be explicitly enabled diff --git a/packages/core/src/lib/plugin/cls-plugin-base.ts b/packages/core/src/lib/plugin/cls-plugin-base.ts index 17335c28..9e896a0e 100644 --- a/packages/core/src/lib/plugin/cls-plugin-base.ts +++ b/packages/core/src/lib/plugin/cls-plugin-base.ts @@ -1,4 +1,8 @@ -import { InjectionToken, Provider } from '@nestjs/common'; +import { + InjectionToken, + OptionalFactoryDependency, + Provider, +} from '@nestjs/common'; import { ClsPlugin, ClsPluginHooks } from './cls-plugin.interface'; /** @@ -41,7 +45,7 @@ export abstract class ClsPluginBase implements ClsPlugin { * ``` */ protected registerHooks(opts: { - inject?: InjectionToken[]; + inject?: Array; useFactory: (...args: any[]) => ClsPluginHooks; }) { this.providers.push({ diff --git a/packages/core/src/lib/proxy-provider/proxy-provider-manager.spec.ts b/packages/core/src/lib/proxy-provider/proxy-provider-manager.spec.ts index 29b565a8..2c02a196 100644 --- a/packages/core/src/lib/proxy-provider/proxy-provider-manager.spec.ts +++ b/packages/core/src/lib/proxy-provider/proxy-provider-manager.spec.ts @@ -1,4 +1,5 @@ import { globalClsService } from '../cls-service.globals'; +import { ProxyProviderInvalidReturnTypeException } from './proxy-provider.exceptions'; import { ProxyProviderManager } from './proxy-provider-manager'; describe('ProxyProviderManager', () => { @@ -24,6 +25,100 @@ describe('ProxyProviderManager', () => { ); }); + describe('factory return type validation', () => { + it.each([ + ['undefined', undefined], + ['null', null], + ['a string', 'hello'], + ['a number', 42], + ['a boolean', true], + ])( + 'throws ProxyProviderInvalidReturnTypeException when factory returns %s', + async (_, returnValue) => { + await globalClsService.run(async () => { + const providerToken = Symbol('example-provider'); + const { useFactory } = + ProxyProviderManager.createProxyProvider({ + provide: providerToken, + useFactory: () => returnValue as any, + }); + + useFactory(); + + ProxyProviderManager.init(); + await expect( + ProxyProviderManager.resolveProxyProviders(), + ).rejects.toThrow( + ProxyProviderInvalidReturnTypeException, + ); + }); + }, + ); + + it('does not throw when factory returns an object', async () => { + await globalClsService.run(async () => { + const providerToken = Symbol('example-provider'); + const { useFactory } = + ProxyProviderManager.createProxyProvider({ + provide: providerToken, + useFactory: () => ({ key: 'value' }), + }); + + useFactory(); + + ProxyProviderManager.init(); + await expect( + ProxyProviderManager.resolveProxyProviders(), + ).resolves.not.toThrow(); + }); + }); + + it('does not throw when factory returns a function', async () => { + await globalClsService.run(async () => { + const providerToken = Symbol('example-provider'); + const { useFactory } = + ProxyProviderManager.createProxyProvider({ + provide: providerToken, + useFactory: () => () => 'result', + type: 'function', + }); + + useFactory(); + + ProxyProviderManager.init(); + await expect( + ProxyProviderManager.resolveProxyProviders(), + ).resolves.not.toThrow(); + }); + }); + }); + + describe('resolution tracking', () => { + it('does not re-resolve an already-resolved provider in the same CLS context', async () => { + await globalClsService.run(async () => { + let callCount = 0; + const providerToken = Symbol('example-provider'); + const { useFactory } = + ProxyProviderManager.createProxyProvider({ + provide: providerToken, + useFactory: () => { + callCount++; + return { value: callCount }; + }, + }); + + useFactory(); + + ProxyProviderManager.init(); + await ProxyProviderManager.resolveProxyProviders(); + await ProxyProviderManager.resolveProxyProviders(); + + // Factory should have only been called once + expect(callCount).toBe(1); + }); + }); + }); + describe('the provider factory', () => { it('allows access to the underlying provider properties', async () => { await globalClsService.run(async () => { diff --git a/packages/core/src/lib/proxy-provider/proxy-provider-resolver.ts b/packages/core/src/lib/proxy-provider/proxy-provider-resolver.ts index 8266258c..0a2705ec 100644 --- a/packages/core/src/lib/proxy-provider/proxy-provider-resolver.ts +++ b/packages/core/src/lib/proxy-provider/proxy-provider-resolver.ts @@ -1,3 +1,4 @@ +import { InjectionToken, OptionalFactoryDependency } from '@nestjs/common'; import { UnknownDependenciesException } from '@nestjs/core/errors/exceptions/unknown-dependencies.exception'; import { ClsService } from '../cls.service'; import { isProxyClassProvider } from './proxy-provider.functions'; @@ -7,6 +8,7 @@ import { ProxyProviderDefinition, } from './proxy-provider.interfaces'; import { + ProxyProviderInvalidReturnTypeException, ProxyProviderNotRegisteredException, ProxyProvidersResolutionTimeoutException, UnknownProxyDependenciesException, @@ -25,6 +27,13 @@ type ProxyProviderPromisesMap = Map>; const CLS_PROXY_PROVIDER_PROMISES_MAP = Symbol('CLS_PROVIDER_PROMISES_MAP'); +/** + * A Set stored in the CLS context that tracks which Proxy Providers have been + * resolved. Tracking resolution separately from the stored value allows + * distinguishing "not resolved yet" from "resolved to null or undefined". + */ +const CLS_PROXY_PROVIDER_RESOLVED_SET = Symbol('CLS_PROVIDER_RESOLVED_SET'); + export class ProxyProvidersResolver { private readonly proxyProviderDependenciesMap = new Map(); @@ -38,6 +47,7 @@ export class ProxyProvidersResolver { this.proxyProviderDependenciesMap.set( provider.symbol, provider.dependencies + .map(extractInjectionToken) .map(getProxyProviderSymbol) .filter((symbol) => !defaultProxyProviderTokens.has(symbol)) .filter((symbol) => proxyProviderMap.has(symbol)), @@ -118,14 +128,34 @@ export class ProxyProvidersResolver { return resolutionPromisesMap; } + /** + * ResolvedSet is a Set scoped to the current CLS context that tracks + * which Proxy Providers have been successfully resolved. Tracking this + * separately from the stored value allows distinguishing "not resolved yet" + * from "resolved to null or undefined" (which would be an error, but we + * use this set so the check is unambiguous regardless of the stored value). + */ + private getOrCreateCurrentResolvedSet(): Set { + const resolvedSet = + this.cls.get>(CLS_PROXY_PROVIDER_RESOLVED_SET) ?? + new Set(); + this.cls.setIfUndefined(CLS_PROXY_PROVIDER_RESOLVED_SET, resolvedSet); + return resolvedSet; + } + /** * Gets a set of all Proxy Provider symbols that need to be resolved * and symbols of their dependencies (that have not been resolved yet) */ private getAllNeededProviderSymbols(providerSymbols: symbol[]) { + const resolvedSet = this.getOrCreateCurrentResolvedSet(); return new Set( providerSymbols - .filter((providerSymbol) => !this.cls.get(providerSymbol)) + .filter( + (providerSymbol) => + !resolvedSet.has(providerSymbol) && + !this.cls.has(providerSymbol), + ) .map((providerSymbol) => { const deps = this.proxyProviderDependenciesMap.get(providerSymbol) ?? @@ -160,7 +190,22 @@ export class ProxyProvidersResolver { await Promise.all(ownDependencyPromises); const providerInstance = await this.resolveProxyProviderInstance(providerDefinition); + + const instanceType = typeof providerInstance; + if ( + providerInstance === null || + providerInstance === undefined || + (instanceType !== 'object' && instanceType !== 'function') + ) { + throw ProxyProviderInvalidReturnTypeException.create( + providerSymbol, + providerInstance, + ); + } + this.cls.set(providerSymbol, providerInstance); + const resolvedSet = this.getOrCreateCurrentResolvedSet(); + resolvedSet.add(providerSymbol); return ownPromise.resolve(); } catch (e) { return ownPromise.reject(e); @@ -198,3 +243,12 @@ export class ProxyProvidersResolver { return await provider.useFactory.apply(null, injected); } } + +function extractInjectionToken( + dep: InjectionToken | OptionalFactoryDependency, +): InjectionToken { + if (dep !== null && typeof dep === 'object' && 'token' in dep) { + return dep.token; + } + return dep as InjectionToken; +} diff --git a/packages/core/src/lib/proxy-provider/proxy-provider.exceptions.ts b/packages/core/src/lib/proxy-provider/proxy-provider.exceptions.ts index 4430fbab..6e1cc721 100644 --- a/packages/core/src/lib/proxy-provider/proxy-provider.exceptions.ts +++ b/packages/core/src/lib/proxy-provider/proxy-provider.exceptions.ts @@ -96,3 +96,15 @@ export class ProxyProvidersResolutionTimeoutException extends ProxyProviderError return new this(message); } } + +export class ProxyProviderInvalidReturnTypeException extends ProxyProviderError { + static create(providerSymbol: symbol, value: unknown) { + const providerName = providerSymbol.description ?? 'unknown'; + const type = value === null ? 'null' : typeof value; + const message = + `The factory for Proxy provider "${providerName}" returned a value of type "${type}", ` + + `but Proxy providers must return an object or a function. ` + + `Primitive values (string, number, boolean, etc.) and null/undefined are not supported.`; + return new this(message); + } +} diff --git a/packages/core/src/lib/proxy-provider/proxy-provider.interfaces.ts b/packages/core/src/lib/proxy-provider/proxy-provider.interfaces.ts index 88b09086..ddccd024 100644 --- a/packages/core/src/lib/proxy-provider/proxy-provider.interfaces.ts +++ b/packages/core/src/lib/proxy-provider/proxy-provider.interfaces.ts @@ -1,4 +1,9 @@ -import { InjectionToken, Provider, Type } from '@nestjs/common'; +import { + InjectionToken, + OptionalFactoryDependency, + Provider, + Type, +} from '@nestjs/common'; import { ModuleRef } from '@nestjs/core'; interface ClsModuleProxyProviderCommonOptions { @@ -54,7 +59,7 @@ export interface ClsModuleProxyFactoryProviderOptions extends ClsModuleProxyProv /** * An array of injection tokens for providers used in the `useFactory`. */ - inject?: InjectionToken[]; + inject?: Array; /** * Factory function that accepts an array of providers in the order of the according tokens in the `inject` array. @@ -98,7 +103,7 @@ export interface ProxyFactoryProviderDefinition { provide: InjectionToken; symbol: symbol; injected: any[]; - dependencies: InjectionToken[]; + dependencies: Array; useFactory: (...args: any[]) => any | Promise; } diff --git a/packages/core/test/proxy-providers/for-feature.spec.ts b/packages/core/test/proxy-providers/for-feature.spec.ts index 5a00c031..ef3a3850 100644 --- a/packages/core/test/proxy-providers/for-feature.spec.ts +++ b/packages/core/test/proxy-providers/for-feature.spec.ts @@ -4,6 +4,8 @@ import { Injectable, Module, ModuleMetadata, + Optional, + OptionalFactoryDependency, } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { @@ -119,6 +121,50 @@ describe('ClsModule', () => { ); }); }); + + it('does not throw when class proxy has @Optional() dep that is missing', async () => { + @Injectable() + class SomeClass { + value = 42; + } + + @InjectableProxy() + class ProxyClass { + constructor(@Optional() public some?: SomeClass) {} + } + + app = await createAndInitTestingApp([ + ClsModule.forFeature(ProxyClass), + ]); + await cls.run(async () => { + await expect(cls.proxy.resolve()).resolves.not.toThrow(); + expect(app.get(ProxyClass).some).toBeUndefined(); + }); + }); + + it('injects the provider when @Optional() dep exists', async () => { + @Injectable() + class SomeClass { + value = 42; + } + + @InjectableProxy() + class ProxyClass { + constructor(@Optional() public some?: SomeClass) {} + } + + app = await createAndInitTestingApp([ + ClsModule.forFeatureAsync({ + extraProviders: [SomeClass], + useClass: ProxyClass, + }), + ]); + await cls.run(async () => { + await expect(cls.proxy.resolve()).resolves.not.toThrow(); + expect(app.get(ProxyClass).some).toBeInstanceOf(SomeClass); + expect(app.get(ProxyClass).some?.value).toBe(42); + }); + }); }); describe('forFeatureAsync', () => { @@ -274,5 +320,92 @@ describe('ClsModule', () => { expect(app.get(TOKEN)()).toBeInstanceOf(SomeClass); }); }); + + describe('OptionalFactoryDependency', () => { + it('injects undefined when optional dep is missing', async () => { + const MISSING_TOKEN = 'MISSING'; + const TOKEN = 'PROXY'; + const optDep: OptionalFactoryDependency = { + token: MISSING_TOKEN, + optional: true, + }; + app = await createAndInitTestingApp([ + ClsModule.forFeatureAsync({ + provide: TOKEN, + inject: [optDep], + useFactory: (dep: unknown) => ({ dep }), + }), + ]); + await cls.run(async () => { + await expect(cls.proxy.resolve()).resolves.not.toThrow(); + expect(app.get(TOKEN).dep).toBeUndefined(); + }); + }); + + it('injects the provider when optional dep exists', async () => { + @Injectable() + class SomeClass { + value = 42; + } + + const TOKEN = 'PROXY'; + const optDep: OptionalFactoryDependency = { + token: SomeClass, + optional: true, + }; + app = await createAndInitTestingApp([ + ClsModule.forFeatureAsync({ + provide: TOKEN, + extraProviders: [SomeClass], + inject: [optDep], + useFactory: (dep: SomeClass) => ({ dep }), + }), + ]); + await cls.run(async () => { + await expect(cls.proxy.resolve()).resolves.not.toThrow(); + expect(app.get(TOKEN).dep).toBeInstanceOf(SomeClass); + expect(app.get(TOKEN).dep.value).toBe(42); + }); + }); + + it('resolves proxy providers in correct order when optional dep is another proxy provider', async () => { + const PROXY_A = 'PROXY_A_OPT'; + const PROXY_B = 'PROXY_B_OPT'; + let resolveOrder: string[] = []; + + const optDep: OptionalFactoryDependency = { + token: PROXY_A, + optional: true, + }; + app = await createAndInitTestingApp([ + // global: true makes PROXY_A injectable by PROXY_B's module + ClsModule.forFeatureAsync({ + provide: PROXY_A, + global: true, + // async factory to ensure ordering matters (not just luck) + useFactory: async () => { + await new Promise((r) => setImmediate(r)); + resolveOrder.push('A'); + return { name: 'A' }; + }, + }), + ClsModule.forFeatureAsync({ + provide: PROXY_B, + inject: [optDep], + useFactory: (a: { name: string }) => { + resolveOrder.push('B'); + return { name: 'B', dep: a?.name }; + }, + }), + ]); + await cls.run(async () => { + resolveOrder = []; + await cls.proxy.resolve(); + expect(resolveOrder[0]).toBe('A'); + expect(resolveOrder[1]).toBe('B'); + expect(app.get(PROXY_B).dep).toBe('A'); + }); + }); + }); }); });