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.28k stars 7.59k forks source link

grpcService stream useless #1260

Closed muyu66 closed 5 years ago

muyu66 commented 5 years ago

[ ] Regression 
[X] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.
  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)))
        .forEach(data => call.write(data));
      call.end();
    };
  }

Should replace Promise with Array

  public transformToObservable<T = any>(resultOrDeffered): Observable<T> {
    if (resultOrDeffered instanceof Promise) {
      return fromPromise(resultOrDeffered);
    } else if (!(resultOrDeffered && isFunction(resultOrDeffered.subscribe))) {
      return of(resultOrDeffered);
    }
    return resultOrDeffered;
  }

When I use grpc stream, I return an array on the server side, and the client receives a problem. According to my analysis, there should be a problem with the transformToObservable code.

Be aware that it is impossible to have a Promise type under await handler.

If it is necessary, I will provide the minimum source to reproduce

Environment

Nest version: 5.3.8 (Microservice)

adubrov commented 5 years ago

@muyu66 why do you use array? If you return array then it just message with repeated field. If you want to return stream on your grpc server - use Observable. For instance

@GrpcMethod("NodeService", "testStream")
  testStream(inp: any, metadata: any): Observable<StreamVal> {
    return range(1, 5).pipe(map(val => ({height: val.height+1})))
  } 

But actually there is one issue in GRPC stream support - there is no client-side streaming support. So I can't send stream on client side.

muyu66 commented 5 years ago

@adubrov Maybe it's because I didn't use pipe to do this thing. I will try it later

Because I don't know much about the internal mechanism of GRPC and NESTJS. I might mistakenly think that I return an array on the server side, nestjs will automatically convert it to stream to the client. Or I use from(Array) on the server side, and nestjs will do something automatically.

As for why I want to return an array, because I don't want to wrap the array in an object. I want to return an array directly instead of an object containing an array

such as: service FindAll (req) returns (stream Book) {} Instead of service FindAll (req) returns (res) {}

message res {
repeated Book books = 1;
}
adubrov commented 5 years ago

@muyu66 yes, it will not covert your array to GRPC stream, you should do it in your controller method

@GrpcMethod("MyService") public findAll(req: any): Observable { return Observable.from(array) }

muyu66 commented 5 years ago

bad ,,,, sad ,,,

I tried it. If the Server uses from or from pipe, the Client will never receive the entire array. I can only receive the last one.

muyu66 commented 5 years ago

I checked it out, it should be what the ClassSerializerInterceptor does to the grpc stream return value, causing the stream to return only the last element in the array. @kamilmysliwiec @adubrov

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.