Skip to content

Commit 4404a7f

Browse files
committed
Fix panic in shouldRetry type assertion and remove deprecated rand.Seed
- Use comma-ok idiom in shouldRetry to handle non-*Err error types safely - Remove deprecated rand.Seed call that was re-seeding on every backoff - Add test coverage for shouldRetry with various error types The shouldRetry function previously used a direct type assertion that would panic if passed an error not of type *Err. While this function is internal and typically receives *Err values, the type assertion should be safe to handle unexpected inputs. The rand.Seed call has been deprecated since Go 1.20 as the global random number generator is now automatically seeded. Additionally, calling Seed on every retry attempt was wasteful.
1 parent 3df8fa8 commit 4404a7f

2 files changed

Lines changed: 19 additions & 3 deletions

File tree

retries/retries.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@ func backoff(attempt int) time.Duration {
7777
wait = maxWait
7878
}
7979
// add some random jitter
80-
rand.Seed(time.Now().UnixNano())
8180
jitter := rand.Intn(int(maxJitter)-int(minJitter)+1) + int(minJitter)
8281
wait += time.Duration(jitter)
8382
return wait
@@ -233,8 +232,8 @@ func shouldRetry(err error) bool {
233232
if err == nil {
234233
return false
235234
}
236-
e := err.(*Err)
237-
if e == nil {
235+
e, ok := err.(*Err)
236+
if !ok || e == nil {
238237
return false
239238
}
240239
return !e.Halt

retries/retries_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,23 @@ func TestRetriesNegativeTimeout(t *testing.T) {
127127
assert.Equal(t, r.config.Timeout(), time.Duration(-1))
128128
}
129129

130+
func TestShouldRetryWithNonErrType(t *testing.T) {
131+
// Test that shouldRetry handles non-*Err types gracefully without panicking
132+
regularError := errors.New("a regular error")
133+
assert.False(t, shouldRetry(regularError))
134+
135+
// Test with nil
136+
assert.False(t, shouldRetry(nil))
137+
138+
// Test with *Err that should not retry
139+
haltErr := Halt(errors.New("halt error"))
140+
assert.False(t, shouldRetry(haltErr))
141+
142+
// Test with *Err that should retry
143+
continueErr := Continue(errors.New("continue error"))
144+
assert.True(t, shouldRetry(continueErr))
145+
}
146+
130147
func ExampleNew() {
131148
// Default retries for 20 minutes on any error
132149
New[int]()

0 commit comments

Comments
 (0)