Merge pull request #97 from buildkite/0.6.x

Release 0.6.1

CHANGELOG.md

@@ -1,5 +1,8 @@
 # Changelog
 
+## 0.6.1 - 2024-06-21
+- Ignore request body when it is empty or when the request is a GET request.
+
 ## 0.6.0 - 2024-06-21
 
 - ⚠️ **BREAKING** Remove support for the undocumented `--files` flag.

internal/api/client.go

@@ -85,7 +85,7 @@ type httpRequest struct {
 // After reaching the retry timeout, the function will return ErrRetryTimeout.
 // The request will not be retried when the server returns 4xx status code,
 // and the error message will be returned as an error.
-func (c *Client) DoWithRetry(ctx context.Context, req httpRequest, v interface{}) (*http.Response, error) {
+func (c *Client) DoWithRetry(ctx context.Context, reqOptions httpRequest, v interface{}) (*http.Response, error) {
 	r := roko.NewRetrier(
 		roko.TryForever(),
 		roko.WithStrategy(roko.ExponentialSubsecond(initialDelay)),
@@ -96,28 +96,33 @@ func (c *Client) DoWithRetry(ctx context.Context, req httpRequest, v interface{}
 	defer cancelRetryContext()
 
 	// retry loop
-	debug.Printf("Sending request %s %s", req.Method, req.URL)
+	debug.Printf("Sending request %s %s", reqOptions.Method, reqOptions.URL)
 	resp, err := roko.DoFunc(retryContext, r, func(r *roko.Retrier) (*http.Response, error) {
 		if r.AttemptCount() > 0 {
 			debug.Printf("Retrying requests, attempt %d", r.AttemptCount())
 		}
 
-		reqBody, err := json.Marshal(req.Body)
-		if err != nil {
-			r.Break()
-			return nil, fmt.Errorf("converting body to json: %w", err)
-		}
-
 		// Each request times out after 15 seconds, chosen to provide some
 		// headroom on top of the goal p99 time to fetch of 10s.
 		reqContext, cancelReqContext := context.WithTimeout(ctx, 15*time.Second)
 		defer cancelReqContext()
 
-		req, err := http.NewRequestWithContext(reqContext, req.Method, req.URL, bytes.NewBuffer(reqBody))
+		req, err := http.NewRequestWithContext(reqContext, reqOptions.Method, reqOptions.URL, nil)
 		if err != nil {
 			r.Break()
 			return nil, fmt.Errorf("creating request: %w", err)
 		}
+
+		if reqOptions.Method != http.MethodGet && reqOptions.Body != nil {
+			// add body to request
+			reqBody, err := json.Marshal(reqOptions.Body)
+			if err != nil {
+				r.Break()
+				return nil, fmt.Errorf("converting body to json: %w", err)
+			}
+			req.Body = io.NopCloser(bytes.NewReader(reqBody))
+		}
+
 		req.Header.Add("Content-Type", "application/json")
 
 		resp, err := c.httpClient.Do(req)
@@ -164,9 +169,11 @@ func (c *Client) DoWithRetry(ctx context.Context, req httpRequest, v interface{}
 		}
 
 		// parse response
-		err = json.Unmarshal(responseBody, v)
-		if err != nil {
-			return nil, fmt.Errorf("parsing response: %w", err)
+		if v != nil && len(responseBody) > 0 {
+			err = json.Unmarshal(responseBody, v)
+			if err != nil {
+				return nil, fmt.Errorf("parsing response: %w", err)
+			}
 		}
 
 		return resp, nil

internal/api/client_test.go

@@ -82,7 +82,7 @@ func TestHttpClient_AttachUserAgentToRequest(t *testing.T) {
 	}
 }
 
-func TestDoWithRetry_Success(t *testing.T) {
+func TestDoWithRetry_Succesful_POST(t *testing.T) {
 	svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusOK)
 		io.Copy(w, r.Body)
@@ -116,6 +116,43 @@ func TestDoWithRetry_Success(t *testing.T) {
 	}
 }
 
+func TestDoWithRetry_Succesful_GET(t *testing.T) {
+	svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+
+		bodyBytes, err := io.ReadAll(r.Body)
+		if err != nil {
+			t.Error(err)
+		}
+
+		if len(bodyBytes) > 0 {
+			t.Errorf("request body = %q, want empty", string(bodyBytes))
+		}
+
+	}))
+	defer svr.Close()
+
+	cfg := ClientConfig{
+		ServerBaseUrl: svr.URL,
+	}
+	c := NewClient(cfg)
+
+	var got map[string]string
+
+	resp, err := c.DoWithRetry(context.Background(), httpRequest{
+		Method: http.MethodGet,
+		URL:    svr.URL,
+	}, &got)
+
+	if err != nil {
+		t.Errorf("DoWithRetry() error = %v", err)
+	}
+
+	if resp.StatusCode != http.StatusOK {
+		t.Errorf("DoWithRetry() status code = %v, want %v", resp.StatusCode, http.StatusOK)
+	}
+}
+
 func TestDoWithRetry_RequestError(t *testing.T) {
 	originalTimeout := retryTimeout
 	retryTimeout = 300 * time.Millisecond

version/VERSION

@@ -1 +1 @@
-0.6.0
+0.6.1