Test HTTP code to Connect code mappings (#762)

This adds some additional tests to test that the mapping of HTTP code to Connect code is correct for clients according to the protocol: https://connectrpc.com/docs/protocol#http-to-error-code This also adds usage of `connect.ErrorWriter` to the raw response middleware (although is admittedly maybe no longer needed since the header issue has been fixed). --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Joshua Humphries <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
connectrpc · Jan 30, 2024 · 672b90e · 672b90e
1 parent e002318
commit 672b90e
Show file tree

Hide file tree

Showing 3 changed files with 242 additions and 3 deletions.
diff --git a/internal/app/connectconformance/test_case_library.go b/internal/app/connectconformance/test_case_library.go
@@ -332,6 +332,10 @@ func parseTestSuites(testFileData map[string][]byte) (map[string]*conformancev1.
 				return nil, fmt.Errorf("%s: test case %q has raw response, but that is only allowed when mode is TEST_MODE_CLIENT",
 					testFilePath, testCase.Request.TestName)
 			}
+			if hasRawResponse(testCase.Request.RequestMessages) && testCase.ExpectedResponse == nil {
+				return nil, fmt.Errorf("%s: test case %q has raw response, but does not specify an explicit expected response",
+					testFilePath, testCase.Request.TestName)
+			}
 			// The expand request directive uses the proto codec for size calculations, so it doesn't make sense to test with other codecs
 			if len(testCase.ExpandRequests) > 0 && (len(suite.RelevantCodecs) > 1 || !hasCodec(suite.RelevantCodecs, conformancev1.Codec_CODEC_PROTO)) {
 				return nil, fmt.Errorf("%s: test case %q specifies expand requests directive, but includes codecs other than CODEC_PROTO",

diff --git a/internal/app/connectconformance/testsuites/http_to_connect_code.yaml b/internal/app/connectconformance/testsuites/http_to_connect_code.yaml
@@ -0,0 +1,225 @@
+name: HTTP to Connect Code Mapping
+mode: TEST_MODE_CLIENT
+relevantProtocols:
+  - PROTOCOL_CONNECT
+# If a Connect server returns a non-200 HTTP code for unary requests without an
+# explicit Connect error code, the client must synthesize the HTTP code to a 
+# Connect code. These tests cases verify that mapping by forcing the server to
+# return a specified HTTP code and then test whether the client correct returns
+# the required Connect code.
+testCases:
+- request:
+    testName: bad request
+    service: connectrpc.conformance.v1.ConformanceService
+    method: Unary
+    streamType: STREAM_TYPE_UNARY
+    requestMessages:
+    - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest
+      responseDefinition:
+        responseData: "dGVzdCByZXNwb25zZQ=="
+        rawResponse:
+          statusCode: 400
+  expectedResponse:
+    error:
+      # invalid_argument
+      code: 3
+- request:
+    testName: unauthorized
+    service: connectrpc.conformance.v1.ConformanceService
+    method: Unary
+    streamType: STREAM_TYPE_UNARY
+    requestMessages:
+    - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest
+      responseDefinition:
+        rawResponse:
+          statusCode: 401
+  expectedResponse:
+    error:
+      # unauthenticated
+      code: 16
+- request:
+    testName: forbidden
+    service: connectrpc.conformance.v1.ConformanceService
+    method: Unary
+    streamType: STREAM_TYPE_UNARY
+    requestMessages:
+    - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest
+      responseDefinition:
+        rawResponse:
+          statusCode: 403
+  expectedResponse:
+    error:
+      # permission_denied
+      code: 7
+- request:
+    testName: not found
+    service: connectrpc.conformance.v1.ConformanceService
+    method: Unary
+    streamType: STREAM_TYPE_UNARY
+    requestMessages:
+    - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest
+      responseDefinition:
+        rawResponse:
+          statusCode: 404
+  expectedResponse:
+    error:
+      # unimplemented
+      code: 12
+- request:
+    testName: request timeout
+    service: connectrpc.conformance.v1.ConformanceService
+    method: Unary
+    streamType: STREAM_TYPE_UNARY
+    requestMessages:
+    - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest
+      responseDefinition:
+        rawResponse:
+          statusCode: 408
+  expectedResponse:
+    error:
+      # deadline_exceeded
+      code: 4
+# TODO - uncomment when conformance is updated to the latest release of connect-go
+# that includes this fix
+# - request:
+#     testName: conflict
+#     service: connectrpc.conformance.v1.ConformanceService
+#     method: Unary
+#     streamType: STREAM_TYPE_UNARY
+#     requestMessages:
+#     - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest
+#       responseDefinition:
+#         rawResponse:
+#           statusCode: 409
+#  expectedResponse:
+#    error:
+#     # aborted
+#      code: 10
+- request:
+    testName: precondition failed
+    service: connectrpc.conformance.v1.ConformanceService
+    method: Unary
+    streamType: STREAM_TYPE_UNARY
+    requestMessages:
+    - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest
+      responseDefinition:
+        rawResponse:
+          statusCode: 412
+  expectedResponse:
+    error:
+      # failed_precondition
+      code: 9
+- request:
+    testName: payload too large
+    service: connectrpc.conformance.v1.ConformanceService
+    method: Unary
+    streamType: STREAM_TYPE_UNARY
+    requestMessages:
+    - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest
+      responseDefinition:
+        rawResponse:
+          statusCode: 413
+  expectedResponse:
+    error:
+      # resource_exhausted
+      code: 8
+# TODO - uncomment when conformance is updated to the latest release of connect-go
+# that includes this fix
+# - request:
+#     testName: unsupported media type
+#     service: connectrpc.conformance.v1.ConformanceService
+#     method: Unary
+#     streamType: STREAM_TYPE_UNARY
+#     requestMessages:
+#     - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest
+#       responseDefinition:
+#         rawResponse:
+#           statusCode: 415
+#  expectedResponse:
+#    error:
+#     # internal
+#      code: 13
+- request:
+    testName: too many requests
+    service: connectrpc.conformance.v1.ConformanceService
+    method: Unary
+    streamType: STREAM_TYPE_UNARY
+    requestMessages:
+    - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest
+      responseDefinition:
+        rawResponse:
+          statusCode: 429
+  expectedResponse:
+    error:
+      # unavailable
+      code: 14
+- request:
+    testName: request header fields too large
+    service: connectrpc.conformance.v1.ConformanceService
+    method: Unary
+    streamType: STREAM_TYPE_UNARY
+    requestMessages:
+    - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest
+      responseDefinition:
+        rawResponse:
+          statusCode: 431
+  expectedResponse:
+    error:
+      # resource_exhausted
+      code: 8
+- request:
+    testName: bad gateway
+    service: connectrpc.conformance.v1.ConformanceService
+    method: Unary
+    streamType: STREAM_TYPE_UNARY
+    requestMessages:
+    - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest
+      responseDefinition:
+        rawResponse:
+          statusCode: 502
+  expectedResponse:
+    error:
+      # unavailable
+      code: 14
+- request:
+    testName: service unavailable
+    service: connectrpc.conformance.v1.ConformanceService
+    method: Unary
+    streamType: STREAM_TYPE_UNARY
+    requestMessages:
+    - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest
+      responseDefinition:
+        rawResponse:
+          statusCode: 503
+  expectedResponse:
+    error:
+      # unavailable
+      code: 14
+- request:
+    testName: gateway timeout
+    service: connectrpc.conformance.v1.ConformanceService
+    method: Unary
+    streamType: STREAM_TYPE_UNARY
+    requestMessages:
+    - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest
+      responseDefinition:
+        rawResponse:
+          statusCode: 504
+  expectedResponse:
+    error:
+      # unavailable
+      code: 14
+- request:
+    testName: im a teapot
+    service: connectrpc.conformance.v1.ConformanceService
+    method: Unary
+    streamType: STREAM_TYPE_UNARY
+    requestMessages:
+    - "@type": type.googleapis.com/connectrpc.conformance.v1.UnaryRequest
+      responseDefinition:
+        rawResponse:
+          statusCode: 418
+  expectedResponse:
+    error:
+      # unknown
+      code: 2
diff --git a/internal/app/referenceserver/raw_response.go b/internal/app/referenceserver/raw_response.go
@@ -39,12 +39,22 @@ type rawResponseKey struct{}
 // been stored, those response writer interactions take precedence and no
 // raw response can be sent.)
 func rawResponder(handler http.Handler, errPrinter internal.Printer) http.Handler {
+	errorWriter := connect.NewErrorWriter()
 	return http.HandlerFunc(func(respWriter http.ResponseWriter, req *http.Request) {
 		testCaseName := req.Header.Get("x-test-case-name")
+		// This is the only hard failure. Without it, we cannot provide feedback.
+		// All other checks below write to stderr to provide feedback.
 		if testCaseName == "" {
-			// This is the only hard failure. Without it, we cannot provide feedback.
-			// All other checks below write to stderr to provide feedback.
-			http.Error(respWriter, "missing x-test-case-name header", http.StatusBadRequest)
+			const msg = "missing x-test-case-name header"
+			if errorWriter.IsSupported(req) {
+				invalidArg := connect.NewError(connect.CodeInvalidArgument, errors.New(msg))
+				err := errorWriter.Write(respWriter, req, invalidArg)
+				if err != nil {
+					http.Error(respWriter, err.Error(), http.StatusInternalServerError)
+				}
+			} else {
+				http.Error(respWriter, msg, http.StatusBadRequest)
+			}
 			return
 		}
 		feedback := &feedbackPrinter{p: errPrinter, testCaseName: testCaseName}