From 9e65bdc024c564f7e50999c5060067d8fbed5f5d Mon Sep 17 00:00:00 2001 From: Naufan Rizal Date: Fri, 21 Jun 2024 13:30:35 +1200 Subject: [PATCH 1/3] Ignore empty body when sending request --- internal/api/client.go | 31 +++++++++++++++++++------------ internal/api/client_test.go | 35 ++++++++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 13 deletions(-) diff --git a/internal/api/client.go b/internal/api/client.go index 25c08fbf..3723de05 100644 --- a/internal/api/client.go +++ b/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 diff --git a/internal/api/client_test.go b/internal/api/client_test.go index 6eddd025..5b5ef95b 100644 --- a/internal/api/client_test.go +++ b/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,39 @@ 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, _ := io.ReadAll(r.Body) + 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 From 72a30206433168e4db530ad453a5767da52fb163 Mon Sep 17 00:00:00 2001 From: Naufan Rizal Date: Fri, 21 Jun 2024 13:42:30 +1200 Subject: [PATCH 2/3] handle body read error in test --- internal/api/client_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/api/client_test.go b/internal/api/client_test.go index 5b5ef95b..6c90ff7a 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -120,7 +120,11 @@ func TestDoWithRetry_Succesful_GET(t *testing.T) { svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) - bodyBytes, _ := io.ReadAll(r.Body) + 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)) } From 46f5091037f6beb2934a4438dc66c259c6459b19 Mon Sep 17 00:00:00 2001 From: Naufan Rizal Date: Fri, 21 Jun 2024 13:56:38 +1200 Subject: [PATCH 3/3] update changelog and version --- CHANGELOG.md | 3 +++ version/VERSION | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 24ccd345..995033dc 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/version/VERSION b/version/VERSION index a918a2aa..ee6cdce3 100644 --- a/version/VERSION +++ b/version/VERSION @@ -1 +1 @@ -0.6.0 +0.6.1