Skip to content

Commit

Permalink
Fix cancellation and timeout failures (#734)
Browse files Browse the repository at this point in the history
This makes various improvements to fix the failing cancellation and
timeout tests:

* Removes `ctx.Err()` checks in the clients since this can be an
ambiguous error on the client and could potentially be unrelated to the
error we received from the RPC framework.
* Adds logic to immediately send response headers on server and bidi
streams so that clients can be unblocked reading this data. Previously
this was causing clients to block on reading this data and was
preventing them from cancelling at the intended time.
* Adds docs to the protos specifying the above.
* Adds cancellation logic to gRPC `BidiStream`
* Configures some tests with appropriate delays so that the cancellation
is properly communicated.

All of the tests in `bidi-cancellation.yaml` and `bidi-timeouts.yaml`
have been folded into the `basic.yaml` suite of tests since these apply
to and pass all permutations.
  • Loading branch information
smaye81 authored Dec 12, 2023
1 parent 3e6cbb4 commit 4c7be66
Show file tree
Hide file tree
Showing 12 changed files with 454 additions and 385 deletions.
54 changes: 5 additions & 49 deletions internal/app/connectconformance/testsuites/basic.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
name: Basic
testCases:
# Unary Tests -----------------------------------------------------------------
- request:
testName: unary success
service: connectrpc.conformance.v1.ConformanceService
Expand Down Expand Up @@ -73,21 +74,7 @@ testCases:
responseTrailers:
- name: x-custom-trailer
value: ["bing","quux"]
- request:
testName: unary timeout
service: connectrpc.conformance.v1.ConformanceService
method: Unary
streamType: STREAM_TYPE_UNARY
timeoutMs: 200
requestMessages:
- "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest
responseDefinition:
responseData: "dGVzdCByZXNwb25zZQ=="
responseDelayMs: 500
# Override
expectedResponse:
error:
code: 4
# Client Stream Tests ---------------------------------------------------------
- request:
testName: client stream success
service: connectrpc.conformance.v1.ConformanceService
Expand Down Expand Up @@ -145,23 +132,7 @@ testCases:
streamType: STREAM_TYPE_CLIENT_STREAM
requestMessages:
- "@type": type.googleapis.com/connectrpc.conformance.v1.ClientStreamRequest
- request:
testName: client stream timeout
service: connectrpc.conformance.v1.ConformanceService
method: ClientStream
streamType: STREAM_TYPE_CLIENT_STREAM
timeoutMs: 200
requestMessages:
- "@type": type.googleapis.com/connectrpc.conformance.v1.ClientStreamRequest
responseDefinition:
responseData: "dGVzdCByZXNwb25zZQ=="
responseDelayMs: 500
- "@type": type.googleapis.com/connectrpc.conformance.v1.ClientStreamRequest
requestData: "dGVzdCByZXNwb25zZQ=="
# Override
expectedResponse:
error:
code: 4
# Server Stream Tests ---------------------------------------------------------
- request:
testName: server stream success
service: connectrpc.conformance.v1.ConformanceService
Expand Down Expand Up @@ -246,23 +217,7 @@ testCases:
responseTrailers:
- name: x-custom-trailer
value: ["bing","quux"]
- request:
testName: server stream timeout
service: connectrpc.conformance.v1.ConformanceService
method: ServerStream
streamType: STREAM_TYPE_SERVER_STREAM
timeoutMs: 200
requestMessages:
- "@type": type.googleapis.com/connectrpc.conformance.v1.ServerStreamRequest
responseDefinition:
responseDelayMs: 500
responseData:
- "dGVzdCByZXNwb25zZQ=="
- "dGVzdCByZXNwb25zZQ=="
# Override
expectedResponse:
error:
code: 4
# Bidi Stream Tests -----------------------------------------------------------
- request:
testName: bidi full duplex stream success
service: connectrpc.conformance.v1.ConformanceService
Expand Down Expand Up @@ -414,6 +369,7 @@ testCases:
value: ["Value1","Value2"]
requestMessages:
- "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest
# Misc Tests ------------------------------------------------------------------
- request:
testName: unimplemented
service: connectrpc.conformance.v1.ConformanceService
Expand Down
223 changes: 0 additions & 223 deletions internal/app/connectconformance/testsuites/bidi-cancellation.yaml

This file was deleted.

46 changes: 0 additions & 46 deletions internal/app/connectconformance/testsuites/bidi-timeouts.yaml

This file was deleted.

Loading

0 comments on commit 4c7be66

Please sign in to comment.