From 50abccf7e4907e85ec82534572f12af3dbfe4ee1 Mon Sep 17 00:00:00 2001 From: Craig Pastro Date: Mon, 10 Apr 2023 21:34:50 -0700 Subject: [PATCH 01/11] Add DoWithData function --- go.mod | 8 ++++++- retry.go | 58 +++++++++++++++++++++++++++++++++------------------ retry_test.go | 19 ++++++++++++++--- 3 files changed, 61 insertions(+), 24 deletions(-) diff --git a/go.mod b/go.mod index f9cc207..ea0a83a 100644 --- a/go.mod +++ b/go.mod @@ -1,5 +1,11 @@ module github.com/avast/retry-go/v4 -go 1.13 +go 1.18 require github.com/stretchr/testify v1.8.1 + +require ( + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect +) diff --git a/retry.go b/retry.go index 13bee07..0fd7d6e 100644 --- a/retry.go +++ b/retry.go @@ -8,25 +8,27 @@ slightly inspired by [Try::Tiny::Retry](https://metacpan.org/pod/Try::Tiny::Retr http get with retry: url := "http://example.com" - var body []byte - err := retry.Do( - func() error { + body, err := retry.DoWithData( + func() ([]byte, error) { resp, err := http.Get(url) if err != nil { - return err + return nil, err } defer resp.Body.Close() - body, err = ioutil.ReadAll(resp.Body) + body, err := ioutil.ReadAll(resp.Body) if err != nil { - return err + return nil, err } - return nil + return body, nil }, ) + if err != nil { + // handle error + } - fmt.Println(body) + fmt.Println(string(body)) [next examples](https://github.com/avast/retry-go/tree/master/examples) @@ -72,6 +74,8 @@ import ( // Function signature of retryable function type RetryableFunc func() error +type RetryableFuncWithData[T any] func() (T, error) + // Default timer is a wrapper around time.After type timerImpl struct{} @@ -80,7 +84,18 @@ func (t *timerImpl) After(d time.Duration) <-chan time.Time { } func Do(retryableFunc RetryableFunc, opts ...Option) error { + retryableFuncWithData := func() (any, error) { + err := retryableFunc() + return nil, err + } + + _, err := DoWithData(retryableFuncWithData, opts...) + return err +} + +func DoWithData[T any](retryableFunc RetryableFuncWithData[T], opts ...Option) (T, error) { var n uint + var emptyT T // default config := newDefaultRetryConfig() @@ -91,27 +106,30 @@ func Do(retryableFunc RetryableFunc, opts ...Option) error { } if err := config.context.Err(); err != nil { - return err + return emptyT, err } // Setting attempts to 0 means we'll retry until we succeed if config.attempts == 0 { - for err := retryableFunc(); err != nil; err = retryableFunc() { + for { + t, err := retryableFunc() + if err == nil { + return t, nil + } + n++ if !IsRecoverable(err) { - return err + return emptyT, err } config.onRetry(n, err) select { case <-config.timer.After(delay(config, n, err)): case <-config.context.Done(): - return config.context.Err() + return emptyT, config.context.Err() } } - - return nil } var errorLog Error @@ -129,7 +147,7 @@ func Do(retryableFunc RetryableFunc, opts ...Option) error { lastErrIndex := n shouldRetry := true for shouldRetry { - err := retryableFunc() + t, err := retryableFunc() if err != nil { errorLog[lastErrIndex] = unpackUnrecoverable(err) @@ -157,15 +175,15 @@ func Do(retryableFunc RetryableFunc, opts ...Option) error { case <-config.timer.After(delay(config, n, err)): case <-config.context.Done(): if config.lastErrorOnly { - return config.context.Err() + return emptyT, config.context.Err() } n++ errorLog[n] = config.context.Err() - return errorLog[:lenWithoutNil(errorLog)] + return emptyT, errorLog[:lenWithoutNil(errorLog)] } } else { - return nil + return t, nil } n++ @@ -177,9 +195,9 @@ func Do(retryableFunc RetryableFunc, opts ...Option) error { } if config.lastErrorOnly { - return errorLog[lastErrIndex] + return emptyT, errorLog[lastErrIndex] } - return errorLog[:lenWithoutNil(errorLog)] + return emptyT, errorLog[:lenWithoutNil(errorLog)] } func newDefaultRetryConfig() *Config { diff --git a/retry_test.go b/retry_test.go index 57f2a87..4cb0566 100644 --- a/retry_test.go +++ b/retry_test.go @@ -11,14 +11,15 @@ import ( "github.com/stretchr/testify/assert" ) -func TestDoAllFailed(t *testing.T) { +func TestDoWithDataAllFailed(t *testing.T) { var retrySum uint - err := Do( - func() error { return errors.New("test") }, + v, err := DoWithData( + func() (int, error) { return 7, errors.New("test") }, OnRetry(func(n uint, err error) { retrySum += n }), Delay(time.Nanosecond), ) assert.Error(t, err) + assert.Equal(t, 0, v) expectedErrorFormat := `All attempts fail: #1: test @@ -44,7 +45,19 @@ func TestDoFirstOk(t *testing.T) { ) assert.NoError(t, err) assert.Equal(t, uint(0), retrySum, "no retry") +} +func TestDoWithDataFirstOk(t *testing.T) { + returnVal := 1 + + var retrySum uint + val, err := DoWithData( + func() (int, error) { return returnVal, nil }, + OnRetry(func(n uint, err error) { retrySum += n }), + ) + assert.NoError(t, err) + assert.Equal(t, returnVal, val) + assert.Equal(t, uint(0), retrySum, "no retry") } func TestRetryIf(t *testing.T) { From b3a963add5c31889f882ba4df4981097132aa70a Mon Sep 17 00:00:00 2001 From: Craig Pastro Date: Mon, 10 Apr 2023 21:45:47 -0700 Subject: [PATCH 02/11] Generate Readme --- README.md | 32 ++++++++++++++++++++++++-------- retry.go | 1 + 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index dcda9a1..d878f0a 100644 --- a/README.md +++ b/README.md @@ -18,25 +18,27 @@ slightly inspired by http get with retry: url := "http://example.com" - var body []byte - err := retry.Do( - func() error { + body, err := retry.DoWithData( + func() ([]byte, error) { resp, err := http.Get(url) if err != nil { - return err + return nil, err } defer resp.Body.Close() - body, err = ioutil.ReadAll(resp.Body) + body, err := ioutil.ReadAll(resp.Body) if err != nil { - return err + return nil, err } - return nil + return body, nil }, ) + if err != nil { + // handle error + } - fmt.Println(body) + fmt.Println(string(body)) [next examples](https://github.com/avast/retry-go/tree/master/examples) @@ -94,6 +96,12 @@ BackOffDelay is a DelayType which increases delay between consecutive retries func Do(retryableFunc RetryableFunc, opts ...Option) error ``` +#### func DoWithData + +```go +func DoWithData[T any](retryableFunc RetryableFuncWithData[T], opts ...Option) (T, error) +``` + #### func FixedDelay ```go @@ -385,6 +393,14 @@ type RetryableFunc func() error Function signature of retryable function +#### type RetryableFuncWithData + +```go +type RetryableFuncWithData[T any] func() (T, error) +``` + +Function signature of retryable function with data + #### type Timer ```go diff --git a/retry.go b/retry.go index 0fd7d6e..9f4bbde 100644 --- a/retry.go +++ b/retry.go @@ -74,6 +74,7 @@ import ( // Function signature of retryable function type RetryableFunc func() error +// Function signature of retryable function with data type RetryableFuncWithData[T any] func() (T, error) // Default timer is a wrapper around time.After From deed8e1b9837ec7a0b27ea22bb8ce9134b41c226 Mon Sep 17 00:00:00 2001 From: Craig Pastro Date: Sat, 22 Apr 2023 09:25:03 -0700 Subject: [PATCH 03/11] Add both examples to README --- README.md | 26 ++++++++++++++++++++++++++ retry.go | 26 ++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/README.md b/README.md index d878f0a..0f5719c 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,32 @@ slightly inspired by http get with retry: + url := "http://example.com" + var body []byte + + err := retry.Do( + func() error { + resp, err := http.Get(url) + if err != nil { + return err + } + defer resp.Body.Close() + body, err = ioutil.ReadAll(resp.Body) + if err != nil { + return err + } + + return nil + }, + ) + if err != nil { + // handle error + } + + fmt.Println(string(body)) + +http get with retry with data: + url := "http://example.com" body, err := retry.DoWithData( diff --git a/retry.go b/retry.go index 9f4bbde..40f69be 100644 --- a/retry.go +++ b/retry.go @@ -7,6 +7,32 @@ slightly inspired by [Try::Tiny::Retry](https://metacpan.org/pod/Try::Tiny::Retr http get with retry: + url := "http://example.com" + var body []byte + + err := retry.Do( + func() error { + resp, err := http.Get(url) + if err != nil { + return err + } + defer resp.Body.Close() + body, err = ioutil.ReadAll(resp.Body) + if err != nil { + return err + } + + return nil + }, + ) + if err != nil { + // handle error + } + + fmt.Println(string(body)) + +http get with retry with data: + url := "http://example.com" body, err := retry.DoWithData( From e8270ce8d47eb0282f9988c1914605bc633f832c Mon Sep 17 00:00:00 2001 From: Craig Pastro Date: Sat, 22 Apr 2023 12:41:36 -0700 Subject: [PATCH 04/11] Update CI --- .github/workflows/workflow.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/workflow.yaml b/.github/workflows/workflow.yaml index a48fe87..4e19b2f 100644 --- a/.github/workflows/workflow.yaml +++ b/.github/workflows/workflow.yaml @@ -8,7 +8,7 @@ jobs: steps: - uses: actions/setup-go@v3 with: - go-version: 1.17 + go-version: 1.20 - uses: actions/checkout@v3 - name: golangci-lint uses: golangci/golangci-lint-action@v3 @@ -19,7 +19,7 @@ jobs: strategy: fail-fast: false matrix: - go-version: [ '1.16', '1.17', '1.18', '1.19' ] + go-version: [ '1.18', '1.19', '1.20' ] os: [ubuntu-latest, macos-latest, windows-latest] env: OS: ${{ matrix.os }} From afe8e1307004f71f99d80c9b65b8a25d173eefe2 Mon Sep 17 00:00:00 2001 From: Craig Pastro Date: Sat, 22 Apr 2023 12:44:26 -0700 Subject: [PATCH 05/11] Update CI to run on pull_request --- .github/workflows/workflow.yaml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/workflow.yaml b/.github/workflows/workflow.yaml index 4e19b2f..e67147d 100644 --- a/.github/workflows/workflow.yaml +++ b/.github/workflows/workflow.yaml @@ -1,6 +1,10 @@ name: Go -on: [push] +on: + push: + pull_request: + branches: + - main jobs: golangci-lint: From 377d1c3945493a60bbb5666738182ce9948416cd Mon Sep 17 00:00:00 2001 From: Craig Pastro Date: Sat, 22 Apr 2023 12:48:09 -0700 Subject: [PATCH 06/11] Run on 'master' branch --- .github/workflows/workflow.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/workflow.yaml b/.github/workflows/workflow.yaml index e67147d..2759339 100644 --- a/.github/workflows/workflow.yaml +++ b/.github/workflows/workflow.yaml @@ -4,7 +4,7 @@ on: push: pull_request: branches: - - main + - master jobs: golangci-lint: From fe38392f9f180de190f740ceb857dfabec03cdda Mon Sep 17 00:00:00 2001 From: Craig Pastro Date: Sat, 22 Apr 2023 12:53:36 -0700 Subject: [PATCH 07/11] Try v4 of setup-go --- .github/workflows/workflow.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/workflow.yaml b/.github/workflows/workflow.yaml index 2759339..5661d13 100644 --- a/.github/workflows/workflow.yaml +++ b/.github/workflows/workflow.yaml @@ -10,7 +10,7 @@ jobs: golangci-lint: runs-on: ubuntu-latest steps: - - uses: actions/setup-go@v3 + - uses: actions/setup-go@v4 with: go-version: 1.20 - uses: actions/checkout@v3 @@ -31,7 +31,7 @@ jobs: steps: - uses: actions/checkout@v3 - name: Setup Go ${{ matrix.go-version }} - uses: actions/setup-go@v3 + uses: actions/setup-go@v4 with: go-version: ${{ matrix.go-version }} check-latest: true From 55a665af130ce3b477a95cf30e5149d189ed963d Mon Sep 17 00:00:00 2001 From: Craig Pastro Date: Sat, 22 Apr 2023 12:56:30 -0700 Subject: [PATCH 08/11] Back to 1.18 --- .github/workflows/workflow.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/workflow.yaml b/.github/workflows/workflow.yaml index 5661d13..c8e6b08 100644 --- a/.github/workflows/workflow.yaml +++ b/.github/workflows/workflow.yaml @@ -12,7 +12,7 @@ jobs: steps: - uses: actions/setup-go@v4 with: - go-version: 1.20 + go-version: 1.18 - uses: actions/checkout@v3 - name: golangci-lint uses: golangci/golangci-lint-action@v3 From af7c10fe7251427f3ed5bd90d6a06bea80b46730 Mon Sep 17 00:00:00 2001 From: Craig Pastro Date: Thu, 3 Aug 2023 13:16:37 -0700 Subject: [PATCH 09/11] a little refactor --- retry.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/retry.go b/retry.go index dd3d986..20ad70f 100644 --- a/retry.go +++ b/retry.go @@ -112,8 +112,7 @@ func (t *timerImpl) After(d time.Duration) <-chan time.Time { func Do(retryableFunc RetryableFunc, opts ...Option) error { retryableFuncWithData := func() (any, error) { - err := retryableFunc() - return nil, err + return nil, retryableFunc() } _, err := DoWithData(retryableFuncWithData, opts...) From 6d7600c3c42505f555a5d4d89ca7d7e8a7b705e2 Mon Sep 17 00:00:00 2001 From: Craig Pastro Date: Thu, 3 Aug 2023 13:17:49 -0700 Subject: [PATCH 10/11] avoid lint errors --- retry_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/retry_test.go b/retry_test.go index 931fb87..8308bfe 100644 --- a/retry_test.go +++ b/retry_test.go @@ -543,7 +543,7 @@ func BenchmarkDo(b *testing.B) { testError := errors.New("test error") for i := 0; i < b.N; i++ { - Do( + _ = Do( func() error { return testError }, @@ -555,7 +555,7 @@ func BenchmarkDo(b *testing.B) { func BenchmarkDoNoErrors(b *testing.B) { for i := 0; i < b.N; i++ { - Do( + _ = Do( func() error { return nil }, From d0901280d98d5838a7118b84f25bd2825be7c683 Mon Sep 17 00:00:00 2001 From: Craig Pastro Date: Thu, 3 Aug 2023 15:42:46 -0700 Subject: [PATCH 11/11] add benchmarks --- retry_test.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/retry_test.go b/retry_test.go index 8308bfe..10cd4ef 100644 --- a/retry_test.go +++ b/retry_test.go @@ -553,6 +553,20 @@ func BenchmarkDo(b *testing.B) { } } +func BenchmarkDoWithData(b *testing.B) { + testError := errors.New("test error") + + for i := 0; i < b.N; i++ { + _, _ = DoWithData( + func() (int, error) { + return 0, testError + }, + Attempts(10), + Delay(0), + ) + } +} + func BenchmarkDoNoErrors(b *testing.B) { for i := 0; i < b.N; i++ { _ = Do( @@ -565,6 +579,18 @@ func BenchmarkDoNoErrors(b *testing.B) { } } +func BenchmarkDoWithDataNoErrors(b *testing.B) { + for i := 0; i < b.N; i++ { + _, _ = DoWithData( + func() (int, error) { + return 0, nil + }, + Attempts(10), + Delay(0), + ) + } +} + func TestIsRecoverable(t *testing.T) { err := errors.New("err") assert.True(t, IsRecoverable(err))