Remove backoff dependency and deprecate RetryOptions

CHANGELOG.md

@@ -6,7 +6,21 @@
 - **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:** Mark method `config.WithCaptureHTTPResponse` as deprecated, to avoid confusion due to it not being a configuration option. Use `runtime.WithCaptureHTTPResponse` instead.
 - **Deprecation:** Mark method `config.WithJWKSEndpoint` and field `config.Configuration.JWKSCustomUrl` as deprecated. Validation using JWKS was removed, for being redundant with token validation done in the APIs. These have no effect.
-- **Breaking Change:** Remove method `KeyFlow.Clone`, that was no longer being used.
+- **Deprecation:**
+  - Methods:
+    - `config.WithMaxRetries`
+    - `config.WithWaitBetweenCalls`
+    - `config.WithRetryTimeout`
+    - `clients.NewRetryConfig`
+  - Fields:
+    - `clients.KeyFlowConfig.ClientRetry`
+    - `clients.TokenFlowConfig.ClientRetry`
+    - `clients.NoAuthFlowConfig.ClientRetry`
+    - `clients.RetryConfig`
+  - Retry options were removed to reduce complexity of the clients. If this functionality is needed, you can provide your own custom HTTP client.
+- **Breaking Change:** Remove method `clients.KeyFlow.Clone`, `clients.TokenFlow.Clone`, `clients.NoAuthFlow.Clone` and `clients.Do`, that are no longer being used.
+- **Breaking Change:** Removed fields `clients.DefaultRetryMaxRetries`, `clients.DefaultRetryWaitBetweenCalls`and `clients.DefaultRetryTimeout`. Removed constants `clients.ClientTimeoutErr`,`clients.ClientContextDeadlineErr`, `clients.ClientConnectionRefusedErr`, `clients.ClientEOFError`. These are no longer being used.
+- **Breaking Change:** Change signature of `auth.NoAuth`, which no longer takes `clients.RetryConfig` as argument.
 
 ## Release (2024-02-07)
 

core/CHANGELOG.md

@@ -1,6 +1,20 @@
 ## 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`
+    - `clients.NewRetryConfig`
+  - Fields:
+    - `clients.KeyFlowConfig.ClientRetry`
+    - `clients.TokenFlowConfig.ClientRetry`
+    - `clients.NoAuthFlowConfig.ClientRetry`
+    - `clients.RetryConfig`
+  - Retry options were removed to reduce complexity of the clients. If this functionality is needed, you can provide your own custom HTTP client.
+- **Breaking Change:** Remove methods `clients.TokenFlow.Clone`, `clients.NoAuthFlow.Clone`, `clients.Do`. Removed fields `clients.DefaultRetryMaxRetries`, `clients.DefaultRetryWaitBetweenCalls`and`clients.DefaultRetryTimeout`. Removed constants `clients.ClientTimeoutErr`,`clients.ClientContextDeadlineErr`, `clients.ClientConnectionRefusedErr`, `clients.ClientEOFError`. These are no longer being used.
+- **Breaking Change:** Change signature of `auth.NoAuth`, which no longer takes `clients.RetryConfig` as argument
 
 ## 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()
 		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() (rt http.RoundTripper, err error) {
+	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/auth/auth_test.go

@@ -558,7 +558,7 @@ func TestNoAuth(t *testing.T) {
 	} {
 		t.Run(test.desc, func(t *testing.T) {
 			// Get the default authentication client and ensure that it's not nil
-			authClient, err := NoAuth(nil)
+			authClient, err := NoAuth()
 			if err != nil {
 				t.Fatalf("Test returned error on valid test case: %v", err)
 			}

core/clients/clients.go

@@ -1,13 +1,7 @@
 package clients
 
 import (
-	"context"
-	"fmt"
-	"net/http"
-	"strings"
 	"time"
-
-	"github.com/cenkalti/backoff/v4"
 )
 
 const (
@@ -16,90 +10,18 @@ const (
 )
 
 const (
-	// Known error messages
-	ClientTimeoutErr           = "Client.Timeout exceeded while awaiting headers"
-	ClientContextDeadlineErr   = "context deadline exceeded"
-	ClientConnectionRefusedErr = "connection refused"
-	ClientEOFError             = "unexpected EOF"
-)
-
-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.
 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.
 func NewRetryConfig() *RetryConfig {
-	return &RetryConfig{
-		MaxRetries:       DefaultRetryMaxRetries,
-		WaitBetweenCalls: DefaultRetryWaitBetweenCalls,
-		RetryTimeout:     DefaultRetryTimeout,
-		ClientTimeout:    DefaultClientTimeout,
-	}
-}
-
-// 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()
-	}
-	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
-		}
-	}
-	return false
+	return &RetryConfig{}
 }