search

connectrpc / connect-swift

The Swift implementation of Connect: Protobuf RPC that works.
https://connectrpc.com/docs/swift/getting-started
Apache License 2.0
93 stars 17 forks source link

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

Closed bourquep closed 5 days ago

bourquep commented 3 weeks ago

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 commented 3 weeks ago

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 commented 2 weeks ago

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 commented 6 days ago

https://github.com/connectrpc/connect-swift/pull/298