Review and improve test coverage of grpc-web

[jrudolph]

As observed in https://github.com/akka/akka-grpc/issues/1459, I introduced a regression in grpc-web that wasn't covered by tests (but which a simple smoke test should have uncovered).

[wb14123]

I found in the implementation #1462 , trailers are not sent with HTTP/1.1 . I'm not sure if I'm understanding the GRPC web protocol correctly but based on the doc: https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-WEB.md , it seems trailers should be encoded in the response body.

[jrudolph]

Thanks for the report, @wb14123. Do you have example code or a curl invocation which would should show that behavior?

[UnsolvedCypher]

@jrudolph I have run into this issue as well. If you pull the quickstart Dockers here then you can run curl 'http://localhost:8080/grpc.gateway.testing.EchoService/Echo' -X POST -H 'User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:96.0) Gecko/20100101 Firefox/96.0' -H 'Accept: application/grpc-web-text' -H 'Accept-Language: en-US,en;q=0.5' -H 'Accept-Encoding: gzip, deflate' -H 'custom-header-1: value1' -H 'Content-Type: application/grpc-web-text' -H 'X-User-Agent: grpc-web-javascript/0.1' -H 'X-Grpc-Web: 1' -H 'Origin: http://localhost:8081' -H 'Connection: keep-alive' -H 'Referer: http://localhost:8081/' -H 'Pragma: no-cache' -H 'Cache-Control: no-cache' --data-raw 'AAAAAAYKBGFvZXU=' to see what a proper response looks like.

There is a Scala example of what is needed to respond correctly here: https://github.com/improbable-eng/grpc-web/issues/194#issuecomment-970503606 . If there's any other information I can provide, please let me know!

[jrudolph]

Thanks, @UnsolvedCypher. I ran your instructions and got this response:

λ curl 'http://localhost:8080/grpc.gateway.testing.EchoService/Echo' -X POST -H 'User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:96.0) Gecko/20100101 Firefox/96.0' -H 'Accept: application/grpc-web-text' -H 'Accept-Language: en-US,en;q=0.5' -H 'Accept-Encoding: gzip, deflate' -H 'custom-header-1: value1' -H 'Content-Type: application/grpc-web-text' -H 'X-User-Agent: grpc-web-javascript/0.1' -H 'X-Grpc-Web: 1' -H 'Origin: http://localhost:8081' -H 'Connection: keep-alive' -H 'Referer: http://localhost:8081/' -H 'Pragma: no-cache' -H 'Cache-Control: no-cache' --data-raw 'AAAAAAYKBGFvZXU=' | 
  base64 -d | 
  hexdump -C

00000000  00 00 00 00 06 0a 04 61  6f 65 75 80 00 00 01 b1  |.......aoeu.....|
00000010  67 72 70 63 2d 73 74 61  74 75 73 3a 30 0d 0a 67  |grpc-status:0..g|
00000020  72 70 63 2d 6d 65 73 73  61 67 65 3a 4f 4b 0d 0a  |rpc-message:OK..|
00000030  75 73 65 72 2d 61 67 65  6e 74 3a 4d 6f 7a 69 6c  |user-agent:Mozil|
00000040  6c 61 2f 35 2e 30 20 28  58 31 31 3b 20 4c 69 6e  |la/5.0 (X11; Lin|
00000050  75 78 20 78 38 36 5f 36  34 3b 20 72 76 3a 39 36  |ux x86_64; rv:96|
00000060  2e 30 29 20 47 65 63 6b  6f 2f 32 30 31 30 30 31  |.0) Gecko/201001|
00000070  30 31 20 46 69 72 65 66  6f 78 2f 39 36 2e 30 0d  |01 Firefox/96.0.|
00000080  0a 61 63 63 65 70 74 3a  61 70 70 6c 69 63 61 74  |.accept:applicat|
00000090  69 6f 6e 2f 67 72 70 63  2d 77 65 62 2d 74 65 78  |ion/grpc-web-tex|
000000a0  74 0d 0a 61 63 63 65 70  74 2d 6c 61 6e 67 75 61  |t..accept-langua|
000000b0  67 65 3a 65 6e 2d 55 53  2c 65 6e 3b 71 3d 30 2e  |ge:en-US,en;q=0.|
000000c0  35 0d 0a 63 75 73 74 6f  6d 2d 68 65 61 64 65 72  |5..custom-header|
000000d0  2d 31 3a 76 61 6c 75 65  31 0d 0a 78 2d 75 73 65  |-1:value1..x-use|
000000e0  72 2d 61 67 65 6e 74 3a  67 72 70 63 2d 77 65 62  |r-agent:grpc-web|
000000f0  2d 6a 61 76 61 73 63 72  69 70 74 2f 30 2e 31 0d  |-javascript/0.1.|
00000100  0a 78 2d 67 72 70 63 2d  77 65 62 3a 31 0d 0a 6f  |.x-grpc-web:1..o|
00000110  72 69 67 69 6e 3a 68 74  74 70 3a 2f 2f 6c 6f 63  |rigin:http://loc|
00000120  61 6c 68 6f 73 74 3a 38  30 38 31 0d 0a 72 65 66  |alhost:8081..ref|
00000130  65 72 65 72 3a 68 74 74  70 3a 2f 2f 6c 6f 63 61  |erer:http://loca|
00000140  6c 68 6f 73 74 3a 38 30  38 31 2f 0d 0a 70 72 61  |lhost:8081/..pra|
00000150  67 6d 61 3a 6e 6f 2d 63  61 63 68 65 0d 0a 63 61  |gma:no-cache..ca|
00000160  63 68 65 2d 63 6f 6e 74  72 6f 6c 3a 6e 6f 2d 63  |che-control:no-c|
00000170  61 63 68 65 0d 0a 78 2d  66 6f 72 77 61 72 64 65  |ache..x-forwarde|
00000180  64 2d 70 72 6f 74 6f 3a  68 74 74 70 0d 0a 78 2d  |d-proto:http..x-|
00000190  72 65 71 75 65 73 74 2d  69 64 3a 32 34 33 36 39  |request-id:24369|
000001a0  35 39 34 2d 33 64 33 37  2d 34 35 37 35 2d 38 64  |594-3d37-4575-8d|
000001b0  32 31 2d 66 36 37 35 63  63 66 33 33 32 35 33 0d  |21-f675ccf33253.|
000001c0  0a                                                |.|
000001c1

Right now no one is looking into this. If someone could provide an equivalent akka-grpc example to compare against, that would be great!

[jrudolph]

Ok, I see the problem, since the Strict optimization the gRPC-web part of the pipeline doesn't see the trailer anymore for non-streamed responses.

[jrudolph]

The best solution would probably be to change GrpcProtocolWriter.encodeDataToFrameBytes into a new method that renders the full entity, taking into account the trailers.

[jrudolph]

I posted a preliminary fix at https://github.com/akka/akka-grpc/pull/1552/files, maybe someone wants to build it and test if it works?

[UnsolvedCypher]

@jrudolph I would be happy to test this out, how do I do that? I have cloned the repository, is there an sbt task I can run to install your PR version locally? And how do I then include it in my project?

[jrudolph]

You can try sbt +publishLocal and see which version that publishes.

[wb14123]

@jrudolph I tried the fix in my project and it works! I'm using Dart (https://github.com/grpc/grpc-dart) as the GRPC client.

[UnsolvedCypher]

@jrudolph I am sorry for the late reply. Unfortunately I am still having trouble even after your fix. I have created a repository that reproduces this issue here.

To see the issue, you can first start the Scala project with sbt at the root of the repository, then in the client folder, run npm install and then node client.js (you will need NodeJS installed for this). You will see that the attempt to contact the grpc-web server results in a crash.

If there is any way I can help with reproducing or testing this issue, please let me know!

[UnsolvedCypher]

Sorry, please disregard my above comment! I double-checked and I built your fix incorrectly- it does in fact work. Thanks for the quick fix and sorry for the false alarm! It would be great to get a new release soon though so I do not have to build against a locally built plugin.

[jrudolph]

Sorry, please disregard my above comment! I double-checked and I built your fix incorrectly- it does in fact work. Thanks for the quick fix and sorry for the false alarm! It would be great to get a new release soon though so I do not have to build against a locally built plugin.

Great, thanks for checking again!

[jrudolph]

In the meantime, there are snapshots from main here: https://oss.sonatype.org/content/repositories/snapshots/com/lightbend/akka/grpc/

[raboof]

(I did some additional manual testing, #1585, and it works for me as well. Let's keep this issue open to improve that test coverage, though ;) - anyone who would like gRPC-Web support not to break again in the future is welcome to contribute tests :smile: )