Fix double retry loop with no backoff

.github/workflows/golangci-lint.yml

@@ -13,7 +13,7 @@ jobs:
       - uses: actions/checkout@v4
       - uses: actions/setup-go@v5
         with:
-          go-version: stable
+          go-version-file: go.mod
       - name: golangci-lint
         uses: golangci/golangci-lint-action@v8
         with:

README.md

@@ -113,6 +113,7 @@ All inputs are optional and map to the environment variables documented in
 | `verbosity`         | `TEST_VERBOSITY`         | `0`     | Increase logging output for tests.                                      |
 | `enable-retries`    | `ENABLE_RETRIES`         | `false` | Enable retrying requests if a test does not succeed.                    |
 | `retry-count`       | `DEFAULT_RETRY_COUNT`    | `2`     | Number of retries. Only applied when `enable-retries` is `true`.        |
+| `retry-backoff-seconds` | `RETRY_BACKOFF_SECONDS` | `2` | Delay, in seconds, before each retry. Only applied when `enable-retries` is `true`. |
 
 The action fails the workflow step when any test fails.
 
@@ -167,6 +168,9 @@ environment variables:
    the initial request does not succeed. Only applied if `ENABLE_RETRIES` is set
    to `true` Defaults: `2`.
 
+- `RETRY_BACKOFF_SECONDS`: Specify the delay, in seconds, before each retry
+   attempt. Only applied if `ENABLE_RETRIES` is set to `true`. Defaults: `2`.
+
 ### Environment variable substitution
 
 This program supports variable substitution from environment variables in YML

action.yml

@@ -40,6 +40,10 @@ inputs:
     description: 'Number of times to retry a test request. Only applied when enable-retries is "true".'
     required: false
     default: '2'
+  retry-backoff-seconds:
+    description: 'Delay, in seconds, before each retry attempt. Only applied when enable-retries is "true".'
+    required: false
+    default: '2'
 
 runs:
   using: 'composite'
@@ -62,5 +66,6 @@ runs:
         TEST_VERBOSITY: ${{ inputs.verbosity }}
         ENABLE_RETRIES: ${{ inputs.enable-retries }}
         DEFAULT_RETRY_COUNT: ${{ inputs.retry-count }}
+        RETRY_BACKOFF_SECONDS: ${{ inputs.retry-backoff-seconds }}
         CGO_ENABLED: '0'
       run: go run "${{ github.action_path }}"

go.mod

@@ -5,7 +5,6 @@ go 1.25.6
 require (
 	github.com/drone/envsubst v1.0.3
 	github.com/fatih/color v1.18.0
-	github.com/hashicorp/go-retryablehttp v0.7.8
 	github.com/tidwall/gjson v1.18.0
 	github.com/tidwall/pretty v1.2.1
 	github.com/youmark/pkcs8 v0.0.0-20240726163527-a2c0da244d78
@@ -14,7 +13,6 @@ require (
 )
 
 require (
-	github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
 	github.com/mattn/go-colorable v0.1.14 // indirect
 	github.com/mattn/go-isatty v0.0.20 // indirect
 	github.com/tidwall/match v1.2.0 // indirect

go.sum

@@ -6,12 +6,6 @@ github.com/fatih/color v1.18.0 h1:S8gINlzdQ840/4pfAwic/ZE0djQEH3wM94VfqLTZcOM=
 github.com/fatih/color v1.18.0/go.mod h1:4FelSpRwEGDpQ12mAdzqdOukCy4u8WUtOY6lkT/6HfU=
 github.com/google/go-cmp v0.2.0 h1:+dTQ8DZQJz0Mb/HjFlkptS1FeQ4cWSnN941F8aEG4SQ=
 github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M=
-github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ=
-github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48=
-github.com/hashicorp/go-hclog v1.6.3 h1:Qr2kF+eVWjTiYmU7Y31tYlP1h0q/X3Nl3tPGdaB11/k=
-github.com/hashicorp/go-hclog v1.6.3/go.mod h1:W4Qnvbt70Wk/zYJryRzDRU/4r0kIg0PVHBcfoyhpF5M=
-github.com/hashicorp/go-retryablehttp v0.7.8 h1:ylXZWnqa7Lhqpk0L1P1LzDtGcCR0rPVUrx/c8Unxc48=
-github.com/hashicorp/go-retryablehttp v0.7.8/go.mod h1:rjiScheydd+CxvumBsIrFKlx3iS0jrZ7LvzFGFmuKbw=
 github.com/mattn/go-colorable v0.1.14 h1:9A9LHSqF/7dyVVX6g0U9cwm9pG3kP9gSzcuIPHPsaIE=
 github.com/mattn/go-colorable v0.1.14/go.mod h1:6LmQG8QLFO4G5z1gPvYEzlUgJ2wF+stgPZH1UqBm1s8=
 github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY=

internal/config.go

@@ -18,8 +18,13 @@ import (
 	"fmt"
 	"os"
 	"strconv"
+	"time"
 )
 
+// DefaultRetryBackoff is the delay between retry attempts used when
+// RETRY_BACKOFF_SECONDS is not set.
+const DefaultRetryBackoff = 2 * time.Second
+
 // Config stores application configuration
 type Config struct {
 	Concurrency          int
@@ -30,6 +35,7 @@ type Config struct {
 	Verbosity            int
 	EnableRetries        bool
 	RetryCount           int
+	RetryBackoff         time.Duration
 }
 
 // FromEnv returns config read from environment variables
@@ -66,6 +72,15 @@ func FromEnv() (*Config, error) {
 		return nil, fmt.Errorf("invalid default retry count value: %d", retryCount)
 	}
 
+	retryBackoffSeconds, err := strconv.Atoi(getEnv("RETRY_BACKOFF_SECONDS", strconv.Itoa(int(DefaultRetryBackoff.Seconds()))))
+	if err != nil {
+		return nil, fmt.Errorf("invalid retry backoff seconds value: %s", err)
+	}
+
+	if retryBackoffSeconds < 0 {
+		return nil, fmt.Errorf("invalid retry backoff seconds value: %d", retryBackoffSeconds)
+	}
+
 	return &Config{
 		Concurrency:          concurrency,
 		Host:                 getEnv("TEST_HOST", ""),
@@ -75,6 +90,7 @@ func FromEnv() (*Config, error) {
 		Verbosity:            verbosity,
 		EnableRetries:        enableRetries,
 		RetryCount:           retryCount,
+		RetryBackoff:         time.Duration(retryBackoffSeconds) * time.Second,
 	}, nil
 }
 

internal/coordinator.go

@@ -39,7 +39,7 @@ func RunTests(tests []*Test, config *Config) bool {
 				maxRetries = config.RetryCount
 			}
 
-			result := RunTest(t, config.Host, maxRetries)
+			result := RunTest(t, config.Host, maxRetries, config.RetryBackoff)
 
 			// Acquire lock before accessing shared variables and writing output.
 			// Code in critical section should not perform network I/O.

internal/http.go

@@ -16,14 +16,11 @@ package internal
 
 import (
 	"bytes"
-	"context"
 	"crypto/tls"
 	"fmt"
 	"io"
 	"net/http"
 	"time"
-
-	"github.com/hashicorp/go-retryablehttp"
 )
 
 // HTTPRequestConfig type
@@ -37,8 +34,6 @@ type HTTPRequestConfig struct {
 	Body                 io.Reader
 	Timeout              time.Duration
 	SkipCertVerification bool
-	MaxRetries           int
-	RetryCallback        func(ctx context.Context, resp *http.Response, err error) (bool, error)
 }
 
 // SendHTTPRequest sends an HTTP request and returns response body and status
@@ -61,7 +56,7 @@ func SendHTTPRequest(config *HTTPRequestConfig) (*http.Response, []byte, error)
 	}
 
 	// Create request
-	req, err := retryablehttp.NewRequest(
+	req, err := http.NewRequest(
 		config.Method,
 		config.URL,
 		config.Body,
@@ -97,27 +92,15 @@ func SendHTTPRequest(config *HTTPRequestConfig) (*http.Response, []byte, error)
 		req.Header.Add(k, v)
 	}
 
-	client := retryablehttp.Client{
-		HTTPClient: &http.Client{
-			CheckRedirect: func(_ *http.Request, _ []*http.Request) error {
-				return http.ErrUseLastResponse
-			},
-			Transport: &http.Transport{
-				//nolint:gosec
-				TLSClientConfig: &tls.Config{InsecureSkipVerify: config.SkipCertVerification},
-			},
-			Timeout: config.Timeout * time.Second,
+	client := &http.Client{
+		CheckRedirect: func(_ *http.Request, _ []*http.Request) error {
+			return http.ErrUseLastResponse
 		},
-	}
-
-	// Enable retries
-	if config.MaxRetries > 0 {
-		client.RetryMax = config.MaxRetries
-		client.CheckRetry = config.RetryCallback
-		client.Backoff = retryablehttp.DefaultBackoff
-	} else {
-		// Don't retry requests
-		client.CheckRetry = func(_ context.Context, _ *http.Response, _ error) (bool, error) { return false, nil }
+		Transport: &http.Transport{
+			//nolint:gosec
+			TLSClientConfig: &tls.Config{InsecureSkipVerify: config.SkipCertVerification},
+		},
+		Timeout: config.Timeout * time.Second,
 	}
 
 	// Start sending request

internal/tester.go

@@ -15,13 +15,13 @@
 package internal
 
 import (
-	"context"
 	"fmt"
 	"io"
 	"net/http"
 	"os"
 	"regexp"
 	"strings"
+	"time"
 
 	"go.uber.org/zap"
 )
@@ -33,8 +33,10 @@ type TestResult struct {
 	Errors  []error
 }
 
-// RunTest runs a single test
-func RunTest(test *Test, defaultHost string, maxRetries int) *TestResult {
+// RunTest runs a single test, sleeping retryBackoff between retry attempts to
+// give the origin/edge time to recover instead of hammering it with
+// back-to-back requests.
+func RunTest(test *Test, defaultHost string, maxRetries int, retryBackoff time.Duration) *TestResult {
 	result := &TestResult{}
 
 	// Validate test and assign default values
@@ -63,39 +65,24 @@ func RunTest(test *Test, defaultHost string, maxRetries int) *TestResult {
 		body = strings.NewReader(test.Request.Body)
 	}
 
-	retryCallback := func(_ context.Context, resp *http.Response, inErr error) (bool, error) {
-		if inErr != nil {
-			// retry is there is an error with the request
-			return true, nil
-		}
-
-		errs := validateResponseStatus(test, resp)
-
-		if len(errs) >= 1 {
-			// retry if there is an error
-			return true, nil
-		}
-
-		// stop retrying
-		return false, nil
-	}
-
 	reqConfig := &HTTPRequestConfig{
 		Method:               test.Request.Method,
 		URL:                  url,
 		Headers:              test.Request.Headers,
 		Body:                 body,
 		Timeout:              60,
 		SkipCertVerification: test.SkipCertVerification,
-		RetryCallback:        retryCallback,
-		MaxRetries:           maxRetries,
 	}
 
 	zap.L().Info("sending request",
 		zap.Any("request", reqConfig),
 	)
 
 	for i := 0; i <= maxRetries; i++ {
+		if i > 0 {
+			time.Sleep(retryBackoff)
+		}
+
 		result.Errors = []error{}
 		result.Retries = i