grpc / grpc-web

gRPC for Web Clients
https://grpc.io
Apache License 2.0
8.65k stars 764 forks source link

Implement proper server-side streaming for `application/grpc-web+proto` content-type #344

Open johanbrandhorst opened 6 years ago

johanbrandhorst commented 6 years ago

As mentioned in the README, the grpcweb mode only supports unary method calls. It should at the very least also support server streaming calls.

stanley-cheung commented 6 years ago

The problem there is the binary format - currently the XHR API does not return back partial chunked response if the response body is binary. Hence the current default mode for gRPC-Web is grpcwebtext (or basically application/grpc-web-text). This is somewhat covered in this old issue #87.

pumano commented 6 years ago

@stanley-cheung It's possible to use fetch for server side stream? Yes, some inconsistency, but feature will be works

johanbrandhorst commented 6 years ago

@pumano my understanding is that relying on fetch will compromise browser support. I think the more interesting angle right now is trying to get something like https://github.com/grpc/grpc-web/issues/87#issuecomment-290545726 merged into the closure XHR library. What do you think @stanley-cheung?

stanley-cheung commented 6 years ago

Actually I tried that 'hack' a while ago and it didn't work. Do you have more details about that?

johanbrandhorst commented 6 years ago

Pinging @jonny-improbable, @marcuslongmuir, @easyCZ, @mwitkow

jared2501 commented 5 years ago

I'd be really interested in seeing a fetch implementation, even if it requires some sort of flag to select. Afaik XHR will keep the full response in-memory as long as the request is in progress, therefore for long-lived server streaming methods (such as subscription-style notifications endpoints) grpc-web is untenable.

We're using improbably-eng's grpc-web fetch transport and it works great. Would move over to code from this repository as soon as there's a fetch implementation!

wenbozhu commented 5 years ago

@jared2501 Yes, we are working on this.

Binary streams support hasn't been a priority for us as we use a yet-to-be-published text based encoding format for protobuf. Also cookie domains can't serve binary data due to security issues.

The main use case to support fetch is to run grpc-web from service workers. The buffering issue is definitely a concern too.

OTOH, long-lived server-push will have other issues to deal with, e.g. keep-alive. We plan to publish a spec for this soon.

descalzi commented 3 years ago

Any ETA on implementing this please ? I can see 'accept': 'application/grpc-web-text' reaching the server, but on streaming the browser does not get the "data" callbacks on a message, until the stream ends or the next message arrived.

Envoy proxy is allowing it through as below, protoc used with "mode=grpcwebtext" allow_headers: keep-alive,user-agent,cache-control,content-type,content-transfer-encoding,custom-header-1,x-accept-content-transfer-encoding,x-accept-response-streaming,x-user-agent,x-grpc-web,grpc-timeout expose_headers: grpc-status,grpc-message

niloc132 commented 3 years ago

@descalzi you might have envoy misconfigured if it isn't handling translating the grpc-web traffic into grpc.

That said, this specific issue is about supporting the binary format for streaming though - your comment suggests you are using grpc-text, which should definitely be supported.

descalzi commented 3 years ago

I see whats going on, even with grpc-text, the xhr call doesnt look at partial data, so it doesnt work for real streaming scenarios, where the connection is open for long. The data callback is received when the final headers are sent, the streaming stops, or enough data is received that the "buffer?" is full and its then read. But for a single message, say, in a long sleep while loop .. it doesnt callback