Skip to content

Commit

Permalink
Fix regression in retries
Browse files Browse the repository at this point in the history
  • Loading branch information
KonradStaniec committed Aug 7, 2024
1 parent d63c121 commit a320356
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 5 deletions.
10 changes: 6 additions & 4 deletions types/retry/retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ func isExpectedErr(err error) bool {
}

// Do executes a func with retry
// TODO: check if this is needed, because is not being used.
// TODO: Remove this function, and make our programs to depend on retires based
// on some standard retry library
func Do(sleep time.Duration, maxSleepTime time.Duration, retryableFunc func() error) error {
if err := retryableFunc(); err != nil {
if isUnrecoverableErr(err) {
Expand All @@ -66,9 +67,10 @@ func Do(sleep time.Duration, maxSleepTime time.Duration, retryableFunc func() er
}

// Add some randomness to prevent thrashing
jitter, err := randDuration(int64(sleep))
if err != nil {
return err
// TODO: This duration should be passed by the caller
jitter, randomnessErr := randDuration(int64(sleep))
if randomnessErr != nil {
return randomnessErr
}
sleep = sleep + jitter/2

Expand Down
14 changes: 13 additions & 1 deletion types/retry/retry_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package retry

import (
"github.com/stretchr/testify/require"
"errors"
"testing"
"time"

"github.com/stretchr/testify/require"
)

func TestUnrecoverableError(t *testing.T) {
Expand All @@ -19,3 +21,13 @@ func TestExpectedError(t *testing.T) {
})
require.NoError(t, err)
}

func TestDoNotShadowAnError(t *testing.T) {
var expectedError = errors.New("expected error")

err := Do(1*time.Second, 1*time.Second, func() error {
return expectedError
})
require.Error(t, err)
require.ErrorIs(t, err, expectedError)
}

0 comments on commit a320356

Please sign in to comment.