fix(core): fail at startup for request-scoped class global enhancers (#18104)

* fix(core): fail at startup for request-scoped class global enhancers

* test(core): rename the request-scoped global enhancer describes

---------

Co-authored-by: Kamil Myśliwiec <mail@kamilmysliwiec.com>

integration/scopes/e2e/existing-global-enhancers.spec.ts

@@ -146,7 +146,7 @@ const createApp = async (module: Type<unknown>) => {
   return app;
 };
 
-describe('Global enhancers registered with useExisting or useFactory', () => {
+describe('Global enhancers that depend on request-scoped providers', () => {
   describe('http', () => {
     let app: INestApplication;
 
@@ -284,6 +284,33 @@ describe('Global enhancers registered with useExisting or useFactory', () => {
       );
     });
 
+    it('should fail at startup for a class', async () => {
+      await expect(
+        compile(
+          createModule([
+            { provide: APP_GUARD, useClass: ImplicitlyScopedDenyGuard },
+          ]),
+        ),
+      ).rejects.toThrow(
+        'The APP_GUARD provider (useClass: ImplicitlyScopedDenyGuard) depends on a request-scoped provider, so it would run with its dependencies undefined. Mark the class with @Injectable({ scope: Scope.REQUEST }), or set "scope: Scope.REQUEST" on the provider.',
+      );
+    });
+
+    it('should deny the request when the class provider sets the scope', async () => {
+      const app = await createApp(
+        createModule([
+          {
+            provide: APP_GUARD,
+            useClass: ImplicitlyScopedDenyGuard,
+            scope: Scope.REQUEST,
+          },
+        ]),
+      );
+
+      await request(app.getHttpServer()).get('/first').expect(403);
+      await app.close();
+    });
+
     it('should fail at startup for a factory', async () => {
       await expect(
         compile(

packages/core/errors/exceptions/request-scoped-global-enhancer.exception.ts

@@ -2,9 +2,13 @@ import { REQUEST_SCOPED_GLOBAL_ENHANCER_MESSAGE } from '../messages.js';
 import { RuntimeException } from './runtime.exception.js';
 
 export class RequestScopedGlobalEnhancerException extends RuntimeException {
-  constructor(enhancerToken: string, provider: string, isAlias: boolean) {
+  constructor(
+    enhancerToken: string,
+    provider: string,
+    kind: 'useClass' | 'useExisting' | 'useFactory',
+  ) {
     super(
-      REQUEST_SCOPED_GLOBAL_ENHANCER_MESSAGE(enhancerToken, provider, isAlias),
+      REQUEST_SCOPED_GLOBAL_ENHANCER_MESSAGE(enhancerToken, provider, kind),
     );
   }
 }

packages/core/errors/messages.ts

@@ -263,12 +263,14 @@ export const INVALID_CLASS_SCOPE_MESSAGE = (
 export const REQUEST_SCOPED_GLOBAL_ENHANCER_MESSAGE = (
   enhancerToken: string,
   provider: string,
-  isAlias: boolean,
+  kind: 'useClass' | 'useExisting' | 'useFactory',
 ) =>
-  `The ${enhancerToken} provider (${provider}) depends on a request-scoped provider, so it would never run. ${
-    isAlias
-      ? `Point "useExisting" at a class marked with @Injectable({ scope: Scope.REQUEST }), or use a factory provider with "scope: Scope.REQUEST".`
-      : `Set "scope: Scope.REQUEST" on the factory provider.`
+  `The ${enhancerToken} provider (${provider}) depends on a request-scoped provider, ${
+    {
+      useClass: `so it would run with its dependencies undefined. Mark the class with @Injectable({ scope: Scope.REQUEST }), or set "scope: Scope.REQUEST" on the provider.`,
+      useExisting: `so it would never run. Point "useExisting" at a class marked with @Injectable({ scope: Scope.REQUEST }), or use a factory provider with "scope: Scope.REQUEST".`,
+      useFactory: `so it would never run. Set "scope: Scope.REQUEST" on the factory provider.`,
+    }[kind]
   }`;
 
 export const UNKNOWN_REQUEST_MAPPING = (metatype: Type) => {

packages/core/scanner.ts

@@ -703,18 +703,18 @@ export class DependenciesScanner {
           'providers',
         )!;
         this.graphInspector.insertAttachedEnhancer(instanceWrapper);
-        // An alias or a factory that depends on a request-scoped provider has no
-        // static instance. Registering `null` would drop the enhancer silently
-        // (a global guard would never run), so fail at startup instead. A static
-        // factory that returns nothing on purpose is still skipped.
-        if (
-          isNil(instanceWrapper.instance) &&
-          !instanceWrapper.isDependencyTreeStatic()
-        ) {
+        // An enhancer registered as static whose dependency tree is
+        // request-scoped has no usable static instance: an alias or a factory
+        // has none, and a class has only a prototype whose constructor never
+        // ran. Fail at startup instead of dropping it or running it half-built.
+        // A static factory that returns nothing on purpose is still skipped.
+        if (!instanceWrapper.isDependencyTreeStatic()) {
+          const [kind, provider] =
+            this.describeEnhancerProvider(instanceWrapper);
           throw new RequestScopedGlobalEnhancerException(
             type as string,
-            this.describeEnhancerProvider(instanceWrapper),
-            instanceWrapper.isAlias,
+            provider,
+            kind,
           );
         }
         applyProvidersMap[type as string](instanceWrapper.instance);
@@ -722,13 +722,18 @@ export class DependenciesScanner {
     );
   }
 
-  private describeEnhancerProvider(wrapper: InstanceWrapper): string {
-    if (!wrapper.isAlias) {
-      return 'useFactory';
+  private describeEnhancerProvider(
+    wrapper: InstanceWrapper,
+  ): ['useClass' | 'useExisting' | 'useFactory', string] {
+    if (wrapper.isAlias) {
+      const [target] = wrapper.inject ?? [];
+      const name = isFunction(target) ? target.name : String(target);
+      return ['useExisting', `useExisting: ${name}`];
     }
-    const [target] = wrapper.inject ?? [];
-    const name = isFunction(target) ? target.name : String(target);
-    return `useExisting: ${name}`;
+    if (isNil(wrapper.inject)) {
+      return ['useClass', `useClass: ${wrapper.metatype!.name}`];
+    }
+    return ['useFactory', 'useFactory'];
   }
 
   public getApplyProvidersMap(): { [type: string]: Function } {

packages/core/test/scanner.spec.ts

@@ -577,6 +577,7 @@ describe('DependenciesScanner', () => {
       const expectedInstance = {};
       const instanceWrapper = {
         instance: expectedInstance,
+        isDependencyTreeStatic: () => true,
       } as unknown as InstanceWrapper;
       vi.spyOn(container, 'getModules').mockImplementation(
         () =>
@@ -602,7 +603,7 @@ describe('DependenciesScanner', () => {
       expect(applySpy).toHaveBeenCalledWith(expectedInstance);
       expect(insertAttachedEnhancerStub).toHaveBeenCalledWith(instanceWrapper);
     });
-    describe('when a provider has no instance because it depends on a request-scoped provider', () => {
+    describe('when a provider depends on a request-scoped provider', () => {
       class AuthGuard {}
 
       const mockWrapper = (wrapper: Partial<InstanceWrapper>) => {
@@ -638,7 +639,7 @@ describe('DependenciesScanner', () => {
           new RequestScopedGlobalEnhancerException(
             APP_GUARD,
             'useExisting: AuthGuard',
-            true,
+            'useExisting',
           ),
         );
       });
@@ -654,7 +655,23 @@ describe('DependenciesScanner', () => {
           new RequestScopedGlobalEnhancerException(
             APP_GUARD,
             'useFactory',
-            false,
+            'useFactory',
+          ),
+        );
+      });
+      it('should throw for a class whose dependency is request-scoped', () => {
+        mockWrapper({
+          instance: Object.create(AuthGuard.prototype),
+          isAlias: false,
+          metatype: AuthGuard,
+          isDependencyTreeStatic: () => false,
+        });
+
+        expect(() => scanner.applyApplicationProviders()).toThrow(
+          new RequestScopedGlobalEnhancerException(
+            APP_GUARD,
+            'useClass: AuthGuard',
+            'useClass',
           ),
         );
       });