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

grpcwebproxy: Delayed leading headers in bidir streaming rpcs on websocket transport #791

Open
Francois-air opened this issue Oct 28, 2020 · 2 comments
Labels
help-wanted We'd like your help! kind/bug Categorizes issue or PR as related to a bug.

Comments

@Francois-air
Copy link

Francois-air commented Oct 28, 2020

Versions of relevant software used
go get -u github.com/improbable-eng/grpc-web/go/grpcwebproxy
@improbable-eng/grpc-web": "^0.13.0
ts-protoc-gen": "^0.13.0

What happened

When using the websocket transport, on a bidirectional streaming RPC where the server explicitly sends metada before any message, grpcwebproxy will delay the dispatching of the metadata until the first message is sent.

What you expected to happen
I would expect the metadata to be dispatched immediately.

How to reproduce it (as minimally and precisely as possible):

On the grpc backend:

func bidirstreamhandler(srv pb.Service_MethodServer) error {
  
  // Send leading metadata
  meta := make(map[string][]string)
  meta["key"] = append(meta["key"], "value")
  srv.SendHeader(meta)

  // swallow messages until the client gives up
  for {
    in, err := srv.Recv()
    if err != nil {
      break
    }
    // Only send a message once something has been received.
    srv.Send(&pb.ReplyMessage{})
  }

  return nil
}

On the client:

const ws_transport = grpc.WebsocketTransport();
grpc.setDefaultTransport(ws_transport);

const msgStream = grpc.client(Service.Method, {host: getGrpcHost()})
msgStream.onHeaders((headers) => {
    // Only send messages once headers have been received. Deadlock!
    msgStream.send(new pb.Request())
 })

msgstream.start()

Anything else we need to know

I have confirmed that the backend does send the headers when a golang grpc client uses the service.
I have also confirmed that the browser does not receive any websocket message.
I have not tested this on either unary, unary-stream or stream-unary methods, and am not 100% certain what the expected behavior would be.

Obviously, this can be easily worked around by sending and cleanly handling noop messages, but I shouldn't have to do this :)

@johanbrandhorst johanbrandhorst added kind/bug Categorizes issue or PR as related to a bug. help-wanted We'd like your help! labels Oct 29, 2020
@johanbrandhorst
Copy link
Contributor

Yep, that sounds like a bug. The grpc-go documentation says that SendHeader should send out the header as soon as it is call, not when the first message is sent. You can see the logic for our websocket translation layer here: https://github.com/improbable-eng/grpc-web/blob/master/go/grpcweb/websocket_wrapper.go. If you can produce a failing test case it should be easier to work on fixing this.

@aikoven
Copy link

aikoven commented Jun 18, 2021

This is actually a bug in grpc-proxy. It is present for HTTP transport as well.

Found a PR that contains a fix: mwitkow/grpc-proxy#47

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help-wanted We'd like your help! kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

3 participants