Skip to content

Commit

Permalink
*: fix panic in get cause error (tikv#1344)
Browse files Browse the repository at this point in the history
Signed-off-by: crazycs520 <crazycs520@gmail.com>
  • Loading branch information
crazycs520 committed May 16, 2024
1 parent 2b01126 commit 5602666
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 2 deletions.
19 changes: 17 additions & 2 deletions internal/locate/region_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,17 @@ func (r *RequestErrorStats) RecordRPCErrorStats(errLabel string) {
}
}

// getErrMsg returns error message. if the error has cause error, then return cause error message.
func getErrMsg(err error) string {
if err == nil {
return ""
}
if causeErr := errors.Cause(err); causeErr != nil {
return causeErr.Error()
}
return err.Error()
}

// String implements fmt.Stringer interface.
func (r *RegionRequestRuntimeStats) String() string {
if r == nil {
Expand Down Expand Up @@ -1794,7 +1805,7 @@ func (s *RegionRequestSender) sendReqToRegion(bo *retry.Backoffer, rpcCtx *RPCCo
if err != nil {
s.rpcError = err
if s.Stats != nil {
errStr := errors.Cause(err).Error()
errStr := getErrMsg(err)
s.Stats.RecordRPCErrorStats(errStr)
s.recordRPCAccessInfo(req, rpcCtx, errStr)
}
Expand Down Expand Up @@ -1881,7 +1892,11 @@ func (s *RegionRequestSender) onSendFail(bo *retry.Backoffer, ctx *RPCContext, r
logutil.Logger(bo.GetCtx()).Warn("receive a grpc cancel signal from remote", zap.Error(err))
}
}
metrics.TiKVRPCErrorCounter.WithLabelValues(errors.Cause(err).Error(), storeLabel).Inc()
if errStr := getErrMsg(err); len(errStr) > 0 {
metrics.TiKVRPCErrorCounter.WithLabelValues(errStr, storeLabel).Inc()
} else {
metrics.TiKVRPCErrorCounter.WithLabelValues("unknown", storeLabel).Inc()
}

if ctx.Store != nil && ctx.Store.storeType == tikvrpc.TiFlashCompute {
s.regionCache.InvalidateTiFlashComputeStoresIfGRPCError(err)
Expand Down
18 changes: 18 additions & 0 deletions internal/locate/region_request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import (
"github.com/pingcap/kvproto/pkg/mpp"
"github.com/pingcap/kvproto/pkg/tikvpb"
"github.com/pkg/errors"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
"github.com/tikv/client-go/v2/config"
"github.com/tikv/client-go/v2/internal/client"
Expand Down Expand Up @@ -744,3 +745,20 @@ func (s *testRegionRequestToSingleStoreSuite) TestBatchClientSendLoopPanic() {
// batchSendLoop should not panic.
s.Equal(atomic.LoadInt64(&client.BatchSendLoopPanicCounter), int64(0))
}

type noCauseError struct {
error
}

func (_ noCauseError) Cause() error {
return nil
}

func TestGetErrMsg(t *testing.T) {
err := noCauseError{error: errors.New("no cause err")}
require.Equal(t, nil, errors.Cause(err))
require.Panicsf(t, func() {
_ = errors.Cause(err).Error()
}, "should panic")
require.Equal(t, "no cause err", getErrMsg(err))
}

0 comments on commit 5602666

Please sign in to comment.