Skip to content

Commit

Permalink
Merge pull request #16 from gubernator-io/thrawn/fix-nil-error
Browse files Browse the repository at this point in the history
Fixed possible nil err in asyncRequest
  • Loading branch information
thrawn01 authored May 20, 2024
2 parents 81e0745 + 40b0ddb commit 44ed3be
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 11 deletions.
9 changes: 5 additions & 4 deletions functional_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -743,11 +743,12 @@ func TestLeakyBucketGregorian(t *testing.T) {
},
}

// Truncate to the nearest minute
// Truncate to the nearest minute.
now := clock.Now()
now = now.Truncate(1 * time.Minute)
// So we don't start on the minute boundary
now = now.Add(time.Millisecond * 100)
trunc := now.Truncate(time.Hour)
trunc = now.Add(now.Sub(trunc))
clock.Advance(now.Sub(trunc))

name := t.Name()
key := guber.RandomString(10)

Expand Down
15 changes: 8 additions & 7 deletions gubernator.go
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ func (s *V1Instance) asyncRequest(ctx context.Context, req *AsyncReq) {
WithField("key", req.Key).
Error("GetPeer() returned peer that is not connected")
countError(err, "Peer not connected")
err = errors.Wrapf(err, "GetPeer() keeps returning peers that are not connected for '%s'", req.Key)
err = fmt.Errorf("GetPeer() keeps returning peers that are not connected for '%s': %w", req.Key, err)
resp.Resp = &RateLimitResp{Error: err.Error()}
break
}
Expand All @@ -344,25 +344,26 @@ func (s *V1Instance) asyncRequest(ctx context.Context, req *AsyncReq) {
WithError(err).
WithField("key", req.Key).
Error("Error applying rate limit")
err = errors.Wrapf(err, "Error in getLocalRateLimit for '%s'", req.Key)
err = fmt.Errorf("during getLocalRateLimit() for '%s': %w", req.Key, err)
resp.Resp = &RateLimitResp{Error: err.Error()}
}
break
}
}

// Make an RPC call to the peer that owns this rate limit
r, err := req.Peer.GetPeerRateLimit(ctx, req.Req)
var r *RateLimitResp
r, err = req.Peer.GetPeerRateLimit(ctx, req.Req)
if err != nil {
if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
attempts++
metricBatchSendRetries.WithLabelValues(req.Req.Name).Inc()
req.Peer, err = s.GetPeer(ctx, req.Key)
if err != nil {
errPart := fmt.Sprintf("Error finding peer that owns rate limit '%s'", req.Key)
errPart := fmt.Sprintf("while finding peer that owns rate limit '%s'", req.Key)
s.log.WithContext(ctx).WithError(err).WithField("key", req.Key).Error(errPart)
countError(err, "Error in GetPeer")
err = errors.Wrap(err, errPart)
countError(err, "during GetPeer()")
err = fmt.Errorf("%s: %w", errPart, err)
resp.Resp = &RateLimitResp{Error: err.Error()}
break
}
Expand All @@ -371,7 +372,7 @@ func (s *V1Instance) asyncRequest(ctx context.Context, req *AsyncReq) {

// Not calling `countError()` because we expect the remote end to
// report this error.
err = errors.Wrap(err, fmt.Sprintf("Error while fetching rate limit '%s' from peer", req.Key))
err = fmt.Errorf("while fetching rate limit '%s' from peer: %w", req.Key, err)
resp.Resp = &RateLimitResp{Error: err.Error()}
break
}
Expand Down

0 comments on commit 44ed3be

Please sign in to comment.