Changed encoding/decoding error code from InvalidArgument to Internal

CHANGELOG.md

@@ -8,7 +8,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ## [Unreleased]
 ### Fixed
 - Return correct error code for ctx Cancelled error in http outbound.
+### Added
 - Make tchannel outbound satisfy Namer interface
+### Changed
+- errors: encoding/decoding (marshalling/unmarshalling) errors are now returned as CodeInternal (13) instead of CodeInvalidArgument (3).
 
 ## [1.73.2] - 2024-09-09
 ### Added
@@ -22,9 +25,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ## [1.73.0] - 2024-05-31
 - Upgraded go version to 1.21, set toolchain version.
 - Reverted rpc-caller-procedure value setting.
-- Reverted header renaming for tchannel: caller-procedure header changed back from `rpc-caller-procedure` to `$rpc$-caller-procedure`.
-- Upgraded grpc-go to v1.44.0
-- Fixed grpc status mapper, IDLE connections are now considered as available.
 
 ## [1.72.1] - 2024-03-14
 - tchannel: Renamed caller-procedure header from `$rpc$-caller-procedure` to `rpc-caller-procedure`.

internal/crossdock/client/errorshttpclient/behavior.go

@@ -283,7 +283,7 @@ func Run(t crossdock.T) {
 				"Context-TTL-MS": "100",
 			},
 			body:       "{}",
-			wantStatus: 400,
+			wantStatus: 500,
 			wantBodyContains: `failed to encode "json" response ` +
 				`body for procedure "bad-response" of service "yarpc-test" ` +
 				`from caller "yarpc-test":`,

internal/crossdock/client/errorstchclient/behavior.go

@@ -131,7 +131,7 @@ func Run(t crossdock.T) {
 					return
 				}
 				code := tchannel.GetSystemErrorCode(err)
-				assert.Equal(tchannel.ErrCodeBadRequest, code, "bad-response must produce unexpected error")
+				assert.Equal(tchannel.ErrCodeUnexpected, code, "bad-response must produce unexpected error")
 				assert.Contains(err.Error(), `failed to encode "json"`, "must mention failure to encode JSON in error message")
 			},
 		},

pkg/errors/client.go

@@ -29,28 +29,28 @@ import (
 )
 
 // RequestBodyEncodeError builds a YARPC error with code
-// yarpcerrors.CodeInvalidArgument that represents a failure to encode
+// yarpcerrors.CodeInternal that represents a failure to encode
 // the request body.
 func RequestBodyEncodeError(req *transport.Request, err error) error {
 	return newClientEncodingError(req, false /*isResponse*/, false /*isHeader*/, err)
 }
 
 // ResponseBodyDecodeError builds a YARPC error with code
-// yarpcerrors.CodeInvalidArgument that represents a failure to decode
+// yarpcerrors.CodeInternal that represents a failure to decode
 // the response body.
 func ResponseBodyDecodeError(req *transport.Request, err error) error {
 	return newClientEncodingError(req, true /*isResponse*/, false /*isHeader*/, err)
 }
 
 // RequestHeadersEncodeError builds a YARPC error with code
-// yarpcerrors.CodeInvalidArgument that represents a failure to
+// yarpcerrors.CodeInternal that represents a failure to
 // encode the request headers.
 func RequestHeadersEncodeError(req *transport.Request, err error) error {
 	return newClientEncodingError(req, false /*isResponse*/, true /*isHeader*/, err)
 }
 
 // ResponseHeadersDecodeError builds a YARPC error with code
-// yarpcerrors.CodeInvalidArgument that represents a failure to
+// yarpcerrors.CodeInternal that represents a failure to
 // decode the response headers.
 func ResponseHeadersDecodeError(req *transport.Request, err error) error {
 	return newClientEncodingError(req, true /*isResponse*/, true /*isHeader*/, err)
@@ -71,5 +71,5 @@ func newClientEncodingError(req *transport.Request, isResponse bool, isHeader bo
 	parts = append(parts,
 		fmt.Sprintf("for procedure %q of service %q: %v",
 			req.Procedure, req.Service, err))
-	return yarpcerrors.Newf(yarpcerrors.CodeInvalidArgument, strings.Join(parts, " "))
+	return yarpcerrors.Newf(yarpcerrors.CodeInternal, strings.Join(parts, " "))
 }

pkg/errors/errors_test.go

@@ -32,15 +32,16 @@ import (
 func TestWrapHandlerError(t *testing.T) {
 	assert.Nil(t, WrapHandlerError(nil, "foo", "bar"))
 	assert.Equal(t, yarpcerrors.CodeInvalidArgument, yarpcerrors.FromError(WrapHandlerError(yarpcerrors.Newf(yarpcerrors.CodeInvalidArgument, ""), "foo", "bar")).Code())
+	assert.Equal(t, yarpcerrors.CodeInternal, yarpcerrors.FromError(WrapHandlerError(yarpcerrors.Newf(yarpcerrors.CodeInternal, ""), "foo", "bar")).Code())
 	assert.Equal(t, yarpcerrors.CodeUnknown, yarpcerrors.FromError(WrapHandlerError(errors.New(""), "foo", "bar")).Code())
 }
 
 func TestExpectEncodings(t *testing.T) {
-	assert.Error(t, ExpectEncodings(&transport.Request{}, "foo"))
+	assertError(t, ExpectEncodings(&transport.Request{}, "foo"), yarpcerrors.CodeInvalidArgument, "request", "encoding")
 	assert.NoError(t, ExpectEncodings(&transport.Request{Encoding: "foo"}, "foo"))
 	assert.NoError(t, ExpectEncodings(&transport.Request{Encoding: "foo"}, "foo", "bar"))
-	assert.Error(t, ExpectEncodings(&transport.Request{Encoding: "foo"}, "bar"))
-	assert.Error(t, ExpectEncodings(&transport.Request{Encoding: "foo"}, "bar", "baz"))
+	assertError(t, ExpectEncodings(&transport.Request{Encoding: "foo"}, "bar"), yarpcerrors.CodeInvalidArgument, "request", "encoding")
+	assertError(t, ExpectEncodings(&transport.Request{Encoding: "foo"}, "bar", "baz"), yarpcerrors.CodeInvalidArgument, "request", "encoding")
 }
 
 func TestEncodeErrors(t *testing.T) {
@@ -51,12 +52,12 @@ func TestEncodeErrors(t *testing.T) {
 	}{
 		{
 			errorFunc:     RequestBodyEncodeError,
-			expectedCode:  yarpcerrors.CodeInvalidArgument,
+			expectedCode:  yarpcerrors.CodeInternal,
 			expectedWords: []string{"request", "body", "encode"},
 		},
 		{
 			errorFunc:     RequestHeadersEncodeError,
-			expectedCode:  yarpcerrors.CodeInvalidArgument,
+			expectedCode:  yarpcerrors.CodeInternal,
 			expectedWords: []string{"request", "headers", "encode"},
 		},
 		{
@@ -71,22 +72,22 @@ func TestEncodeErrors(t *testing.T) {
 		},
 		{
 			errorFunc:     ResponseBodyEncodeError,
-			expectedCode:  yarpcerrors.CodeInvalidArgument,
+			expectedCode:  yarpcerrors.CodeInternal,
 			expectedWords: []string{"response", "body", "encode"},
 		},
 		{
 			errorFunc:     ResponseHeadersEncodeError,
-			expectedCode:  yarpcerrors.CodeInvalidArgument,
+			expectedCode:  yarpcerrors.CodeInternal,
 			expectedWords: []string{"response", "headers", "encode"},
 		},
 		{
 			errorFunc:     ResponseBodyDecodeError,
-			expectedCode:  yarpcerrors.CodeInvalidArgument,
+			expectedCode:  yarpcerrors.CodeInternal,
 			expectedWords: []string{"response", "body", "decode"},
 		},
 		{
 			errorFunc:     ResponseHeadersDecodeError,
-			expectedCode:  yarpcerrors.CodeInvalidArgument,
+			expectedCode:  yarpcerrors.CodeInternal,
 			expectedWords: []string{"response", "headers", "decode"},
 		},
 	}

pkg/errors/server.go

@@ -29,36 +29,36 @@ import (
 )
 
 // RequestBodyDecodeError builds a YARPC error with code
-// yarpcerrors.CodeInvalidArgument that represents a failure to decode
+// yarpcerrors.CodeInternal that represents a failure to decode
 // the request body.
 func RequestBodyDecodeError(req *transport.Request, err error) error {
 	return newServerEncodingError(req, nil, false /*isResponse*/, false /*isHeader*/, err)
 }
 
 // ResponseBodyEncodeError builds a YARPC error with code
-// yarpcerrors.CodeInvalidArgument that represents a failure to encode
+// yarpcerrors.CodeInternal that represents a failure to encode
 // the response body.
 func ResponseBodyEncodeError(req *transport.Request, err error) error {
 	return newServerEncodingError(req, nil, true /*isResponse*/, false /*isHeader*/, err)
 }
 
 // RequestHeadersDecodeError builds a YARPC error with code
-// yarpcerrors.CodeInvalidArgument that represents a failure to
+// yarpcerrors.CodeInternal that represents a failure to
 // decode the request headers.
 func RequestHeadersDecodeError(req *transport.Request, err error) error {
 	return newServerEncodingError(req, nil, false /*isResponse*/, true /*isHeader*/, err)
 }
 
 // ResponseHeadersEncodeError builds a YARPC error with code
-// yarpcerrors.CodeInvalidArgument that represents a failure to
+// yarpcerrors.CodeInternal that represents a failure to
 // encode the response headers.
 func ResponseHeadersEncodeError(req *transport.Request, err error) error {
 	return newServerEncodingError(req, nil, true /*isResponse*/, true /*isHeader*/, err)
 }
 
 // ExpectEncodings verifies that the given request has one of the given
 // encodings, otherwise it returns a YARPC error with code
-// yarpcerrors.CodeInvalidArgument.
+// yarpcerrors.CodeInternal.
 func ExpectEncodings(req *transport.Request, want ...transport.Encoding) error {
 	got := req.Encoding
 	for _, w := range want {
@@ -74,6 +74,7 @@ func newServerEncodingError(req *transport.Request, encodings []transport.Encodi
 	if len(encodings) == 0 {
 		encodings = []transport.Encoding{req.Encoding}
 	}
+	errCode := yarpcerrors.CodeInternal
 	parts := []string{"failed to"}
 	if isResponse {
 		switch len(encodings) {
@@ -83,6 +84,7 @@ func newServerEncodingError(req *transport.Request, encodings []transport.Encodi
 			parts = append(parts, fmt.Sprintf("encode %v response", encodings))
 		}
 	} else {
+		errCode = yarpcerrors.CodeInvalidArgument
 		switch len(encodings) {
 		case 1:
 			parts = append(parts, fmt.Sprintf("decode %q request", string(encodings[0])))
@@ -98,7 +100,7 @@ func newServerEncodingError(req *transport.Request, encodings []transport.Encodi
 	parts = append(parts,
 		fmt.Sprintf("for procedure %q of service %q from caller %q: %v",
 			req.Procedure, req.Service, req.Caller, err))
-	return yarpcerrors.Newf(yarpcerrors.CodeInvalidArgument, strings.Join(parts, " "))
+	return yarpcerrors.Newf(errCode, strings.Join(parts, " "))
 }
 
 func newEncodingMismatchError(want []transport.Encoding, got transport.Encoding) error {

transport/tchannel/handler_test.go

@@ -306,7 +306,7 @@ func TestHandlerFailures(t *testing.T) {
 						"serialization derp",
 					)))
 			},
-			wantStatus:        tchannel.ErrCodeBadRequest,
+			wantStatus:        tchannel.ErrCodeUnexpected,
 			newResponseWriter: newHandlerWriter,
 			recorder:          newResponseRecorder(),
 			wantLogLevel:      zapcore.ErrorLevel,
@@ -391,7 +391,7 @@ func TestHandlerFailures(t *testing.T) {
 				arg2:    nil,
 				arg3:    []byte{0x00},
 			},
-			wantStatus:        tchannel.ErrCodeBadRequest,
+			wantStatus:        tchannel.ErrCodeBadRequest, // This is an actual error for request, but we're testing problem with response writing
 			newResponseWriter: newHandlerWriter,
 			recorder:          newFaultyResponseRecorder(),
 			wantLogLevel:      zapcore.ErrorLevel,