From 3f31e076a5a02aff463372872b8a5ffebcfc040c Mon Sep 17 00:00:00 2001 From: Joshua Humphries <2035234+jhump@users.noreply.github.com> Date: Mon, 18 Mar 2024 13:39:11 -0400 Subject: [PATCH] Relax expectation for unspecified errors, fix expectation for cardinality violations (#833) There are numerous error conditions in gRPC where the actual behavior of a client or server in the face of such an error is unspecified. In particular, there is no specification for how a client or server should react to an invalid/unexpected HTTP method or content-type. So this adds a mechanism to supply additional acceptable error codes. One that thing that **is** specified, but was codified with the wrong expectation in these tests, is related to cardinality violations. A cardinality violation is when exactly one request or response is expected (for a unary RPC or the non-streaming half of a non-bidi streaming RPC), but there is actually zero or more than one. This is specified in a table that accompanies the docs for gRPC codes. Search for "cardinality violation" in this page: https://grpc.github.io/grpc/core/md_doc_statuscodes.html --- Makefile | 3 + docs/authoring_test_cases.md | 53 +++++--- internal/app/connectconformance/results.go | 40 +++++- .../app/connectconformance/results_test.go | 43 ++++++- .../testsuites/data/client_unexpected.yaml | 15 --- .../data/connect_client_unexpected.yaml | 71 ++++++++++ .../data/connect_server_unexpected.yaml | 39 ++++++ .../data/grpc_client_unexpected.yaml | 98 ++++++++++++-- .../data/grpc_server_unexpected.yaml | 78 +++++++++++ .../data/grpc_web_client_unexpected.yaml | 121 +++++++++++++++++- .../data/grpc_web_server_unexpected.yaml | 78 +++++++++++ .../go/connectrpc/conformance/v1/suite.pb.go | 81 +++++++----- proto/connectrpc/conformance/v1/suite.proto | 7 + testing/grpcclient-known-failing.txt | 11 +- testing/grpcserver-known-failing.txt | 8 ++ testing/grpcserver-web-known-failing.txt | 11 +- testing/grpcwebclient-known-failing.txt | 7 +- .../connectrpc/conformance/v1/suite_pb.d.ts | 6 + .../connectrpc/conformance/v1/suite_pb.js | 55 +++++++- testing/referenceclient-known-failing.txt | 11 ++ testing/referenceserver-known-failing.txt | 8 ++ 21 files changed, 745 insertions(+), 99 deletions(-) create mode 100644 testing/grpcserver-known-failing.txt create mode 100644 testing/referenceclient-known-failing.txt create mode 100644 testing/referenceserver-known-failing.txt diff --git a/Makefile b/Makefile index 3197a90f..d6176192 100644 --- a/Makefile +++ b/Makefile @@ -96,8 +96,10 @@ runconformance: runservertests runclienttests .PHONY: runservertests runservertests: $(BIN)/connectconformance $(BIN)/referenceserver $(BIN)/grpcserver $(BIN)/connectconformance -v --conf ./testing/reference-impls-config.yaml --mode server --trace \ + --known-failing @./testing/referenceserver-known-failing.txt \ -- $(BIN)/referenceserver $(BIN)/connectconformance -v --conf ./testing/grpc-impls-config.yaml --mode server --trace \ + --known-failing @./testing/grpcserver-known-failing.txt \ -- $(BIN)/grpcserver $(BIN)/connectconformance -v --conf ./testing/grpc-web-server-impl-config.yaml --mode server --trace \ --known-failing @./testing/grpcserver-web-known-failing.txt \ @@ -106,6 +108,7 @@ runservertests: $(BIN)/connectconformance $(BIN)/referenceserver $(BIN)/grpcserv .PHONY: runclienttests runclienttests: $(BIN)/connectconformance $(BIN)/referenceclient $(BIN)/grpcclient buildgrpcweb $(BIN)/connectconformance -v --conf ./testing/reference-impls-config.yaml --mode client --trace \ + --known-failing @./testing/referenceclient-known-failing.txt \ -- $(BIN)/referenceclient $(BIN)/connectconformance -v --conf ./testing/grpc-impls-config.yaml --mode client --trace \ --known-failing @./testing/grpcclient-known-failing.txt \ diff --git a/docs/authoring_test_cases.md b/docs/authoring_test_cases.md index b9af6c77..9cd392fc 100644 --- a/docs/authoring_test_cases.md +++ b/docs/authoring_test_cases.md @@ -101,6 +101,40 @@ or must be specified together. If they are omitted, the runner will auto-populat > > If a test is specific to one of the first four fields, it should instead be indicated in the directives for the test suite itself. +### Expected responses + +The expected response for a test, in the `expectedResponse` field, can be auto-generated based on the request details. +The conformance runner will determine what the response should be according to the values specified in the individual +test case requests. + +You also have the ability to explicitly specify your own expected response directly in the test definition. However, +this is typically only needed for exception test cases. If the expected response is mostly re-stating the response +definition that appears in the requests, you should rely on the auto-generation if possible. Otherwise, specifying +an expected response can make the test YAML overly verbose and harder to read, write, and maintain. + +If the test induces behavior that prevents the server from sending or client from receiving the full response +definition, it will be necessary to define the expected response explicitly. Timeouts, cancellations, and exceeding +message size limits are good examples of this. + +If you do need to specify an explicit response, simply define an `expectedResponse` block for your test case and +this will override the auto-generated expected response in the test runner. + +To see tests denoting an explicit response, search the [test suites][test-suite-dir] directory for the word `expectedResponse`. + +#### Lenience in Expected Error Codes + +There are some cases where a condition is obviously an error, based on the protocol specification, but that +specification does not actually indicate how a client or server should behave in the face of that error. One +example is if a gRPC server receives an HTTP/2 request that has an unexpected content-type or an HTTP method +other than POST. + +For these cases, the test author should put the most sensible error code as the expected error code in the +`expectedResponse` field of the test case, but then can also add other acceptable error codes in the +`otherAllowedErrorCodes` field. Even when the error code is not specified, it is not good practice to allow +_any_ error code since many error codes will obviously not apply, and some error codes should never be used +by a client or server implementation library but are reserved for use by actual application logic (see the +table at the bottom of the [gRPC status codes documentation](https://grpc.github.io/grpc/core/md_doc_statuscodes.html)). + ### Raw Payloads There are two message types in the test case schema worth noting here - [`RawHTTPRequest`][raw-http-request] and @@ -169,25 +203,6 @@ for a specific protocol or only running the unary tests within a suite. Aside from the `/` for separating elements on the name, test case names should use `kebab-case` convention. -## Expected responses - -The expected response for a test is auto-generated based on the request details. The conformance runner will determine -what the response should be according to the values specified in the test suite and individual test cases. - -You also have the ability to explicitly specify your own expected response directly in the test definition itself. However, -this is typically only needed for exception test cases. If the expected response is mostly re-stating the response definition -that appears in the requests, you should rely on the auto-generation if possible. Otherwise, specifying an expected response -can make the test YAML overly verbose and harder to read, write, and maintain. - -If the test induces behavior that prevents the server from sending or client from receiving the full response definition, it -will be necessary to define the expected response explicitly. Timeouts, cancellations, and exceeding message size limits are -good examples of this. - -If you do need to specify an explicit response, simply define an `expectedResponse` block for your test case and this will -override the auto-generated expected response in the test runner. - -To see tests denoting an explicit response, search the [test suites][test-suite-dir] directory for the word `expectedResponse`. - ## Running and Debugging New Tests To test new test cases, you can use `make runconformance`, to run the reference implementations against the new test diff --git a/internal/app/connectconformance/results.go b/internal/app/connectconformance/results.go index 03281df4..93220589 100644 --- a/internal/app/connectconformance/results.go +++ b/internal/app/connectconformance/results.go @@ -158,7 +158,7 @@ func (r *testResults) assert( expected := definition.ExpectedResponse var errs multiErrors - errs = append(errs, checkError(expected.Error, actual.Error)...) + errs = append(errs, checkError(expected.Error, actual.Error, definition.OtherAllowedErrorCodes)...) errs = append(errs, checkPayloads(expected.Payloads, actual.Payloads)...) if len(expected.Payloads) == 0 && @@ -459,7 +459,7 @@ func checkPayloads(expected, actual []*conformancev1.ConformancePayload) multiEr return errs } -func checkError(expected, actual *conformancev1.Error) multiErrors { +func checkError(expected, actual *conformancev1.Error, otherCodes []conformancev1.Code) multiErrors { switch { case expected == nil && actual == nil: // nothing to do @@ -471,13 +471,14 @@ func checkError(expected, actual *conformancev1.Error) multiErrors { } var errs multiErrors - if expected.Code != actual.Code { - errs = append(errs, fmt.Errorf("actual error code %d (%s) does not match expected code %d (%s)", - actual.Code, connect.Code(actual.Code).String(), expected.Code, connect.Code(expected.Code).String())) + if expected.Code != actual.Code && !inSlice(actual.Code, otherCodes) { + expectedCodes := expectedCodeString(expected.Code, otherCodes) + errs = append(errs, fmt.Errorf("actual error {code: %d (%s), message: %q} does not match expected code %s", + actual.Code, connect.Code(actual.Code).String(), actual.GetMessage(), expectedCodes)) } if expected.Message != nil && expected.GetMessage() != actual.GetMessage() { - errs = append(errs, fmt.Errorf("actual error message %q does not match expected message %q", - actual.GetMessage(), expected.GetMessage())) + errs = append(errs, fmt.Errorf("actual error {code: %d (%s), message: %q} does not match expected message %q", + actual.Code, connect.Code(actual.Code).String(), actual.GetMessage(), expected.GetMessage())) } if len(expected.Details) != len(actual.Details) { // TODO: Should this be more lenient? Are we okay with a Connect implementation adding extra @@ -530,3 +531,28 @@ func indent(s string) string { } return strings.Join(lines, "\n") } + +func inSlice[T comparable](elem T, slice []T) bool { + // TODO: delete this function when this repo is using Go 1.21 + // and update call sites to instead use slices.Contains + for _, item := range slice { + if item == elem { + return true + } + } + return false +} + +func expectedCodeString(expectedCode conformancev1.Code, otherAllowedCodes []conformancev1.Code) string { + allowedCodes := make([]string, len(otherAllowedCodes)+1) + for i, code := range append([]conformancev1.Code{expectedCode}, otherAllowedCodes...) { + allowedCodes[i] = fmt.Sprintf("%d (%s)", code, connect.Code(code).String()) + if i == len(allowedCodes)-1 && i != 0 { + allowedCodes[i] = "or " + allowedCodes[i] + } + } + if len(allowedCodes) < 3 { + return strings.Join(allowedCodes, " ") + } + return strings.Join(allowedCodes, ", ") +} diff --git a/internal/app/connectconformance/results_test.go b/internal/app/connectconformance/results_test.go index 006df300..42ad543d 100644 --- a/internal/app/connectconformance/results_test.go +++ b/internal/app/connectconformance/results_test.go @@ -16,6 +16,7 @@ package connectconformance import ( "errors" + "fmt" "strings" "testing" @@ -329,7 +330,7 @@ func TestResults_Assert_ReportsAllErrors(t *testing.T) { } }`, expectedErrors: []string{ - "actual error code 11 (out_of_range) does not match expected code 5 (not_found)", + `actual error {code: 11 (out_of_range), message: "foobar"} does not match expected code 5 (not_found)`, }, }, { @@ -347,7 +348,7 @@ func TestResults_Assert_ReportsAllErrors(t *testing.T) { } }`, expectedErrors: []string{ - `actual error message "oof!" does not match expected message "foobar"`, + `actual error {code: 5 (not_found), message: "oof!"} does not match expected message "foobar"`, }, }, { @@ -842,6 +843,44 @@ func TestCanonicalizeHeaderVals(t *testing.T) { } } +func TestExpectedCodeString(t *testing.T) { + t.Parallel() + testCases := []struct { + expectedCode conformancev1.Code + otherCodes []conformancev1.Code + expectedString string + }{ + { + expectedCode: conformancev1.Code_CODE_ABORTED, + expectedString: "10 (aborted)", + }, + { + expectedCode: conformancev1.Code_CODE_ABORTED, + otherCodes: []conformancev1.Code{conformancev1.Code_CODE_INTERNAL}, + expectedString: "10 (aborted) or 13 (internal)", + }, + { + expectedCode: conformancev1.Code_CODE_ABORTED, + otherCodes: []conformancev1.Code{conformancev1.Code_CODE_INTERNAL, conformancev1.Code_CODE_CANCELED}, + expectedString: "10 (aborted), 13 (internal), or 1 (canceled)", + }, + { + expectedCode: conformancev1.Code_CODE_ABORTED, + otherCodes: []conformancev1.Code{ + conformancev1.Code_CODE_INTERNAL, conformancev1.Code_CODE_CANCELED, conformancev1.Code_CODE_ALREADY_EXISTS, + }, + expectedString: "10 (aborted), 13 (internal), 1 (canceled), or 6 (already_exists)", + }, + } + for _, testCase := range testCases { + testCase := testCase + t.Run(fmt.Sprintf("%d_other_codes", len(testCase.otherCodes)), func(t *testing.T) { + t.Parallel() + require.Equal(t, testCase.expectedString, expectedCodeString(testCase.expectedCode, testCase.otherCodes)) + }) + } +} + func makeKnownFailing() *testTrie { return parsePatterns([]string{"known-to-fail/**"}) } diff --git a/internal/app/connectconformance/testsuites/data/client_unexpected.yaml b/internal/app/connectconformance/testsuites/data/client_unexpected.yaml index 85fa8cf0..346de060 100644 --- a/internal/app/connectconformance/testsuites/data/client_unexpected.yaml +++ b/internal/app/connectconformance/testsuites/data/client_unexpected.yaml @@ -27,18 +27,3 @@ testCases: expectedResponse: error: code: CODE_UNKNOWN -- request: - testName: unexpected-content-type - service: connectrpc.conformance.v1.ConformanceService - method: Unary - streamType: STREAM_TYPE_UNARY - requestMessages: - - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest - responseDefinition: - rawResponse: - headers: - - name: Content-Type - value: ["image/jpeg"] - expectedResponse: - error: - code: CODE_UNKNOWN diff --git a/internal/app/connectconformance/testsuites/data/connect_client_unexpected.yaml b/internal/app/connectconformance/testsuites/data/connect_client_unexpected.yaml index 1af94e58..478ebafb 100644 --- a/internal/app/connectconformance/testsuites/data/connect_client_unexpected.yaml +++ b/internal/app/connectconformance/testsuites/data/connect_client_unexpected.yaml @@ -24,6 +24,77 @@ testCases: error: # mapped from 412 status code (invalid body ignored) code: CODE_FAILED_PRECONDITION + + - request: + testName: client-stream/ok-but-no-response + streamType: STREAM_TYPE_CLIENT_STREAM + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.ClientStreamRequest + responseDefinition: + rawResponse: + status_code: 200 + headers: + - name: content-type + value: [ "application/connect+proto" ] + stream: + items: + - flags: 2 + payload: + text: | + { + "code": "out_of_range", + "message": "oops", + } + expectedResponse: + error: + code: CODE_UNIMPLEMENTED + - request: + testName: client-stream/multiple-responses + streamType: STREAM_TYPE_CLIENT_STREAM + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.ClientStreamRequest + responseDefinition: + rawResponse: + status_code: 200 + headers: + - name: content-type + value: [ "application/connect+proto" ] + stream: + items: + - flags: 0 + payload: + binary_message: + "@type": "type.googleapis.com/connectrpc.conformance.v1.ClientStreamResponse" + - flags: 0 + payload: + binary_message: + "@type": "type.googleapis.com/connectrpc.conformance.v1.ClientStreamResponse" + - flags: 2 + payload: + text: | + { + "code": "out_of_range", + "message": "oops", + } + expectedResponse: + error: + code: CODE_UNIMPLEMENTED + + - request: + testName: unexpected-content-type + service: connectrpc.conformance.v1.ConformanceService + method: Unary + streamType: STREAM_TYPE_UNARY + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest + responseDefinition: + rawResponse: + headers: + - name: Content-Type + value: ["image/jpeg"] + expectedResponse: + error: + code: CODE_UNKNOWN - request: testName: unexpected-codec streamType: STREAM_TYPE_UNARY diff --git a/internal/app/connectconformance/testsuites/data/connect_server_unexpected.yaml b/internal/app/connectconformance/testsuites/data/connect_server_unexpected.yaml index ef761ffe..719af4b8 100644 --- a/internal/app/connectconformance/testsuites/data/connect_server_unexpected.yaml +++ b/internal/app/connectconformance/testsuites/data/connect_server_unexpected.yaml @@ -7,6 +7,45 @@ relevantCompressions: relevantCodecs: - CODEC_PROTO testCases: + - request: + testName: server-stream/no-request + streamType: STREAM_TYPE_SERVER_STREAM + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.ServerStreamRequest + rawRequest: + verb: POST + uri: /connectrpc.conformance.v1.ConformanceService/ServerStream + headers: + - name: content-type + value: [ "application/connect+proto" ] + expectedResponse: + error: + code: CODE_UNIMPLEMENTED + - request: + testName: server-stream/multiple-requests + streamType: STREAM_TYPE_SERVER_STREAM + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.ServerStreamRequest + rawRequest: + verb: POST + uri: /connectrpc.conformance.v1.ConformanceService/ServerStream + headers: + - name: content-type + value: [ "application/connect+proto" ] + stream: + items: + - flags: 0 + payload: + binary_message: + "@type": "type.googleapis.com/connectrpc.conformance.v1.ServerStreamRequest" + - flags: 0 + payload: + binary_message: + "@type": "type.googleapis.com/connectrpc.conformance.v1.ServerStreamRequest" + expectedResponse: + error: + code: CODE_UNIMPLEMENTED + - request: testName: unexpected-verb streamType: STREAM_TYPE_UNARY diff --git a/internal/app/connectconformance/testsuites/data/grpc_client_unexpected.yaml b/internal/app/connectconformance/testsuites/data/grpc_client_unexpected.yaml index 7a37ea29..2553e0d9 100644 --- a/internal/app/connectconformance/testsuites/data/grpc_client_unexpected.yaml +++ b/internal/app/connectconformance/testsuites/data/grpc_client_unexpected.yaml @@ -24,16 +24,16 @@ testCases: trailers: - name: "grpc-message" value: ["error"] - stream: - items: - - flags: 128 - payload: - text: "grpc-message: error\r\n" + otherAllowedErrorCodes: + # Not actually specified what error code to use, but only + # internal and unknown really make any sense. + - CODE_INTERNAL expectedResponse: error: - code: CODE_INTERNAL + code: CODE_UNKNOWN + - request: - testName: unary-ok-but-no-response + testName: unary/ok-but-no-response streamType: STREAM_TYPE_UNARY requestMessages: - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest @@ -48,10 +48,9 @@ testCases: value: ["0"] expectedResponse: error: - code: CODE_UNKNOWN - + code: CODE_UNIMPLEMENTED - request: - testName: unary-multiple-responses + testName: unary/multiple-responses streamType: STREAM_TYPE_UNARY requestMessages: - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest @@ -76,7 +75,52 @@ testCases: value: ["0"] expectedResponse: error: - code: CODE_UNKNOWN + code: CODE_UNIMPLEMENTED + + - request: + testName: client-stream/ok-but-no-response + streamType: STREAM_TYPE_CLIENT_STREAM + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.ClientStreamRequest + responseDefinition: + rawResponse: + status_code: 200 + headers: + - name: content-type + value: [ "application/grpc" ] + trailers: + - name: "grpc-status" + value: ["0"] + expectedResponse: + error: + code: CODE_UNIMPLEMENTED + - request: + testName: client-stream/multiple-responses + streamType: STREAM_TYPE_CLIENT_STREAM + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.ClientStreamRequest + responseDefinition: + rawResponse: + status_code: 200 + headers: + - name: content-type + value: [ "application/grpc" ] + stream: + items: + - flags: 0 + payload: + binary_message: + "@type": "type.googleapis.com/connectrpc.conformance.v1.ClientStreamResponse" + - flags: 0 + payload: + binary_message: + "@type": "type.googleapis.com/connectrpc.conformance.v1.ClientStreamResponse" + trailers: + - name: "grpc-status" + value: ["0"] + expectedResponse: + error: + code: CODE_UNIMPLEMENTED - request: testName: trailers-only/ignore-header-if-body-present @@ -97,6 +141,10 @@ testCases: payload: binary_message: "@type": "type.googleapis.com/connectrpc.conformance.v1.UnaryResponse" + otherAllowedErrorCodes: + # Not actually specified what error code to use, but only + # internal and unknown really make any sense. + - CODE_UNKNOWN expectedResponse: error: code: CODE_INTERNAL # internal since trailers are entirely missing @@ -122,6 +170,28 @@ testCases: error: code: 9 + # Other anomalous responses. + - request: + testName: unexpected-content-type + service: connectrpc.conformance.v1.ConformanceService + method: Unary + streamType: STREAM_TYPE_UNARY + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest + responseDefinition: + rawResponse: + headers: + - name: Content-Type + value: [ "image/jpeg" ] + otherAllowedErrorCodes: + # Not actually specified what error code to use, but only + # internal and unknown really make any sense. There may be + # an argument for unimplemented, too. + - CODE_INTERNAL + - CODE_UNIMPLEMENTED + expectedResponse: + error: + code: CODE_UNKNOWN - request: testName: unexpected-codec streamType: STREAM_TYPE_UNARY @@ -140,6 +210,12 @@ testCases: trailers: - name: grpc-status value: [ "0" ] + otherAllowedErrorCodes: + # Not actually specified what error code to use, but only + # internal and unknown really make any sense. There may be + # an argument for unimplemented, too. + - CODE_UNKNOWN + - CODE_UNIMPLEMENTED expectedResponse: error: code: CODE_INTERNAL diff --git a/internal/app/connectconformance/testsuites/data/grpc_server_unexpected.yaml b/internal/app/connectconformance/testsuites/data/grpc_server_unexpected.yaml index b30238fc..c3dbe89f 100644 --- a/internal/app/connectconformance/testsuites/data/grpc_server_unexpected.yaml +++ b/internal/app/connectconformance/testsuites/data/grpc_server_unexpected.yaml @@ -7,6 +7,84 @@ relevantCompressions: relevantCodecs: - CODEC_PROTO testCases: + - request: + testName: unary/no-request + streamType: STREAM_TYPE_UNARY + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest + rawRequest: + verb: POST + uri: /connectrpc.conformance.v1.ConformanceService/Unary + headers: + - name: content-type + value: [ "application/grpc+proto" ] + expectedResponse: + error: + code: CODE_UNIMPLEMENTED + - request: + testName: unary/multiple-requests + streamType: STREAM_TYPE_UNARY + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest + rawRequest: + verb: POST + uri: /connectrpc.conformance.v1.ConformanceService/Unary + headers: + - name: content-type + value: [ "application/grpc+proto" ] + stream: + items: + - flags: 0 + payload: + binary_message: + "@type": "type.googleapis.com/connectrpc.conformance.v1.UnaryRequest" + - flags: 0 + payload: + binary_message: + "@type": "type.googleapis.com/connectrpc.conformance.v1.UnaryRequest" + expectedResponse: + error: + code: CODE_UNIMPLEMENTED + + - request: + testName: server-stream/no-request + streamType: STREAM_TYPE_SERVER_STREAM + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.ServerStreamRequest + rawRequest: + verb: POST + uri: /connectrpc.conformance.v1.ConformanceService/ServerStream + headers: + - name: content-type + value: [ "application/grpc+proto" ] + expectedResponse: + error: + code: CODE_UNIMPLEMENTED + - request: + testName: server-stream/multiple-requests + streamType: STREAM_TYPE_SERVER_STREAM + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.ServerStreamRequest + rawRequest: + verb: POST + uri: /connectrpc.conformance.v1.ConformanceService/ServerStream + headers: + - name: content-type + value: [ "application/grpc+proto" ] + stream: + items: + - flags: 0 + payload: + binary_message: + "@type": "type.googleapis.com/connectrpc.conformance.v1.ServerStreamRequest" + - flags: 0 + payload: + binary_message: + "@type": "type.googleapis.com/connectrpc.conformance.v1.ServerStreamRequest" + expectedResponse: + error: + code: CODE_UNIMPLEMENTED + - request: testName: unexpected-verb streamType: STREAM_TYPE_UNARY diff --git a/internal/app/connectconformance/testsuites/data/grpc_web_client_unexpected.yaml b/internal/app/connectconformance/testsuites/data/grpc_web_client_unexpected.yaml index fc4f66cf..93b6d178 100644 --- a/internal/app/connectconformance/testsuites/data/grpc_web_client_unexpected.yaml +++ b/internal/app/connectconformance/testsuites/data/grpc_web_client_unexpected.yaml @@ -27,11 +27,16 @@ testCases: - flags: 128 payload: text: "grpc-message: error\r\n" + otherAllowedErrorCodes: + # Not actually specified what error code to use, but only + # internal and unknown really make any sense. + - CODE_INTERNAL expectedResponse: error: code: CODE_UNKNOWN + - request: - testName: trailers-in-body/unary-ok-but-no-response + testName: trailers-in-body/unary/ok-but-no-response streamType: STREAM_TYPE_UNARY requestMessages: - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest @@ -52,9 +57,9 @@ testCases: text: "grpc-status: 0\r\n" expectedResponse: error: - code: CODE_UNKNOWN + code: CODE_UNIMPLEMENTED - request: - testName: trailers-in-body/unary-multiple-responses + testName: trailers-in-body/unary/multiple-responses streamType: STREAM_TYPE_UNARY requestMessages: - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest @@ -79,9 +84,59 @@ testCases: text: "grpc-status: 0\r\n" expectedResponse: error: - code: CODE_UNKNOWN + code: CODE_UNIMPLEMENTED + + - request: + testName: trailers-in-body/client-stream/ok-but-no-response + streamType: STREAM_TYPE_CLIENT_STREAM + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.ClientStreamRequest + responseDefinition: + rawResponse: + statusCode: 200 + headers: + - name: "access-control-allow-origin" + value: [ "*" ] + - name: "access-control-expose-headers" + value: [ "*" ] + - name: content-type + value: [ "application/grpc-web" ] + stream: + items: + - flags: 128 + payload: + text: "grpc-status: 0\r\n" + expectedResponse: + error: + code: CODE_UNIMPLEMENTED + - request: + testName: trailers-in-body/client-stream/multiple-responses + streamType: STREAM_TYPE_CLIENT_STREAM + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.ClientStreamRequest + responseDefinition: + rawResponse: + statusCode: 200 + headers: + - name: content-type + value: [ "application/grpc-web" ] + stream: + items: + - flags: 0 + payload: + binary_message: + "@type": "type.googleapis.com/connectrpc.conformance.v1.ClientStreamResponse" + - flags: 0 + payload: + binary_message: + "@type": "type.googleapis.com/connectrpc.conformance.v1.ClientStreamResponse" + - flags: 128 + payload: + text: "grpc-status: 0\r\n" + expectedResponse: + error: + code: CODE_UNIMPLEMENTED - # TODO: Figure out why these next two cause JS gRPC web client to crash. - request: testName: trailers-only/missing-status streamType: STREAM_TYPE_UNARY @@ -99,11 +154,16 @@ testCases: value: [ "application/grpc-web" ] - name: grpc-message value: [ "error" ] + otherAllowedErrorCodes: + # Not actually specified what error code to use, but only + # internal and unknown really make any sense. + - CODE_INTERNAL expectedResponse: error: code: CODE_UNKNOWN + - request: - testName: trailers-only/unary-ok-but-no-response + testName: trailers-only/unary/ok-but-no-response streamType: STREAM_TYPE_UNARY requestMessages: - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest @@ -121,7 +181,27 @@ testCases: value: [ "0" ] expectedResponse: error: - code: CODE_UNKNOWN + code: CODE_UNIMPLEMENTED + - request: + testName: trailers-only/client-stream/ok-but-no-response + streamType: STREAM_TYPE_CLIENT_STREAM + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.ClientStreamRequest + responseDefinition: + rawResponse: + statusCode: 200 + headers: + - name: "access-control-allow-origin" + value: [ "*" ] + - name: "access-control-expose-headers" + value: [ "*" ] + - name: content-type + value: [ "application/grpc-web" ] + - name: grpc-status + value: [ "0" ] + expectedResponse: + error: + code: CODE_UNIMPLEMENTED - request: testName: trailers-only/ignore-header-if-body-present @@ -150,6 +230,27 @@ testCases: code: 9 # Other anomalous responses. + - request: + testName: unexpected-content-type + service: connectrpc.conformance.v1.ConformanceService + method: Unary + streamType: STREAM_TYPE_UNARY + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest + responseDefinition: + rawResponse: + headers: + - name: Content-Type + value: [ "image/jpeg" ] + otherAllowedErrorCodes: + # Not actually specified what error code to use, but only + # internal and unknown really make any sense. There may be + # an argument for unimplemented, too. + - CODE_INTERNAL + - CODE_UNIMPLEMENTED + expectedResponse: + error: + code: CODE_UNKNOWN - request: testName: unexpected-codec streamType: STREAM_TYPE_UNARY @@ -168,6 +269,12 @@ testCases: - flags: 128 payload: text: "grpc-status: 0\r\n" + otherAllowedErrorCodes: + # Not actually specified what error code to use, but only + # internal and unknown really make any sense. There may be + # an argument for unimplemented, too. + - CODE_UNKNOWN + - CODE_UNIMPLEMENTED expectedResponse: error: code: CODE_INTERNAL diff --git a/internal/app/connectconformance/testsuites/data/grpc_web_server_unexpected.yaml b/internal/app/connectconformance/testsuites/data/grpc_web_server_unexpected.yaml index d5ea9cf8..6b14c3f0 100644 --- a/internal/app/connectconformance/testsuites/data/grpc_web_server_unexpected.yaml +++ b/internal/app/connectconformance/testsuites/data/grpc_web_server_unexpected.yaml @@ -7,6 +7,84 @@ relevantCompressions: relevantCodecs: - CODEC_PROTO testCases: + - request: + testName: unary/no-request + streamType: STREAM_TYPE_UNARY + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest + rawRequest: + verb: POST + uri: /connectrpc.conformance.v1.ConformanceService/Unary + headers: + - name: content-type + value: [ "application/grpc-web+proto" ] + expectedResponse: + error: + code: CODE_UNIMPLEMENTED + - request: + testName: unary/multiple-requests + streamType: STREAM_TYPE_UNARY + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest + rawRequest: + verb: POST + uri: /connectrpc.conformance.v1.ConformanceService/Unary + headers: + - name: content-type + value: [ "application/grpc-web+proto" ] + stream: + items: + - flags: 0 + payload: + binary_message: + "@type": "type.googleapis.com/connectrpc.conformance.v1.UnaryRequest" + - flags: 0 + payload: + binary_message: + "@type": "type.googleapis.com/connectrpc.conformance.v1.UnaryRequest" + expectedResponse: + error: + code: CODE_UNIMPLEMENTED + + - request: + testName: server-stream/no-request + streamType: STREAM_TYPE_SERVER_STREAM + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.ServerStreamRequest + rawRequest: + verb: POST + uri: /connectrpc.conformance.v1.ConformanceService/ServerStream + headers: + - name: content-type + value: [ "application/grpc-web+proto" ] + expectedResponse: + error: + code: CODE_UNIMPLEMENTED + - request: + testName: server-stream/multiple-requests + streamType: STREAM_TYPE_SERVER_STREAM + requestMessages: + - "@type": type.googleapis.com/connectrpc.conformance.v1.ServerStreamRequest + rawRequest: + verb: POST + uri: /connectrpc.conformance.v1.ConformanceService/ServerStream + headers: + - name: content-type + value: [ "application/grpc-web+proto" ] + stream: + items: + - flags: 0 + payload: + binary_message: + "@type": "type.googleapis.com/connectrpc.conformance.v1.ServerStreamRequest" + - flags: 0 + payload: + binary_message: + "@type": "type.googleapis.com/connectrpc.conformance.v1.ServerStreamRequest" + expectedResponse: + error: + code: CODE_UNIMPLEMENTED + - request: testName: unexpected-verb streamType: STREAM_TYPE_UNARY diff --git a/internal/gen/proto/go/connectrpc/conformance/v1/suite.pb.go b/internal/gen/proto/go/connectrpc/conformance/v1/suite.pb.go index 49cf2565..732aef33 100644 --- a/internal/gen/proto/go/connectrpc/conformance/v1/suite.pb.go +++ b/internal/gen/proto/go/connectrpc/conformance/v1/suite.pb.go @@ -360,6 +360,12 @@ type TestCase struct { // Specifying an expected response explicitly in test definitions will override // the auto-generation of the test runner. ExpectedResponse *ClientResponseResult `protobuf:"bytes,3,opt,name=expected_response,json=expectedResponse,proto3" json:"expected_response,omitempty"` + // When expected_response indicates that an error is expected, in some cases, the + // actual error code returned may be flexible. In that case, this field provides + // other acceptable error codes, in addition to the one indicated in the + // expected_response. As long as the actual error's code matches any of these, the + // error is considered conformant, and the test case can pass. + OtherAllowedErrorCodes []Code `protobuf:"varint,4,rep,packed,name=other_allowed_error_codes,json=otherAllowedErrorCodes,proto3,enum=connectrpc.conformance.v1.Code" json:"other_allowed_error_codes,omitempty"` } func (x *TestCase) Reset() { @@ -415,6 +421,13 @@ func (x *TestCase) GetExpectedResponse() *ClientResponseResult { return nil } +func (x *TestCase) GetOtherAllowedErrorCodes() []Code { + if x != nil { + return x.OtherAllowedErrorCodes + } + return nil +} + type TestCase_ExpandedSize struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache @@ -544,7 +557,7 @@ var file_connectrpc_conformance_v1_suite_proto_rawDesc = []byte{ 0x4f, 0x4e, 0x5f, 0x4d, 0x4f, 0x44, 0x45, 0x5f, 0x52, 0x45, 0x51, 0x55, 0x49, 0x52, 0x45, 0x10, 0x01, 0x12, 0x1f, 0x0a, 0x1b, 0x43, 0x4f, 0x4e, 0x4e, 0x45, 0x43, 0x54, 0x5f, 0x56, 0x45, 0x52, 0x53, 0x49, 0x4f, 0x4e, 0x5f, 0x4d, 0x4f, 0x44, 0x45, 0x5f, 0x49, 0x47, 0x4e, 0x4f, 0x52, 0x45, - 0x10, 0x02, 0x22, 0xf2, 0x02, 0x0a, 0x08, 0x54, 0x65, 0x73, 0x74, 0x43, 0x61, 0x73, 0x65, 0x12, + 0x10, 0x02, 0x22, 0xce, 0x03, 0x0a, 0x08, 0x54, 0x65, 0x73, 0x74, 0x43, 0x61, 0x73, 0x65, 0x12, 0x48, 0x0a, 0x07, 0x72, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x2e, 0x2e, 0x63, 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x72, 0x70, 0x63, 0x2e, 0x63, 0x6f, 0x6e, 0x66, 0x6f, 0x72, 0x6d, 0x61, 0x6e, 0x63, 0x65, 0x2e, 0x76, 0x31, 0x2e, 0x43, 0x6c, 0x69, @@ -561,30 +574,36 @@ var file_connectrpc_conformance_v1_suite_proto_rawDesc = []byte{ 0x66, 0x6f, 0x72, 0x6d, 0x61, 0x6e, 0x63, 0x65, 0x2e, 0x76, 0x31, 0x2e, 0x43, 0x6c, 0x69, 0x65, 0x6e, 0x74, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x52, 0x65, 0x73, 0x75, 0x6c, 0x74, 0x52, 0x10, 0x65, 0x78, 0x70, 0x65, 0x63, 0x74, 0x65, 0x64, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, - 0x73, 0x65, 0x1a, 0x63, 0x0a, 0x0c, 0x45, 0x78, 0x70, 0x61, 0x6e, 0x64, 0x65, 0x64, 0x53, 0x69, - 0x7a, 0x65, 0x12, 0x38, 0x0a, 0x16, 0x73, 0x69, 0x7a, 0x65, 0x5f, 0x72, 0x65, 0x6c, 0x61, 0x74, - 0x69, 0x76, 0x65, 0x5f, 0x74, 0x6f, 0x5f, 0x6c, 0x69, 0x6d, 0x69, 0x74, 0x18, 0x01, 0x20, 0x01, - 0x28, 0x05, 0x48, 0x00, 0x52, 0x13, 0x73, 0x69, 0x7a, 0x65, 0x52, 0x65, 0x6c, 0x61, 0x74, 0x69, - 0x76, 0x65, 0x54, 0x6f, 0x4c, 0x69, 0x6d, 0x69, 0x74, 0x88, 0x01, 0x01, 0x42, 0x19, 0x0a, 0x17, - 0x5f, 0x73, 0x69, 0x7a, 0x65, 0x5f, 0x72, 0x65, 0x6c, 0x61, 0x74, 0x69, 0x76, 0x65, 0x5f, 0x74, - 0x6f, 0x5f, 0x6c, 0x69, 0x6d, 0x69, 0x74, 0x42, 0x8b, 0x02, 0x0a, 0x1d, 0x63, 0x6f, 0x6d, 0x2e, - 0x63, 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x72, 0x70, 0x63, 0x2e, 0x63, 0x6f, 0x6e, 0x66, 0x6f, - 0x72, 0x6d, 0x61, 0x6e, 0x63, 0x65, 0x2e, 0x76, 0x31, 0x42, 0x0a, 0x53, 0x75, 0x69, 0x74, 0x65, - 0x50, 0x72, 0x6f, 0x74, 0x6f, 0x50, 0x01, 0x5a, 0x58, 0x63, 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, - 0x72, 0x70, 0x63, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x63, 0x6f, 0x6e, 0x66, 0x6f, 0x72, 0x6d, 0x61, - 0x6e, 0x63, 0x65, 0x2f, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x67, 0x65, 0x6e, - 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2f, 0x67, 0x6f, 0x2f, 0x63, 0x6f, 0x6e, 0x6e, 0x65, 0x63, - 0x74, 0x72, 0x70, 0x63, 0x2f, 0x63, 0x6f, 0x6e, 0x66, 0x6f, 0x72, 0x6d, 0x61, 0x6e, 0x63, 0x65, - 0x2f, 0x76, 0x31, 0x3b, 0x63, 0x6f, 0x6e, 0x66, 0x6f, 0x72, 0x6d, 0x61, 0x6e, 0x63, 0x65, 0x76, - 0x31, 0xa2, 0x02, 0x03, 0x43, 0x43, 0x58, 0xaa, 0x02, 0x19, 0x43, 0x6f, 0x6e, 0x6e, 0x65, 0x63, - 0x74, 0x72, 0x70, 0x63, 0x2e, 0x43, 0x6f, 0x6e, 0x66, 0x6f, 0x72, 0x6d, 0x61, 0x6e, 0x63, 0x65, - 0x2e, 0x56, 0x31, 0xca, 0x02, 0x19, 0x43, 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x72, 0x70, 0x63, - 0x5c, 0x43, 0x6f, 0x6e, 0x66, 0x6f, 0x72, 0x6d, 0x61, 0x6e, 0x63, 0x65, 0x5c, 0x56, 0x31, 0xe2, - 0x02, 0x25, 0x43, 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x72, 0x70, 0x63, 0x5c, 0x43, 0x6f, 0x6e, - 0x66, 0x6f, 0x72, 0x6d, 0x61, 0x6e, 0x63, 0x65, 0x5c, 0x56, 0x31, 0x5c, 0x47, 0x50, 0x42, 0x4d, - 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0xea, 0x02, 0x1b, 0x43, 0x6f, 0x6e, 0x6e, 0x65, 0x63, - 0x74, 0x72, 0x70, 0x63, 0x3a, 0x3a, 0x43, 0x6f, 0x6e, 0x66, 0x6f, 0x72, 0x6d, 0x61, 0x6e, 0x63, - 0x65, 0x3a, 0x3a, 0x56, 0x31, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x73, 0x65, 0x12, 0x5a, 0x0a, 0x19, 0x6f, 0x74, 0x68, 0x65, 0x72, 0x5f, 0x61, 0x6c, 0x6c, 0x6f, + 0x77, 0x65, 0x64, 0x5f, 0x65, 0x72, 0x72, 0x6f, 0x72, 0x5f, 0x63, 0x6f, 0x64, 0x65, 0x73, 0x18, + 0x04, 0x20, 0x03, 0x28, 0x0e, 0x32, 0x1f, 0x2e, 0x63, 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x72, + 0x70, 0x63, 0x2e, 0x63, 0x6f, 0x6e, 0x66, 0x6f, 0x72, 0x6d, 0x61, 0x6e, 0x63, 0x65, 0x2e, 0x76, + 0x31, 0x2e, 0x43, 0x6f, 0x64, 0x65, 0x52, 0x16, 0x6f, 0x74, 0x68, 0x65, 0x72, 0x41, 0x6c, 0x6c, + 0x6f, 0x77, 0x65, 0x64, 0x45, 0x72, 0x72, 0x6f, 0x72, 0x43, 0x6f, 0x64, 0x65, 0x73, 0x1a, 0x63, + 0x0a, 0x0c, 0x45, 0x78, 0x70, 0x61, 0x6e, 0x64, 0x65, 0x64, 0x53, 0x69, 0x7a, 0x65, 0x12, 0x38, + 0x0a, 0x16, 0x73, 0x69, 0x7a, 0x65, 0x5f, 0x72, 0x65, 0x6c, 0x61, 0x74, 0x69, 0x76, 0x65, 0x5f, + 0x74, 0x6f, 0x5f, 0x6c, 0x69, 0x6d, 0x69, 0x74, 0x18, 0x01, 0x20, 0x01, 0x28, 0x05, 0x48, 0x00, + 0x52, 0x13, 0x73, 0x69, 0x7a, 0x65, 0x52, 0x65, 0x6c, 0x61, 0x74, 0x69, 0x76, 0x65, 0x54, 0x6f, + 0x4c, 0x69, 0x6d, 0x69, 0x74, 0x88, 0x01, 0x01, 0x42, 0x19, 0x0a, 0x17, 0x5f, 0x73, 0x69, 0x7a, + 0x65, 0x5f, 0x72, 0x65, 0x6c, 0x61, 0x74, 0x69, 0x76, 0x65, 0x5f, 0x74, 0x6f, 0x5f, 0x6c, 0x69, + 0x6d, 0x69, 0x74, 0x42, 0x8b, 0x02, 0x0a, 0x1d, 0x63, 0x6f, 0x6d, 0x2e, 0x63, 0x6f, 0x6e, 0x6e, + 0x65, 0x63, 0x74, 0x72, 0x70, 0x63, 0x2e, 0x63, 0x6f, 0x6e, 0x66, 0x6f, 0x72, 0x6d, 0x61, 0x6e, + 0x63, 0x65, 0x2e, 0x76, 0x31, 0x42, 0x0a, 0x53, 0x75, 0x69, 0x74, 0x65, 0x50, 0x72, 0x6f, 0x74, + 0x6f, 0x50, 0x01, 0x5a, 0x58, 0x63, 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x72, 0x70, 0x63, 0x2e, + 0x63, 0x6f, 0x6d, 0x2f, 0x63, 0x6f, 0x6e, 0x66, 0x6f, 0x72, 0x6d, 0x61, 0x6e, 0x63, 0x65, 0x2f, + 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x67, 0x65, 0x6e, 0x2f, 0x70, 0x72, 0x6f, + 0x74, 0x6f, 0x2f, 0x67, 0x6f, 0x2f, 0x63, 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x72, 0x70, 0x63, + 0x2f, 0x63, 0x6f, 0x6e, 0x66, 0x6f, 0x72, 0x6d, 0x61, 0x6e, 0x63, 0x65, 0x2f, 0x76, 0x31, 0x3b, + 0x63, 0x6f, 0x6e, 0x66, 0x6f, 0x72, 0x6d, 0x61, 0x6e, 0x63, 0x65, 0x76, 0x31, 0xa2, 0x02, 0x03, + 0x43, 0x43, 0x58, 0xaa, 0x02, 0x19, 0x43, 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x72, 0x70, 0x63, + 0x2e, 0x43, 0x6f, 0x6e, 0x66, 0x6f, 0x72, 0x6d, 0x61, 0x6e, 0x63, 0x65, 0x2e, 0x56, 0x31, 0xca, + 0x02, 0x19, 0x43, 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x72, 0x70, 0x63, 0x5c, 0x43, 0x6f, 0x6e, + 0x66, 0x6f, 0x72, 0x6d, 0x61, 0x6e, 0x63, 0x65, 0x5c, 0x56, 0x31, 0xe2, 0x02, 0x25, 0x43, 0x6f, + 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x72, 0x70, 0x63, 0x5c, 0x43, 0x6f, 0x6e, 0x66, 0x6f, 0x72, 0x6d, + 0x61, 0x6e, 0x63, 0x65, 0x5c, 0x56, 0x31, 0x5c, 0x47, 0x50, 0x42, 0x4d, 0x65, 0x74, 0x61, 0x64, + 0x61, 0x74, 0x61, 0xea, 0x02, 0x1b, 0x43, 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x72, 0x70, 0x63, + 0x3a, 0x3a, 0x43, 0x6f, 0x6e, 0x66, 0x6f, 0x72, 0x6d, 0x61, 0x6e, 0x63, 0x65, 0x3a, 0x3a, 0x56, + 0x31, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( @@ -613,6 +632,7 @@ var file_connectrpc_conformance_v1_suite_proto_goTypes = []interface{}{ (Compression)(0), // 8: connectrpc.conformance.v1.Compression (*ClientCompatRequest)(nil), // 9: connectrpc.conformance.v1.ClientCompatRequest (*ClientResponseResult)(nil), // 10: connectrpc.conformance.v1.ClientResponseResult + (Code)(0), // 11: connectrpc.conformance.v1.Code } var file_connectrpc_conformance_v1_suite_proto_depIdxs = []int32{ 0, // 0: connectrpc.conformance.v1.TestSuite.mode:type_name -> connectrpc.conformance.v1.TestSuite.TestMode @@ -625,11 +645,12 @@ var file_connectrpc_conformance_v1_suite_proto_depIdxs = []int32{ 9, // 7: connectrpc.conformance.v1.TestCase.request:type_name -> connectrpc.conformance.v1.ClientCompatRequest 4, // 8: connectrpc.conformance.v1.TestCase.expand_requests:type_name -> connectrpc.conformance.v1.TestCase.ExpandedSize 10, // 9: connectrpc.conformance.v1.TestCase.expected_response:type_name -> connectrpc.conformance.v1.ClientResponseResult - 10, // [10:10] is the sub-list for method output_type - 10, // [10:10] is the sub-list for method input_type - 10, // [10:10] is the sub-list for extension type_name - 10, // [10:10] is the sub-list for extension extendee - 0, // [0:10] is the sub-list for field type_name + 11, // 10: connectrpc.conformance.v1.TestCase.other_allowed_error_codes:type_name -> connectrpc.conformance.v1.Code + 11, // [11:11] is the sub-list for method output_type + 11, // [11:11] is the sub-list for method input_type + 11, // [11:11] is the sub-list for extension type_name + 11, // [11:11] is the sub-list for extension extendee + 0, // [0:11] is the sub-list for field type_name } func init() { file_connectrpc_conformance_v1_suite_proto_init() } diff --git a/proto/connectrpc/conformance/v1/suite.proto b/proto/connectrpc/conformance/v1/suite.proto index 1a3cc95d..5fda271e 100644 --- a/proto/connectrpc/conformance/v1/suite.proto +++ b/proto/connectrpc/conformance/v1/suite.proto @@ -141,4 +141,11 @@ message TestCase { // Specifying an expected response explicitly in test definitions will override // the auto-generation of the test runner. ClientResponseResult expected_response = 3; + + // When expected_response indicates that an error is expected, in some cases, the + // actual error code returned may be flexible. In that case, this field provides + // other acceptable error codes, in addition to the one indicated in the + // expected_response. As long as the actual error's code matches any of these, the + // error is considered conformant, and the test case can pass. + repeated Code other_allowed_error_codes = 4; } diff --git a/testing/grpcclient-known-failing.txt b/testing/grpcclient-known-failing.txt index 0b94eb52..9d640e09 100644 --- a/testing/grpcclient-known-failing.txt +++ b/testing/grpcclient-known-failing.txt @@ -1,3 +1,12 @@ # The grpc-go library should return "internal" but instead returns "unimplemented". # https://github.com/grpc/grpc-go/issues/6987 -gRPC Unexpected Responses/**/unexpected-compression \ No newline at end of file +gRPC Unexpected Responses/**/unexpected-compression + +# The entries for "cardinality violation" in the following doc indicate that +# these cases should fail with "unimplemented": +# https://grpc.github.io/grpc/core/md_doc_statuscodes.html +# But the grpc-go client instead fails with "unknown". +**/unary/multiple-responses +**/unary/ok-but-no-response +**/client-stream/multiple-responses +**/client-stream/ok-but-no-response diff --git a/testing/grpcserver-known-failing.txt b/testing/grpcserver-known-failing.txt new file mode 100644 index 00000000..54441fe2 --- /dev/null +++ b/testing/grpcserver-known-failing.txt @@ -0,0 +1,8 @@ +# The entries for "cardinality violation" in the following doc indicate that +# these cases should fail with "unimplemented": +# https://grpc.github.io/grpc/core/md_doc_statuscodes.html +# But the grpc-go client instead fails with "unknown". +**/unary/multiple-requests +**/unary/no-request +**/server-stream/multiple-requests +**/server-stream/no-request diff --git a/testing/grpcserver-web-known-failing.txt b/testing/grpcserver-web-known-failing.txt index 0b8eb658..c3c2f1f1 100644 --- a/testing/grpcserver-web-known-failing.txt +++ b/testing/grpcserver-web-known-failing.txt @@ -1,3 +1,12 @@ # This returns 400 instead of 405. This is an inconsistency in grpc-go. # https://github.com/grpc/grpc-go/pull/6989 - gRPC-Web Unexpected Requests/**/unexpected-verb \ No newline at end of file + gRPC-Web Unexpected Requests/**/unexpected-verb + + # The entries for "cardinality violation" in the following doc indicate that + # these cases should fail with "unimplemented": + # https://grpc.github.io/grpc/core/md_doc_statuscodes.html + # But the grpc-go client instead fails with "unknown". + **/unary/multiple-requests + **/unary/no-request + **/server-stream/multiple-requests + **/server-stream/no-request diff --git a/testing/grpcwebclient-known-failing.txt b/testing/grpcwebclient-known-failing.txt index 56f58e15..1e6b15a4 100644 --- a/testing/grpcwebclient-known-failing.txt +++ b/testing/grpcwebclient-known-failing.txt @@ -16,9 +16,8 @@ Duplicate Metadata/**/server-stream/** gRPC-Web Trailers/**/trailers-in-body/duplicate-metadata gRPC-Web Trailers/**/trailers-in-body/mixed-case -# The gRPC-Web client also does not use the expected error codes for some error situations. -# It appears to use "unknown" for nearly (which likely means it's just not classifying them -# at all, which is certainly a bug). -gRPC-Web Unexpected Responses/**/unexpected-codec +# The gRPC-Web client reports "unknown" for these cases. gRPC-Web Unexpected Responses/**/unexpected-compressed-message gRPC-Web Unexpected Responses/**/unexpected-compression +gRPC-Web Unexpected Responses/**/multiple-responses +gRPC-Web Unexpected Responses/**/ok-but-no-response diff --git a/testing/grpcwebclient/gen/proto/connectrpc/conformance/v1/suite_pb.d.ts b/testing/grpcwebclient/gen/proto/connectrpc/conformance/v1/suite_pb.d.ts index dbcddd6a..f3211ad8 100644 --- a/testing/grpcwebclient/gen/proto/connectrpc/conformance/v1/suite_pb.d.ts +++ b/testing/grpcwebclient/gen/proto/connectrpc/conformance/v1/suite_pb.d.ts @@ -118,6 +118,11 @@ export class TestCase extends jspb.Message { hasExpectedResponse(): boolean; clearExpectedResponse(): TestCase; + getOtherAllowedErrorCodesList(): Array; + setOtherAllowedErrorCodesList(value: Array): TestCase; + clearOtherAllowedErrorCodesList(): TestCase; + addOtherAllowedErrorCodes(value: connectrpc_conformance_v1_config_pb.Code, index?: number): TestCase; + serializeBinary(): Uint8Array; toObject(includeInstance?: boolean): TestCase.AsObject; static toObject(includeInstance: boolean, msg: TestCase): TestCase.AsObject; @@ -131,6 +136,7 @@ export namespace TestCase { request?: connectrpc_conformance_v1_client_compat_pb.ClientCompatRequest.AsObject, expandRequestsList: Array, expectedResponse?: connectrpc_conformance_v1_client_compat_pb.ClientResponseResult.AsObject, + otherAllowedErrorCodesList: Array, } export class ExpandedSize extends jspb.Message { diff --git a/testing/grpcwebclient/gen/proto/connectrpc/conformance/v1/suite_pb.js b/testing/grpcwebclient/gen/proto/connectrpc/conformance/v1/suite_pb.js index b89fd413..83c1b0c1 100644 --- a/testing/grpcwebclient/gen/proto/connectrpc/conformance/v1/suite_pb.js +++ b/testing/grpcwebclient/gen/proto/connectrpc/conformance/v1/suite_pb.js @@ -705,7 +705,7 @@ proto.connectrpc.conformance.v1.TestSuite.prototype.setReliesOnMessageReceiveLim * @private {!Array} * @const */ -proto.connectrpc.conformance.v1.TestCase.repeatedFields_ = [2]; +proto.connectrpc.conformance.v1.TestCase.repeatedFields_ = [2,4]; @@ -741,7 +741,8 @@ proto.connectrpc.conformance.v1.TestCase.toObject = function(includeInstance, ms request: (f = msg.getRequest()) && connectrpc_conformance_v1_client_compat_pb.ClientCompatRequest.toObject(includeInstance, f), expandRequestsList: jspb.Message.toObjectList(msg.getExpandRequestsList(), proto.connectrpc.conformance.v1.TestCase.ExpandedSize.toObject, includeInstance), - expectedResponse: (f = msg.getExpectedResponse()) && connectrpc_conformance_v1_client_compat_pb.ClientResponseResult.toObject(includeInstance, f) + expectedResponse: (f = msg.getExpectedResponse()) && connectrpc_conformance_v1_client_compat_pb.ClientResponseResult.toObject(includeInstance, f), + otherAllowedErrorCodesList: (f = jspb.Message.getRepeatedField(msg, 4)) == null ? undefined : f }; if (includeInstance) { @@ -793,6 +794,12 @@ proto.connectrpc.conformance.v1.TestCase.deserializeBinaryFromReader = function( reader.readMessage(value,connectrpc_conformance_v1_client_compat_pb.ClientResponseResult.deserializeBinaryFromReader); msg.setExpectedResponse(value); break; + case 4: + var values = /** @type {!Array} */ (reader.isDelimited() ? reader.readPackedEnum() : [reader.readEnum()]); + for (var i = 0; i < values.length; i++) { + msg.addOtherAllowedErrorCodes(values[i]); + } + break; default: reader.skipField(); break; @@ -846,6 +853,13 @@ proto.connectrpc.conformance.v1.TestCase.serializeBinaryToWriter = function(mess connectrpc_conformance_v1_client_compat_pb.ClientResponseResult.serializeBinaryToWriter ); } + f = message.getOtherAllowedErrorCodesList(); + if (f.length > 0) { + writer.writePackedEnum( + 4, + f + ); + } }; @@ -1109,4 +1123,41 @@ proto.connectrpc.conformance.v1.TestCase.prototype.hasExpectedResponse = functio }; +/** + * repeated Code other_allowed_error_codes = 4; + * @return {!Array} + */ +proto.connectrpc.conformance.v1.TestCase.prototype.getOtherAllowedErrorCodesList = function() { + return /** @type {!Array} */ (jspb.Message.getRepeatedField(this, 4)); +}; + + +/** + * @param {!Array} value + * @return {!proto.connectrpc.conformance.v1.TestCase} returns this + */ +proto.connectrpc.conformance.v1.TestCase.prototype.setOtherAllowedErrorCodesList = function(value) { + return jspb.Message.setField(this, 4, value || []); +}; + + +/** + * @param {!proto.connectrpc.conformance.v1.Code} value + * @param {number=} opt_index + * @return {!proto.connectrpc.conformance.v1.TestCase} returns this + */ +proto.connectrpc.conformance.v1.TestCase.prototype.addOtherAllowedErrorCodes = function(value, opt_index) { + return jspb.Message.addToRepeatedField(this, 4, value, opt_index); +}; + + +/** + * Clears the list making it empty but non-null. + * @return {!proto.connectrpc.conformance.v1.TestCase} returns this + */ +proto.connectrpc.conformance.v1.TestCase.prototype.clearOtherAllowedErrorCodesList = function() { + return this.setOtherAllowedErrorCodesList([]); +}; + + goog.object.extend(exports, proto.connectrpc.conformance.v1); diff --git a/testing/referenceclient-known-failing.txt b/testing/referenceclient-known-failing.txt new file mode 100644 index 00000000..26413120 --- /dev/null +++ b/testing/referenceclient-known-failing.txt @@ -0,0 +1,11 @@ +# Currently, connect-go returns an "internal" error when either of these occurs. +# However, the gRPC specs state that a problem with message cardinality (where +# a single message is expected but stream actually has a different number), in +# both clients and servers, should be an "unimplemented" error. +# +# Search for "cardinality violation" in this doc: +# https://grpc.github.io/grpc/core/md_doc_statuscodes.html +**/unary/multiple-responses +**/unary/ok-but-no-response +**/client-stream/multiple-responses +**/client-stream/ok-but-no-response \ No newline at end of file diff --git a/testing/referenceserver-known-failing.txt b/testing/referenceserver-known-failing.txt new file mode 100644 index 00000000..54441fe2 --- /dev/null +++ b/testing/referenceserver-known-failing.txt @@ -0,0 +1,8 @@ +# The entries for "cardinality violation" in the following doc indicate that +# these cases should fail with "unimplemented": +# https://grpc.github.io/grpc/core/md_doc_statuscodes.html +# But the grpc-go client instead fails with "unknown". +**/unary/multiple-requests +**/unary/no-request +**/server-stream/multiple-requests +**/server-stream/no-request