Restructure retry mechanism (#108)

* chore: update dependencies

* feat: add wrapper over new backoff pkg

* feat: modify retryPolicy to use new backoff pkg

* test: update tests

* feat: use builder pattern for backoff

* fix: remove unused code

* feat: add IsError method to the response struct

Author: ziscky

go.mod

@@ -32,6 +32,7 @@ require (
 	github.com/alicebob/gopher-json v0.0.0-20200520072559-a9ecdc9d1d3a // indirect
 	github.com/andybalholm/brotli v1.1.0 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
+	github.com/cenkalti/backoff/v4 v4.3.0
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect
 	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
 	github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect

go.sum

@@ -744,6 +744,8 @@ github.com/cenkalti/backoff/v4 v4.1.2/go.mod h1:scbssz8iZGpm3xbr14ovlUdkxfGXNInq
 github.com/cenkalti/backoff/v4 v4.1.3/go.mod h1:scbssz8iZGpm3xbr14ovlUdkxfGXNInqkPWOWmG2CLw=
 github.com/cenkalti/backoff/v4 v4.2.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE=
 github.com/cenkalti/backoff/v4 v4.2.1/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE=
+github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8=
+github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE=
 github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
 github.com/census-instrumentation/opencensus-proto v0.3.0/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
 github.com/census-instrumentation/opencensus-proto v0.4.1/go.mod h1:4T9NM4+4Vw91VeyqjLS6ao50K5bOcLKN6Q42XnYaRYw=

pkg/zhttpclient/backoff/backoff.go

@@ -0,0 +1,48 @@
+package backoff
+
+import (
+	"time"
+
+	"github.com/cenkalti/backoff/v4"
+)
+
+const (
+	exponentialMultiplier = 2
+)
+
+type BackOff struct {
+	maxAttempts     int
+	maxDuration     time.Duration
+	initialDuration time.Duration
+}
+
+func New() *BackOff {
+	return &BackOff{}
+}
+
+func (b *BackOff) WithMaxAttempts(maxAttempts int) *BackOff {
+	b.maxAttempts = maxAttempts
+	return b
+}
+func (b *BackOff) WithMaxDuration(max time.Duration) *BackOff {
+	b.maxDuration = max
+	return b
+}
+func (b *BackOff) WithInitialDuration(initial time.Duration) *BackOff {
+	b.initialDuration = initial
+	return b
+}
+
+func (b *BackOff) Exponential() backoff.BackOff {
+	tmp := backoff.NewExponentialBackOff(backoff.WithInitialInterval(b.initialDuration), backoff.WithMaxElapsedTime(b.maxDuration), backoff.WithMultiplier(exponentialMultiplier))
+	return backoff.WithMaxRetries(tmp, uint64(b.maxAttempts))
+}
+
+func (b *BackOff) Linear() backoff.BackOff {
+	return backoff.WithMaxRetries(backoff.NewConstantBackOff(b.initialDuration), uint64(b.maxAttempts))
+}
+
+// Do retries op if it returns an error according to the provided backoff
+func Do(op func() error, b backoff.BackOff) error {
+	return backoff.Retry(op, b)
+}

pkg/zhttpclient/client_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"errors"
+	"fmt"
 	"io"
 	"net"
 	"net/http"
@@ -13,6 +14,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/cenkalti/backoff/v4"
 	"github.com/zondax/golem/pkg/zhttpclient"
 
 	"github.com/stretchr/testify/assert"
@@ -345,10 +347,10 @@ func TestHTTPClient_Retry(t *testing.T) {
 		// the request should be retried exponentialy starting from 100ms i.e 100ms * (2 ^ attempt) for a max of 2 times
 		// the total time of the request should be:
 		// 		total = 0ms  // attempt 1
-		//      total += 100ms * (2 ^ 1) = 200ms  // attempt 1
-		// 		total += 100ms * (2 ^ 2) = 400ms // attempt 2
-		//      total = 600ms
-		// the time between retries ( we will only check the last retry attempt) = 400ms
+		//      total += 100ms * (2 ^ 0) = 100ms  // attempt 1
+		// 		total += 100ms * (2 ^ 1) = 200ms // attempt 2
+		//      total = 300ms
+		// the time between retries ( we will only check the last retry attempt) = 200ms
 		{
 			name: "exponential retry",
 			srv:  newTestSrv(t, http.StatusInternalServerError, nil, 0),
@@ -361,14 +363,25 @@ func TestHTTPClient_Retry(t *testing.T) {
 					MaxWaitBeforeRetry: 2 * time.Second,
 				}
 				r.WithCodes(http.StatusInternalServerError)
-				r.SetExponentialBackoff(100 * time.Millisecond)
+
+				tmp := backoff.NewExponentialBackOff(backoff.WithInitialInterval(100*time.Millisecond),
+					backoff.WithMaxElapsedTime(r.MaxWaitBeforeRetry),
+					backoff.WithMultiplier(2),
+				)
+				tmp.RandomizationFactor = 0
+				b := backoff.WithMaxRetries(tmp, uint64(r.MaxAttempts))
+
+				r.SetBackoff(func(_ uint, _ *http.Response, _ error) time.Duration {
+					return b.NextBackOff()
+				})
+
 				return r
 			},
 			wantRetry:       true,
-			wantWaitBetween: 400 * time.Millisecond,
+			wantWaitBetween: 200 * time.Millisecond,
 			wantCalled:      3,
 			wantCode:        http.StatusInternalServerError,
-			wantTotalWait:   600 * time.Millisecond,
+			wantTotalWait:   300 * time.Millisecond,
 			wantBody:        []byte{},
 		},
 	}
@@ -422,6 +435,7 @@ func TestHTTPClient_Retry(t *testing.T) {
 			assert.Equal(t, tt.wantCalled, tt.srv.called)
 			if tt.wantRetry {
 				// ignore minor deviations in millisecond values
+				fmt.Println(end, start, end-start, tt.wantTotalWait.Milliseconds())
 				assert.Equal(t, (end-start)/tt.wantTotalWait.Milliseconds(), int64(1))
 				assert.Equal(t, tt.srv.waitBetweenCalls/tt.wantWaitBetween.Milliseconds(), int64(1))
 			}

pkg/zhttpclient/request.go

@@ -33,6 +33,11 @@ type zRequest struct {
 	url     string
 }
 
+// IsError returns true if the statusCode is >= 400
+func (r *Response) IsError() bool {
+	return r.Code > 399
+}
+
 func newZRequest(client *zHTTPClient) ZRequest {
 	// only used to enforce retry policies at the request level
 	c := New(*client.config).(*zHTTPClient)

pkg/zhttpclient/retry.go

@@ -1,9 +1,11 @@
 package zhttpclient
 
 import (
-	"math"
 	"net/http"
 	"time"
+
+	"github.com/cenkalti/backoff/v4"
+	zbackoff "github.com/zondax/golem/pkg/zhttpclient/backoff"
 )
 
 // BackoffFn is a function that returns a backoff duration.
@@ -16,6 +18,8 @@ type RetryPolicy struct {
 	WaitBeforeRetry time.Duration
 	// MaxWaitBeforeRetry is the maximum cap for the wait before retry
 	MaxWaitBeforeRetry time.Duration
+	// b is a wrapped backoff functions provider
+	b backoff.BackOff
 	// backoffFn is a function that returns a custom sleep duration before a retry.
 	// It is capped between WaitBeforeRetry and MaxWaitBeforeRetry
 	backoffFn        BackoffFn
@@ -38,15 +42,16 @@ func (r *RetryPolicy) SetBackoff(fn BackoffFn) {
 
 // SetLinearBackoff sets a constant sleep duration between retries.
 func (r *RetryPolicy) SetLinearBackoff(duration time.Duration) {
+	r.b = zbackoff.New().WithInitialDuration(duration).WithMaxAttempts(r.MaxAttempts).Linear()
 	r.backoffFn = func(uint, *http.Response, error) time.Duration {
-		return duration
+		return r.b.NextBackOff()
 	}
 }
 
 // SetExponentialBackoff sets an exponential base 2 delay ( duration * 2 ^ attempt ) for each attempt.
 func (r *RetryPolicy) SetExponentialBackoff(duration time.Duration) {
-	r.backoffFn = func(attempt uint, _ *http.Response, _ error) time.Duration {
-		mul := int64(math.Pow(2.0, float64(attempt)))
-		return time.Millisecond * time.Duration(duration.Milliseconds()*mul)
+	r.b = zbackoff.New().WithInitialDuration(duration).WithMaxAttempts(r.MaxAttempts).WithMaxDuration(r.MaxWaitBeforeRetry).Exponential()
+	r.backoffFn = func(_ uint, _ *http.Response, _ error) time.Duration {
+		return r.b.NextBackOff()
 	}
 }