Skip to content

Commit

Permalink
Fix gRPC-Web incorrect empty response handling (#298)
Browse files Browse the repository at this point in the history
gRPC-Web was incorrectly dropping empty responses. This PR resolves
#292.

A test was added in connectrpc/conformance#918
which reproduced this behavior when running locally:

```
% make testconformance
swift build -c release --product ConnectConformanceClient
Building for production...
[7/7] Linking ConnectConformanceClient
Build complete! (7.42s)
mv ./.build/release/ConnectConformanceClient .tmp/bin
PATH="/Users/michael/Development/connect-swift/.tmp/bin:/Users/michael/google-cloud-sdk/bin:/Users/michael/Library/Android/sdk/tools:/Users/michael/Library/Android/sdk/emulator:/Users/michael/Library/Android/sdk/platform-tools:/Users/michael/Library/Android/sdk/tools:/Users/michael/Library/Python/3.8/bin:/usr/local/bin:/usr/local/opt/libpq/bin:/usr/local/opt/ruby/bin:/Users/michael/depot_tools:/Users/michael/Development/bin:/Users/michael/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/Library/Apple/usr/bin:/usr/local/go/bin" connectconformance --trace --conf ./Tests/ConformanceClient/InvocationConfigs/urlsession.yaml --mode client .tmp/bin/ConnectConformanceClient httpclient=urlsession
FAILED: gRPC-Web Empty Responses/HTTPVersion:1/TLS:false/unary/empty-response:
	received an unexpected error:
	code:CODE_UNIMPLEMENTED message:"unary response has no message"
	expecting 1 response messages but instead got 0
---- HTTP Trace ----
 request>     0.000ms POST http://.../connectrpc.conformance.v1.ConformanceService/Unary HTTP/1.1
 request>             Accept: */*
 request>             Accept-Encoding: gzip, deflate
 request>             Accept-Language: en-US,en;q=0.9
 request>             Connection: keep-alive
 request>             Content-Length: 139
 request>             Content-Type: application/grpc-web+proto
 request>             Grpc-Accept-Encoding: gzip
 request>             User-Agent: ConnectConformanceClient (unknown version) CFNetwork/1498.700.2 Darwin/23.6.0
 request>             X-Expect-Codec: 1
 request>             X-Expect-Compression: 1
 request>             X-Expect-Http-Method: POST
 request>             X-Expect-Http-Version: 1
 request>             X-Expect-Protocol: 3
 request>             X-Expect-Tls: false
 request>             X-Test-Case-Name: gRPC-Web Empty Responses/HTTPVersion:1/TLS:false/unary/empty-response
 request>             X-User-Agent: @connectrpc/connect-swift
 request>
 request>     0.109ms message #1: prefix: flags=0, len=134
 request>             message #1: data: 134/134 bytes
 request>     0.110ms body end
response<     0.154ms 200 OK
response<             Content-Type: application/grpc-web+proto
response<             Vary: Origin
response<             X-Custom-Header: foo
response<
response<     0.155ms message #1: prefix: flags=0, len=0
response<     0.158ms message #2: prefix: flags=128, len=40
response<             message #2: data: 40/40 bytes
response<               eos: grpc-status: 0
response<               eos: x-custom-trailer: bing
response<               eos: 
response<     0.161ms body end
--------------------

Total cases: 904
903 passed, 1 failed
make: *** [testconformance] Error 1
```

With this change, the tests pass:

```
% make testconformance
swift build -c release --product ConnectConformanceClient
Building for production...
[5/5] Linking ConnectConformanceClient
Build complete! (3.80s)
mv ./.build/release/ConnectConformanceClient .tmp/bin
PATH="/Users/michael/Development/connect-swift/.tmp/bin:/Users/michael/google-cloud-sdk/bin:/Users/michael/Library/Android/sdk/tools:/Users/michael/Library/Android/sdk/emulator:/Users/michael/Library/Android/sdk/platform-tools:/Users/michael/Library/Android/sdk/tools:/Users/michael/Library/Python/3.8/bin:/usr/local/bin:/usr/local/opt/libpq/bin:/usr/local/opt/ruby/bin:/Users/michael/depot_tools:/Users/michael/Development/bin:/Users/michael/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/Library/Apple/usr/bin:/usr/local/go/bin" connectconformance --trace --conf ./Tests/ConformanceClient/InvocationConfigs/urlsession.yaml --mode client .tmp/bin/ConnectConformanceClient httpclient=urlsession
Total cases: 904
904 passed, 0 failed
PATH="/Users/michael/Development/connect-swift/.tmp/bin:/Users/michael/google-cloud-sdk/bin:/Users/michael/Library/Android/sdk/tools:/Users/michael/Library/Android/sdk/emulator:/Users/michael/Library/Android/sdk/platform-tools:/Users/michael/Library/Android/sdk/tools:/Users/michael/Library/Python/3.8/bin:/usr/local/bin:/usr/local/opt/libpq/bin:/usr/local/opt/ruby/bin:/Users/michael/depot_tools:/Users/michael/Development/bin:/Users/michael/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/Library/Apple/usr/bin:/usr/local/go/bin" connectconformance --trace --conf ./Tests/ConformanceClient/InvocationConfigs/nio.yaml --mode client .tmp/bin/ConnectConformanceClient httpclient=nio
Total cases: 1595
1595 passed, 0 failed
```

A release has not yet been tagged in the conformance repo, but once it
is I'll pull that test into Connect-Swift.

Signed-off-by: Michael Rebello <[email protected]>
  • Loading branch information
rebello95 authored Sep 8, 2024
1 parent 1c595ad commit 37e817f
Showing 1 changed file with 1 addition and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ private extension HTTPResponse {
error: error,
tracingInfo: self.tracingInfo
)
} else if message?.isEmpty != false {
} else if message == nil {
return HTTPResponse(
code: .unimplemented,
headers: self.headers,
Expand Down

0 comments on commit 37e817f

Please sign in to comment.