Skip to content

Commit

Permalink
Fix panic on zero send from server (#398)
Browse files Browse the repository at this point in the history
- Maps a zero code of `connectWireError` to a CodeUnknown in
`connectWireError) asError() *Error `
This could have been done in the unmarshaling to `connectWireError` but
I chose to leave that to be exactly what was read off the wire.
- Adds nil guard in `validateResponse` for asError
- Adds tests for error mapping

Fixes: https://github.com/bufbuild/connect-go/issues/396
  • Loading branch information
joshcarp authored Nov 18, 2022
1 parent c6b5562 commit 8e45b51
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 23 deletions.
44 changes: 22 additions & 22 deletions connect_ext_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1661,85 +1661,85 @@ func TestConnectHTTPErrorCodes(t *testing.T) {
req.Header.Set("Content-Type", "application/json")
resp, err := server.Client().Do(req)
assert.Nil(t, err)
assert.Equal(t, wantHttpStatus, resp.StatusCode)
defer resp.Body.Close()
assert.Equal(t, wantHttpStatus, resp.StatusCode)
connectClient := pingv1connect.NewPingServiceClient(server.Client(), server.URL)
connectResp, err := connectClient.Ping(context.Background(), connect.NewRequest(&pingv1.PingRequest{}))
assert.NotNil(t, err)
assert.Nil(t, connectResp)
}
t.Run("connect.CodeCanceled, 408", func(t *testing.T) {
t.Run("CodeCanceled-408", func(t *testing.T) {
t.Parallel()
checkHTTPStatus(t, connect.CodeCanceled, 408)
})
t.Run("connect.CodeUnknown, 500", func(t *testing.T) {
t.Run("CodeUnknown-500", func(t *testing.T) {
t.Parallel()
checkHTTPStatus(t, connect.CodeUnknown, 500)
})
t.Run("connect.CodeInvalidArgument, 400", func(t *testing.T) {
t.Run("CodeInvalidArgument-400", func(t *testing.T) {
t.Parallel()
checkHTTPStatus(t, connect.CodeInvalidArgument, 400)
})
t.Run("connect.CodeDeadlineExceeded, 408", func(t *testing.T) {
t.Run("CodeDeadlineExceeded-408", func(t *testing.T) {
t.Parallel()
checkHTTPStatus(t, connect.CodeDeadlineExceeded, 408)
})
t.Run("connect.CodeNotFound, 404", func(t *testing.T) {
t.Run("CodeNotFound-404", func(t *testing.T) {
t.Parallel()
checkHTTPStatus(t, connect.CodeNotFound, 404)
})
t.Run("connect.CodeAlreadyExists, 409", func(t *testing.T) {
t.Run("CodeAlreadyExists-409", func(t *testing.T) {
t.Parallel()
checkHTTPStatus(t, connect.CodeAlreadyExists, 409)
})
t.Run("connect.CodePermissionDenied, 403", func(t *testing.T) {
t.Run("CodePermissionDenied-403", func(t *testing.T) {
t.Parallel()
checkHTTPStatus(t, connect.CodePermissionDenied, 403)
})
t.Run("connect.CodeResourceExhausted, 429", func(t *testing.T) {
t.Run("CodeResourceExhausted-429", func(t *testing.T) {
t.Parallel()
checkHTTPStatus(t, connect.CodeResourceExhausted, 429)
})
t.Run("connect.CodeFailedPrecondition, 412", func(t *testing.T) {
t.Run("CodeFailedPrecondition-412", func(t *testing.T) {
t.Parallel()
checkHTTPStatus(t, connect.CodeFailedPrecondition, 412)
})
t.Run("connect.CodeAborted, 409", func(t *testing.T) {
t.Run("CodeAborted-409", func(t *testing.T) {
t.Parallel()
checkHTTPStatus(t, connect.CodeAborted, 409)
})
t.Run("connect.CodeOutOfRange, 400", func(t *testing.T) {
t.Run("CodeOutOfRange-400", func(t *testing.T) {
t.Parallel()
checkHTTPStatus(t, connect.CodeOutOfRange, 400)
})
t.Run("connect.CodeUnimplemented, 404", func(t *testing.T) {
t.Run("CodeUnimplemented-404", func(t *testing.T) {
t.Parallel()
checkHTTPStatus(t, connect.CodeUnimplemented, 404)
})
t.Run("connect.CodeInternal, 500", func(t *testing.T) {
t.Run("CodeInternal-500", func(t *testing.T) {
t.Parallel()
checkHTTPStatus(t, connect.CodeInternal, 500)
})
t.Run("connect.CodeUnavailable, 503", func(t *testing.T) {
t.Run("CodeUnavailable-503", func(t *testing.T) {
t.Parallel()
checkHTTPStatus(t, connect.CodeUnavailable, 503)
})
t.Run("connect.CodeDataLoss, 500", func(t *testing.T) {
t.Run("CodeDataLoss-500", func(t *testing.T) {
t.Parallel()
checkHTTPStatus(t, connect.CodeDataLoss, 500)
})
t.Run("connect.CodeUnauthenticated, 401", func(t *testing.T) {
t.Run("CodeUnauthenticated-401", func(t *testing.T) {
t.Parallel()
checkHTTPStatus(t, connect.CodeUnauthenticated, 401)
})
t.Run("100, 500", func(t *testing.T) {
t.Run("100-500", func(t *testing.T) {
t.Parallel()
checkHTTPStatus(t, 100, 500)
})
// t.Run("0, 500", func(t *testing.T) { //TODO: enable this when
// t.Parallel()
// checkHTTPStatus(t, 0, 500)
// })
t.Run("0-500", func(t *testing.T) {
t.Parallel()
checkHTTPStatus(t, 0, 500)
})
}

func TestFailCompression(t *testing.T) {
Expand Down
8 changes: 7 additions & 1 deletion protocol_connect.go
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,9 @@ func (cc *connectUnaryClientConn) validateResponse(response *http.Response) *Err
)
}
serverErr := wireErr.asError()
if serverErr == nil {
return nil
}
serverErr.meta = cc.responseHeader.Clone()
mergeHeaders(serverErr.meta, cc.responseTrailer)
return serverErr
Expand Down Expand Up @@ -919,9 +922,12 @@ func newConnectWireError(err error) *connectWireError {
}

func (e *connectWireError) asError() *Error {
if e == nil || e.Code == 0 {
if e == nil {
return nil
}
if e.Code < minCode || e.Code > maxCode {
e.Code = CodeUnknown
}
err := NewError(e.Code, errors.New(e.Message))
err.wireErr = true
if len(e.Details) > 0 {
Expand Down

0 comments on commit 8e45b51

Please sign in to comment.