Fix %w verb for error wrapping in grpcclient (#17484)

Signed-off-by: Congqi Xia <congqi.xia@zilliz.com>
2024-12-02 20:09:57 +08:00 · 2022-06-13 12:48:08 +08:00 · 2022-06-13 12:48:08 +08:00 · d1de8cabdd
commit d1de8cabdd
parent a206f7274b
2 changed files with 200 additions and 5 deletions
--- a/internal/util/grpcclient/client.go
+++ b/internal/util/grpcclient/client.go
@ -223,8 +223,8 @@ func (c *ClientBase) Call(ctx context.Context, caller func(client interface{}) (

 	ret, err := c.callOnce(ctx, caller)
 	if err != nil {
-		traceErr := fmt.Errorf("err: %s\n, %s", err.Error(), trace.StackTrace())
-		log.Error(c.GetRole()+" ClientBase Call grpc first call get error ", zap.Error(traceErr))
+		traceErr := fmt.Errorf("err: %w\n, %s", err, trace.StackTrace())
+		log.Error("ClientBase Call grpc first call get error", zap.String("role", c.GetRole()), zap.Error(traceErr))
 		return nil, traceErr
 	}
 	return ret, err
@ -241,7 +241,7 @@ func (c *ClientBase) ReCall(ctx context.Context, caller func(client interface{})
 		return ret, nil
 	}

-	traceErr := fmt.Errorf("err: %s\n, %s", err.Error(), trace.StackTrace())
+	traceErr := fmt.Errorf("err: %w\n, %s", err, trace.StackTrace())
 	log.Warn(c.GetRole()+" ClientBase ReCall grpc first call get error ", zap.Error(traceErr))

 	if !funcutil.CheckCtxValid(ctx) {
@ -250,8 +250,8 @@ func (c *ClientBase) ReCall(ctx context.Context, caller func(client interface{})

 	ret, err = c.callOnce(ctx, caller)
 	if err != nil {
-		traceErr = fmt.Errorf("err: %s\n, %s", err.Error(), trace.StackTrace())
-		log.Error(c.GetRole()+" ClientBase ReCall grpc second call get error ", zap.Error(traceErr))
+		traceErr = fmt.Errorf("err: %w\n, %s", err, trace.StackTrace())
+		log.Error("ClientBase ReCall grpc second call get error", zap.String("role", c.GetRole()), zap.Error(traceErr))
 		return nil, traceErr
 	}
 	return ret, err
--- a/internal/util/grpcclient/client_test.go
+++ b/internal/util/grpcclient/client_test.go
@ -19,10 +19,13 @@ package grpcclient
 import (
 	"context"
 	"errors"
+	"sync"
 	"testing"
 	"time"

 	"github.com/stretchr/testify/assert"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )

 func TestClientBase_SetRole(t *testing.T) {
@ -49,4 +52,196 @@ func TestClientBase_connect(t *testing.T) {
 		assert.Error(t, err)
 		assert.True(t, errors.Is(err, ErrConnect))
 	})
+
+	t.Run("failed to get addr", func(t *testing.T) {
+		errMock := errors.New("mocked")
+		base := ClientBase{
+			getAddrFunc: func() (string, error) {
+				return "", errMock
+			},
+			DialTimeout: time.Millisecond,
+		}
+		err := base.connect(context.Background())
+		assert.Error(t, err)
+		assert.True(t, errors.Is(err, errMock))
+	})
+}
+
+func TestClientBase_Call(t *testing.T) {
+	// mock client with nothing
+	base := ClientBase{}
+	base.grpcClientMtx.Lock()
+	base.grpcClient = struct{}{}
+	base.grpcClientMtx.Unlock()
+
+	t.Run("Call normal return", func(t *testing.T) {
+		_, err := base.Call(context.Background(), func(client interface{}) (interface{}, error) {
+			return struct{}{}, nil
+		})
+		assert.NoError(t, err)
+	})
+
+	t.Run("Call with canceled context", func(t *testing.T) {
+		ctx, cancel := context.WithCancel(context.Background())
+		cancel()
+		_, err := base.Call(ctx, func(client interface{}) (interface{}, error) {
+			return struct{}{}, nil
+		})
+		assert.Error(t, err)
+		assert.True(t, errors.Is(err, context.Canceled))
+	})
+
+	t.Run("Call canceled in caller func", func(t *testing.T) {
+		ctx, cancel := context.WithCancel(context.Background())
+		errMock := errors.New("mocked")
+		_, err := base.Call(ctx, func(client interface{}) (interface{}, error) {
+			cancel()
+			return nil, errMock
+		})
+
+		assert.Error(t, err)
+		assert.True(t, errors.Is(err, errMock))
+		base.grpcClientMtx.RLock()
+		// client shall not be reset
+		assert.NotNil(t, base.grpcClient)
+		base.grpcClientMtx.RUnlock()
+	})
+
+	t.Run("Call canceled in caller func", func(t *testing.T) {
+		ctx, cancel := context.WithCancel(context.Background())
+		errMock := errors.New("mocked")
+		_, err := base.Call(ctx, func(client interface{}) (interface{}, error) {
+			cancel()
+			return nil, errMock
+		})
+
+		assert.Error(t, err)
+		assert.True(t, errors.Is(err, errMock))
+		base.grpcClientMtx.RLock()
+		// client shall not be reset
+		assert.NotNil(t, base.grpcClient)
+		base.grpcClientMtx.RUnlock()
+	})
+
+	t.Run("Call returns non-grpc error", func(t *testing.T) {
+		ctx, cancel := context.WithCancel(context.Background())
+		defer cancel()
+		errMock := errors.New("mocked")
+		_, err := base.Call(ctx, func(client interface{}) (interface{}, error) {
+			return nil, errMock
+		})
+
+		assert.Error(t, err)
+		assert.True(t, errors.Is(err, errMock))
+		base.grpcClientMtx.RLock()
+		// client shall not be reset
+		assert.NotNil(t, base.grpcClient)
+		base.grpcClientMtx.RUnlock()
+	})
+
+	t.Run("Call returns grpc error", func(t *testing.T) {
+		ctx, cancel := context.WithCancel(context.Background())
+		defer cancel()
+		errGrpc := status.Error(codes.Unknown, "mocked")
+		_, err := base.Call(ctx, func(client interface{}) (interface{}, error) {
+			return nil, errGrpc
+		})
+
+		assert.Error(t, err)
+		assert.True(t, errors.Is(err, errGrpc))
+		base.grpcClientMtx.RLock()
+		// client shall not be reset
+		assert.Nil(t, base.grpcClient)
+		base.grpcClientMtx.RUnlock()
+
+	})
+
+	base.grpcClientMtx.Lock()
+	base.grpcClient = nil
+	base.grpcClientMtx.Unlock()
+	base.SetGetAddrFunc(func() (string, error) { return "", nil })
+
+	t.Run("Call with connect failure", func(t *testing.T) {
+		ctx, cancel := context.WithCancel(context.Background())
+		defer cancel()
+		_, err := base.Call(ctx, func(client interface{}) (interface{}, error) {
+			return struct{}{}, nil
+		})
+		assert.Error(t, err)
+		assert.True(t, errors.Is(err, ErrConnect))
+	})
+}
+
+func TestClientBase_Recall(t *testing.T) {
+	// mock client with nothing
+	base := ClientBase{}
+	base.grpcClientMtx.Lock()
+	base.grpcClient = struct{}{}
+	base.grpcClientMtx.Unlock()
+
+	t.Run("Recall normal return", func(t *testing.T) {
+		_, err := base.ReCall(context.Background(), func(client interface{}) (interface{}, error) {
+			return struct{}{}, nil
+		})
+		assert.NoError(t, err)
+	})
+
+	t.Run("ReCall with canceled context", func(t *testing.T) {
+		ctx, cancel := context.WithCancel(context.Background())
+		cancel()
+		_, err := base.ReCall(ctx, func(client interface{}) (interface{}, error) {
+			return struct{}{}, nil
+		})
+		assert.Error(t, err)
+		assert.True(t, errors.Is(err, context.Canceled))
+	})
+
+	t.Run("ReCall fails first and success second", func(t *testing.T) {
+		ctx, cancel := context.WithCancel(context.Background())
+		defer cancel()
+		flag := false
+		var mut sync.Mutex
+		_, err := base.ReCall(ctx, func(client interface{}) (interface{}, error) {
+			mut.Lock()
+			defer mut.Unlock()
+			if flag {
+				return struct{}{}, nil
+			}
+			flag = true
+			return nil, errors.New("mock first")
+		})
+		assert.NoError(t, err)
+	})
+
+	t.Run("ReCall canceled in caller func", func(t *testing.T) {
+		ctx, cancel := context.WithCancel(context.Background())
+		errMock := errors.New("mocked")
+		_, err := base.ReCall(ctx, func(client interface{}) (interface{}, error) {
+			cancel()
+			return nil, errMock
+		})
+
+		assert.Error(t, err)
+		assert.True(t, errors.Is(err, context.Canceled))
+		base.grpcClientMtx.RLock()
+		// client shall not be reset
+		assert.NotNil(t, base.grpcClient)
+		base.grpcClientMtx.RUnlock()
+	})
+
+	base.grpcClientMtx.Lock()
+	base.grpcClient = nil
+	base.grpcClientMtx.Unlock()
+	base.SetGetAddrFunc(func() (string, error) { return "", nil })
+
+	t.Run("ReCall with connect failure", func(t *testing.T) {
+		ctx, cancel := context.WithCancel(context.Background())
+		defer cancel()
+		_, err := base.ReCall(ctx, func(client interface{}) (interface{}, error) {
+			return struct{}{}, nil
+		})
+		assert.Error(t, err)
+		assert.True(t, errors.Is(err, ErrConnect))
+	})
+
 }