From 38e0dca30c8cb30365d058607c47a96cafaa2e30 Mon Sep 17 00:00:00 2001 From: crazycs Date: Thu, 16 May 2024 11:19:22 +0800 Subject: [PATCH] *: fix panic in get cause error (#1344) Signed-off-by: crazycs520 --- internal/locate/region_request.go | 19 +++++++++++++++++-- internal/locate/region_request_test.go | 18 ++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/internal/locate/region_request.go b/internal/locate/region_request.go index 07dda82e5af..04d0d12c59e 100644 --- a/internal/locate/region_request.go +++ b/internal/locate/region_request.go @@ -180,6 +180,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 { @@ -2115,7 +2126,7 @@ func (s *RegionRequestSender) sendReqToRegion( 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) } @@ -2211,7 +2222,11 @@ func (s *RegionRequestSender) onSendFail(bo *retry.Backoffer, ctx *RPCContext, r } } } - metrics.TiKVRPCErrorCounter.WithLabelValues(errors.Cause(err).Error(), storeLabel).Inc() + if errStr := getErrMsg(err); len(errStr) > 0 { + metrics.TiKVRPCErrorCounter.WithLabelValues(getErrMsg(err), storeLabel).Inc() + } else { + metrics.TiKVRPCErrorCounter.WithLabelValues("unknown", storeLabel).Inc() + } if ctx.Store != nil && ctx.Store.storeType == tikvrpc.TiFlashCompute { s.regionCache.InvalidateTiFlashComputeStoresIfGRPCError(err) diff --git a/internal/locate/region_request_test.go b/internal/locate/region_request_test.go index 797b739b57a..230bb9a5124 100644 --- a/internal/locate/region_request_test.go +++ b/internal/locate/region_request_test.go @@ -56,6 +56,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/config/retry" @@ -872,3 +873,20 @@ func (s *testRegionRequestToSingleStoreSuite) TestRegionRequestStats() { } s.Contains(expecteds, access.String()) } + +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)) +}