search

connectrpc / connect-swift

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

Properly trigger client-side timeouts per-stream #272

Closed rebello95 closed 5 months ago

rebello95 commented 5 months ago

Per the discussion in https://github.com/connectrpc/connect-swift/pull/265#issuecomment-2138141935, this PR avoids setting timeouts on the HTTP client instantiated for each conformance test (URLSessionHTTPClient or NIOHTTPClient), and instead relies on the ProtocolClient to trigger timeouts via request headers (server-triggered timeouts) and client-side timers (client-triggered timeouts). This caused some conformance tests for stream timeouts to fail because the HTTP clients were no longer triggering timeouts internally.

This PR updates the ProtocolClient to handle timeouts for streams correctly (both setting outbound request headers and starting local timers for unary requests and streams). Without this change, it was possible for consumers to configure a timeout on the HTTP client (i.e., using URLSessionConfiguration), but this would only partially work since outbound request headers such as grpc-timeout would not be set. After this change, Connect will handle both cases if timeout is set on ProtocolClientConfig.

This is similar to the logic in connect-kotlin here: https://github.com/connectrpc/connect-kotlin/commit/76b89e7fc716addd6c80bc4977de6bfbd7241959#diff-cc7f51337bad192f2603a53312146bcd36a7c2bc3fc860327dee53cb3e326749R333-R341