Remove backoff dependency and deprecate RetryOptions

core/CHANGELOG.md

@@ -1,6 +1,19 @@
 ## v0.10.0 (YYYY-MM-DD)
 
 - **Feature:** Add configuration option that, for the key flow, enables a goroutine to be spawned that will refresh the access token when it's close to expiring
+- **Deprecation:**
+  - Methods:
+    - `config.WithMaxRetries`
+    - `config.WithWaitBetweenCalls`
+    - `config.WithRetryTimeout`
+    - `client.NewRetryConfig()`
+  - Fields:
+    - `client.KeyFlowConfig.ClientRetry`
+    - `client.TokenFlowConfig.ClientRetry`
+    - `client.NoAuthFlowConfig.ClientRetry`
+    - `client.RetryConfig`
+  - Retry options were removed to reduce complexity of the client. If this functionality is needed, you can provide your own custom HTTP client. An option to add HTTP middleware will be introduced soon.
+- **Breaking Change:** Remove methods `client.TokenFlow.Clone` and `client.NoAuthFlow.Clone`. Removed fields `client.DefaultRetryMaxRetries`, `client.DefaultRetryWaitBetweenCalls` and `client.DefaultRetryTimeout`. These are no longer being used.
 
 ## v0.9.0 (2024-02-19)
 

core/auth/auth.go

@@ -39,7 +39,7 @@ func SetupAuth(cfg *config.Configuration) (rt http.RoundTripper, err error) {
 	if cfg.CustomAuth != nil {
 		return cfg.CustomAuth, nil
 	} else if cfg.NoAuth {
-		noAuthRoundTripper, err := NoAuth(cfg.RetryOptions)
+		noAuthRoundTripper, err := NoAuth(cfg.RetryOptions) //nolint:staticcheck //will be removed in a later update
 		if err != nil {
 			return nil, fmt.Errorf("configuring no auth client: %w", err)
 		}
@@ -93,10 +93,8 @@ func DefaultAuth(cfg *config.Configuration) (rt http.RoundTripper, err error) {
 
 // NoAuth configures a flow without authentication and returns an http.RoundTripper
 // that can be used to make unauthenticated requests
-func NoAuth(retryOptions *clients.RetryConfig) (rt http.RoundTripper, err error) {
-	noAuthConfig := clients.NoAuthFlowConfig{
-		ClientRetry: retryOptions,
-	}
+func NoAuth(_ *clients.RetryConfig) (rt http.RoundTripper, err error) { //nolint:staticcheck //will be removed in a later update
+	noAuthConfig := clients.NoAuthFlowConfig{}
 	noAuthRoundTripper := &clients.NoAuthFlow{}
 	if err := noAuthRoundTripper.Init(noAuthConfig); err != nil {
 		return nil, fmt.Errorf("initializing client: %w", err)
@@ -125,7 +123,6 @@ func TokenAuth(cfg *config.Configuration) (http.RoundTripper, error) {
 
 	tokenCfg := clients.TokenFlowConfig{
 		ServiceAccountToken: cfg.Token,
-		ClientRetry:         cfg.RetryOptions,
 	}
 
 	client := &clients.TokenFlow{}
@@ -181,7 +178,6 @@ func KeyAuth(cfg *config.Configuration) (http.RoundTripper, error) {
 	keyCfg := clients.KeyFlowConfig{
 		ServiceAccountKey:             serviceAccountKey,
 		PrivateKey:                    cfg.PrivateKey,
-		ClientRetry:                   cfg.RetryOptions,
 		TokenUrl:                      cfg.TokenCustomUrl,
 		BackgroundTokenRefreshContext: cfg.BackgroundTokenRefreshContext,
 	}

core/clients/clients.go

@@ -1,13 +1,8 @@
 package clients
 
 import (
-	"context"
-	"fmt"
 	"net/http"
-	"strings"
 	"time"
-
-	"github.com/cenkalti/backoff/v4"
 )
 
 const (
@@ -24,82 +19,26 @@ const (
 )
 
 const (
-	DefaultClientTimeout         = time.Minute
-	DefaultRetryMaxRetries       = 3
-	DefaultRetryWaitBetweenCalls = 30 * time.Second
-	DefaultRetryTimeout          = 2 * time.Minute
+	DefaultClientTimeout = time.Minute
 )
 
+// Deprecated: retry options were removed to reduce complexity of the client. If this functionality is needed, you can provide your own custom HTTP client. An option to add HTTP middleware will be introduced soon
 type RetryConfig struct {
 	MaxRetries       int           // Max retries
 	WaitBetweenCalls time.Duration // Time to wait between requests
 	RetryTimeout     time.Duration // Max time to re-try
 	ClientTimeout    time.Duration // HTTP Client timeout
 }
 
+// Deprecated: retry options were removed to reduce complexity of the client. If this functionality is needed, you can provide your own custom HTTP client. An option to add HTTP middleware will be introduced soon
 func NewRetryConfig() *RetryConfig {
-	return &RetryConfig{
-		MaxRetries:       DefaultRetryMaxRetries,
-		WaitBetweenCalls: DefaultRetryWaitBetweenCalls,
-		RetryTimeout:     DefaultRetryTimeout,
-		ClientTimeout:    DefaultClientTimeout,
-	}
+	return &RetryConfig{}
 }
 
-// Do performs the request, retrying according to the configurations provided
-func Do(client *http.Client, req *http.Request, cfg *RetryConfig) (resp *http.Response, err error) {
-	if cfg == nil {
-		cfg = NewRetryConfig()
-	}
+// Do performs the request
+func Do(client *http.Client, req *http.Request) (resp *http.Response, err error) {
 	if client == nil {
-		client = &http.Client{}
-	}
-	client.Timeout = cfg.ClientTimeout
-
-	maxRetries := cfg.MaxRetries
-	ctx, cancel := context.WithTimeout(req.Context(), cfg.RetryTimeout)
-	defer cancel()
-	b := backoff.WithContext(backoff.NewConstantBackOff(cfg.WaitBetweenCalls), ctx)
-
-	err = backoff.Retry(func() error {
-		resp, err = client.Do(req) //nolint:bodyclose // body is closed by the caller functions
-		if err != nil {
-			if maxRetries > 0 {
-				if errorIsOneOf(err, ClientTimeoutErr, ClientContextDeadlineErr, ClientConnectionRefusedErr) ||
-					(req.Method == http.MethodGet && strings.Contains(err.Error(), ClientEOFError)) {
-					// reduce retries counter and retry
-					maxRetries--
-					return err
-				}
-			}
-			return backoff.Permanent(err)
-		}
-		if maxRetries > 0 && resp != nil {
-			if resp.StatusCode == http.StatusBadGateway ||
-				resp.StatusCode == http.StatusGatewayTimeout {
-				maxRetries--
-				return fmt.Errorf("request returned a gateway error")
-			}
-		}
-		return nil
-	}, b)
-	if err != nil {
-		return resp, fmt.Errorf("url: %s\nmethod: %s\n err: %w", req.URL.String(), req.Method, err)
-	}
-
-	return resp, err
-}
-
-// ErrorIsOneOf checks if a given error message
-// has one of the provided sub strings
-func errorIsOneOf(err error, msgs ...string) bool {
-	if err == nil {
-		return false
-	}
-	for _, m := range msgs {
-		if strings.Contains(err.Error(), m) {
-			return true
-		}
+		client = http.DefaultClient
 	}
-	return false
+	return client.Do(req)
 }

core/clients/clients_test.go

@@ -9,17 +9,11 @@ import (
 	"reflect"
 	"strings"
 	"testing"
-	"time"
 )
 
 func TestNewRetryConfig(t *testing.T) {
 	got := NewRetryConfig()
-	want := RetryConfig{
-		MaxRetries:       DefaultRetryMaxRetries,
-		WaitBetweenCalls: DefaultRetryWaitBetweenCalls,
-		RetryTimeout:     DefaultRetryTimeout,
-		ClientTimeout:    DefaultClientTimeout,
-	}
+	want := RetryConfig{}
 	if got == nil {
 		t.Error("NewRetryConfig returned nil")
 	} else if !reflect.DeepEqual(*got, want) {
@@ -29,7 +23,6 @@ func TestNewRetryConfig(t *testing.T) {
 
 func TestDo(t *testing.T) {
 	type args struct {
-		cfg            *RetryConfig
 		serverStatus   int
 		serverResponse string
 	}
@@ -41,22 +34,18 @@ func TestDo(t *testing.T) {
 		errMsg   string
 	}{
 		{"all ok", args{
-			cfg:            &RetryConfig{0, time.Microsecond, time.Second, DefaultClientTimeout},
 			serverStatus:   http.StatusOK,
 			serverResponse: `{"status":"ok", "testing": "%s"}`,
 		}, &http.Response{StatusCode: http.StatusOK}, false, ""},
 		{"all ok nil client", args{
-			cfg:            &RetryConfig{0, time.Microsecond, time.Second, DefaultClientTimeout},
 			serverStatus:   http.StatusOK,
 			serverResponse: `{"status":"ok", "testing": "%s"}`,
 		}, &http.Response{StatusCode: http.StatusOK}, false, ""},
 		{"fail 1", args{
-			cfg:            &RetryConfig{1, time.Microsecond, time.Second, DefaultClientTimeout},
 			serverStatus:   http.StatusInternalServerError,
 			serverResponse: `{"status":"error 1", "testing": "%s"}`,
 		}, &http.Response{StatusCode: http.StatusInternalServerError}, false, ""},
 		{"fail 2 - timeout error", args{
-			cfg:            &RetryConfig{3, time.Microsecond, time.Second, DefaultClientTimeout},
 			serverStatus:   http.StatusOK,
 			serverResponse: `{"status":"ok", "testing": "%s"}`,
 		}, &http.Response{StatusCode: http.StatusOK}, true, "no such host"},
@@ -88,7 +77,7 @@ func TestDo(t *testing.T) {
 			if tt.name == "all ok nil client" {
 				c = nil
 			}
-			gotResp, err := Do(c, req, tt.args.cfg)
+			gotResp, err := Do(c, req)
 			if err == nil {
 				// Defer discard and close the body
 				defer func() {

core/clients/key_flow.go

@@ -36,7 +36,7 @@ const (
 type KeyFlow struct {
 	client        *http.Client
 	config        *KeyFlowConfig
-	doer          func(client *http.Client, req *http.Request, cfg *RetryConfig) (resp *http.Response, err error)
+	doer          func(client *http.Client, req *http.Request) (resp *http.Response, err error)
 	key           *ServiceAccountKeyResponse
 	privateKey    *rsa.PrivateKey
 	privateKeyPEM []byte
@@ -47,8 +47,9 @@ type KeyFlow struct {
 
 // KeyFlowConfig is the flow config
 type KeyFlowConfig struct {
-	ServiceAccountKey             *ServiceAccountKeyResponse
-	PrivateKey                    string
+	ServiceAccountKey *ServiceAccountKeyResponse
+	PrivateKey        string
+	// Deprecated: retry options were removed to reduce complexity of the client. If this functionality is needed, you can provide your own custom HTTP client. An option to add HTTP middleware will be introduced soon
 	ClientRetry                   *RetryConfig
 	TokenUrl                      string
 	BackgroundTokenRefreshContext context.Context // Functionality is enabled if this isn't nil
@@ -125,9 +126,6 @@ func (c *KeyFlow) Init(cfg *KeyFlowConfig) error {
 		c.config.TokenUrl = tokenAPI
 	}
 	c.configureHTTPClient()
-	if c.config.ClientRetry == nil {
-		c.config.ClientRetry = NewRetryConfig()
-	}
 	err := c.validate()
 	if err != nil {
 		return err
@@ -175,7 +173,7 @@ func (c *KeyFlow) RoundTrip(req *http.Request) (*http.Response, error) {
 		return nil, err
 	}
 	req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", accessToken))
-	return c.doer(c.client, req, c.config.ClientRetry)
+	return c.doer(c.client, req)
 }
 
 // GetAccessToken returns a short-lived access token and saves the access and refresh tokens in the token field
@@ -319,7 +317,7 @@ func (c *KeyFlow) requestToken(grant, assertion string) (*http.Response, error)
 		return nil, err
 	}
 	req.Header.Add("Content-Type", "application/x-www-form-urlencoded")
-	return c.doer(&http.Client{}, req, c.config.ClientRetry)
+	return c.doer(&http.Client{}, req)
 }
 
 // parseTokenResponse parses the response from the server

core/clients/key_flow_continuous_refresh_test.go

@@ -86,7 +86,7 @@ func TestContinuousRefreshToken(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
 			numberDoCalls := 0
-			mockDo := func(client *http.Client, req *http.Request, cfg *RetryConfig) (resp *http.Response, err error) {
+			mockDo := func(client *http.Client, req *http.Request) (resp *http.Response, err error) {
 				numberDoCalls++
 
 				if tt.doError != nil {
@@ -127,7 +127,6 @@ func TestContinuousRefreshToken(t *testing.T) {
 
 			keyFlow := &KeyFlow{
 				config: &KeyFlowConfig{
-					ClientRetry:                   NewRetryConfig(),
 					BackgroundTokenRefreshContext: ctx,
 				},
 				doer: mockDo,
@@ -212,7 +211,7 @@ func TestContinuousRefreshTokenConcurrency(t *testing.T) {
 	doTestPhase1RequestDone := false
 	doTestPhase2RequestDone := false
 	doTestPhase4RequestDone := false
-	mockDo := func(client *http.Client, req *http.Request, cfg *RetryConfig) (resp *http.Response, err error) {
+	mockDo := func(client *http.Client, req *http.Request) (resp *http.Response, err error) {
 		switch currentTestPhase {
 		default:
 			t.Fatalf("Do call: unexpected request during test phase %d", currentTestPhase)
@@ -298,7 +297,6 @@ func TestContinuousRefreshTokenConcurrency(t *testing.T) {
 	keyFlow := &KeyFlow{
 		client: &http.Client{},
 		config: &KeyFlowConfig{
-			ClientRetry:                   NewRetryConfig(),
 			BackgroundTokenRefreshContext: ctx,
 		},
 		doer: mockDo,

core/clients/key_flow_test.go

@@ -268,12 +268,12 @@ func TestRequestToken(t *testing.T) {
 
 	for _, tt := range testCases {
 		t.Run(tt.name, func(t *testing.T) {
-			mockDo := func(client *http.Client, req *http.Request, cfg *RetryConfig) (resp *http.Response, err error) {
+			mockDo := func(client *http.Client, req *http.Request) (resp *http.Response, err error) {
 				return tt.mockResponse, tt.mockError
 			}
 
 			c := &KeyFlow{
-				config: &KeyFlowConfig{ClientRetry: NewRetryConfig()},
+				config: &KeyFlowConfig{},
 				doer:   mockDo,
 			}
 

core/clients/no_auth_flow.go

@@ -12,6 +12,7 @@ type NoAuthFlow struct {
 
 // NoAuthFlowConfig holds the configuration for the unauthenticated flow
 type NoAuthFlowConfig struct {
+	// Deprecated: retry options were removed to reduce complexity of the client. If this functionality is needed, you can provide your own custom HTTP client. An option to add HTTP middleware will be introduced soon
 	ClientRetry *RetryConfig
 }
 
@@ -23,34 +24,18 @@ func (c *NoAuthFlow) GetConfig() NoAuthFlowConfig {
 	return *c.config
 }
 
-func (c *NoAuthFlow) Init(cfg NoAuthFlowConfig) error {
-	c.config = &NoAuthFlowConfig{
-		ClientRetry: cfg.ClientRetry,
-	}
+func (c *NoAuthFlow) Init(_ NoAuthFlowConfig) error {
+	c.config = &NoAuthFlowConfig{}
 	c.client = &http.Client{
 		Timeout: DefaultClientTimeout,
 	}
-	if c.config.ClientRetry == nil {
-		c.config.ClientRetry = NewRetryConfig()
-	}
 	return nil
 }
 
-// Clone creates a clone of the client
-func (c *NoAuthFlow) Clone() interface{} {
-	sc := *c
-	nc := &sc
-	cl := *nc.client
-	cf := *nc.config
-	nc.client = &cl
-	nc.config = &cf
-	return c
-}
-
 // Roundtrip performs the request
 func (c *NoAuthFlow) RoundTrip(req *http.Request) (*http.Response, error) {
 	if c.client == nil {
 		return nil, fmt.Errorf("please run Init()")
 	}
-	return Do(c.client, req, c.config.ClientRetry)
+	return Do(c.client, req)
 }

core/clients/no_auth_flow_test.go

@@ -59,9 +59,7 @@ func TestNoAuthFlow_Do(t *testing.T) {
 			name: "success",
 			fields: fields{
 				&http.Client{},
-				&NoAuthFlowConfig{
-					ClientRetry: &RetryConfig{},
-				},
+				&NoAuthFlowConfig{},
 			},
 			args:    args{},
 			want:    http.StatusOK,