From 9f058e46a79d2adc4eccd5c0158a4501ff07f4a8 Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Mon, 23 Oct 2023 14:36:40 +0300 Subject: [PATCH] allowed to create xerrors.TransportError() from grpc status error + refactoring of xerrors tests --- internal/xerrors/operation_test.go | 79 +++++++-- internal/xerrors/pessimized_error_test.go | 106 ++++++++++++ internal/xerrors/transport.go | 21 ++- internal/xerrors/transport_test.go | 202 +++++++++++----------- 4 files changed, 285 insertions(+), 123 deletions(-) create mode 100644 internal/xerrors/pessimized_error_test.go diff --git a/internal/xerrors/operation_test.go b/internal/xerrors/operation_test.go index 593578010..41d735957 100644 --- a/internal/xerrors/operation_test.go +++ b/internal/xerrors/operation_test.go @@ -10,21 +10,72 @@ import ( ) func TestIsOperationError(t *testing.T) { - for _, code := range [...]Ydb.StatusIds_StatusCode{ - Ydb.StatusIds_BAD_REQUEST, - Ydb.StatusIds_BAD_SESSION, + for _, tt := range []struct { + err error + codes []Ydb.StatusIds_StatusCode + match bool + }{ + // check only operation error with any ydb status code + { + err: &operationError{code: Ydb.StatusIds_BAD_REQUEST}, + match: true, + }, + { + err: fmt.Errorf("wrapped: %w", &operationError{code: Ydb.StatusIds_BAD_REQUEST}), + match: true, + }, + { + err: Join( + fmt.Errorf("test"), + &operationError{code: Ydb.StatusIds_BAD_REQUEST}, + Retryable(fmt.Errorf("test")), + ), + match: true, + }, + // match ydb status code + { + err: &operationError{code: Ydb.StatusIds_BAD_REQUEST}, + codes: []Ydb.StatusIds_StatusCode{Ydb.StatusIds_BAD_REQUEST}, + match: true, + }, + { + err: fmt.Errorf("wrapped: %w", &operationError{code: Ydb.StatusIds_BAD_REQUEST}), + codes: []Ydb.StatusIds_StatusCode{Ydb.StatusIds_BAD_REQUEST}, + match: true, + }, + { + err: Join( + fmt.Errorf("test"), + &operationError{code: Ydb.StatusIds_BAD_REQUEST}, + Retryable(fmt.Errorf("test")), + ), + codes: []Ydb.StatusIds_StatusCode{Ydb.StatusIds_BAD_REQUEST}, + match: true, + }, + // no match ydb status code + { + err: &operationError{code: Ydb.StatusIds_BAD_REQUEST}, + codes: []Ydb.StatusIds_StatusCode{Ydb.StatusIds_ABORTED}, + match: false, + }, + { + err: fmt.Errorf("wrapped: %w", &operationError{code: Ydb.StatusIds_BAD_REQUEST}), + codes: []Ydb.StatusIds_StatusCode{Ydb.StatusIds_ABORTED}, + match: false, + }, + { + err: Join( + fmt.Errorf("test"), + &operationError{code: Ydb.StatusIds_BAD_REQUEST}, + Retryable(fmt.Errorf("test")), + ), + codes: []Ydb.StatusIds_StatusCode{Ydb.StatusIds_ABORTED}, + match: false, + }, } { - for _, err := range []error{ - &operationError{code: code}, - Operation(WithStatusCode(code)), - fmt.Errorf("wrapped: %w", &operationError{code: code}), - } { - t.Run("", func(t *testing.T) { - if !IsOperationError(err, code) { - t.Errorf("expected %v to be operationError with code=%v", err, code) - } - }) - } + t.Run("", func(t *testing.T) { + require.Equal(t, tt.match, IsOperationError(tt.err, tt.codes...)) + }) } } diff --git a/internal/xerrors/pessimized_error_test.go b/internal/xerrors/pessimized_error_test.go new file mode 100644 index 000000000..403587849 --- /dev/null +++ b/internal/xerrors/pessimized_error_test.go @@ -0,0 +1,106 @@ +package xerrors + +import ( + "context" + "errors" + "fmt" + "testing" + + grpcCodes "google.golang.org/grpc/codes" + grpcStatus "google.golang.org/grpc/status" +) + +func TestMustPessimizeEndpoint(t *testing.T) { + for _, test := range []struct { + error error + pessimize bool + }{ + { + error: Transport(grpcStatus.Error(grpcCodes.Canceled, "")), + pessimize: true, + }, + { + error: Transport(grpcStatus.Error(grpcCodes.Unknown, "")), + pessimize: true, + }, + { + error: Transport(grpcStatus.Error(grpcCodes.InvalidArgument, "")), + pessimize: true, + }, + { + error: Transport(grpcStatus.Error(grpcCodes.DeadlineExceeded, "")), + pessimize: true, + }, + { + error: Transport(grpcStatus.Error(grpcCodes.NotFound, "")), + pessimize: true, + }, + { + error: Transport(grpcStatus.Error(grpcCodes.AlreadyExists, "")), + pessimize: true, + }, + { + error: Transport(grpcStatus.Error(grpcCodes.PermissionDenied, "")), + pessimize: true, + }, + { + error: Transport(grpcStatus.Error(grpcCodes.ResourceExhausted, "")), + pessimize: false, + }, + { + error: Transport(grpcStatus.Error(grpcCodes.FailedPrecondition, "")), + pessimize: true, + }, + { + error: Transport(grpcStatus.Error(grpcCodes.Aborted, "")), + pessimize: true, + }, + { + error: Transport(grpcStatus.Error(grpcCodes.OutOfRange, "")), + pessimize: false, + }, + { + error: Transport(grpcStatus.Error(grpcCodes.Unimplemented, "")), + pessimize: true, + }, + { + error: Transport(grpcStatus.Error(grpcCodes.Internal, "")), + pessimize: true, + }, + { + error: Transport(grpcStatus.Error(grpcCodes.Unavailable, "")), + pessimize: true, + }, + { + error: Transport(grpcStatus.Error(grpcCodes.DataLoss, "")), + pessimize: true, + }, + { + error: Transport(grpcStatus.Error(grpcCodes.Unauthenticated, "")), + pessimize: true, + }, + { + error: context.Canceled, + pessimize: false, + }, + { + error: context.DeadlineExceeded, + pessimize: false, + }, + { + error: fmt.Errorf("user error"), + pessimize: false, + }, + } { + err := errors.Unwrap(test.error) + if err == nil { + err = test.error + } + t.Run(err.Error(), func(t *testing.T) { + pessimize := MustPessimizeEndpoint(test.error) + if pessimize != test.pessimize { + t.Errorf("unexpected pessimization status for error `%v`: %t, exp: %t", test.error, pessimize, test.pessimize) + } + }) + } +} diff --git a/internal/xerrors/transport.go b/internal/xerrors/transport.go index 6390c2a32..0a42b8b68 100644 --- a/internal/xerrors/transport.go +++ b/internal/xerrors/transport.go @@ -113,19 +113,23 @@ func IsTransportError(err error, codes ...grpcCodes.Code) bool { if err == nil { return false } + var status *grpcStatus.Status if t := (*transportError)(nil); errors.As(err, &t) { + status = t.status + } else if t, has := grpcStatus.FromError(err); has { + status = t + } + if status != nil { if len(codes) == 0 { return true } for _, code := range codes { - if t.status.Code() == code { + if status.Code() == code { return true } } - return false } - _, has := grpcStatus.FromError(err) - return has + return false } // Transport returns a new transport error with given options @@ -178,9 +182,18 @@ func MustPessimizeEndpoint(err error, codes ...grpcCodes.Code) bool { } func TransportError(err error) Error { + if err == nil { + return nil + } var t *transportError if errors.As(err, &t) { return t } + if s, ok := grpcStatus.FromError(err); ok { + return &transportError{ + status: s, + err: err, + } + } return nil } diff --git a/internal/xerrors/transport_test.go b/internal/xerrors/transport_test.go index c681ad39f..8cfa9af58 100644 --- a/internal/xerrors/transport_test.go +++ b/internal/xerrors/transport_test.go @@ -1,152 +1,108 @@ package xerrors import ( - "context" - "errors" "fmt" "testing" "github.com/stretchr/testify/require" - "github.com/ydb-platform/ydb-go-genproto/protos/Ydb" grpcCodes "google.golang.org/grpc/codes" grpcStatus "google.golang.org/grpc/status" ) func TestIsTransportError(t *testing.T) { - code := grpcCodes.Canceled - for _, err := range []error{ - &transportError{status: grpcStatus.New(code, "")}, - fmt.Errorf("wrapped: %w", &transportError{status: grpcStatus.New(code, "")}), - } { - t.Run("", func(t *testing.T) { - if !IsTransportError(err, code) { - t.Errorf("expected %v to be transportError with code=%v", err, code) - } - }) - } -} - -func TestIsNonTransportError(t *testing.T) { - code := grpcCodes.Canceled - for _, err := range []error{ - &transportError{status: grpcStatus.New(grpcCodes.Aborted, "")}, - fmt.Errorf("wrapped: %w", &transportError{status: grpcStatus.New(grpcCodes.Aborted, "")}), - &operationError{code: Ydb.StatusIds_BAD_REQUEST}, - } { - t.Run("", func(t *testing.T) { - if IsTransportError(err, code) { - t.Errorf("expected %v not to be transportError with code=%v", err, code) - } - }) - } -} - -func TestIsNonOperationError(t *testing.T) { - code := Ydb.StatusIds_BAD_REQUEST - for _, err := range []error{ - &operationError{code: Ydb.StatusIds_TIMEOUT}, - fmt.Errorf("wrapped: %w", &operationError{code: Ydb.StatusIds_TIMEOUT}), - &transportError{status: grpcStatus.New(grpcCodes.Aborted, "")}, - } { - t.Run("", func(t *testing.T) { - if IsOperationError(err, code) { - t.Errorf("expected %v not to be operationError with code=%v", err, code) - } - }) - } -} - -func TestMustPessimizeEndpoint(t *testing.T) { - for _, test := range []struct { - error error - pessimize bool + for _, tt := range []struct { + err error + codes []grpcCodes.Code + match bool }{ + // check only transport error with any grpc status code { - error: Transport(grpcStatus.Error(grpcCodes.Canceled, "")), - pessimize: true, - }, - { - error: Transport(grpcStatus.Error(grpcCodes.Unknown, "")), - pessimize: true, - }, - { - error: Transport(grpcStatus.Error(grpcCodes.InvalidArgument, "")), - pessimize: true, - }, - { - error: Transport(grpcStatus.Error(grpcCodes.DeadlineExceeded, "")), - pessimize: true, + err: grpcStatus.Error(grpcCodes.Canceled, ""), + match: true, }, { - error: Transport(grpcStatus.Error(grpcCodes.NotFound, "")), - pessimize: true, + err: &transportError{status: grpcStatus.New(grpcCodes.Canceled, "")}, + match: true, }, { - error: Transport(grpcStatus.Error(grpcCodes.AlreadyExists, "")), - pessimize: true, + err: fmt.Errorf("wrapped: %w", &transportError{status: grpcStatus.New(grpcCodes.Canceled, "")}), + match: true, }, { - error: Transport(grpcStatus.Error(grpcCodes.PermissionDenied, "")), - pessimize: true, + err: fmt.Errorf("wrapped: %w", grpcStatus.Error(grpcCodes.Canceled, "")), + match: true, }, { - error: Transport(grpcStatus.Error(grpcCodes.ResourceExhausted, "")), - pessimize: false, + err: Join( + fmt.Errorf("test"), + grpcStatus.Error(grpcCodes.Canceled, ""), + Retryable(fmt.Errorf("test")), + ), + match: true, }, + // match grpc status code { - error: Transport(grpcStatus.Error(grpcCodes.FailedPrecondition, "")), - pessimize: true, + err: grpcStatus.Error(grpcCodes.Canceled, ""), + codes: []grpcCodes.Code{grpcCodes.Canceled}, + match: true, }, { - error: Transport(grpcStatus.Error(grpcCodes.Aborted, "")), - pessimize: true, + err: &transportError{status: grpcStatus.New(grpcCodes.Canceled, "")}, + codes: []grpcCodes.Code{grpcCodes.Canceled}, + match: true, }, { - error: Transport(grpcStatus.Error(grpcCodes.OutOfRange, "")), - pessimize: false, + err: fmt.Errorf("wrapped: %w", &transportError{status: grpcStatus.New(grpcCodes.Canceled, "")}), + codes: []grpcCodes.Code{grpcCodes.Canceled}, + match: true, }, { - error: Transport(grpcStatus.Error(grpcCodes.Unimplemented, "")), - pessimize: true, + err: fmt.Errorf("wrapped: %w", grpcStatus.Error(grpcCodes.Canceled, "")), + codes: []grpcCodes.Code{grpcCodes.Canceled}, + match: true, }, { - error: Transport(grpcStatus.Error(grpcCodes.Internal, "")), - pessimize: true, + err: Join( + fmt.Errorf("test"), + grpcStatus.Error(grpcCodes.Canceled, ""), + Retryable(fmt.Errorf("test")), + ), + codes: []grpcCodes.Code{grpcCodes.Canceled}, + match: true, }, + // no match grpc status code { - error: Transport(grpcStatus.Error(grpcCodes.Unavailable, "")), - pessimize: true, + err: grpcStatus.Error(grpcCodes.Canceled, ""), + codes: []grpcCodes.Code{grpcCodes.Aborted}, + match: false, }, { - error: Transport(grpcStatus.Error(grpcCodes.DataLoss, "")), - pessimize: true, + err: &transportError{status: grpcStatus.New(grpcCodes.Canceled, "")}, + codes: []grpcCodes.Code{grpcCodes.Aborted}, + match: false, }, { - error: Transport(grpcStatus.Error(grpcCodes.Unauthenticated, "")), - pessimize: true, + err: fmt.Errorf("wrapped: %w", &transportError{status: grpcStatus.New(grpcCodes.Canceled, "")}), + codes: []grpcCodes.Code{grpcCodes.Aborted}, + match: false, }, { - error: context.Canceled, - pessimize: false, + err: fmt.Errorf("wrapped: %w", grpcStatus.Error(grpcCodes.Canceled, "")), + codes: []grpcCodes.Code{grpcCodes.Aborted}, + match: false, }, { - error: context.DeadlineExceeded, - pessimize: false, - }, - { - error: fmt.Errorf("user error"), - pessimize: false, + err: Join( + fmt.Errorf("test"), + grpcStatus.Error(grpcCodes.Canceled, ""), + Retryable(fmt.Errorf("test")), + ), + codes: []grpcCodes.Code{grpcCodes.Aborted}, + match: false, }, } { - err := errors.Unwrap(test.error) - if err == nil { - err = test.error - } - t.Run(err.Error(), func(t *testing.T) { - pessimize := MustPessimizeEndpoint(test.error) - if pessimize != test.pessimize { - t.Errorf("unexpected pessimization status for error `%v`: %t, exp: %t", test.error, pessimize, test.pessimize) - } + t.Run("", func(t *testing.T) { + require.Equal(t, tt.match, IsTransportError(tt.err, tt.codes...)) }) } } @@ -188,3 +144,39 @@ func Test_transportError_Error(t *testing.T) { }) } } + +func TestTransportErrorName(t *testing.T) { + for _, tt := range []struct { + err error + name string + }{ + { + err: nil, + name: "", + }, + { + err: grpcStatus.Error(grpcCodes.Aborted, ""), + name: "transport/Aborted", + }, + { + err: TransportError(grpcStatus.Error(grpcCodes.Aborted, "")), + name: "transport/Aborted", + }, + { + err: WithStackTrace(grpcStatus.Error(grpcCodes.Aborted, "")), + name: "transport/Aborted", + }, + { + err: WithStackTrace(TransportError(grpcStatus.Error(grpcCodes.Aborted, ""))), + name: "transport/Aborted", + }, + } { + t.Run("", func(t *testing.T) { + if tt.err == nil { + require.Nil(t, TransportError(tt.err)) + } else { + require.Equal(t, tt.name, TransportError(tt.err).Name()) + } + }) + } +}