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 server does not handle async interceptor. #11429

Closed julien-sarazin closed 1 year ago

julien-sarazin commented 1 year ago

Is there an existing issue for this?

Current behavior

In a gRPC controller, when using an interceptor that executes async methods, the payload returned by the server is empty.

Minimum reproduction code

https://github.com/julien-sarazin/nest-grpc-interceptor-issue

Steps to reproduce

  1. npm i

  2. npm run start:dev

  3. GET http://localhost:3333 See payload is properly filled

    {
    "foo": "foo",
    "bar": "bar",
    "date": "2023-04-05T11:56:56.445Z"
    }
  4. Same query via gRPC request localhost:50033 ItemService.First() See payload is properly filled

    {
    "item": {
        "foo": "foo",
        "bar": "bar",
        "date": "0"
    }
    }
  5. Uncomment app.controller.ts (line 22)

    @GrpcMethod('ItemService', 'First')
    // @UseInterceptors(new AsyncInterceptor())
    first(
    data: item.ItemById,
    metadata?: Metadata,
    ...rest: any[]
    ): Observable<any> {
    return from(props({ item: this.appService.getItem() }));
    }
    }
  6. Same query via gRPC request localhost:50033 ItemService.First() see payload incorrect

    {}

Expected behavior

Expecting the payload to be the same with or without the interceptor, to match the behavior of classic HTTP controller.

Package

Other package

No response

NestJS version

9.4.0

Packages versions

[System Information]
OS Version     : macOS Monterey
NodeJS Version : v18.15.0
NPM Version    : 9.5.0 

[Nest CLI]
Nest CLI Version : 9.3.0 

[Nest Platform Information]
platform-express version : 9.4.0
microservices version    : 9.4.0
schematics version       : 9.1.0
testing version          : 9.4.0
common version           : 9.4.0
core version             : 9.4.0
cli version              : 9.3.0

Node.js version

18.15.0

In which operating systems have you tested?

Other

Looking at https://github.com/nestjs/nest/blob/0d8c618c76af29cecf86d4a6f0a7ab8cb7471f0a/packages/microservices/server/server-grpc.ts#L220

making the subscriber async resolve the issue :

...
            this.transformToObservable(result).subscribe({
                next: async data => callback(null, await data),
                error: (err) => callback(err),
            });
kamilmysliwiec commented 1 year ago

Would you like to create a PR for this issue?

julien-sarazin commented 1 year ago

Sure thing. It would be my first, would you suggest any reading other than the contributing.md?

kamilmysliwiec commented 1 year ago

Going through the contribution file should be enough! Otherwise, look at scripts defined in the package.json file, they should be pretty self explanatory (build, test, integration tests)

kamilmysliwiec commented 1 year ago

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