nestjs / nest

A progressive Node.js framework for building efficient, scalable, and enterprise-grade server-side applications with TypeScript/JavaScript 🚀
https://nestjs.com
MIT License
67.29k stars 7.59k forks source link

GRPC client interceptor does not get called #9079

Closed ckfngod closed 2 years ago

ckfngod commented 2 years ago

Is there an existing issue for this?

Current behavior

GRPC client interceptor not called when passed through ClientsModule options but is working when passed through options.channelOptions:

This does not work:

ClientsModule.register([
  {
    name: 'HERO_PACKAGE',
    transport: Transport.GRPC,
    options: {
      package: 'hero',
      protoPath: join(__dirname, 'hero.proto'),
      interceptors: [
        (options, nextCall) => {
          console.log('CALLED INTERCEPTOR');
          return new InterceptingCall(nextCall(options));
        },
      ],
    },
  },
])

This is working:

ClientsModule.register([
  {
    name: 'HERO_PACKAGE',
    transport: Transport.GRPC,
    options: {
      package: 'hero',
      protoPath: join(__dirname, 'hero.proto'),
      channelOptions: {
        interceptors: [
          (options, nextCall) => {
            console.log('CALLED INTERCEPTOR');
            return new InterceptingCall(nextCall(options));
          },
        ],
      },
    },
  },
])

Minimum reproduction code

https://github.com/ckfngod/grpc-client-interceptor-issue

Steps to reproduce

  1. npm i
  2. npm run start
  3. curl localhost:3000

GRPC request is successful but there is no console output from application when interceptor should be logging.

Expected behavior

GRPC client interceptor should be called.

Package

Other package

No response

NestJS version

8.2.6

Packages versions

[System Information] OS Version : macOS Monterey NodeJS Version : v16.13.2 NPM Version : 8.1.2

[Nest CLI] Nest CLI Version : 8.2.0

[Nest Platform Information] platform-express version : 8.2.6 microservices version : 8.2.6 schematics version : 8.0.5 testing version : 8.2.6 common version : 8.2.6 core version : 8.2.6 cli version : 8.2.0

Node.js version

16.13.2

In which operating systems have you tested?

Other

No response

dacdodinh99 commented 2 years ago

I had the exact same issue. Does anyone know how to solve this?

vietkute02 commented 2 years ago

This error happen because interceptors option not pass to grpc-js client

https://github.com/nestjs/nest/blob/master/packages/microservices/client/client-grpc.ts#L65

tuananhlai commented 2 years ago

It's so weird, we already have an PR to expose interceptors option for gRPC but it doesn't get passed to grpc-js client?

https://github.com/nestjs/nest/pull/8231

kamilmysliwiec commented 2 years ago

Yeah, it turns out #8231 was an invalid PR that exposed interceptors option in the wrong place. You should pass interceptors as part of the channelOptions object, as in this example:

ClientsModule.register([
  {
    name: 'HERO_PACKAGE',
    transport: Transport.GRPC,
    options: {
      package: 'hero',
      protoPath: join(__dirname, 'hero.proto'),
      channelOptions: {
        interceptors: [
          (options, nextCall) => {
            console.log('CALLED INTERCEPTOR');
            return new InterceptingCall(nextCall(options));
          },
        ],
      },
    },
  },
])

8231 will be reverted

leonidasv commented 2 years ago

@kamilmysliwiec channelOptions need a proper typing in order to support this:

TS2322: Type '(options: any, nextCall: any) => any' is not assignable to type 'string'.

image

nekooei commented 2 years ago

@kamilmysliwiec channelOptions need a proper typing in order to support this:

TS2322: Type '(options: any, nextCall: any) => any' is not assignable to type 'string'.

image

Yes, I have same problem

hyunjinjeong commented 2 years ago

We can use it with type casting as a workaround.

const options = {
  ...
} as unknown as GrpcOptions

// or more specifically
const options = {
  options: {
    channelOptions: {
      interceptors: [SomeInterceptor],
    } as unknown as GrpcOptions['options']['channelOptions']
  }
}

We need a proper typing though...

kamilmysliwiec commented 2 years ago

Let's track this here https://github.com/nestjs/nest/pull/9587