Skip to content

Commit 31dfd66

Browse files
authored
Merge pull request #4 from codeGROOVE-dev/merge_129
merge: No delay after final retry on max attempts avast#129
2 parents ea9d019 + 5aa569e commit 31dfd66

File tree

2 files changed

+52
-6
lines changed

2 files changed

+52
-6
lines changed

retry.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -175,8 +175,8 @@ func DoWithData[T any](retryableFunc RetryableFuncWithData[T], opts ...Option) (
175175
attemptsForError[err] = attempts
176176
}
177177

178-
shouldRetry := true
179-
for shouldRetry {
178+
shouldRetry:
179+
for {
180180
t, err := retryableFunc()
181181
if err == nil {
182182
return t, nil
@@ -192,13 +192,15 @@ func DoWithData[T any](retryableFunc RetryableFuncWithData[T], opts ...Option) (
192192
if errors.Is(err, errToCheck) {
193193
attempts--
194194
attemptsForError[errToCheck] = attempts
195-
shouldRetry = shouldRetry && attempts > 0
195+
if attempts <= 0 {
196+
break shouldRetry
197+
}
196198
}
197199
}
198200

199201
// if this is last attempt - don't wait
200202
if n == config.attempts-1 {
201-
break
203+
break shouldRetry
202204
}
203205

204206
config.onRetry(n, err)
@@ -213,8 +215,6 @@ func DoWithData[T any](retryableFunc RetryableFuncWithData[T], opts ...Option) (
213215

214216
return emptyT, append(errorLog, context.Cause(config.context))
215217
}
216-
217-
shouldRetry = shouldRetry && n < config.attempts
218218
}
219219

220220
if config.lastErrorOnly {

retry_test.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -633,6 +633,52 @@ func BenchmarkDoWithDataNoErrors(b *testing.B) {
633633
}
634634
}
635635

636+
type attemptsForErrorTestError struct{}
637+
638+
func (attemptsForErrorTestError) Error() string { return "test error" }
639+
640+
func TestAttemptsForErrorNoDelayAfterFinalAttempt(t *testing.T) {
641+
var count uint64
642+
var timestamps []time.Time
643+
644+
startTime := time.Now()
645+
646+
err := Do(
647+
func() error {
648+
count++
649+
timestamps = append(timestamps, time.Now())
650+
return attemptsForErrorTestError{}
651+
},
652+
Attempts(3),
653+
Delay(200*time.Millisecond),
654+
DelayType(FixedDelay),
655+
AttemptsForError(2, attemptsForErrorTestError{}),
656+
LastErrorOnly(true),
657+
Context(context.Background()),
658+
)
659+
660+
endTime := time.Now()
661+
662+
assert.Error(t, err)
663+
assert.Equal(t, uint64(2), count, "should attempt exactly 2 times")
664+
assert.Len(t, timestamps, 2, "should have 2 timestamps")
665+
666+
// Verify timing: first attempt at ~0ms, second at ~200ms, end immediately after second attempt
667+
firstAttemptTime := timestamps[0].Sub(startTime)
668+
secondAttemptTime := timestamps[1].Sub(startTime)
669+
totalTime := endTime.Sub(startTime)
670+
671+
// First attempt should be immediate
672+
assert.Less(t, firstAttemptTime, 50*time.Millisecond, "first attempt should be immediate")
673+
674+
// Second attempt should be after delay
675+
assert.Greater(t, secondAttemptTime, 150*time.Millisecond, "second attempt should be after delay")
676+
assert.Less(t, secondAttemptTime, 250*time.Millisecond, "second attempt should not be too delayed")
677+
678+
// Total time should not include delay after final attempt
679+
assert.Less(t, totalTime, 300*time.Millisecond, "should not delay after final attempt")
680+
}
681+
636682
func TestOnRetryNotCalledOnLastAttempt(t *testing.T) {
637683
callCount := 0
638684
onRetryCalls := make([]uint, 0)

0 commit comments

Comments
 (0)