From 4c7be66322f8b87e53a7901977f3824a7940f7f3 Mon Sep 17 00:00:00 2001 From: Steve Ayers Date: Tue, 12 Dec 2023 16:52:38 -0500 Subject: [PATCH] Fix cancellation and timeout failures (#734) 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. --- .../connectconformance/testsuites/basic.yaml | 54 +-- .../testsuites/bidi-cancellation.yaml | 223 ------------ .../testsuites/bidi-timeouts.yaml | 46 --- .../testsuites/cancellation.yaml | 322 +++++++++++++++--- .../testsuites/timeouts.yaml | 94 +++++ internal/app/grpcclient/impl.go | 35 +- internal/app/grpcserver/impl.go | 9 +- internal/app/referenceclient/impl.go | 5 - internal/app/referenceserver/impl.go | 11 + .../conformancev1connect/service.connect.go | 16 + .../conformance/v1/service_grpc.pb.go | 16 + proto/connectrpc/conformance/v1/service.proto | 8 + 12 files changed, 454 insertions(+), 385 deletions(-) delete mode 100644 internal/app/connectconformance/testsuites/bidi-cancellation.yaml delete mode 100644 internal/app/connectconformance/testsuites/bidi-timeouts.yaml create mode 100644 internal/app/connectconformance/testsuites/timeouts.yaml diff --git a/internal/app/connectconformance/testsuites/basic.yaml b/internal/app/connectconformance/testsuites/basic.yaml index 2b67e75b..042f1164 100644 --- a/internal/app/connectconformance/testsuites/basic.yaml +++ b/internal/app/connectconformance/testsuites/basic.yaml @@ -1,5 +1,6 @@ name: Basic testCases: +# Unary Tests ----------------------------------------------------------------- - request: testName: unary success service: connectrpc.conformance.v1.ConformanceService @@ -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 @@ -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 @@ -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 @@ -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 diff --git a/internal/app/connectconformance/testsuites/bidi-cancellation.yaml b/internal/app/connectconformance/testsuites/bidi-cancellation.yaml deleted file mode 100644 index fb0453d6..00000000 --- a/internal/app/connectconformance/testsuites/bidi-cancellation.yaml +++ /dev/null @@ -1,223 +0,0 @@ -name: Bidi Cancellation -# TODO - Need to fix the timeout issues with wrapping context deadline. Right now -# these tests break for gRPC until that issue is addressed -relevantProtocols: - - PROTOCOL_CONNECT - - PROTOCOL_GRPC_WEB -testCases: -- request: - testName: bidi full duplex cancel after zero responses - service: connectrpc.conformance.v1.ConformanceService - method: BidiStream - streamType: STREAM_TYPE_FULL_DUPLEX_BIDI_STREAM - cancel: - afterNumResponses: 0 - requestMessages: - - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest - responseDefinition: - responseData: - - "dGVzdCByZXNwb25zZQ==" - - "dGVzdCByZXNwb25zZQ==" - fullDuplex: true - - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest - requestData: "dGVzdCByZXNwb25zZQ==" - # Override - expectedResponse: - error: - code: 1 -testCases: -- request: - testName: bidi full duplex cancel after responses - service: connectrpc.conformance.v1.ConformanceService - method: BidiStream - streamType: STREAM_TYPE_FULL_DUPLEX_BIDI_STREAM - cancel: - afterNumResponses: 1 - requestMessages: - - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest - responseDefinition: - responseData: - - "dGVzdCByZXNwb25zZQ==" - - "dGVzdCByZXNwb25zZQ==" - fullDuplex: true - - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest - requestData: "dGVzdCByZXNwb25zZQ==" - # Override - expectedResponse: - payloads: - - data: "dGVzdCByZXNwb25zZQ==" - requestInfo: - requests: - - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest - responseDefinition: - responseData: - - "dGVzdCByZXNwb25zZQ==" - - "dGVzdCByZXNwb25zZQ==" - fullDuplex: true - error: - code: 1 -# TODO - This is returning a Code Unknown (prob related to context bugs?) -# TODO - also is 2 responses correct? I believe so. -# - request: -# testName: bidi full duplex cancel before close send -# service: connectrpc.conformance.v1.ConformanceService -# method: BidiStream -# streamType: STREAM_TYPE_FULL_DUPLEX_BIDI_STREAM -# cancel: -# beforeCloseSend: -# requestMessages: -# - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest -# responseDefinition: -# responseData: -# - "dGVzdCByZXNwb25zZQ==" -# - "dGVzdCByZXNwb25zZQ==" -# fullDuplex: true -# - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest -# requestData: "dGVzdCByZXNwb25zZQ==" -# # Override -# expectedResponse: -# payloads: -# - data: "dGVzdCByZXNwb25zZQ==" -# requestInfo: -# requests: -# - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest -# responseDefinition: -# responseData: -# - "dGVzdCByZXNwb25zZQ==" -# - "dGVzdCByZXNwb25zZQ==" -# fullDuplex: true -# - data: "dGVzdCByZXNwb25zZQ==" -# requestInfo: -# requests: -# - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest -# requestData: "dGVzdCByZXNwb25zZQ==" -# error: -# code: 1 -# TODO - This is returning a Code Unknown (prob related to context bugs?) -# - request: -# testName: bidi full duplex cancel after close send -# service: connectrpc.conformance.v1.ConformanceService -# method: BidiStream -# streamType: STREAM_TYPE_FULL_DUPLEX_BIDI_STREAM -# cancel: -# afterCloseSendMs: 100 -# requestMessages: -# - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest -# responseDefinition: -# responseDelayMs: 250 -# responseData: -# - "dGVzdCByZXNwb25zZQ==" -# - "dGVzdCByZXNwb25zZQ==" -# fullDuplex: true -# - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest -# requestData: "dGVzdCByZXNwb25zZQ==" -# # Override -# expectedResponse: -# payloads: -# - data: "dGVzdCByZXNwb25zZQ==" -# requestInfo: -# requests: -# - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest -# responseDefinition: -# responseDelayMs: 250 -# responseData: -# - "dGVzdCByZXNwb25zZQ==" -# - "dGVzdCByZXNwb25zZQ==" -# fullDuplex: true -# - data: "dGVzdCByZXNwb25zZQ==" -# requestInfo: -# requests: -# - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest -# requestData: "dGVzdCByZXNwb25zZQ==" -# error: -# code: 1 -- request: - testName: bidi half duplex cancel after zero responses - service: connectrpc.conformance.v1.ConformanceService - method: BidiStream - streamType: STREAM_TYPE_HALF_DUPLEX_BIDI_STREAM - cancel: - afterNumResponses: 0 - requestMessages: - - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest - responseDefinition: - responseData: - - "dGVzdCByZXNwb25zZQ==" - - "dGVzdCByZXNwb25zZQ==" - - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest - requestData: "dGVzdCByZXNwb25zZQ==" - # Override - expectedResponse: - error: - code: 1 -- request: - testName: bidi half duplex cancel after responses - service: connectrpc.conformance.v1.ConformanceService - method: BidiStream - streamType: STREAM_TYPE_HALF_DUPLEX_BIDI_STREAM - cancel: - afterNumResponses: 1 - requestMessages: - - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest - responseDefinition: - responseData: - - "dGVzdCByZXNwb25zZQ==" - - "dGVzdCByZXNwb25zZQ==" - - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest - requestData: "dGVzdCByZXNwb25zZQ==" - # Override - expectedResponse: - payloads: - - data: "dGVzdCByZXNwb25zZQ==" - requestInfo: - requests: - - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest - responseDefinition: - responseData: - - "dGVzdCByZXNwb25zZQ==" - - "dGVzdCByZXNwb25zZQ==" - - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest - requestData: "dGVzdCByZXNwb25zZQ==" - error: - code: 1 -# TODO - This is returning a Code Unknown (prob related to context bugs?) -# - request: -# testName: bidi half duplex cancel before close send -# service: connectrpc.conformance.v1.ConformanceService -# method: BidiStream -# streamType: STREAM_TYPE_HALF_DUPLEX_BIDI_STREAM -# cancel: -# beforeCloseSend: -# requestMessages: -# - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest -# responseDefinition: -# responseData: -# - "dGVzdCByZXNwb25zZQ==" -# - "dGVzdCByZXNwb25zZQ==" -# - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest -# requestData: "dGVzdCByZXNwb25zZQ==" -# # Override -# expectedResponse: -# error: -# code: 1 -# TODO - This is returning a Code Unknown (prob related to context bugs?) -# - request: -# testName: bidi half duplex cancel after close send -# service: connectrpc.conformance.v1.ConformanceService -# method: BidiStream -# streamType: STREAM_TYPE_HALF_DUPLEX_BIDI_STREAM -# cancel: -# afterCloseSendMs: 100 -# requestMessages: -# - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest -# responseDefinition: -# responseDelayMs: 250 -# responseData: -# - "dGVzdCByZXNwb25zZQ==" -# - "dGVzdCByZXNwb25zZQ==" -# - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest -# requestData: "dGVzdCByZXNwb25zZQ==" -# # Override -# expectedResponse: -# error: -# code: 1 diff --git a/internal/app/connectconformance/testsuites/bidi-timeouts.yaml b/internal/app/connectconformance/testsuites/bidi-timeouts.yaml deleted file mode 100644 index eb3b2889..00000000 --- a/internal/app/connectconformance/testsuites/bidi-timeouts.yaml +++ /dev/null @@ -1,46 +0,0 @@ -name: Bidi Timeouts -# TODO - Need to fix the timeout issues with wrapping context deadline. Right now -# these tests break for gRPC until that issue is addressed -relevantProtocols: - - PROTOCOL_CONNECT - - PROTOCOL_GRPC_WEB -testCases: -- request: - testName: bidi full duplex timeout - service: connectrpc.conformance.v1.ConformanceService - method: BidiStream - streamType: STREAM_TYPE_FULL_DUPLEX_BIDI_STREAM - timeoutMs: 200 - requestMessages: - - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest - responseDefinition: - responseDelayMs: 500 - responseData: - - "dGVzdCByZXNwb25zZQ==" - - "dGVzdCByZXNwb25zZQ==" - fullDuplex: true - - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest - requestData: "dGVzdCByZXNwb25zZQ==" - # Override - expectedResponse: - error: - code: 4 -- request: - testName: bidi half duplex timeout - service: connectrpc.conformance.v1.ConformanceService - method: BidiStream - streamType: STREAM_TYPE_HALF_DUPLEX_BIDI_STREAM - timeoutMs: 200 - requestMessages: - - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest - responseDefinition: - responseDelayMs: 500 - responseData: - - "dGVzdCByZXNwb25zZQ==" - - "dGVzdCByZXNwb25zZQ==" - - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest - requestData: "dGVzdCByZXNwb25zZQ==" - # Override - expectedResponse: - error: - code: 4 diff --git a/internal/app/connectconformance/testsuites/cancellation.yaml b/internal/app/connectconformance/testsuites/cancellation.yaml index 612a0590..10420728 100644 --- a/internal/app/connectconformance/testsuites/cancellation.yaml +++ b/internal/app/connectconformance/testsuites/cancellation.yaml @@ -1,56 +1,8 @@ name: Cancellation +# Cancellations only apply to clients under test +mode: TEST_MODE_CLIENT testCases: -# TODO - The server stream tests are failing with grpc due to issues with -# blocking on sender header metadata. -# - request: -# testName: server stream cancel after zero responses -# service: connectrpc.conformance.v1.ConformanceService -# method: ServerStream -# streamType: STREAM_TYPE_SERVER_STREAM -# cancel: -# afterNumResponses: 0 -# requestMessages: -# - "@type": type.googleapis.com/connectrpc.conformance.v1.ServerStreamRequest -# responseDefinition: -# responseHeaders: -# - name: x-custom-header -# value: ["foo","bar","baz"] -# responseData: -# - "dGVzdCByZXNwb25zZQ==" -# - "dGVzdCByZXNwb25zZQ==" -# responseTrailers: -# - name: x-custom-trailer -# value: ["bing","quux"] -# # Override -# expectedResponse: -# error: -# code: 1 -# - request: -# testName: server stream cancel after responses -# service: connectrpc.conformance.v1.ConformanceService -# method: ServerStream -# streamType: STREAM_TYPE_SERVER_STREAM -# cancel: -# afterNumResponses: 1 -# requestMessages: -# - "@type": type.googleapis.com/connectrpc.conformance.v1.ServerStreamRequest -# responseDefinition: -# responseData: -# - "dGVzdCByZXNwb25zZQ==" -# - "dGVzdCByZXNwb25zZQ==" -# # Override -# expectedResponse: -# payloads: -# - data: "dGVzdCByZXNwb25zZQ==" -# requestInfo: -# requests: -# - "@type": type.googleapis.com/connectrpc.conformance.v1.ServerStreamRequest -# responseDefinition: -# responseData: -# - "dGVzdCByZXNwb25zZQ==" -# - "dGVzdCByZXNwb25zZQ==" -# error: -# code: 1 +# Client Stream Tests --------------------------------------------------------- - request: testName: client stream cancel before close send service: connectrpc.conformance.v1.ConformanceService @@ -98,3 +50,271 @@ testCases: expectedResponse: error: code: 1 +# Server Stream Tests --------------------------------------------------------- +- request: + testName: server stream cancel after zero responses + service: connectrpc.conformance.v1.ConformanceService + method: ServerStream + streamType: STREAM_TYPE_SERVER_STREAM + cancel: + afterNumResponses: 0 + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.ServerStreamRequest + responseDefinition: + responseDelayMs: 100 + responseHeaders: + - name: x-custom-header + value: ["foo","bar","baz"] + responseData: + - "dGVzdCByZXNwb25zZQ==" + - "dGVzdCByZXNwb25zZQ==" + responseTrailers: + - name: x-custom-trailer + value: ["bing","quux"] + # Override + expectedResponse: + error: + code: 1 +- request: + testName: server stream cancel after responses + service: connectrpc.conformance.v1.ConformanceService + method: ServerStream + streamType: STREAM_TYPE_SERVER_STREAM + cancel: + afterNumResponses: 1 + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.ServerStreamRequest + responseDefinition: + responseDelayMs: 100 + responseData: + - "dGVzdCByZXNwb25zZQ==" + - "dGVzdCByZXNwb25zZQ==" + # Override + expectedResponse: + payloads: + - data: "dGVzdCByZXNwb25zZQ==" + requestInfo: + requests: + - "@type": type.googleapis.com/connectrpc.conformance.v1.ServerStreamRequest + responseDefinition: + responseDelayMs: 100 + responseData: + - "dGVzdCByZXNwb25zZQ==" + - "dGVzdCByZXNwb25zZQ==" + error: + code: 1 +# Bidi Stream Tests ----------------------------------------------------------- +- request: + testName: bidi full duplex cancel after zero responses + service: connectrpc.conformance.v1.ConformanceService + method: BidiStream + streamType: STREAM_TYPE_FULL_DUPLEX_BIDI_STREAM + cancel: + afterNumResponses: 0 + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + responseDefinition: + responseDelayMs: 100 + responseData: + - "dGVzdCByZXNwb25zZQ==" + - "dGVzdCByZXNwb25zZQ==" + fullDuplex: true + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + requestData: "dGVzdCByZXNwb25zZQ==" + # Override + expectedResponse: + error: + code: 1 +- request: + testName: bidi full duplex cancel after responses + service: connectrpc.conformance.v1.ConformanceService + method: BidiStream + streamType: STREAM_TYPE_FULL_DUPLEX_BIDI_STREAM + cancel: + afterNumResponses: 1 + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + responseDefinition: + responseDelayMs: 100 + responseData: + - "dGVzdCByZXNwb25zZQ==" + - "dGVzdCByZXNwb25zZQ==" + fullDuplex: true + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + requestData: "dGVzdCByZXNwb25zZQ==" + # Override + expectedResponse: + payloads: + - data: "dGVzdCByZXNwb25zZQ==" + requestInfo: + requests: + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + responseDefinition: + responseDelayMs: 100 + responseData: + - "dGVzdCByZXNwb25zZQ==" + - "dGVzdCByZXNwb25zZQ==" + fullDuplex: true + error: + code: 1 +- request: + testName: bidi full duplex cancel before close send + service: connectrpc.conformance.v1.ConformanceService + method: BidiStream + streamType: STREAM_TYPE_FULL_DUPLEX_BIDI_STREAM + cancel: + beforeCloseSend: + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + responseDefinition: + responseData: + - "dGVzdCByZXNwb25zZQ==" + - "dGVzdCByZXNwb25zZQ==" + fullDuplex: true + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + requestData: "dGVzdCByZXNwb25zZQ==" + # Override + expectedResponse: + payloads: + - data: "dGVzdCByZXNwb25zZQ==" + requestInfo: + requests: + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + responseDefinition: + responseData: + - "dGVzdCByZXNwb25zZQ==" + - "dGVzdCByZXNwb25zZQ==" + fullDuplex: true + - data: "dGVzdCByZXNwb25zZQ==" + requestInfo: + requests: + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + requestData: "dGVzdCByZXNwb25zZQ==" + error: + code: 1 +- request: + testName: bidi full duplex cancel after close send + service: connectrpc.conformance.v1.ConformanceService + method: BidiStream + streamType: STREAM_TYPE_FULL_DUPLEX_BIDI_STREAM + cancel: + afterCloseSendMs: 0 + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + responseDefinition: + responseData: + - "dGVzdCByZXNwb25zZQ==" + - "dGVzdCByZXNwb25zZQ==" + fullDuplex: true + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + requestData: "dGVzdCByZXNwb25zZQ==" + # Override + expectedResponse: + payloads: + - data: "dGVzdCByZXNwb25zZQ==" + requestInfo: + requests: + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + responseDefinition: + responseData: + - "dGVzdCByZXNwb25zZQ==" + - "dGVzdCByZXNwb25zZQ==" + fullDuplex: true + - data: "dGVzdCByZXNwb25zZQ==" + requestInfo: + requests: + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + requestData: "dGVzdCByZXNwb25zZQ==" + error: + code: 1 +- request: + testName: bidi half duplex cancel after zero responses + service: connectrpc.conformance.v1.ConformanceService + method: BidiStream + streamType: STREAM_TYPE_HALF_DUPLEX_BIDI_STREAM + cancel: + afterNumResponses: 0 + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + responseDefinition: + responseDelayMs: 100 + responseData: + - "dGVzdCByZXNwb25zZQ==" + - "dGVzdCByZXNwb25zZQ==" + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + requestData: "dGVzdCByZXNwb25zZQ==" + # Override + expectedResponse: + error: + code: 1 +- request: + testName: bidi half duplex cancel after responses + service: connectrpc.conformance.v1.ConformanceService + method: BidiStream + streamType: STREAM_TYPE_HALF_DUPLEX_BIDI_STREAM + cancel: + afterNumResponses: 1 + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + responseDefinition: + responseDelayMs: 100 + responseData: + - "dGVzdCByZXNwb25zZQ==" + - "dGVzdCByZXNwb25zZQ==" + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + requestData: "dGVzdCByZXNwb25zZQ==" + # Override + expectedResponse: + payloads: + - data: "dGVzdCByZXNwb25zZQ==" + requestInfo: + requests: + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + responseDefinition: + responseDelayMs: 100 + responseData: + - "dGVzdCByZXNwb25zZQ==" + - "dGVzdCByZXNwb25zZQ==" + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + requestData: "dGVzdCByZXNwb25zZQ==" + error: + code: 1 +- request: + testName: bidi half duplex cancel before close send + service: connectrpc.conformance.v1.ConformanceService + method: BidiStream + streamType: STREAM_TYPE_HALF_DUPLEX_BIDI_STREAM + cancel: + beforeCloseSend: + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + responseDefinition: + responseData: + - "dGVzdCByZXNwb25zZQ==" + - "dGVzdCByZXNwb25zZQ==" + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + requestData: "dGVzdCByZXNwb25zZQ==" + # Override + expectedResponse: + error: + code: 1 +- request: + testName: bidi half duplex cancel after close send + service: connectrpc.conformance.v1.ConformanceService + method: BidiStream + streamType: STREAM_TYPE_HALF_DUPLEX_BIDI_STREAM + cancel: + afterCloseSendMs: 0 + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + responseDefinition: + responseDelayMs: 250 + responseData: + - "dGVzdCByZXNwb25zZQ==" + - "dGVzdCByZXNwb25zZQ==" + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + requestData: "dGVzdCByZXNwb25zZQ==" + # Override + expectedResponse: + error: + code: 1 diff --git a/internal/app/connectconformance/testsuites/timeouts.yaml b/internal/app/connectconformance/testsuites/timeouts.yaml new file mode 100644 index 00000000..d628f2ed --- /dev/null +++ b/internal/app/connectconformance/testsuites/timeouts.yaml @@ -0,0 +1,94 @@ +name: Timeouts +testCases: +# Unary Tests ----------------------------------------------------------------- +- 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 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 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 timeout + service: connectrpc.conformance.v1.ConformanceService + method: BidiStream + streamType: STREAM_TYPE_FULL_DUPLEX_BIDI_STREAM + timeoutMs: 200 + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + responseDefinition: + responseDelayMs: 500 + responseData: + - "dGVzdCByZXNwb25zZQ==" + - "dGVzdCByZXNwb25zZQ==" + fullDuplex: true + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + requestData: "dGVzdCByZXNwb25zZQ==" + # Override + expectedResponse: + error: + code: 4 +- request: + testName: bidi half duplex timeout + service: connectrpc.conformance.v1.ConformanceService + method: BidiStream + streamType: STREAM_TYPE_HALF_DUPLEX_BIDI_STREAM + timeoutMs: 200 + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + responseDefinition: + responseDelayMs: 500 + responseData: + - "dGVzdCByZXNwb25zZQ==" + - "dGVzdCByZXNwb25zZQ==" + - "@type": type.googleapis.com/connectrpc.conformance.v1.BidiStreamRequest + requestData: "dGVzdCByZXNwb25zZQ==" + # Override + expectedResponse: + error: + code: 4 diff --git a/internal/app/grpcclient/impl.go b/internal/app/grpcclient/impl.go index 6a7c4d4a..9628ec98 100644 --- a/internal/app/grpcclient/impl.go +++ b/internal/app/grpcclient/impl.go @@ -271,6 +271,9 @@ func (i *invoker) bidiStream( ctx context.Context, ccr *v1.ClientCompatRequest, ) (result *v1.ClientResponseResult, retErr error) { + ctx, cancel := context.WithCancel(ctx) + defer cancel() + result = &v1.ClientResponseResult{ ConnectErrorRaw: nil, // TODO } @@ -285,13 +288,15 @@ func (i *invoker) bidiStream( fullDuplex := ccr.StreamType == v1.StreamType_STREAM_TYPE_FULL_DUPLEX_BIDI_STREAM + // Cancellation timing + timing, err := internal.GetCancelTiming(ccr.Cancel) + if err != nil { + return nil, err + } + var protoErr *v1.Error + totalRcvd := 0 for _, msg := range ccr.RequestMessages { - if err := ctx.Err(); err != nil { - // If an error was returned, convert it to a proto Error - protoErr = grpcutil.ConvertGrpcToProtoError(err) - break - } bsr := &v1.BidiStreamRequest{} if err := msg.UnmarshalTo(bsr); err != nil { // Return the error and nil result because this is an @@ -312,8 +317,12 @@ func (i *invoker) bidiStream( break } if fullDuplex { + if totalRcvd == timing.AfterNumResponses { + cancel() + } // If this is a full duplex stream, receive a response for each request msg, err := stream.Recv() + totalRcvd++ if err != nil { if !errors.Is(err, io.EOF) { // If an error was returned that is not an EOF, convert it @@ -339,12 +348,21 @@ func (i *invoker) bidiStream( } }() + if timing.BeforeCloseSend != nil { + cancel() + } + // Sends are done, close the send side of the stream err = stream.CloseSend() if err != nil && protoErr == nil { protoErr = grpcutil.ConvertGrpcToProtoError(err) } + if timing.AfterCloseSendMs >= 0 { + time.Sleep(time.Duration(timing.AfterCloseSendMs) * time.Millisecond) + cancel() + } + // Once the send side is closed, header metadata is ready to be read hdr, err = stream.Header() if err != nil && protoErr == nil { @@ -359,12 +377,11 @@ func (i *invoker) bidiStream( // Receive any remaining responses for { - if err := ctx.Err(); err != nil { - // If an error was returned, convert it to a proto Error - protoErr = grpcutil.ConvertGrpcToProtoError(err) - break + if totalRcvd == timing.AfterNumResponses { + cancel() } msg, err := stream.Recv() + totalRcvd++ if err != nil { if !errors.Is(err, io.EOF) { // If an error was returned that is not an EOF, convert it diff --git a/internal/app/grpcserver/impl.go b/internal/app/grpcserver/impl.go index 158bc9e0..8ae68b5e 100644 --- a/internal/app/grpcserver/impl.go +++ b/internal/app/grpcserver/impl.go @@ -150,7 +150,8 @@ func (c *conformanceServiceServer) ServerStream( responseDefinition := req.ResponseDefinition if responseDefinition != nil { //nolint:nestif headerMD := grpcutil.ConvertProtoHeaderToMetadata(responseDefinition.ResponseHeaders) - if err := stream.SetHeader(headerMD); err != nil { + // Immediately send the headers on the stream so that metadata can be read by the client + if err := stream.SendHeader(headerMD); err != nil { return err } @@ -235,7 +236,8 @@ func (c *conformanceServiceServer) BidiStream( if responseDefinition != nil { headerMD := grpcutil.ConvertProtoHeaderToMetadata(responseDefinition.ResponseHeaders) - if err := stream.SetHeader(headerMD); err != nil { + // Immediately send the headers on the stream so that metadata can be read by the client + if err := stream.SendHeader(headerMD); err != nil { return err } @@ -286,6 +288,9 @@ func (c *conformanceServiceServer) BidiStream( // where the requested responses are greater than the total requests. if responseDefinition != nil { //nolint:nestif for ; respNum < len(responseDefinition.ResponseData); respNum++ { + if err := stream.Context().Err(); err != nil { + return err + } resp := &v1.BidiStreamResponse{ Payload: &v1.ConformancePayload{ Data: responseDefinition.ResponseData[respNum], diff --git a/internal/app/referenceclient/impl.go b/internal/app/referenceclient/impl.go index 1708267a..97c29f87 100644 --- a/internal/app/referenceclient/impl.go +++ b/internal/app/referenceclient/impl.go @@ -448,11 +448,6 @@ func (i *invoker) bidiStream( // Receive any remaining responses for { - if err := ctx.Err(); err != nil { - // If an error was returned, convert it to a proto Error - protoErr = internal.ConvertErrorToProtoError(err) - break - } if totalRcvd == timing.AfterNumResponses { cancel() } diff --git a/internal/app/referenceserver/impl.go b/internal/app/referenceserver/impl.go index 34d8de98..6feaee48 100644 --- a/internal/app/referenceserver/impl.go +++ b/internal/app/referenceserver/impl.go @@ -187,6 +187,10 @@ func (s *conformanceServer) ServerStream( if responseDefinition != nil { //nolint:nestif internal.AddHeaders(responseDefinition.ResponseHeaders, stream.ResponseHeader()) internal.AddHeaders(responseDefinition.ResponseTrailers, stream.ResponseTrailer()) + // Immediately send the headers/trailers on the stream so that they can be read by the client + if err := stream.Send(nil); err != nil { + return connect.NewError(connect.CodeInternal, fmt.Errorf("error sending on stream: %w", err)) + } // Calculate the response delay if specified responseDelay := time.Duration(responseDefinition.ResponseDelayMs) * time.Millisecond @@ -273,6 +277,10 @@ func (s *conformanceServer) BidiStream( if responseDefinition != nil { internal.AddHeaders(responseDefinition.ResponseHeaders, stream.ResponseHeader()) internal.AddHeaders(responseDefinition.ResponseTrailers, stream.ResponseTrailer()) + // Immediately send the headers on the stream so that they can be read by the client + if err := stream.Send(nil); err != nil { + return connect.NewError(connect.CodeInternal, fmt.Errorf("error sending on stream: %w", err)) + } // Calculate a response delay if specified responseDelay = time.Duration(responseDefinition.ResponseDelayMs) * time.Millisecond @@ -323,6 +331,9 @@ func (s *conformanceServer) BidiStream( // where the requested responses are greater than the total requests. if responseDefinition != nil { //nolint:nestif for ; respNum < len(responseDefinition.ResponseData); respNum++ { + if err := ctx.Err(); err != nil { + return err + } resp := &v1.BidiStreamResponse{ Payload: &v1.ConformancePayload{ Data: responseDefinition.ResponseData[respNum], diff --git a/internal/gen/proto/go/connectrpc/conformance/v1/conformancev1connect/service.connect.go b/internal/gen/proto/go/connectrpc/conformance/v1/conformancev1connect/service.connect.go index 6ab6118c..0fca1f17 100644 --- a/internal/gen/proto/go/connectrpc/conformance/v1/conformancev1connect/service.connect.go +++ b/internal/gen/proto/go/connectrpc/conformance/v1/conformancev1connect/service.connect.go @@ -99,6 +99,10 @@ type ConformanceServiceClient interface { // message data in the data field. Subsequent messages after the first one // should contain only the data field. // + // Servers should immediately send response headers on the stream before sleeping + // for any specified response delay and/or sending the first message so that + // clients can be unblocked reading response headers. + // // If a response definition is not specified OR is specified, but response data // is empty, the server should skip sending anything on the stream. When there // are no responses to send, servers should throw an error if one is provided @@ -137,6 +141,10 @@ type ConformanceServiceClient interface { // vs. full duplex streams. Once all responses are sent, the server should either // return an error if specified or close the stream without error. // + // Servers should immediately send response headers on the stream before sleeping + // for any specified response delay and/or sending the first message so that + // clients can be unblocked reading response headers. + // // If a response definition is not specified OR is specified, but response data // is empty, the server should skip sending anything on the stream. Stream // headers and trailers should always be set on the stream if provided @@ -290,6 +298,10 @@ type ConformanceServiceHandler interface { // message data in the data field. Subsequent messages after the first one // should contain only the data field. // + // Servers should immediately send response headers on the stream before sleeping + // for any specified response delay and/or sending the first message so that + // clients can be unblocked reading response headers. + // // If a response definition is not specified OR is specified, but response data // is empty, the server should skip sending anything on the stream. When there // are no responses to send, servers should throw an error if one is provided @@ -328,6 +340,10 @@ type ConformanceServiceHandler interface { // vs. full duplex streams. Once all responses are sent, the server should either // return an error if specified or close the stream without error. // + // Servers should immediately send response headers on the stream before sleeping + // for any specified response delay and/or sending the first message so that + // clients can be unblocked reading response headers. + // // If a response definition is not specified OR is specified, but response data // is empty, the server should skip sending anything on the stream. Stream // headers and trailers should always be set on the stream if provided diff --git a/internal/gen/proto/go/connectrpc/conformance/v1/service_grpc.pb.go b/internal/gen/proto/go/connectrpc/conformance/v1/service_grpc.pb.go index b03ac61b..5518546b 100644 --- a/internal/gen/proto/go/connectrpc/conformance/v1/service_grpc.pb.go +++ b/internal/gen/proto/go/connectrpc/conformance/v1/service_grpc.pb.go @@ -63,6 +63,10 @@ type ConformanceServiceClient interface { // message data in the data field. Subsequent messages after the first one // should contain only the data field. // + // Servers should immediately send response headers on the stream before sleeping + // for any specified response delay and/or sending the first message so that + // clients can be unblocked reading response headers. + // // If a response definition is not specified OR is specified, but response data // is empty, the server should skip sending anything on the stream. When there // are no responses to send, servers should throw an error if one is provided @@ -101,6 +105,10 @@ type ConformanceServiceClient interface { // vs. full duplex streams. Once all responses are sent, the server should either // return an error if specified or close the stream without error. // + // Servers should immediately send response headers on the stream before sleeping + // for any specified response delay and/or sending the first message so that + // clients can be unblocked reading response headers. + // // If a response definition is not specified OR is specified, but response data // is empty, the server should skip sending anything on the stream. Stream // headers and trailers should always be set on the stream if provided @@ -296,6 +304,10 @@ type ConformanceServiceServer interface { // message data in the data field. Subsequent messages after the first one // should contain only the data field. // + // Servers should immediately send response headers on the stream before sleeping + // for any specified response delay and/or sending the first message so that + // clients can be unblocked reading response headers. + // // If a response definition is not specified OR is specified, but response data // is empty, the server should skip sending anything on the stream. When there // are no responses to send, servers should throw an error if one is provided @@ -334,6 +346,10 @@ type ConformanceServiceServer interface { // vs. full duplex streams. Once all responses are sent, the server should either // return an error if specified or close the stream without error. // + // Servers should immediately send response headers on the stream before sleeping + // for any specified response delay and/or sending the first message so that + // clients can be unblocked reading response headers. + // // If a response definition is not specified OR is specified, but response data // is empty, the server should skip sending anything on the stream. Stream // headers and trailers should always be set on the stream if provided diff --git a/proto/connectrpc/conformance/v1/service.proto b/proto/connectrpc/conformance/v1/service.proto index d7106477..b5b73ccf 100644 --- a/proto/connectrpc/conformance/v1/service.proto +++ b/proto/connectrpc/conformance/v1/service.proto @@ -50,6 +50,10 @@ service ConformanceService { // message data in the data field. Subsequent messages after the first one // should contain only the data field. // + // Servers should immediately send response headers on the stream before sleeping + // for any specified response delay and/or sending the first message so that + // clients can be unblocked reading response headers. + // // If a response definition is not specified OR is specified, but response data // is empty, the server should skip sending anything on the stream. When there // are no responses to send, servers should throw an error if one is provided @@ -88,6 +92,10 @@ service ConformanceService { // vs. full duplex streams. Once all responses are sent, the server should either // return an error if specified or close the stream without error. // + // Servers should immediately send response headers on the stream before sleeping + // for any specified response delay and/or sending the first message so that + // clients can be unblocked reading response headers. + // // If a response definition is not specified OR is specified, but response data // is empty, the server should skip sending anything on the stream. Stream // headers and trailers should always be set on the stream if provided