Open
Conversation
|
I got stuck for the server streaming not responding, until I saw this PR. This solves my problem. Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There was a connection leak issue with
grpc-gatewaythat affected all clients using Unary and ServerStreaming methods (i.e. methods in which gRPC clients send a single request, instead of a stream), both HTTP and WebSocket proxy - when client sent more data than server read (anything at all with no body-annotation, and 512 bytes in case of small requests with body-annotation), EOF / context cancellation never happened.These PRs fixed the connection leak issues:
But they also broke WebSocket, because:
req.Bodyas to not leave any data unreadreq.Bodyuntil conn is closed, at which point the whole request is cancelled tooRelated issue: grpc-ecosystem/grpc-gateway#5326
To properly work with the changes, we need a way to distinguish between:
This PR adds a query parameter, similar to
method, calledmethodTypewhich can have one of the following values:Specifying the correct type of method will close the request at appropriate time:
One issue that comes to mind:
What if some e.g. ClientStreaming method expects request body to be closed before sending a response?
Many WebSocket client libraries can only send Close Message when closing the whole connection, therefore unable to read response that comes after close.
An off-the-dome example would be a "Fingerprint" method for which typical request size is e.g. bigger than RAM of client, and needs streaming, and whose server waits for EOF to respond with a fingerprint of the whole stream.
We would need a way for WebSocket client to send EOF without closing the connection, maybe via special inbound message?