Support Scope Injection (Scope.REQUEST) for global Interceptor, middleware, etc.

[ghost]

I'm submitting a...

[ ] Regression 
[ ] Bug report
[x ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

• Currently, Nesjts has no support for Scope Injector for example: main.ts

  
  app.useGlobalInterceptors(app.get(MultiTenancyHTTPInterceptor))

MultiTenancyHTTP.interceptor.ts

@Injectable({scope: Scope.REQUEST})
export class MultiTenancyHTTPInterceptor{
     @Inject(REQUEST)
     private context:any;

}

- There are no CONTEXT request for Microservices ## Expected behavior Support Scope Injection (Scope.REQUEST) for global Interceptor, middleware, etc. ## Environment

Nest version: 6.0.5


For Tooling issues:
- Node version: 11
- Platform:  Mac

Others:

[ulrichb]

@kamilmysliwiec : Is there a possibility to workaround the constructor injection, by using the service locator pattern within intercept()?

Something like

class MyGlobalInterceptor implements NestInterceptor {

    intercept(context: ExecutionContext, next: CallHandler<unknown>): Observable<unknown> {

        const myRequestScopeService = 
                      nestApp.<<<getForCurrentRequest>>>(MyRequestScopeService);
        // ... 
    }
}
nestApp.useGlobalInterceptors(new MyGlobalInterceptor());

[duro]

I am running into this issue. I have a logger service which is request scoped:

@Injectable({ scope: Scope.REQUEST })
export class APILoggingService extends APILogger {
  constructor(@Inject(REQUEST) request: IAPIGatewayRequest) {
    if (
      !request ||
      !request.apiGateway ||
      !request.apiGateway.event ||
      !request.apiGateway.context
    ) {
      throw new Error(
        'You are trying to use the API Gateway logger without having used the aws-serverless-express middleware',
      )
    }

    super(request.apiGateway.event, request.apiGateway.context)
  }
}

And I am trying to inject it into a global interceptor like so:

@Injectable()
export class LoggingInterceptor implements NestInterceptor {
  constructor(private readonly log: APILoggingService) {}

  public intercept(context: ExecutionContext, next: CallHandler): Observable<any> {
    const req = context.switchToHttp().getRequest<Request>()
    this.log.debug('Incoming Request', new RequestLog(req))

    const now = Date.now()
    return next.handle().pipe(tap(() => console.log(`After... ${Date.now() - now}ms`)))
  }
}

Which I have mounted in a module like so:

import { APP_INTERCEPTOR } from '@nestjs/core'
import { Module, Provider } from '@nestjs/common'
import { APILoggingService } from './logger.service'
import { LoggingInterceptor } from './logger.interceptor'

const globalRouteLogger: Provider = {
  provide: APP_INTERCEPTOR,
  useClass: LoggingInterceptor,
}

@Module({
  providers: [APILoggingService, globalRouteLogger],
  exports: [APILoggingService],
})
export class LambdaModule {}

And when ever I hit a route, I get the following error:

TypeError: Cannot read property 'debug' of undefined

[duro]

To add to this, I can confirm that if I use the @UseInterceptor decorator on a controller, everything works fine. It is only when the interceptor is mounted globally.

[yostools]

Is a solution planned?

[duro]

I'm still seeing this tagged with the question label, I think it should be either categorized as a bug or at the very least a feature request. @kamilmysliwiec

[finchen]

Yes it would be good to know if it can be implemented or not. I am trying to log errors with a request unique id. Thanks

[zinzinday]

I tried to find out this problem. But currently cannot be overcome. I had to remove scope.REQUEST and create a service to assign context values ​​from middleware instead of @Inject(REQUEST), before version 6.0 was also the way I handled when recalling the request context.

[kamilmysliwiec]

Actually, it is supported. Example:

{
  provide: APP_INTERCEPTOR,
  scope: Scope.REQUEST,
  useClass: LoggingInterceptor,
}

https://docs.nestjs.com/fundamentals/injection-scopes#usage

[jasoncarty]

@kamilmysliwiec I have the same problem, and when adding the:

{
  provide: APP_INTERCEPTOR,
  scope: Scope.REQUEST,
  useClass: LoggingInterceptor,
}

It still does not work for me.

[eropple]

I also can't verify that this works. When I either set scope: Scope.REQUEST in my provider or I implicitly cause it by passing a request-scoped dependency, it appears that my useGlobalInterceptors call is ignored--the body of my interceptor factory method is never actually evaluated at any point.

I can't spare the time right now to build a minimal example, but I'd be very interested in seeing the disproof of this. Is there an example of request-scoped interceptors working with useGlobalInterceptors?

[eropple]

Update: I found time to build that minimal example: https://github.com/eropple/nestjs-request-scope-test

Something is really weird here. We know the canonical way to register a global interceptor:

app.useGlobalInterceptors(app.get(InterceptorType));

If I attempt to app.get() an @Injectable({ scope: Scope.REQUEST }) object here, I get one. The kicker? Its constructor is never invoked. If you throw an exception in its constructor, that exception never fires. It just creates the class without ever actually injecting anything into it. I get why this happens, but this should just throw an exception, shouldn't it?

It gets weirder, though, and this is the interesting state of the request-scope-test repo linked above. If you use a FactoryProvider with scope: Scope.REQUEST and then attempt to app.get an object, it returns null. This behavior is better than the above behavior...but it's pretty bad behavior. This should probably throw an exception, too. My guess is that this latter behavior is actually fairly related to #2259, where @kamilmysliwiec requested a test repo; intuitively I wonder if fixing this would also fix that.

It seems like the end solution here is that app.useGlobalInterceptors should probably take Array<NestInterceptor | InjectorDefinition and build request-scoped interceptors upon...well...request.

EDIT: I've tried to get started on a PR to move the ball on this...but master appears to be sad right now. Eep.

[eropple]

Further investigation today suggests that request-scoped global interceptors don't and can't work. Unless I'm missing something, it seems like everything that could call them inside the request flow is hard-coded to STATIC_CONTEXT. (The profusion of Consumer and Context classes is making it hard for me to be sure.)

@kamilmysliwiec, I'm happy to do some of the heavy lifting to address this, but it doesn't appear to be a simple fix and to be productive I need some direction and guidance. Who can help me help myself?

[kamilmysliwiec]

You obviously cannot use app.useGlobal[...] for request-scoped enhancers. Only this syntax:

{
  provide: APP_INTERCEPTOR,
  scope: Scope.REQUEST,
  useClass: LoggingInterceptor,
}

is supported because it leaves the class instantiation responsibility to Nest.

Also, this:

app.useGlobalInterceptors(app.get(InterceptorType));

is actually a bad practice, not a canonical way. If you want to bind global enhancers that should live within Nest DI scope, you should use syntax that I shared above. useGlobalInterceptors(), useGlobalGuards() etc. should be used only for static initialization (if you want to create an instance in place on your own). You should never combine these methods with .get() call. When DI is required, use APP_INTERCEPTOR, APP_GUARD, etc token, as stated in the docs.

Regarding get() support for both request and transient-scoped providers, we track this issue here https://github.com/nestjs/nest/issues/2049 and there is a PR https://github.com/nestjs/nest/pull/2387 with a solution already. .get() will also throw an exception when called for request/transient-scoped providers in the future (in progress).

[kaihaase]

@kamilmysliwiec I try to integrate the context for a GraphQL query into a pipe, as described above, to check the permissibility of individual properties of inputs for the resolvers in relation to the current user:

core.module.ts

providers: [
      {
        provide: APP_PIPE,
        scope: Scope.REQUEST,
        useClass: CheckInputPipe,
      }
]

But the context is undefined:

check-input.pipe.ts

@Injectable({ scope: Scope.REQUEST })
export class CheckInputPipe implements PipeTransform<any> {

  constructor(@Inject(CONTEXT) private readonly context) {}

  async transform(value: any, { metatype }: ArgumentMetadata) {

    console.log(this.context); ==> undefined
    ...
  }

What am I doing wrong?

[eropple]

@kamilmysliwiec Thanks for the detailed response! I would take some issue with the use of the word "obviously". It's not obvious to me, as a user, why useGlobalInterceptors can't take an InjectorDefinition and just do the right thing.

I 100% get you when you say that app.get a bad practice, and coming from other frameworks it sure smelled--but it's also the practice that shows up a lot in various places, so maybe it would be worth pushing back against--perhaps a logger warning when useGlobalInterceptors is invoked would help people fall into the pit of success?

Also, I'm not having success using APP_INTERCEPTOR now that I know it exists (I don't recall it in the docs the last time I looked, but I certainly might have glazed over it). Please refer to my test repo, particularly app.module.ts; this seems like it should be conforming to the docs and the magic APP_INTERCEPTOR word--aside: how can we ensure ordering of interceptors when executed? it seems kind of fraught--does not seem to change anything. The interceptor does is not created or applied to the request flow. If I set it instead to Scope.DEFAULT, the interceptor is created and is executed upon request.

I'm sorry to keep pushing, and maybe it's something I'm not understanding, but something still seems really off with this. Can you please take a look and point me (and apparently @kaihaase too) in the right direction?

[kamilmysliwiec]

I 100% get you when you say that app.get a bad practice, and coming from other frameworks it sure smelled--but it's also the practice that shows up a lot in various places, so maybe it would be worth pushing back against--perhaps a logger warning when useGlobalInterceptors is invoked would help people fall into the pit of success?

This change that I've mentioned:

Regarding get() support for both request and transient-scoped providers, we track this issue here #2049 and there is a PR #2387 with a solution already. .get() will also throw an exception when called for request/transient-scoped providers in the future (in progress).

should fix this confusion soon.

I'm sorry to keep pushing, and maybe it's something I'm not understanding, but something still seems really off with this. Can you please take a look and point me (and apparently @kaihaase too) in the right direction?

I'll look at both repos asap :)

[kamilmysliwiec]

@kaihaase @csidell-earny could you please test this in 6.5.0? Errors should be gone now

[eropple]

I'm neither of those folks, but my test app exhibits the expected behavior now. Thanks, @kamilmysliwiec! :)

[jasoncarty]

I can confirm that my interceptor is now able to inject a dependency and it is working as expected. Thank you very much!

[csidell-earny]

@kamilmysliwiec Did you accidentally tag me on this one?

[kaihaase]

@kamilmysliwiec After updating to 6.5.0, the transform method of my pipe is no longer called.

[kamilmysliwiec]

@csidell-earny ahh yeah sorry, I actually wanted to tag @eropple 😄

[kamilmysliwiec]

@kaihaase we should create a separate issue in the github repo. In order to support this functionality by @nestjs/graphql, I'll have to push a few other changes to this package exclusively

[kaihaase]

@kamilmysliwiec I created a separate issue in the repo of @nestjs/graphql: https://github.com/nestjs/graphql/issues/325

[kamilmysliwiec]

Thank you :)

[dbg1995]

@kamilmysliwiec I want to pass transformOptions with groups is roles of current user. But at the constructor of CustomValidationPipe which that user in request is undefined, because the AuthGuard('jwt') ís has not been called yet. I have attached my solution as image. But I'm not satisfied with that solution, can you tell me another way to do what I want (bow).

I wonder if it is possible to initialize pipe after run guard? why Pipe haven't ExecutionContext look like guard, filter, intercoptor?

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.