better handling of 429s

go.mod

@@ -6,13 +6,13 @@ require (
 	github.com/golang-jwt/jwt/v5 v5.2.1
 	github.com/google/uuid v1.6.0
 	github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320
-	github.com/hashicorp/go-retryablehttp v0.7.7
 	github.com/hashicorp/terraform-plugin-docs v0.19.4
 	github.com/hashicorp/terraform-plugin-log v0.9.0
 	github.com/hashicorp/terraform-plugin-sdk/v2 v2.34.0
 	github.com/jarcoal/httpmock v1.3.1
 	github.com/stretchr/testify v1.9.0
 	golang.org/x/crypto v0.28.0
+	golang.org/x/sync v0.8.0
 )
 
 require (
@@ -75,7 +75,6 @@ require (
 	golang.org/x/exp v0.0.0-20230626212559-97b1e661b5df // indirect
 	golang.org/x/mod v0.17.0 // indirect
 	golang.org/x/net v0.25.0 // indirect
-	golang.org/x/sync v0.8.0 // indirect
 	golang.org/x/sys v0.26.0 // indirect
 	golang.org/x/text v0.19.0 // indirect
 	golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect

go.sum

@@ -84,8 +84,6 @@ github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+l
 github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM=
 github.com/hashicorp/go-plugin v1.6.0 h1:wgd4KxHJTVGGqWBq4QPB1i5BZNEx9BR8+OFmHDmTk8A=
 github.com/hashicorp/go-plugin v1.6.0/go.mod h1:lBS5MtSSBZk0SHc66KACcjjlU6WzEVP/8pwz68aMkCI=
-github.com/hashicorp/go-retryablehttp v0.7.7 h1:C8hUCYzor8PIfXHa4UrZkU4VvK8o9ISHxT2Q8+VepXU=
-github.com/hashicorp/go-retryablehttp v0.7.7/go.mod h1:pkQpWZeYWskR+D1tR2O5OcBFOxfA7DoAO6xtkuQnHTk=
 github.com/hashicorp/go-uuid v1.0.0/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro=
 github.com/hashicorp/go-uuid v1.0.3 h1:2gKiV6YVmrJ1i2CKKa9obLvRieoRGviZFL26PcT/Co8=
 github.com/hashicorp/go-uuid v1.0.3/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro=

internal/bitwarden/webapi/client.go

@@ -8,17 +8,23 @@ import (
 	"fmt"
 	"io"
 	"mime/multipart"
+	"net/http"
 	"net/url"
 	"strings"
 	"time"
 
-	"github.com/hashicorp/go-retryablehttp"
 	"github.com/hashicorp/terraform-plugin-log/tflog"
 	"github.com/maxlaverse/terraform-provider-bitwarden/internal/bitwarden/crypto"
 	"github.com/maxlaverse/terraform-provider-bitwarden/internal/bitwarden/crypto/keybuilder"
 	"github.com/maxlaverse/terraform-provider-bitwarden/internal/bitwarden/models"
 )
 
+const (
+	defaultRequestTimeout = 10 * time.Second
+	maxConcurrentRequests = 4
+	maxRetryAttempts      = 3
+)
+
 type Client interface {
 	CreateFolder(ctx context.Context, obj Folder) (*Folder, error)
 	CreateObject(context.Context, models.Object) (*models.Object, error)
@@ -55,22 +61,22 @@ type Client interface {
 
 func NewClient(serverURL, deviceIdentifier, providerVersion string, opts ...Options) Client {
 	c := &client{
-		device:     DeviceInformation(deviceIdentifier, providerVersion),
-		serverURL:  strings.TrimSuffix(serverURL, "/"),
-		httpClient: retryablehttp.NewClient(),
+		device:    DeviceInformation(deviceIdentifier, providerVersion),
+		serverURL: strings.TrimSuffix(serverURL, "/"),
+		httpClient: &http.Client{
+			Transport: NewRetryRoundTripper(maxConcurrentRequests, maxRetryAttempts, defaultRequestTimeout),
+		},
 	}
 	for _, o := range opts {
 		o(c)
 	}
-	c.httpClient.Logger = nil
-	c.httpClient.CheckRetry = CustomRetryPolicy
-	c.httpClient.HTTPClient.Timeout = 10 * time.Second
+
 	return c
 }
 
 type client struct {
 	device             deviceInfoWithOfficialFallback
-	httpClient         *retryablehttp.Client
+	httpClient         *http.Client
 	serverURL          string
 	sessionAccessToken string
 }
@@ -86,7 +92,7 @@ func (c *client) CreateFolder(ctx context.Context, obj Folder) (*Folder, error)
 
 func (c *client) CreateObject(ctx context.Context, obj models.Object) (*models.Object, error) {
 	var err error
-	var httpReq *retryablehttp.Request
+	var httpReq *http.Request
 	if len(obj.CollectionIds) != 0 {
 		cipherCreationRequest := CreateCipherRequest{
 			Cipher:        obj,
@@ -506,8 +512,8 @@ func (c *client) Sync(ctx context.Context) (*SyncResponse, error) {
 	return doRequest[SyncResponse](ctx, c.httpClient, httpReq)
 }
 
-func (c *client) prepareRequest(ctx context.Context, reqMethod, reqUrl string, reqBody interface{}) (*retryablehttp.Request, error) {
-	var httpReq *retryablehttp.Request
+func (c *client) prepareRequest(ctx context.Context, reqMethod, reqUrl string, reqBody interface{}) (*http.Request, error) {
+	var httpReq *http.Request
 	var err error
 
 	if reqBody != nil {
@@ -525,12 +531,12 @@ func (c *client) prepareRequest(ctx context.Context, reqMethod, reqUrl string, r
 				return nil, fmt.Errorf("unable to marshall request body: %w", err)
 			}
 		}
-		httpReq, err = retryablehttp.NewRequestWithContext(ctx, reqMethod, reqUrl, bytes.NewBuffer(bodyBytes))
+		httpReq, err = http.NewRequestWithContext(ctx, reqMethod, reqUrl, bytes.NewBuffer(bodyBytes))
 		if httpReq != nil && len(contentType) > 0 {
 			httpReq.Header.Add("Content-Type", contentType)
 		}
 	} else {
-		httpReq, err = retryablehttp.NewRequestWithContext(ctx, reqMethod, reqUrl, nil)
+		httpReq, err = http.NewRequestWithContext(ctx, reqMethod, reqUrl, nil)
 	}
 
 	if err != nil {
@@ -549,7 +555,7 @@ func (c *client) prepareRequest(ctx context.Context, reqMethod, reqUrl string, r
 	return httpReq, nil
 }
 
-func doRequest[T any](ctx context.Context, httpClient *retryablehttp.Client, httpReq *retryablehttp.Request) (*T, error) {
+func doRequest[T any](ctx context.Context, httpClient *http.Client, httpReq *http.Request) (*T, error) {
 	logRequest(ctx, httpReq)
 
 	resp, err := httpClient.Do(httpReq)
@@ -560,9 +566,12 @@ func doRequest[T any](ctx context.Context, httpClient *retryablehttp.Client, htt
 
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
-		return nil, fmt.Errorf("error reading response body from to '%s': %w", httpReq.URL, err)
+		return nil, fmt.Errorf("error reading response body from '%s %s': %w", httpReq.Method, httpReq.URL, err)
 	}
 
+	debugInfo := map[string]interface{}{"status_code": resp.StatusCode, "url": httpReq.URL.RequestURI(), "body": string(body), "headers": resp.Header}
+	tflog.Trace(ctx, "Response from Bitwarden server", debugInfo)
+
 	if resp.StatusCode != 200 {
 		return nil, fmt.Errorf("bad response status code for '%s': %d!=200, body:%s", httpReq.URL, resp.StatusCode, string(body))
 	}
@@ -580,23 +589,37 @@ func doRequest[T any](ctx context.Context, httpClient *retryablehttp.Client, htt
 		fmt.Printf("Body to unmarshall: %s\n", string(body))
 		return nil, fmt.Errorf("error unmarshalling response from '%s': %w", httpReq.URL, err)
 	}
-	debugInfo := map[string]interface{}{"url": httpReq.URL.RequestURI(), "body": string(body)}
 
-	tflog.Trace(ctx, "Response from Bitwarden server", debugInfo)
 	return &res, nil
 }
 
-func logRequest(ctx context.Context, httpReq *retryablehttp.Request) {
-	bodyCopy, err := httpReq.BodyBytes()
-	if err != nil {
-		tflog.Trace(ctx, "Unable to re-read request body", map[string]interface{}{"error": err})
-	}
+func logRequest(ctx context.Context, httpReq *http.Request) {
 	debugInfo := map[string]interface{}{
 		"url":     httpReq.URL.RequestURI(),
 		"method":  httpReq.Method,
 		"headers": httpReq.Header,
-		"body":    string(bodyCopy),
+	}
+
+	if httpReq.Body != nil {
+		bodyCopy, newBody, err := readAndRestoreBody(httpReq.Body)
+		if err != nil {
+			tflog.Trace(ctx, "Unable to re-read request body", map[string]interface{}{"error": err})
+		}
+		httpReq.Body = newBody
+		debugInfo["body"] = string(bodyCopy)
 	}
 
 	tflog.Trace(ctx, "Request to Bitwarden server ", debugInfo)
 }
+
+func readAndRestoreBody(rc io.ReadCloser) ([]byte, io.ReadCloser, error) {
+	var buf bytes.Buffer
+
+	tee := io.TeeReader(rc, &buf)
+
+	body, err := io.ReadAll(tee)
+	if err != nil {
+		return nil, nil, err
+	}
+	return body, io.NopCloser(bytes.NewReader(buf.Bytes())), nil
+}

internal/bitwarden/webapi/options.go

@@ -6,12 +6,16 @@ type Options func(c Client)
 
 func DisableRetries() Options {
 	return func(c Client) {
-		c.(*client).httpClient.RetryMax = 0
+		roundTripper, ok := c.(*client).httpClient.Transport.(*RetryRoundTripper)
+		if !ok {
+			return
+		}
+		roundTripper.DisableRetries = true
 	}
 }
 
 func WithCustomClient(httpClient http.Client) Options {
 	return func(c Client) {
-		c.(*client).httpClient.HTTPClient = &httpClient
+		c.(*client).httpClient = &httpClient
 	}
 }