diff --git a/connect_ext_test.go b/connect_ext_test.go index 0ada5bbe..fefb9f7d 100644 --- a/connect_ext_test.go +++ b/connect_ext_test.go @@ -1731,9 +1731,9 @@ func TestConnectHTTPErrorCodes(t *testing.T) { assert.NotNil(t, err) assert.Nil(t, connectResp) } - t.Run("CodeCanceled-408", func(t *testing.T) { + t.Run("CodeCanceled-499", func(t *testing.T) { t.Parallel() - checkHTTPStatus(t, connect.CodeCanceled, 408) + checkHTTPStatus(t, connect.CodeCanceled, 499) }) t.Run("CodeUnknown-500", func(t *testing.T) { t.Parallel() @@ -1743,9 +1743,9 @@ func TestConnectHTTPErrorCodes(t *testing.T) { t.Parallel() checkHTTPStatus(t, connect.CodeInvalidArgument, 400) }) - t.Run("CodeDeadlineExceeded-408", func(t *testing.T) { + t.Run("CodeDeadlineExceeded-504", func(t *testing.T) { t.Parallel() - checkHTTPStatus(t, connect.CodeDeadlineExceeded, 408) + checkHTTPStatus(t, connect.CodeDeadlineExceeded, 504) }) t.Run("CodeNotFound-404", func(t *testing.T) { t.Parallel() @@ -1763,9 +1763,9 @@ func TestConnectHTTPErrorCodes(t *testing.T) { t.Parallel() checkHTTPStatus(t, connect.CodeResourceExhausted, 429) }) - t.Run("CodeFailedPrecondition-412", func(t *testing.T) { + t.Run("CodeFailedPrecondition-400", func(t *testing.T) { t.Parallel() - checkHTTPStatus(t, connect.CodeFailedPrecondition, 412) + checkHTTPStatus(t, connect.CodeFailedPrecondition, 400) }) t.Run("CodeAborted-409", func(t *testing.T) { t.Parallel() @@ -1775,9 +1775,9 @@ func TestConnectHTTPErrorCodes(t *testing.T) { t.Parallel() checkHTTPStatus(t, connect.CodeOutOfRange, 400) }) - t.Run("CodeUnimplemented-404", func(t *testing.T) { + t.Run("CodeUnimplemented-501", func(t *testing.T) { t.Parallel() - checkHTTPStatus(t, connect.CodeUnimplemented, 404) + checkHTTPStatus(t, connect.CodeUnimplemented, 501) }) t.Run("CodeInternal-500", func(t *testing.T) { t.Parallel() diff --git a/handler_ext_test.go b/handler_ext_test.go index b01eedd9..f32bd1a5 100644 --- a/handler_ext_test.go +++ b/handler_ext_test.go @@ -231,7 +231,7 @@ func TestHandler_ServeHTTP(t *testing.T) { resp, err := client.Do(req) assert.Nil(t, err) defer resp.Body.Close() - assert.Equal(t, resp.StatusCode, http.StatusNotFound) + assert.Equal(t, resp.StatusCode, http.StatusNotImplemented) type errorMessage struct { Code string `json:"code,omitempty"` diff --git a/internal/conformance/known-failing.txt b/internal/conformance/known-failing.txt index 8acce3f9..83bcb82f 100644 --- a/internal/conformance/known-failing.txt +++ b/internal/conformance/known-failing.txt @@ -11,4 +11,17 @@ # these lines below. Connect Error and End-Stream/**/error/missing-code Connect Error and End-Stream/**/error/null -Connect Error and End-Stream/**/error/null-code \ No newline at end of file +Connect Error and End-Stream/**/error/null-code + +# The current v1.0.0-rc3 of conformance suite has expectations based +# on the old mappings of HTTP to RPC code. The mappings were revised +# in the spec (https://github.com/connectrpc/connectrpc.com/pull/130), +# and this repo implements those new mappings. So test cases based on +# the old mappings fail for right now. +Connect Unexpected Responses/**/unexpected-error-body +HTTP to Connect Code Mapping/**/bad-request +HTTP to Connect Code Mapping/**/conflict +HTTP to Connect Code Mapping/**/payload-too-large +HTTP to Connect Code Mapping/**/precondition-failed +HTTP to Connect Code Mapping/**/request-header-fields-too-large +HTTP to Connect Code Mapping/**/request-timeout diff --git a/protocol.go b/protocol.go index e28a6964..eb1984f0 100644 --- a/protocol.go +++ b/protocol.go @@ -396,3 +396,28 @@ func canonicalizeContentTypeSlow(contentType string) string { } return mime.FormatMediaType(base, params) } + +func httpToCode(httpCode int) Code { + // https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md + // Note that this is NOT the inverse of the gRPC-to-HTTP or Connect-to-HTTP + // mappings. + + // Literals are easier to compare to the specification (vs named + // constants). + switch httpCode { + case 400: + return CodeInternal + case 401: + return CodeUnauthenticated + case 403: + return CodePermissionDenied + case 404: + return CodeUnimplemented + case 429: + return CodeUnavailable + case 502, 503, 504: + return CodeUnavailable + default: + return CodeUnknown + } +} diff --git a/protocol_connect.go b/protocol_connect.go index f07d3002..b53b52eb 100644 --- a/protocol_connect.go +++ b/protocol_connect.go @@ -543,13 +543,13 @@ func (cc *connectUnaryClientConn) validateResponse(response *http.Response) *Err var wireErr connectWireError if err := unmarshaler.UnmarshalFunc(&wireErr, json.Unmarshal); err != nil { return NewError( - connectHTTPToCode(response.StatusCode), + httpToCode(response.StatusCode), errors.New(response.Status), ) } if wireErr.Code == 0 { // code not set? default to one implied by HTTP status - wireErr.Code = connectHTTPToCode(response.StatusCode) + wireErr.Code = httpToCode(response.StatusCode) } serverErr := wireErr.asError() if serverErr == nil { @@ -652,7 +652,7 @@ func (cc *connectStreamingClientConn) onRequestSend(fn func(*http.Request)) { func (cc *connectStreamingClientConn) validateResponse(response *http.Response) *Error { if response.StatusCode != http.StatusOK { - return errorf(connectHTTPToCode(response.StatusCode), "HTTP status %v", response.Status) + return errorf(httpToCode(response.StatusCode), "HTTP status %v", response.Status) } if err := connectValidateStreamResponseContentType( cc.codec.Name(), @@ -1267,13 +1267,13 @@ func connectCodeToHTTP(code Code) int { // it easier to compare this function to the Connect specification. switch code { case CodeCanceled: - return 408 + return 499 case CodeUnknown: return 500 case CodeInvalidArgument: return 400 case CodeDeadlineExceeded: - return 408 + return 504 case CodeNotFound: return 404 case CodeAlreadyExists: @@ -1283,13 +1283,13 @@ func connectCodeToHTTP(code Code) int { case CodeResourceExhausted: return 429 case CodeFailedPrecondition: - return 412 + return 400 case CodeAborted: return 409 case CodeOutOfRange: return 400 case CodeUnimplemented: - return 404 + return 501 case CodeInternal: return 500 case CodeUnavailable: @@ -1303,39 +1303,6 @@ func connectCodeToHTTP(code Code) int { } } -func connectHTTPToCode(httpCode int) Code { - // As above, literals are easier to compare to the specificaton (vs named - // constants). - switch httpCode { - case 400: - return CodeInvalidArgument - case 401: - return CodeUnauthenticated - case 403: - return CodePermissionDenied - case 404: - return CodeUnimplemented - case 408: - return CodeDeadlineExceeded - case 409: - return CodeAborted - case 412: - return CodeFailedPrecondition - case 413: - return CodeResourceExhausted - case 415: - return CodeInternal - case 429: - return CodeUnavailable - case 431: - return CodeResourceExhausted - case 502, 503, 504: - return CodeUnavailable - default: - return CodeUnknown - } -} - func connectCodecFromContentType(streamType StreamType, contentType string) string { if streamType == StreamTypeUnary { return strings.TrimPrefix(contentType, connectUnaryContentTypePrefix) @@ -1393,7 +1360,7 @@ func connectValidateUnaryResponseContentType( return nil } return NewError( - connectHTTPToCode(statusCode), + httpToCode(statusCode), errors.New(statusMsg), ) } diff --git a/protocol_connect_test.go b/protocol_connect_test.go index 5819192f..8d34b3fb 100644 --- a/protocol_connect_test.go +++ b/protocol_connect_test.go @@ -243,19 +243,19 @@ func TestConnectValidateUnaryResponseContentType(t *testing.T) { codecName: codecNameProto, statusCode: http.StatusNotFound, responseContentType: "application/proto", - expectCode: connectHTTPToCode(http.StatusNotFound), + expectCode: httpToCode(http.StatusNotFound), }, { codecName: codecNameJSON, statusCode: http.StatusBadRequest, responseContentType: "some/garbage", - expectCode: connectHTTPToCode(http.StatusBadRequest), + expectCode: httpToCode(http.StatusBadRequest), }, { codecName: codecNameJSON, statusCode: http.StatusTooManyRequests, responseContentType: "some/garbage", - expectCode: connectHTTPToCode(http.StatusTooManyRequests), + expectCode: httpToCode(http.StatusTooManyRequests), }, } for _, testCase := range testCases { diff --git a/protocol_grpc.go b/protocol_grpc.go index b84b9148..5a58cf42 100644 --- a/protocol_grpc.go +++ b/protocol_grpc.go @@ -651,7 +651,7 @@ func grpcValidateResponse( codecName string, ) *Error { if response.StatusCode != http.StatusOK { - return errorf(grpcHTTPToCode(response.StatusCode), "HTTP status %v", response.Status) + return errorf(httpToCode(response.StatusCode), "HTTP status %v", response.Status) } if err := grpcValidateResponseContentType( web, @@ -678,27 +678,6 @@ func grpcValidateResponse( return nil } -func grpcHTTPToCode(httpCode int) Code { - // https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md - // Note that this is not just the inverse of the gRPC-to-HTTP mapping. - switch httpCode { - case 400: - return CodeInternal - case 401: - return CodeUnauthenticated - case 403: - return CodePermissionDenied - case 404: - return CodeUnimplemented - case 429: - return CodeUnavailable - case 502, 503, 504: - return CodeUnavailable - default: - return CodeUnknown - } -} - // The gRPC wire protocol specifies that errors should be serialized using the // binary Protobuf format, even if the messages in the request/response stream // use a different codec. Consequently, this function needs a Protobuf codec to