Merge branch 'develop' into feat/payload_check_crc32c

ppzqh · Mar 14, 2024 · e6ecd8c · e6ecd8c
2 parents 47804a5 + 92f64b0
commit e6ecd8c
Show file tree

Hide file tree

Showing 2 changed files with 104 additions and 20 deletions.
diff --git a/pkg/remote/trans/nphttp2/codes.go b/pkg/remote/trans/nphttp2/codes.go
@@ -26,6 +26,7 @@ import (
 	"github.com/cloudwego/kitex/pkg/remote/trans/nphttp2/status"
 )
 
+// only kitex errors will be converted
 var kitexErrConvTab = map[error]codes.Code{
 	kerrors.ErrInternalException: codes.Internal,
 	kerrors.ErrOverlimit:         codes.ResourceExhausted,
@@ -38,19 +39,22 @@ func convertStatus(err error) *status.Status {
 	if err == nil {
 		return status.New(codes.OK, "")
 	}
-	// biz error should add biz info which is convenient to be recognized by client side
-	var dErr *kerrors.DetailedError
-	if errors.As(err, &dErr) && dErr.Is(kerrors.ErrBiz) {
-		bizErr := dErr.Unwrap()
-		if se, ok := bizErr.(status.Iface); ok {
-			gs := se.GRPCStatus()
-			gs.AppendMessage(fmt.Sprintf("[%s]", kerrors.ErrBiz.Error())).Err()
-			return gs
+	// covert from kitex error
+	if kerrors.IsKitexError(err) {
+		var basicError error
+		var detailedError *kerrors.DetailedError
+		if errors.As(err, &detailedError) {
+			basicError = detailedError.ErrorType()
+			// biz error should add biz info which is convenient to be recognized by client side
+			if st := getStatusForBizErr(detailedError); st != nil {
+				return st
+			}
+		} else {
+			basicError = err
 		}
-		if te, ok := bizErr.(*remote.TransError); ok {
-			return status.New(codes.Code(te.TypeID()), fmt.Sprintf("%s [%s]", bizErr.Error(), kerrors.ErrBiz.Error()))
+		if c, ok := kitexErrConvTab[basicError]; ok {
+			return status.New(c, err.Error())
 		}
-		return status.New(codes.Internal, fmt.Sprintf("%s [%s]", bizErr.Error(), kerrors.ErrBiz.Error()))
 	}
 	// return GRPCStatus() if err is built with status.Error
 	if se, ok := err.(interface{ GRPCStatus() *status.Status }); ok {
@@ -60,9 +64,21 @@ func convertStatus(err error) *status.Status {
 	if te, ok := err.(*remote.TransError); ok {
 		return status.New(codes.Code(te.TypeID()), err.Error())
 	}
-	// covert from kitex error
-	if c, ok := kitexErrConvTab[err]; ok {
-		return status.New(c, err.Error())
-	}
 	return status.New(codes.Internal, err.Error())
 }
+
+func getStatusForBizErr(dErr *kerrors.DetailedError) *status.Status {
+	if !dErr.Is(kerrors.ErrBiz) {
+		return nil
+	}
+	bizErr := dErr.Unwrap()
+	if se, ok := bizErr.(status.Iface); ok {
+		gs := se.GRPCStatus()
+		gs.AppendMessage(fmt.Sprintf("[%s]", kerrors.ErrBiz.Error())).Err()
+		return gs
+	}
+	if te, ok := bizErr.(*remote.TransError); ok {
+		return status.New(codes.Code(te.TypeID()), fmt.Sprintf("%s [%s]", bizErr.Error(), kerrors.ErrBiz.Error()))
+	}
+	return status.New(codes.Internal, fmt.Sprintf("%s [%s]", bizErr.Error(), kerrors.ErrBiz.Error()))
+}
diff --git a/pkg/remote/trans/nphttp2/codes_test.go b/pkg/remote/trans/nphttp2/codes_test.go
@@ -27,10 +27,78 @@ import (
 	"github.com/cloudwego/kitex/pkg/remote/trans/nphttp2/status"
 )
 
+var _ error = (*mockError)(nil)
+
+type mockError struct{}
+
+func (m mockError) Error() string {
+	return "mock"
+}
+
 func TestConvertStatus(t *testing.T) {
-	test.Assert(t, convertStatus(nil).Code() == codes.OK)
-	test.Assert(t, convertStatus(errors.New("mock")).Code() == codes.Internal)
-	test.Assert(t, convertStatus(status.Err(100, "mock")).Code() == 100)
-	test.Assert(t, convertStatus(remote.NewTransErrorWithMsg(100, "mock")).Code() == 100)
-	test.Assert(t, convertStatus(kerrors.ErrInternalException).Code() == codes.Internal)
+	t.Run("nil", func(t *testing.T) {
+		test.Assert(t, convertStatus(nil).Code() == codes.OK)
+	})
+
+	t.Run("Biz-GRPCStatusError", func(t *testing.T) {
+		err := kerrors.ErrBiz.WithCause(status.Err(100, "mock"))
+		test.Assert(t, convertStatus(err).Code() == 100)
+	})
+
+	t.Run("Biz-TransError", func(t *testing.T) {
+		transErr := remote.NewTransError(101, errors.New("mock_trans_err"))
+		err := kerrors.ErrBiz.WithCause(transErr)
+		test.Assert(t, convertStatus(err).Code() == 101)
+	})
+
+	t.Run("Biz-Other", func(t *testing.T) {
+		err := kerrors.ErrBiz.WithCause(errors.New("mock"))
+		test.Assert(t, convertStatus(err).Code() == codes.Internal)
+	})
+
+	t.Run("GRPCStatusError", func(t *testing.T) {
+		test.Assert(t, convertStatus(status.Err(102, "mock")).Code() == 102)
+	})
+
+	t.Run("TransError", func(t *testing.T) {
+		test.Assert(t, convertStatus(remote.NewTransErrorWithMsg(103, "mock")).Code() == 103)
+	})
+
+	t.Run("basicError-conv-table-ACL", func(t *testing.T) {
+		test.Assert(t, convertStatus(kerrors.ErrACL).Code() == codes.PermissionDenied)
+	})
+
+	t.Run("basicError-conv-table-ErrInternalException", func(t *testing.T) {
+		test.Assert(t, convertStatus(kerrors.ErrInternalException).Code() == codes.Internal)
+	})
+
+	t.Run("basicError-conv-table-OverLimit", func(t *testing.T) {
+		test.Assert(t, convertStatus(kerrors.ErrOverlimit).Code() == codes.ResourceExhausted)
+	})
+
+	t.Run("basicError-conv-table-RemoteOrNetwork", func(t *testing.T) {
+		test.Assert(t, convertStatus(kerrors.ErrRemoteOrNetwork).Code() == codes.Unavailable)
+	})
+
+	t.Run("basicError-unknown", func(t *testing.T) {
+		test.Assert(t, convertStatus(kerrors.ErrServiceDiscovery).Code() == codes.Internal)
+	})
+
+	t.Run("DetailedError-with-known-basicError", func(t *testing.T) {
+		err := kerrors.ErrACL.WithCause(errors.New("mock"))
+		test.Assert(t, convertStatus(err).Code() == codes.PermissionDenied)
+	})
+
+	t.Run("DetailedError-with-unknown-basicError", func(t *testing.T) {
+		err := kerrors.ErrServiceDiscovery.WithCause(errors.New("mock"))
+		test.Assert(t, convertStatus(err).Code() == codes.Internal)
+	})
+
+	t.Run("std-error", func(t *testing.T) {
+		test.Assert(t, convertStatus(errors.New("mock")).Code() == codes.Internal)
+	})
+
+	t.Run("user-defined-error", func(t *testing.T) {
+		test.Assert(t, convertStatus(&mockError{}).Code() == codes.Internal)
+	})
 }