devashishdxt / tonic-web-wasm-client

Other
104 stars 28 forks source link

Sending Poll::Pending appears to hang #17

Closed jr200 closed 1 year ago

jr200 commented 2 years ago

Hi,

I'm not sure if I'm using this library correctly, so my apologies upfront...

In the server part of the test-suite, if I change the body of the implementation of the Stream trait to include a Poll::Pending return value, the client indefinitely hangs.

impl Stream for MessageStream {
    type Item = Result<EchoResponse, Status>;

    fn poll_next(mut self: Pin<&mut Self>, _: &mut Context<'_>) -> Poll<Option<Self::Item>> {
        self.count += 1;
        if self.count < 3 {
            Poll::Pending
        } else if self.count < 6 {
            Poll::Ready(Some(Ok(EchoResponse {
                message: format!("echo({})", self.message),
            })))

        } else {
            Poll::Ready(None)
        }
    }
}

My use-case is my MessageStream wraps another which might return Poll::Pending. I should add I am also fairly new to rust.

Thanks!

devashishdxt commented 2 years ago

I this particular example, you need to arrange wake() call for the future so that the runtime knows when to poll the future again.

Read Rust async book for better understanding.

devashishdxt commented 2 years ago

@jr200 Can you confirm if your issue is resolved? If yes, we can close this issue. If not, can you provide more details of what you're trying to do?

jr200 commented 2 years ago

Hi @devashishdxt, I've read the async book now (thanks for the recommendation!). I believe the simple test-case works, but I haven't managed to get my use-case working yet, where I am wrapping a Receiver in a Stream. Although the browser no longer hangs when a Poll::Pending is processed, either the messages do not make it through or I appear to get many more messages than I am expecting.

I am almost definitely still doing something wrong, so I'll try and put together a minimal example and debug it this weekend.

jr200 commented 2 years ago

Had an initial look, and it seems that the server has to send all messages followed by a Poll::Ready(None) (close stream) before the client will begin to process the received messages. So in my case where I have essentially an infinite-stream, the client (browser) never begins to process the first message.

teddyrendahl commented 2 years ago

I'm seeing this as well. My client does not get the stream response until the stream has closed. My server works fine with the Javascript bindings so I don't think the issue is there.

devashishdxt commented 2 years ago

@teddyrendahl Can you post some sample JS code and your server code for my reference?

jr200 commented 2 years ago

I was also able to get server-side streaming to work using rust/tonic-web and a JS grpc-web implementation. However, this was using grpcwebtext. The grpc-web page says Content-type: application/grpc-web+proto only supports unary calls, while Content-type: application/grpc-web-text supports both unary and server-side streaming calls.

Perhaps I was trying to do something that was never possible. Can you confirm this to be the case?

devashishdxt commented 2 years ago

@jr200 application/grpc-web-text is not yet supported on this crate. But, I'd like to implement support for this. For that, can you please share some sample code (server and js-client) that you're using so that I can test this particular usecase?

jr200 commented 2 years ago

Sure - I will create a minimal example tonight and share it here.

But with regards to my original issue, can you confirm that this crate does not support streaming with Content-type: application/grpc-web+proto?

jr200 commented 2 years ago

Hi @devashishdxt, I've put together a minimal example of a rust server exposing a streaming API over gRPC-web, and a with a JS/react frontend consuming the stream (Content-type: application/grpc-web-text).

Code is here: https://github.com/jr200/grpc-web-example.

Please let me know if are any issues...

devashishdxt commented 2 years ago

Hi @devashishdxt, I've put together a minimal example of a rust server exposing a streaming API over gRPC-web, and a with a JS/react frontend consuming the stream (Content-type: application/grpc-web-text).

Code is here: https://github.com/jr200/grpc-web-example.

Please let me know if are any issues...

Thanks for the example. I'll try and add support for this over the weekend.

wackazong commented 1 year ago

I think I can confirm this. When using the routeguide example from tonic and putting a small delay into list_features on the server side I see the messages incoming regularly.

When I do the same with this repo I see the results coming in from the server regularly in the network tab of the dev tools.

However, the messages are only processed after all have been received.

            // this let only returns after all messages have been received
            let mut stream = client
                .many_slow_hello(request)
                .await
                .expect("request successful")
                .into_inner();
            // this loop waiting for messages starts only once the stream has ended            
            loop {
                match stream.message().await {
                    Ok(Some(message)) => {
                        info!("many_slow_hello: {:?}", message.message);
                        received.write().push(message);
                    }
                    Ok(None) => {
                        info!("Stream ended");
                        break;
                    }
                    Err(status) => {
                        error!("Stream error: {}", status);
                    }
                }
            }

I am new to tonic and wasm but I have the hunch this could be the same issue.

Thanks in advance for your support! 🙏

wackazong commented 1 year ago

AFAICS this seems to be impossible in WASM. Getting the chunks of a HTTP response as they come in is not supported in reqwest and also not in other WASM-compatible HTTP clients.

wngr commented 1 year ago

Oh, what a bummer! What makes you think that?

Seems https://github.com/seanmonstar/reqwest/pull/1576 would provide that functionality, so in theory it looks like it's possible.

Edit: I just tried https://github.com/titanous/grpc-web-client/pull/13 as a drop-in-replacement for this crate. Server streaming just works.

wackazong commented 1 year ago

Seems seanmonstar/reqwest#1576 would provide that functionality, so in theory it looks like it's possible.

Yes, I think that is what would need to be enabled.

Edit: I just tried titanous/grpc-web-client#13 as a drop-in-replacement for this crate. Server streaming just works.

Great to hear, thanks!

Edit: grpc-web-client uses web-sys directly, way to go.

Edit: I can confirm, the drop-in replacement works for server streaming.

wngr commented 1 year ago

I think the way forward would be to re-use the whole parsing logic from tonic, and either use web-sys directly or via reqwest with added stream support.

devashishdxt commented 1 year ago

I'm already working on this in next branch of this repo. Will finish it in a few days.

wngr commented 1 year ago

Do you plan on re-using the decoding logic from tonic or stick to rolling your own?

devashishdxt commented 1 year ago

Released a new version. Hopefully it should solve the issues.

wngr commented 1 year ago

I can confirm it works. Thanks for this!