Changed encoding/decoding error code

biosvs · biosvs · commit 4a35a1d60dd7 · 2024-04-30T15:24:24.000+02:00
Signed-off-by: Vitalii Levitskii &lt;vitalii@uber.com&gt;
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ## [Unreleased]
 - Upgraded go version to 1.21, set toolchain version.
 - Reverted rpc-caller-procedure value setting.
+- errors: encoding/decoding (marshalling/unmarshalling) errors are now returned as CodeInternal (13) instead of CodeInvalidArgument (3).
 
 ## [1.72.1] - 2024-03-14
 - tchannel: Renamed caller-procedure header from `$rpc$-caller-procedure` to `rpc-caller-procedure`.
diff --git a/pkg/errors/client.go b/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, " "))
 }
diff --git a/pkg/errors/errors_test.go b/pkg/errors/errors_test.go
@@ -32,6 +32,7 @@ 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())
 }
 
@@ -51,42 +52,42 @@ 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"},
 		},
 		{
 			errorFunc:     RequestBodyDecodeError,
-			expectedCode:  yarpcerrors.CodeInvalidArgument,
+			expectedCode:  yarpcerrors.CodeInternal,
 			expectedWords: []string{"request", "body", "decode"},
 		},
 		{
 			errorFunc:     RequestHeadersDecodeError,
-			expectedCode:  yarpcerrors.CodeInvalidArgument,
+			expectedCode:  yarpcerrors.CodeInternal,
 			expectedWords: []string{"request", "headers", "decode"},
 		},
 		{
 			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"},
 		},
 	}
diff --git a/pkg/errors/server.go b/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 {
@@ -98,7 +98,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(yarpcerrors.CodeInternal, strings.Join(parts, " "))
 }
 
 func newEncodingMismatchError(want []transport.Encoding, got transport.Encoding) error {
diff --git a/transport/grpc/integration_test.go b/transport/grpc/integration_test.go
@@ -285,7 +285,7 @@ func TestApplicationErrorPropagation(t *testing.T) {
 			"bad_encoding",
 			transport.Headers{},
 		)
-		require.True(t, yarpcerrors.IsInvalidArgument(err))
+		require.True(t, yarpcerrors.IsInternal(err))
 		require.False(t, response.ApplicationError)
 	})
 }
diff --git a/transport/tchannel/handler_test.go b/transport/tchannel/handler_test.go
@@ -213,7 +213,7 @@ func TestHandlerFailures(t *testing.T) {
 				arg2:    nil,
 				arg3:    []byte{0x00},
 			},
-			wantStatus:        tchannel.ErrCodeBadRequest,
+			wantStatus:        tchannel.ErrCodeUnexpected,
 			newResponseWriter: newHandlerWriter,
 			recorder:          newResponseRecorder(),
 			wantLogLevel:      zapcore.ErrorLevel,
@@ -228,7 +228,7 @@ func TestHandlerFailures(t *testing.T) {
 				arg2:    []byte("{not valid JSON}"),
 				arg3:    []byte{0x00},
 			},
-			wantStatus:        tchannel.ErrCodeBadRequest,
+			wantStatus:        tchannel.ErrCodeUnexpected,
 			newResponseWriter: newHandlerWriter,
 			recorder:          newResponseRecorder(),
 			wantLogLevel:      zapcore.ErrorLevel,
@@ -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.ErrCodeUnexpected,
 			newResponseWriter: newHandlerWriter,
 			recorder:          newFaultyResponseRecorder(),
 			wantLogLevel:      zapcore.ErrorLevel,

Original file line number	Diff line number	Diff line change
`@@ -285,7 +285,7 @@ func TestApplicationErrorPropagation(t *testing.T) {`
`285`	`285`	`"bad_encoding",`
`286`	`286`	`transport.Headers{},`
`287`	`287`	`)`
`288`		`- require.True(t, yarpcerrors.IsInvalidArgument(err))`
	`288`	`+ require.True(t, yarpcerrors.IsInternal(err))`
`289`	`289`	`require.False(t, response.ApplicationError)`
`290`	`290`	`})`
`291`	`291`	`}`