Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for hybrid decorators with gRPC #4982

Closed
twister21 opened this issue Jun 27, 2020 · 1 comment
Closed

Add support for hybrid decorators with gRPC #4982

twister21 opened this issue Jun 27, 2020 · 1 comment
Labels
needs triage This issue has not been looked into type: enhancement 🐺

Comments

@twister21
Copy link

twister21 commented Jun 27, 2020

Feature Request

Is your feature request related to a problem? Please describe.

Since Nest.js version 7.1.0 it's allowed to inherit the ApplicationConfig for hybrid applications. That's why it's possible to share for example guards or interceptors, as well as controller handler methods. However, it's a bit tricky to extract the request data from the HTTP request object and the gRPC payload.

While HTTP supports params, query and body request data, gRPC requests only have a payload and metadata.

This example works for both HTTP and gRPC.

@Post()
@GrpcMethod("UserService")
createOne(@Body() @Payload() dto: CreateUserDto): Promise<User> {
    return this.userService.createUser(dto);
}

The following only works for HTTP, but the id property is missing in the gRPC request.

@GrpcService("UserService")
@Delete(":id")
deleteUser(@Param() @Payload dto: DeleteUserDto) {
    this.userService.deleteUser(dto.id)
}

Describe the solution you'd like

I would like to have additional decorators that can handle both HTTP and gRPC requests by correctly mapping the request data. For example:

Hybrid REST/gRPC controller method

@GrpcService("UserService")
@Delete(":id")
deleteUser(@ParamOrPayload("id") userId: string) {
    this.userService.deleteUser(userId)
}

For HTTP requests, the userId is populated with the id passed in the request url, while for gRPC requests the userId is extracted from the gRPC payload.

For this to work with pipes, it is necessary to map the HTTP exceptions / gRPC exceptions depending on the execution context. There's already an issue that deals with this:

#1953

Teachability, Documentation, Adoption, Migration Strategy

What is the motivation / use case for changing the behavior?

gRPC supports mapping to REST endpoints in the proto definition file, for example:

rpc DeleteOne(DeleteOneUserRequest) returns (google.protobuf.Empty) {
   option (google.api.http) = {
      delete: "/api/users/{id}"
    };
 }

Then, both services can be generated and serve requests over the over the same port. Here's a short example for GO:
https://grpc.io/blog/coreos/

@twister21 twister21 added needs triage This issue has not been looked into type: enhancement 🐺 labels Jun 27, 2020
@kamilmysliwiec
Copy link
Member

That's why it's possible to share for example guards or interceptors, as well as controller handler methods.

Sharing controller handler methods is not recommended and should be considered as bad practice. Controller represents a transfer layer, it shouldn't be tied to multiple, different transport strategies.

I would like to have additional decorators that can handle both HTTP and gRPC requests by correctly mapping the request data

We don't plan to add such a feature in the near future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage This issue has not been looked into type: enhancement 🐺
Projects
None yet
Development

No branches or pull requests

2 participants