Repository: honeycombio/terraform-provider-honeycombio
Pull request #629 (2025-03-05 22:01:04)

feat(client): improve V1 api client retry behaviour
With rate limit feedback headers now standard across the V1 and V2 APIs, this tiny refactor has the V1 API client share the retry logic with the V2 API client ✨ 


Patch:
diff --git a/client/client.go b/client/client.go
index 5e22397..c2900c8 100644
--- a/client/client.go
+++ b/client/client.go
@@ -19,6 +19,8 @@ import (
 
 	cleanhttp "github.com/hashicorp/go-cleanhttp"
 	retryablehttp "github.com/hashicorp/go-retryablehttp"
+
+	"github.com/honeycombio/terraform-provider-honeycombio/client/internal/limits"
 )
 
 const (
@@ -128,13 +130,13 @@ func NewClientWithConfig(config *Config) (*Client, error) {
 	}
 
 	client.httpClient = &retryablehttp.Client{
-		Backoff:      retryablehttp.DefaultBackoff,
-		CheckRetry:   client.retryHTTPCheck,
+		Backoff:      limits.RetryHTTPBackoff,
+		CheckRetry:   limits.RetryHTTPCheck,
 		ErrorHandler: retryablehttp.PassthroughErrorHandler,
 		HTTPClient:   cfg.HTTPClient,
 		RetryWaitMin: 200 * time.Millisecond,
-		RetryWaitMax: 10 * time.Second,
-		RetryMax:     15,
+		RetryWaitMax: time.Minute,
+		RetryMax:     30,
 	}
 
 	if config.Debug {
@@ -236,25 +238,6 @@ func (c *Client) newRequest(ctx context.Context, method, path string, body any)
 	return req, nil
 }
 
-// retryHTTPCheck provides a callback for Client.CheckRetry which
-// will retry both rate limit (429) and server (5xx) errors.
-func (c *Client) retryHTTPCheck(ctx context.Context, resp *http.Response, err error) (bool, error) {
-	if ctx.Err() != nil {
-		return false, ctx.Err()
-	}
-	if err != nil {
-		return true, err
-	}
-
-	if resp != nil {
-		if resp.StatusCode == http.StatusTooManyRequests || resp.StatusCode >= 500 {
-			return true, nil
-		}
-	}
-
-	return false, nil
-}
-
 // urlEncodeDataset sanitizes the dataset name for when it is used as part of
 // the URL.
 func urlEncodeDataset(dataset string) string {
diff --git a/client/v2/limits.go b/client/internal/limits/limits.go
similarity index 73%
rename from client/v2/limits.go
rename to client/internal/limits/limits.go
index 87dc1eb..71a9d2c 100644
--- a/client/v2/limits.go
+++ b/client/internal/limits/limits.go
@@ -1,6 +1,7 @@
-package v2
+package limits
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"math/rand"
@@ -8,6 +9,7 @@ import (
 	"time"
 
 	"github.com/dunglas/httpsfv"
+	"github.com/hashicorp/go-retryablehttp"
 )
 
 const (
@@ -27,6 +29,26 @@ const (
 	HeaderRetryAfter = "Retry-After"
 )
 
+// RetryHTTPBackoff is a retryablehttp.Backoff function that will
+// use a linear backoff for all status codes except 429, which will
+// attempt to use the rate limit headers to determine the backoff time
+func RetryHTTPBackoff(
+	mini, maxi time.Duration,
+	attemptNum int,
+	r *http.Response,
+) time.Duration {
+	if r != nil && r.StatusCode == http.StatusTooManyRequests {
+		return rateLimitBackoff(mini, maxi, r)
+	}
+
+	// if we've not been rate limited, use a linear backoff
+	// but increase the minimum and maximum backoff times
+	// and hand it off to retryablehttp.LinearJitterBackoff
+	mini = 500 * time.Millisecond
+	maxi = 950 * time.Millisecond
+	return retryablehttp.LinearJitterBackoff(mini, maxi, attemptNum, r)
+}
+
 // rateLimitBackoff calculates the backoff time for a rate limited request
 // based on the possible response headers.
 // The function will first try to get the reset time from the rate limit header.
@@ -63,6 +85,27 @@ func rateLimitBackoff(mini, maxi time.Duration, r *http.Response) time.Duration
 	return mini + jitter
 }
 
+// retryHTTPCheck is a retryablehttp.CheckRetry function that will
+// retry on a 429 or any 5xx status code
+func RetryHTTPCheck(
+	ctx context.Context,
+	r *http.Response,
+	err error,
+) (bool, error) {
+	if ctx.Err() != nil {
+		return false, ctx.Err()
+	}
+	if err != nil {
+		return true, err
+	}
+	if r != nil {
+		if r.StatusCode == http.StatusTooManyRequests || r.StatusCode >= 500 {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
 // parseRateLimitHeader parses the rate limit header into its constituent parts.
 //
 // The header is expected to be in the format "limit=X, remaining=Y, reset=Z".
diff --git a/client/v2/client.go b/client/v2/client.go
index 9156c65..150f91f 100644
--- a/client/v2/client.go
+++ b/client/v2/client.go
@@ -16,6 +16,7 @@ import (
 	"github.com/hashicorp/jsonapi"
 
 	hnyclient "github.com/honeycombio/terraform-provider-honeycombio/client"
+	"github.com/honeycombio/terraform-provider-honeycombio/client/internal/limits"
 )
 
 const (
@@ -94,13 +95,13 @@ func NewClientWithConfig(config *Config) (*Client, error) {
 		},
 	}
 	client.http = &retryablehttp.Client{
-		Backoff:      client.retryHTTPBackoff,
-		CheckRetry:   client.retryHTTPCheck,
+		Backoff:      limits.RetryHTTPBackoff,
+		CheckRetry:   limits.RetryHTTPCheck,
 		ErrorHandler: retryablehttp.PassthroughErrorHandler,
 		HTTPClient:   config.HTTPClient,
 		RetryWaitMin: 200 * time.Millisecond,
-		RetryWaitMax: 10 * time.Second,
-		RetryMax:     15,
+		RetryWaitMax: time.Minute,
+		RetryMax:     30,
 	}
 
 	if config.Debug {
@@ -194,44 +195,3 @@ func (c *Client) newRequest(
 
 	return req, err
 }
-
-// retryHTTPCheck is a retryablehttp.CheckRetry function that will
-// retry on a 429 or any 5xx status code
-func (c *Client) retryHTTPCheck(
-	ctx context.Context,
-	r *http.Response,
-	err error,
-) (bool, error) {
-	if ctx.Err() != nil {
-		return false, ctx.Err()
-	}
-	if err != nil {
-		return true, err
-	}
-	if r != nil {
-		if r.StatusCode == http.StatusTooManyRequests || r.StatusCode >= 500 {
-			return true, nil
-		}
-	}
-	return false, nil
-}
-
-// retryHTTPBackoff is a retryablehttp.Backoff function that will
-// use a linear backoff for all status codes except 429, which will
-// attempt to use the rate limit headers to determine the backoff time
-func (c *Client) retryHTTPBackoff(
-	mini, maxi time.Duration,
-	attemptNum int,
-	r *http.Response,
-) time.Duration {
-	if r != nil && r.StatusCode == http.StatusTooManyRequests {
-		return rateLimitBackoff(mini, maxi, r)
-	}
-
-	// if we've not been rate limited, use a linear backoff
-	// but increase the minimum and maximum backoff times
-	// and hand it off to retryablehttp.LinearJitterBackoff
-	mini = 500 * time.Millisecond
-	maxi = 950 * time.Millisecond
-	return retryablehttp.LinearJitterBackoff(mini, maxi, attemptNum, r)
-}


Test patch:
diff --git a/client/v2/limits_test.go b/client/internal/limits/limits_test.go
similarity index 99%
rename from client/v2/limits_test.go
rename to client/internal/limits/limits_test.go
index 43f7527..c1462f6 100644
--- a/client/v2/limits_test.go
+++ b/client/internal/limits/limits_test.go
@@ -1,4 +1,4 @@
-package v2
+package limits
 
 import (
 	"net/http"