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.

Sign up for GitHub

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

Jump to bottom

Version 0.14.0 breaks some gRPC-Web API calls that return an empty response #292

Closed
bourquep opened this issue Aug 22, 2024 · 3 comments · Fixed by #298
Closed

Version 0.14.0 breaks some gRPC-Web API calls that return an empty response #292

bourquep opened this issue Aug 22, 2024 · 3 comments · Fixed by #298

Comments

@bourquep
Copy link

After updating my buf-generated SDKs from Connect 0.13.0 to 0.14.0, some gRPC-Web API calls started failing with unimplemented - unary response has no message. These same API calls succeed with Connect 0.13.0. My gRPC-Web server is envoy, who proxies the calls to my .Net gRPC services.

Here are the sniffed request/response headers from a call that fails with 0.14.0:

Request

POST /studyo.today.planners.v1.Planners/GetDetailedCourseSections HTTP/1.1
Host: grpc-dev.studyo.today
x-user-agent: @connectrpc/connect-swift
Accept: */*
baggage: sentry-environment=production,sentry-public_key=XXX,sentry-release=XXX,sentry-trace_id=XXX
Authorization: Bearer XXX
sentry-trace: XXX
Accept-Encoding: gzip, deflate, br
Accept-Language: fr
Content-Type: application/grpc-web+proto
X-Today-TimeZone: America/Toronto
Content-Length: 48
User-Agent: Studyo%20Go/0 CFNetwork/1406.0.4 Darwin/23.6.0
grpc-accept-encoding: gzip
Connection: keep-alive

Response

HTTP/1.1 200 OK
content-type: application/grpc-web+proto
date: Thu, 22 Aug 2024 18:51:17 GMT
server: envoy
content-language: fr
x-today-traceid: f785eb8186239a8b73ad50d93b46a8f4
x-envoy-upstream-service-time: 241
strict-transport-security: max-age=31536000; includeSubDomains
x-content-type-options: nosniff
content-encoding: gzip
vary: Accept-Encoding
transfer-encoding: chunked

Response body (binary)

image

It looks like this PR, which was deployed in v0.14.0, introduces extra conformance checks on the response and somehow considers my API’s response as invalid:

https://github.com/connectrpc/connect-swift/pull/271/files#diff-b629e2bc5c08c393cb149ab967c5972c871c26a23889647075103a07d88e0e38R81
@jhump
Copy link
Member

jhump commented Aug 22, 2024

The grpc-web response contains a zero-length response message, followed by the trailers. Perhaps the message being zero length is tickling something that makes the framework think the message was absent. I'll look at adding a case that can exercise this to the conformance test suite.

@rebello95
Copy link
Collaborator

Thanks @jhump - when do you think we can have a test case for that? I think it'd be great to include it with the fix for this so we can verify it with tests.

@rebello95 rebello95 mentioned this issue Sep 7, 2024
Merged
@rebello95
Copy link
Collaborator

#298

rebello95 added a commit that referenced this issue Sep 8, 2024
@rebello95
Fix gRPC-Web incorrect empty response handling (#298)
37e817f 
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]>
rebello95 added a commit that referenced this issue Sep 28, 2024
@rebello95
Update conformance to v1.0.4
255caac 
Updates to https://github.com/connectrpc/conformance/releases/tag/v1.0.4.

This validates the fix in #298 for #292 as well.
@rebello95 rebello95 mentioned this issue Sep 28, 2024
Merged
rebello95 added a commit that referenced this issue Sep 28, 2024
@rebello95
Update conformance to v1.0.4
25b83ee 
Updates to https://github.com/connectrpc/conformance/releases/tag/v1.0.4.

This validates the fix in #298 for #292 as well.

Signed-off-by: Michael Rebello <[email protected]>
rebello95 added a commit that referenced this issue Oct 1, 2024
@rebello95
Update conformance to v1.0.4 (#302)
885a10b 
Updates to
https://github.com/connectrpc/conformance/releases/tag/v1.0.4.

This validates the fix in
#298 for
#292 as well.

Signed-off-by: Michael Rebello <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix gRPC-Web incorrect empty response handling connectrpc/connect-swift
3 participants
@bourquep @jhump @rebello95