grpc-web icon indicating copy to clipboard operation
grpc-web copied to clipboard

[Feature] multiplex over a websocket

Open borissmidt opened this issue 2 years ago • 15 comments

Dear,

I was wondering, could a websocket be multiplexed so it really mirrors the java/go api? So you create 1 connection per stub, this would allow grpc-web to have a huge amount of streams.

Maybe a simple protocol could be defined in protobuf itself to make it easy to implement in other languages to have an InProcessProxy reducing the need for proxies.

The only disadvantage is that the proxy then has to keep state to track which message maps to which stream.

Would you be interested in such a feature or would it be too complex to implement?

For the protocol i was thinking along the lines of:

syntax = "proto3";

option go_package = "./main";

message Message{
    uint32 streamId = 1;
    oneof payload {
        Start start = 2;
        Body body = 3;
        Complete complete = 4;
        Error error = 5;
        Cancel cancel = 6;
    } 
}

message Start {
    string operation = 1;
    repeated Header headers = 2;
}

message Body {
    bytes data = 1;
}

message Complete {
}

message Error {
}


message Cancel {
}

message Header {
    string name = 1;
    bytes data = 2;
}

borissmidt avatar Feb 13 '22 17:02 borissmidt

Hi! I doubt this is something we are going to implement here, browsers are planning on implementing websockets over HTTP/2 AFAIK, which should eventually solve the maximum connection issues.

johanbrandhorst avatar Feb 13 '22 19:02 johanbrandhorst

how would that work then on the go side? would you get a special websocket implementation that then gives back somekind of 'virtual' websocket?

borissmidt avatar Feb 13 '22 20:02 borissmidt

That I do not know

johanbrandhorst avatar Feb 13 '22 20:02 johanbrandhorst

Would it be technically feasible? (i was trying implement it but i have never written go before :smile: )

borissmidt avatar Feb 13 '22 20:02 borissmidt

Oh I'm sure it would be, but the websocket abstraction on the backend would need to perform all the multiplexing work unless the standard library can somehow abstract that. https://datatracker.ietf.org/doc/html/rfc8441 this appears to be the RFC. https://github.com/nhooyr/websocket/issues/4 is the issue for our websocket library, which seems to depend on https://github.com/golang/go/issues/32763.

johanbrandhorst avatar Feb 13 '22 20:02 johanbrandhorst

I will try to implement it in scala, i'm more experienced with that, and if it was easy to implement i will port it to go and the typescript client.

borissmidt avatar Feb 13 '22 20:02 borissmidt

I tried to implement it in go but i get an INFO[0000] streamId:1 header:{headers:{key:"Content-Type" value:"text/plain; charset=utf-8"} headers:{key:"X-Content-Type-Options" value:"nosniff"} status:500} so i can get the headers back to the client which is already a great achievement. https://github.com/borissmidt/grpc-web-1/blob/webscocketChannel/go/test/server_test.go

so somewhere around here it goes wrong and it gives an error but it is probably because the request doesn't have the right headers.

any suggestions how i can find out what is wrongly configured in the req which makes it fail?

borissmidt avatar Feb 16 '22 20:02 borissmidt

I am interested in this work and will attempt to debug thank you for moving this along.

hexfusion avatar Feb 16 '22 23:02 hexfusion

You are making good progress I ran the test through the debugger and found this. Taking a deeper look now.

gRPC requires a ResponseWriter supporting http.Flusher

ref: https://github.com/grpc/grpc-go/blob/ec717cad7395d45698b57c1df1ae36b4dbaa33dd/internal/transport/handler_server.go#L65

hexfusion avatar Feb 17 '22 02:02 hexfusion

Well it seems to be making better progress now that we understand the failure.

diff --git a/go/test/main.go b/go/test/main.go
index 5d893e9..65fc527 100644
--- a/go/test/main.go
+++ b/go/test/main.go
@@ -286,6 +286,8 @@ func hackIntoNormalGrpcRequest(req *http.Request) (*http.Request, bool) {
        return req, false
 }
 
+var _ http.Flusher = &GrpcStream{}
+
 type GrpcStream struct {
        id                uint32
        hasWrittenHeaders bool
@@ -352,6 +354,11 @@ func (stream *GrpcStream) Close() error {
        return nil
 }
 
+func (stream *GrpcStream) Flush() {
+       // This is most certainly not optimal :)
+}
+
time="2022-02-16T22:43:12-05:00" level=info msg="got websocket"
time="2022-02-16T22:43:12-05:00" level=info msg="accept websocket &{GET  HTTP[/1.1]() 1 1 map[Accept-Encoding:[gzip] Connection:[Upgrade] Sec-Websocket-Extensions:[permessage-deflate; client_no_context_takeover; server_no_context_takeover] Sec-Websocket-Key:[dcPgh29cRq20u2R6auJcew==] Sec-Websocket-Protocol:[grpc-websocket] Sec-Websocket-Version:[13] Upgrade:[websocket] User-Agent:[Go-http-client[/1.1]()]] {} <nil> 0 [] false 127.0.0.1:45291 map[] map[] <nil> map[] 127.0.0.1:40010  <nil> <nil> <nil> 0xc00036c2c0}"
time="2022-02-16T22:43:12-05:00" level=info msg="writing message"
time="2022-02-16T22:43:26-05:00" level=info msg="[core] Subchannel Connectivity change to CONNECTING" system=system
time="2022-02-16T22:43:26-05:00" level=info msg="Reading response"
time="2022-02-16T22:43:26-05:00" level=info msg="starting call to http server &{POST [http://localhost:50051/main.Greeter/UnaryHello]()  2 0 map[Content-Type:[application[/grpc]()+proto] Grpc-Encoding:[gzip]] 0xc00029c3c0 <nil> 0 [] false  map[] map[] <nil> map[]   <nil> <nil> <nil> 0xc0002942c0}"
time="2022-02-16T22:43:26-05:00" level=info msg="[core] Channel Connectivity change to CONNECTING" system=system
time="2022-02-16T22:43:26-05:00" level=info msg="[core] Subchannel picks a new address \"localhost:50051\" to connect" system=system
time="2022-02-16T22:43:27-05:00" level=info msg="reading from channel 1"
time="2022-02-16T22:43:27-05:00" level=info msg="[core] Subchannel Connectivity change to READY" system=system
time="2022-02-16T22:43:27-05:00" level=info msg="[core] Channel Connectivity change to READY" system=system

hexfusion avatar Feb 17 '22 03:02 hexfusion

@hexfusion Thank you that really helped, after adding the framing like typescript, i was able to make the requests work (streaming and unary calls).

Currently i have added the 'websocket channel' into the go proxy.

My todo list still is:

  • tests to see if the resources are cleaned up correct (its my first time with go so i might do some weird things).
  • typescript transporter
  • ping/pong for the websocket.
  • integration test, i might need some help with that later on.
  • buffer size settings.
  • backpressure in the protocl since if a client start pushing on 1 stream a lot of data and the server doesn't read it then all streams get stuck. (but is this done with any of the other features?)
  • (extra but might be better for another repo, support in java to avoid using a proxy)

borissmidt avatar Feb 19 '22 13:02 borissmidt

ok great, I would like to try and help move the todo forward how can I collab? on your fork?

hexfusion avatar Feb 19 '22 15:02 hexfusion

I will continue to work today on it, i can open a pr and you can give me some advice? On the go part?

On Sat, Feb 19, 2022, 16:20 Sam Batschelet @.***> wrote:

ok great, I would like to try and help move the todo forward how can I collab? on your fork?

— Reply to this email directly, view it on GitHub https://github.com/improbable-eng/grpc-web/issues/1081#issuecomment-1046041636, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABNXZFQ3RGJFOBZYOAEJWDTU36YLLANCNFSM5OJLQSYA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

borissmidt avatar Feb 19 '22 16:02 borissmidt

@johanbrandhorst I was exploring this library in hope of writing a webapp (for browsers) using gRPC. Since gRPC uses a single multiplexed HTTP2 connection, when WebSocket can be multiplexed, I'd say not multiplexing websocket highly reduce the efficiency of the library (due to WS handshakes for each new function call).

@borissmidt I might not be good enough to help out, but how's your progress? I observed that your branch change (client side) exports. I'd suggest exporting another Transport, say, as MultiplexedWebSocket instead of replacing CrossBrowserHttpTransport would look better for backward-compatibility, unless of course you're planning to create a separate package altogether.

benedictjohannes avatar May 01 '22 10:05 benedictjohannes

I'm planning to go to a separate package to only support the 'grpc-websocket' protocol (instead of grpc-web) to make the scope of the package more clear and significantly reduce the maintenance burden.

At the moment i have not found a lot of time to continue this project, but i would really be glad with some help on the front of integration tests. So currently i have a go server which serves 'grpc-webscoket' and i implemented the typscript client.

The current working branch is here.

I got stuck at implementing integration tests, because of my lack of skill in typescript, frontend code and NodeJS.

Open issues improvements are:

  • Fair multiplexing on the websocket. I.e. have a queue per stream and round robin the messages of each stream into the websocket buffer.
  • Java client for grpc-websocket to be able to skip any proxy for the JDK.
  • Compatiblity with grpc-web and google/grpc-web to make it easy to drop into both frameworks.

borissmidt avatar May 01 '22 10:05 borissmidt