Merge pull request #1371 from ydb-platform/xerrors

xerrors improvements

Author: asmyasnikov

internal/balancer/balancer.go

@@ -291,7 +291,7 @@ func (b *Balancer) wrapCall(ctx context.Context, f func(ctx context.Context, cc
 			if cc.GetState() == conn.Banned {
 				b.pool.Allow(ctx, cc)
 			}
-		} else if xerrors.MustPessimizeEndpoint(err, b.driverConfig.ExcludeGRPCCodesForPessimization()...) {
+		} else if conn.IsBadConn(err, b.driverConfig.ExcludeGRPCCodesForPessimization()...) {
 			b.pool.Ban(ctx, cc, err)
 		}
 	}()

internal/conn/errors.go

@@ -0,0 +1,40 @@
+package conn
+
+import (
+	grpcCodes "google.golang.org/grpc/codes"
+
+	"github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors"
+)
+
+func IsBadConn(err error, goodConnCodes ...grpcCodes.Code) bool {
+	if !xerrors.IsTransportError(err) {
+		return false
+	}
+
+	if xerrors.IsTransportError(err,
+		append(
+			goodConnCodes,
+			grpcCodes.ResourceExhausted,
+			grpcCodes.OutOfRange,
+			grpcCodes.OK,
+			// grpcCodes.Canceled,
+			// grpcCodes.Unknown,
+			// grpcCodes.InvalidArgument,
+			// grpcCodes.DeadlineExceeded,
+			// grpcCodes.NotFound,
+			// grpcCodes.AlreadyExists,
+			// grpcCodes.PermissionDenied,
+			// grpcCodes.FailedPrecondition,
+			// grpcCodes.Aborted,
+			// grpcCodes.OutOfRange,
+			// grpcCodes.Unimplemented,
+			// grpcCodes.Internal,
+			// grpcCodes.DataLoss,
+			// grpcCodes.Unauthenticated,
+		)...,
+	) {
+		return false
+	}
+
+	return true
+}

internal/conn/errors_test.go

@@ -0,0 +1,110 @@
+package conn
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	grpcCodes "google.golang.org/grpc/codes"
+	grpcStatus "google.golang.org/grpc/status"
+
+	"github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors"
+)
+
+func TestIsBadConn(t *testing.T) {
+	for i, tt := range []struct {
+		err           error
+		goodConnCodes []grpcCodes.Code
+		badConn       bool
+	}{
+		{
+			err:     fmt.Errorf("test"),
+			badConn: false,
+		},
+		{
+			err:     xerrors.Operation(),
+			badConn: false,
+		},
+		{
+			err:     xerrors.Transport(grpcStatus.Error(grpcCodes.ResourceExhausted, "")),
+			badConn: false,
+		},
+		{
+			err:     xerrors.Transport(grpcStatus.Error(grpcCodes.Unavailable, "")),
+			badConn: true,
+		},
+		{
+			err:     xerrors.Transport(grpcStatus.Error(grpcCodes.OK, "")),
+			badConn: false,
+		},
+		{
+			err:     xerrors.Transport(grpcStatus.Error(grpcCodes.Canceled, "")),
+			badConn: true,
+		},
+		{
+			err:     xerrors.Transport(grpcStatus.Error(grpcCodes.Unknown, "")),
+			badConn: true,
+		},
+		{
+			err:     xerrors.Transport(grpcStatus.Error(grpcCodes.InvalidArgument, "")),
+			badConn: true,
+		},
+		{
+			err:     xerrors.Transport(grpcStatus.Error(grpcCodes.DeadlineExceeded, "")),
+			badConn: true,
+		},
+		{
+			err:     xerrors.Transport(grpcStatus.Error(grpcCodes.NotFound, "")),
+			badConn: true,
+		},
+		{
+			err:     xerrors.Transport(grpcStatus.Error(grpcCodes.AlreadyExists, "")),
+			badConn: true,
+		},
+		{
+			err:     xerrors.Transport(grpcStatus.Error(grpcCodes.PermissionDenied, "")),
+			badConn: true,
+		},
+		{
+			err:     xerrors.Transport(grpcStatus.Error(grpcCodes.FailedPrecondition, "")),
+			badConn: true,
+		},
+		{
+			err:     xerrors.Transport(grpcStatus.Error(grpcCodes.Aborted, "")),
+			badConn: true,
+		},
+		{
+			err:     xerrors.Transport(grpcStatus.Error(grpcCodes.OutOfRange, "")),
+			badConn: false,
+		},
+		{
+			err:     xerrors.Transport(grpcStatus.Error(grpcCodes.Unimplemented, "")),
+			badConn: true,
+		},
+		{
+			err:     xerrors.Transport(grpcStatus.Error(grpcCodes.Internal, "")),
+			badConn: true,
+		},
+		{
+			err:     xerrors.Transport(grpcStatus.Error(grpcCodes.DataLoss, "")),
+			badConn: true,
+		},
+		{
+			err:     xerrors.Transport(grpcStatus.Error(grpcCodes.Unauthenticated, "")),
+			badConn: true,
+		},
+		{
+			err: xerrors.Transport(grpcStatus.Error(grpcCodes.Unauthenticated, "")),
+			goodConnCodes: []grpcCodes.Code{
+				grpcCodes.Unauthenticated,
+			},
+			badConn: false,
+		},
+	} {
+		t.Run(fmt.Sprintf("%d. %v", i, tt.err), func(t *testing.T) {
+			require.Equal(t, tt.badConn, IsBadConn(tt.err, tt.goodConnCodes...))
+			require.Equal(t, tt.badConn, IsBadConn(xerrors.WithStackTrace(tt.err), tt.goodConnCodes...))
+			require.Equal(t, tt.badConn, IsBadConn(xerrors.Retryable(tt.err), tt.goodConnCodes...))
+		})
+	}
+}

internal/xerrors/pessimized_error_test.go

@@ -97,6 +97,9 @@ func InvalidObject() invalidObjectOption {
 }
 
 func Retryable(err error, opts ...RetryableErrorOption) error {
+	if err == nil {
+		return nil
+	}
 	var (
 		e  Error
 		re = &retryableError{

internal/xerrors/retryable.go

@@ -180,27 +180,6 @@ func Transport(err error, opts ...teOpt) error {
 	return te
 }
 
-func MustPessimizeEndpoint(err error, codes ...grpcCodes.Code) bool {
-	switch {
-	case err == nil:
-		return false
-
-	// all transport errors except selected codes
-	case IsTransportError(err) && !IsTransportError(
-		err,
-		append(
-			codes,
-			grpcCodes.ResourceExhausted,
-			grpcCodes.OutOfRange,
-		)...,
-	):
-		return true
-
-	default:
-		return false
-	}
-}
-
 func TransportError(err error) Error {
 	if err == nil {
 		return nil

internal/xerrors/transport.go

@@ -143,7 +143,7 @@ func TestGrpcError(t *testing.T) {
 	}
 }
 
-func Test_transportError_Error(t *testing.T) {
+func TestTransportErrorString(t *testing.T) {
 	for _, tt := range []struct {
 		err  error
 		text string