Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MethodType query parameter #43

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

paskozdilar
Copy link

@paskozdilar paskozdilar commented Apr 4, 2025

There was a connection leak issue with grpc-gateway that 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:

  1. server now drains the full req.Body as to not leave any data unread
  2. websocket never closes req.Body until conn is closed, at which point the whole request is cancelled too

Related issue: grpc-ecosystem/grpc-gateway#5326


To properly work with the changes, we need a way to distinguish between:

  • methods which send a single request (Unary, ServerStreaming)
  • methods which send a stream of requests (ClientStreaming, DuplexStreaming)

This PR adds a query parameter, similar to method, called methodType which can have one of the following values:

  • Unary
  • ServerStreaming
  • ClientStreaming
  • DuplexStreaming

Specifying the correct type of method will close the request at appropriate time:

  • after first message for Unary/ServerStreaming
  • when client closes conn for ClientStreaming/DuplexStreaming

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant