Skip to content

Commit

Permalink
Revise code mappings per latest changes to spec (#706)
Browse files Browse the repository at this point in the history
This reconciles the implementation in this repo with the recent
adjustments to HTTP<->Connect code mappings
(connectrpc/connectrpc.com#130).

The gRPC and Connect mappings from HTTP code to RPC code are now
aligned, so this unifies them into a single function.
  • Loading branch information
jhump authored Mar 12, 2024
1 parent 75d634f commit e3f35a6
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 76 deletions.
16 changes: 8 additions & 8 deletions connect_ext_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion handler_ext_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
15 changes: 14 additions & 1 deletion internal/conformance/known-failing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
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
25 changes: 25 additions & 0 deletions protocol.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
49 changes: 8 additions & 41 deletions protocol_connect.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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)
Expand Down Expand Up @@ -1393,7 +1360,7 @@ func connectValidateUnaryResponseContentType(
return nil
}
return NewError(
connectHTTPToCode(statusCode),
httpToCode(statusCode),
errors.New(statusMsg),
)
}
Expand Down
6 changes: 3 additions & 3 deletions protocol_connect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
23 changes: 1 addition & 22 deletions protocol_grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down

0 comments on commit e3f35a6

Please sign in to comment.