grpc server streaming error handling

daikiojm commented 5 years ago

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

In server side streaming response, no error response to the client is made.

Even if throwing an exception in the controller as follows, gRPC error will not be returned as a response to the client side.

  @GrpcMethod('SampleService', 'SampleStreamMethod')
  sampleStream(req: SampleRequest): Observable<Sample> {
    throw new RpcException({ code: grpc.status.UNKNOWN, message: 'sample error' });
    // some observable response.
  }

Expected behavior

In server-side streaming, handling Observable errors appropriately and returning it as a response to the client.

The following part

https://github.com/nestjs/nest/blob/5.6.0/packages/microservices/server/server-grpc.ts#L134-L143

I think that it should be modified like this.

  public createStreamServiceMethod(methodHandler): Function {
    return async (call, callback) => {
      const handler = methodHandler(call.request, call.metadata);
      const result$ = this.transformToObservable(await handler);
      await result$
        .pipe(
          takeUntil(fromEvent(call, CANCEL_EVENT)),
          catchError((err) => call.emit('error', err)),
          )
        .forEach(data => call.write(data));
      call.end();
    };
  }

However, I am not familiar with the gPRC protocol, so I do not know exactly.

Environment


Nest version: 5.6.2


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

Others:

AlexDaSoul commented 5 years ago

PR https://github.com/nestjs/nest/pull/1606

kamilmysliwiec commented 5 years ago

Fixed in the latest release

lock[bot] commented 5 years ago

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.

nestjs / nest