timostamm / protobuf-ts

Protobuf and RPC for TypeScript
Apache License 2.0
1.1k stars 130 forks source link

Stream response with grpc-web transport #381

Closed Ludea closed 2 years ago

Ludea commented 2 years ago

Hi, I try to retrieve stream data

const status = async(client: IRepoClient, path: string) => {  
  const call = client.status({
      path: path
    });
      for await (let response of call.responses) {
       console.log("foo "); 
        console.log("got : " + response);
  } 
}

There is no output from browser console. I check with Bloomrpc, the server send the stream as expected. If I print call.response before loop, I get strange json output.

I miss something ?

timostamm commented 2 years ago

Looks correct to me. Are you sure you are hitting a gRCP-web server, not plain gRPC?

Perhaps this example helps to identify a missing piece: https://github.com/timostamm/protobuf-ts/tree/master/packages/example-node-grpcweb-transport-client

Ludea commented 2 years ago

I follow https://github.com/timostamm/protobuf-ts/tree/master/packages/example-browser-grpcweb-transport-client because I use it throuhgh browser.

I enable http1 and grpc-web from server https://github.com/Ludea/speedupdate-rs/blob/tarpc/server/src/main.rs#L235 (it's a rust implementation based on tonic, https://github.com/hyperium/tonic

and the client : https://github.com/Ludea/lucle/blob/main/web/src/views/Speedupdate.tsx#L45

Note: unary call work as expected

the message I get when I print call.response {"_lis":{"nxt":[],"msg":[],"err":[],"cmp":[]},"_closed":true}

timostamm commented 2 years ago

This could be a regression in our grpcweb-transport, a bug in ludea, a misunderstanding between the two protocol implementations, or a simple misconfiguration.

To isolate the problem, I'm afraid you have to try hitting the server with other gRCP-web implementations. It's really unfortunate that there is no formal gRPC-web specification.

Ludea commented 2 years ago

will try with connect-web

jcready commented 2 years ago

The call.responses is only ever going to be the RpcOutputStream which won't give you the messages from the stream. The usage you linked to looks like this:

  const status = async(client: IRepoClient, path: string) => {  
  const call = client.status({
      path: path
    });
    //TODO : Delete thid
    setRepoInit(true);
    const status = await call.status;
    if (status.code === "OK") {
      const trailers = await call.responses;
      alert("g : " + JSON.stringify(trailers));
      for await (let response of call.responses) {
        alert("got : " + response);
        if (response.repoinit) {
          console.log("got response message: ", response)
          setRepoInit(true);
          setCurrentVersion(response.currentVersion);
          setListVersions(response.versions);
      setListPackages(response.packages);
        }
      }
    }
  }

Two odd things I'm seeing here:

  1. You're awaiting the call.status before doing the async iteration.
  2. You're awaiting the call.responses and assigning it to a variable named trailers. This is almost certainly not correct and I'm also not sure if that's messing up the async iterator since you're awaiting it before iterating it.

What happens if you follow the example in the manual and do the async iteration before awaiting the status and then fixing the trailers to actually await the call.trailers?

const call = client.status({ path });
for await (const response of call.responses) {
    console.log("got a message", response);
}
const {status, trailers} = await call;
Ludea commented 2 years ago

Works as expected with

for await (const response of call.responses) {
    console.log("got a message", response);
}
const {status, trailers} = await call;