Don't re-use empty body on retry

goshopify.go

@@ -4,6 +4,7 @@ package goshopify
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -347,8 +348,19 @@ func (c *Client) doGetHeaders(req *http.Request, v interface{}) (http.Header, er
 	c.attempts = 0
 	c.logRequest(req)
 
+	// copy request body so it can be re-used
+	var body []byte
+	if req.Body != nil {
+		body, err = ioutil.ReadAll(req.Body)
+		defer req.Body.Close()
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	for {
 		c.attempts++
+		req.Body = ioutil.NopCloser(bytes.NewBuffer(body))
 		resp, err = c.Client.Do(req)
 		c.logResponse(resp)
 		if err != nil {
@@ -443,7 +455,10 @@ func (c *Client) logBody(body *io.ReadCloser, format string) {
 	if body == nil {
 		return
 	}
-	b, _ := ioutil.ReadAll(*body)
+	b, err := ioutil.ReadAll(*body)
+	if err != nil && !errors.Is(err, io.EOF) {
+		return
+	}
 	if len(b) > 0 {
 		c.log.Debugf(format, string(b))
 	}

goshopify_test.go

@@ -30,8 +30,10 @@ var (
 // errReader can be used to simulate a failed call to response.Body.Read
 type errReader struct{}
 
+var testErr = errors.New("test-error")
+
 func (errReader) Read([]byte) (int, error) {
-	return 0, errors.New("test-error")
+	return 0, testErr
 }
 
 func (errReader) Close() error {
@@ -350,6 +352,24 @@ func TestDo(t *testing.T) {
 	}
 }
 
+func TestDoErrBody(t *testing.T) {
+	shopUrl := "https://fooshop.myshopify.com"
+	httpmock.RegisterResponder("GET", shopUrl, httpmock.NewStringResponder(200, ""))
+
+	req, err := http.NewRequest("GET", shopUrl, errReader{})
+	if err != nil {
+		t.Error("error creating request: ", err)
+	}
+
+	err = client.Do(req, nil)
+	if err == nil {
+		t.Errorf("Do(): expected error test-error, actual nil")
+	}
+	if !errors.Is(err, testErr) {
+		t.Errorf("Do(): expected ResponseDecodingError, actual %#v", err)
+	}
+}
+
 func TestRetry(t *testing.T) {
 	setup()
 	defer teardown()
@@ -462,6 +482,44 @@ func TestRetry(t *testing.T) {
 	}
 }
 
+func TestRetryPost(t *testing.T) {
+	u := "foo/1"
+	responder := func(req *http.Request) (*http.Response, error) {
+		resp := httpmock.NewStringResponse(http.StatusTooManyRequests, `{"errors":"Exceeded 2 calls per second for api client. Reduce request rates to resume uninterrupted service."}`)
+		resp.Header.Add("Retry-After", "2.0")
+		body, err := ioutil.ReadAll(req.Body)
+		if err != nil {
+			return nil, err
+		}
+		if len(body) == 0 {
+			return nil, errors.New("empty body")
+		}
+		return resp, nil
+	}
+	expected := RateLimitError{
+		RetryAfter: 2,
+		ResponseError: ResponseError{
+			Status:  429,
+			Message: "Exceeded 2 calls per second for api client. Reduce request rates to resume uninterrupted service.",
+		},
+	}
+
+	testClient := NewClient(app, "fooshop", "abcd", WithRetry(2))
+	httpmock.ActivateNonDefault(testClient.Client)
+	shopUrl := fmt.Sprintf("https://fooshop.myshopify.com/%v", u)
+	httpmock.RegisterResponder("POST", shopUrl, responder)
+
+	testBody := []byte(`{"foo": "bar"}`)
+	req, err := testClient.NewRequest("POST", u, testBody, nil)
+	if err != nil {
+		t.Errorf("TestRetryPost(): errored %s", err)
+	}
+	err = testClient.Do(req, nil)
+	if !reflect.DeepEqual(err, expected) {
+		t.Errorf("Do(): expected error %#v, actual %#v", expected, err)
+	}
+}
+
 func TestClientDoAutoApiVersion(t *testing.T) {
 	u := "foo/1"
 	responder := func(req *http.Request) (*http.Response, error) {
@@ -736,7 +794,7 @@ func TestCheckResponseError(t *testing.T) {
 		},
 		{
 			&http.Response{StatusCode: 400, Body: errReader{}},
-			errors.New("test-error"),
+			testErr,
 		},
 		{
 			httpmock.NewStringResponse(422, `{"error": "Unprocessable Entity - ok"}`),