websockets in grpcwebproxy #180

JCGrant · Apr 26, 2018

I have used these changes in a project at my job, and they seem to work well!

johanbrandhorst · Apr 26, 2018

johanbrandhorst reviewed Apr 26, 2018

View changes

johanbrandhorst · Apr 26, 2018

go/grpcwebproxy/README.md

+
+### Enabling Websocket Transport
+
+By default, grpcwebproxy will not use websockets as a trasport layer. To enable


johanbrandhorst · Apr 26, 2018

johanbrandhorst reviewed Apr 26, 2018

View changes

johanbrandhorst · Apr 26, 2018

go/grpcwebproxy/main.go

@@ -34,6 +34,8 @@ var (
 	runHttpServer = pflag.Bool("run_http_server", true, "whether to run HTTP server")
 	runTlsServer  = pflag.Bool("run_tls_server", true, "whether to run TLS server")

+	useWebsockets = pflag.Bool("use_websockets", false, "whether to use beta websocket tranport layer")


transport 🤔

I have real issues with that word apparently! 😂

johanbrandhorst · Apr 26, 2018

johanbrandhorst reviewed Apr 26, 2018

View changes

johanbrandhorst · Apr 26, 2018

go/grpcwebproxy/main.go

-	// gRPC-Web compatibility layer with CORS configured to accept on every
-	wrappedGrpc := grpcweb.WrapServer(grpcServer, grpcweb.WithCorsForRegisteredEndpointsOnly(false))
+	options := []grpcweb.Option{
+		// gRPC-Web compatibility layer with CORS configured to accept on every


I think you accidentally a word

johanbrandhorst · Apr 26, 2018

johanbrandhorst reviewed Apr 26, 2018

View changes

johanbrandhorst · Apr 26, 2018

go/grpcwebproxy/README.md

@@ -49,8 +49,7 @@ $GOPATH/bin/grpcwebproxy

 ### Enabling Websocket Transport

-By default, grpcwebproxy will not use websockets as a trasport layer. To enable
-websockets, set the `--use_websockets` flag to true
+By default, grpcwebproxy will not use websockets as a transport layer. To enable websockets, set the `--use_websockets` flag to true


I think a full stop at the end of this sentence would be correct as well.

Thanks for the PR, looks good. Will test it tomorrow.

easyCZ · Apr 26, 2018

Thanks for the PR, looks good. Will test it tomorrow.

friendly ping on this

timotheecour · May 3, 2018

friendly ping on this

easyCZ · May 8, 2018

easyCZ approved these changes May 8, 2018

View changes

LGTM, @MarcusLongmuir do you mind taking a look?

hope merged, thanks!

bianbian-org · May 31, 2018

hope merged, thanks!

oops, websockets cannot work with Java backend (https is ok).

bianbian-org · Jun 5, 2018

oops, websockets cannot work with Java backend (https is ok).

@JCGrant

@JCGrant need help please. May I ask what is your backend server based? I use a Java-based backend, ws could return Status Code: 101 Switching Protocols, but cannot handle any other requests and disconnect when timeout. ie. proxy always got rpc error: code = Canceled:

grpcwsproxy_1           | time="2018-06-14T14:39:48Z" level=info 
msg="finished streaming call" error="rpc error: code = Canceled desc = context canceled" 
grpc.code=Canceled grpc.method=GetXXX grpc.service=xxx.Service grpc.start_time="2018-06-14T14:38:48Z" grpc.time_ms=59971 span.kind=server system=grpc

bianbian-org · Jun 14, 2018

@JCGrant need help please. May I ask what is your backend server based? I use a Java-based backend, ws could return Status Code: 101 Switching Protocols, but cannot handle any other requests and disconnect when timeout. ie. proxy always got rpc error: code = Canceled:

grpcwsproxy_1           | time="2018-06-14T14:39:48Z" level=info 
msg="finished streaming call" error="rpc error: code = Canceled desc = context canceled" 
grpc.code=Canceled grpc.method=GetXXX grpc.service=xxx.Service grpc.start_time="2018-06-14T14:38:48Z" grpc.time_ms=59971 span.kind=server system=grpc

@bianbian-org

@bianbian-org I use a Go backend. Unfortunately, I'm not entirely sure what is causing your issues.

JCGrant · Jun 14, 2018

@bianbian-org I use a Go backend. Unfortunately, I'm not entirely sure what is causing your issues.

@JCGrant

Thanks all the same @JCGrant
I maybe found the problem, finally!
When using grpc-web-client with Websocket as transport, it depends on the finishSendFrame to tell the server that the client has finished sending:

enum WebsocketSignal {
  FINISH_SEND = 1
}

const finishSendFrame = new Uint8Array([1]);
...
    finishSend: () => {
      if (!ws || ws.readyState === ws.CONNECTING) {
        sendQueue.push(WebsocketSignal.FINISH_SEND);
      } else {
        sendToWebsocket(WebsocketSignal.FINISH_SEND);
      }
    },

And in the grpc-web/websocket_wrapper, it waits the signal:

	if len(framePayload) == 1 && framePayload[0] == 1 {
		return 0, io.EOF
	}

However, when call grpc methods, it do not call the finishSend(). So the grpc-web-socket-proxy is always waiting for the finishSendFrame in request till timeout.

When add the finishSendFrame manually, it works!

bianbian-org · Jun 15, 2018

Thanks all the same @JCGrant
I maybe found the problem, finally!
When using grpc-web-client with Websocket as transport, it depends on the finishSendFrame to tell the server that the client has finished sending:

enum WebsocketSignal {
  FINISH_SEND = 1
}

const finishSendFrame = new Uint8Array([1]);
...
    finishSend: () => {
      if (!ws || ws.readyState === ws.CONNECTING) {
        sendQueue.push(WebsocketSignal.FINISH_SEND);
      } else {
        sendToWebsocket(WebsocketSignal.FINISH_SEND);
      }
    },

And in the grpc-web/websocket_wrapper, it waits the signal:

	if len(framePayload) == 1 && framePayload[0] == 1 {
		return 0, io.EOF
	}

However, when call grpc methods, it do not call the finishSend(). So the grpc-web-socket-proxy is always waiting for the finishSendFrame in request till timeout.

When add the finishSendFrame manually, it works!

I've got this working with a Java gRPC server, sent a single request and got a response back correctly.

alexpdp7 · Jun 17, 2018

I've got this working with a Java gRPC server, sent a single request and got a response back correctly.

This works with a C# backend as well, any chance it will be merged soon?

jcloutz · Jul 11, 2018

This works with a C# backend as well, any chance it will be merged soon?

MarcusLongmuir · Aug 15, 2018

MarcusLongmuir approved these changes Aug 15, 2018

View changes

JCGrant added some commits Apr 26, 2018

Add use_websockets flag to enable websocket transport

9f5bb5f

Update README.md with websocket instructions

96d43ed

JCGrant added some commits Apr 26, 2018

Neaten README.md

d945243

Fix typo and missing word

f5fec9a

Add full-stop

3caee14

alexpdp7 referenced this pull request Jun 11, 2018
Closed
Provide complete bidirectional example if possible? #205

bianbian-org referenced this pull request Jun 15, 2018
Merged
grpcClient.finishSend() is essential for websocket transport #211

MarcusLongmuir merged commit 2cf3019 into improbable-eng:master Aug 15, 2018
1 check passed

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

improbable-eng/grpc-web

Join GitHub today

websockets in grpcwebproxy #180

Conversation

JCGrant commented Apr 26, 2018 • edited

JCGrant added some commits Apr 26, 2018

johanbrandhorst reviewed Apr 26, 2018 View changes

This comment has been minimized.

This comment has been minimized.

johanbrandhorst reviewed Apr 26, 2018 View changes

This comment has been minimized.

This comment has been minimized.

johanbrandhorst reviewed Apr 26, 2018 View changes

This comment has been minimized.

JCGrant added some commits Apr 26, 2018

johanbrandhorst reviewed Apr 26, 2018 View changes

This comment has been minimized.

This comment has been minimized.

easyCZ commented Apr 26, 2018

This comment has been minimized.

timotheecour commented May 3, 2018

easyCZ approved these changes May 8, 2018 View changes

This comment has been minimized.

bianbian-org commented May 31, 2018

This comment has been minimized.

bianbian-org commented Jun 5, 2018

alexpdp7 referenced this pull request Jun 11, 2018

This comment has been minimized.

bianbian-org commented Jun 14, 2018 • edited

This comment has been minimized.

JCGrant commented Jun 14, 2018 • edited

This comment has been minimized.

bianbian-org commented Jun 15, 2018

bianbian-org referenced this pull request Jun 15, 2018

This comment has been minimized.

alexpdp7 commented Jun 17, 2018

This comment has been minimized.

jcloutz commented Jul 11, 2018

MarcusLongmuir approved these changes Aug 15, 2018 View changes

Hide details View details MarcusLongmuir merged commit 2cf3019 into improbable-eng:master Aug 15, 2018 1 check passed

1 check passed

JCGrant commented Apr 26, 2018 •

edited

johanbrandhorst reviewed Apr 26, 2018

View changes

johanbrandhorst reviewed Apr 26, 2018

View changes

johanbrandhorst reviewed Apr 26, 2018

View changes

johanbrandhorst reviewed Apr 26, 2018

View changes

easyCZ approved these changes May 8, 2018

View changes

bianbian-org commented Jun 14, 2018 •

edited

JCGrant commented Jun 14, 2018 •

edited

MarcusLongmuir approved these changes Aug 15, 2018

View changes

MarcusLongmuir merged commit `2cf3019` into improbable-eng:master Aug 15, 2018
1 check passed