From e07ce2f40705cff94e525aedfa9b5fd96b18fea1 Mon Sep 17 00:00:00 2001 From: davesavic Date: Sun, 7 Jan 2024 14:12:06 +1000 Subject: [PATCH 1/2] Added persistent body on retry requests --- client.go | 18 ++++++++ client_test.go | 62 ++++++++++++++++++++++++- coverage.out | 123 ++++++++++++++++++++++++++----------------------- 3 files changed, 144 insertions(+), 59 deletions(-) diff --git a/client.go b/client.go index c432a7b..177364b 100644 --- a/client.go +++ b/client.go @@ -1,6 +1,7 @@ package clink import ( + "bytes" "encoding/base64" "encoding/json" "fmt" @@ -53,9 +54,26 @@ func (c *Client) Do(req *http.Request) (*http.Response, error) { } var resp *http.Response + var body []byte var err error + if req.Body != nil && req.Body != http.NoBody { + body, err = io.ReadAll(req.Body) + if err != nil { + return nil, fmt.Errorf("failed to read request body: %w", err) + } + + err = req.Body.Close() + if err != nil { + return nil, fmt.Errorf("failed to close request body: %w", err) + } + } + for attempt := 0; attempt <= c.MaxRetries; attempt++ { + if len(body) > 0 { + req.Body = io.NopCloser(bytes.NewReader(body)) + } + resp, err = c.HttpClient.Do(req) if req.Context().Err() != nil { diff --git a/client_test.go b/client_test.go index 2b44976..6aa3168 100644 --- a/client_test.go +++ b/client_test.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "encoding/json" "errors" + "fmt" "io" "net/http" "net/http/httptest" @@ -347,7 +348,6 @@ func TestClient_Methods(t *testing.T) { if !tc.resultFunc(resp, tc.method) { t.Errorf("expected result to be successful") } - }) } } @@ -524,6 +524,66 @@ func TestUnsuccessfulRetries(t *testing.T) { } } +// TestRequestBodyEmptyOnRetries tests that the request body on a custom io.Reader wrapper is NOT empty on retries. +type oneTimeReaderWrapper struct { + data []byte + consumed bool +} + +func (r *oneTimeReaderWrapper) Read(p []byte) (n int, err error) { + if r.consumed { + return 0, fmt.Errorf("body already read") + } + n = copy(p, r.data) + r.consumed = true + return n, io.EOF +} + +func TestRequestBodyNotEmptyOnRetries(t *testing.T) { + var requestCount int + var lastRequestBody string + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestCount++ + + bodyBytes, err := io.ReadAll(r.Body) + if err != nil { + t.Fatalf("failed to read request body: %v", err) + } + lastRequestBody = string(bodyBytes) + + w.WriteHeader(http.StatusInternalServerError) + })) + defer server.Close() + + client := clink.NewClient( + clink.WithRetries(1, func(request *http.Request, response *http.Response, err error) bool { + return true + }), + clink.WithClient(server.Client()), + ) + + requestBody := []byte("test body") + req, err := http.NewRequest(http.MethodPost, server.URL, &oneTimeReaderWrapper{data: requestBody}) + if err != nil { + t.Fatalf("failed to create request: %v", err) + } + + _, err = client.Do(req) + if err != nil { + t.Fatalf("failed to make request: %v", err) + } + + if requestCount != 2 { + t.Fatalf("expected 2 requests due to retry, but got %d", requestCount) + } + + expectedBody := string(requestBody) + if lastRequestBody != expectedBody { + t.Errorf("expected request body to be '%s' on retry, got '%s'", expectedBody, lastRequestBody) + } +} + func TestContextCancellationDuringRetries(t *testing.T) { var requestCount int server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/coverage.out b/coverage.out index 8c29e91..560cf6c 100644 --- a/coverage.out +++ b/coverage.out @@ -1,69 +1,76 @@ mode: count -github.com/davesavic/clink/client.go:24.40,27.27 2 26 -github.com/davesavic/clink/client.go:27.27,29.3 1 31 -github.com/davesavic/clink/client.go:31.2,31.10 1 26 -github.com/davesavic/clink/client.go:34.30,39.2 1 26 -github.com/davesavic/clink/client.go:44.64,45.36 1 18 -github.com/davesavic/clink/client.go:45.36,47.3 1 2 -github.com/davesavic/clink/client.go:49.2,49.26 1 18 -github.com/davesavic/clink/client.go:49.26,50.59 1 2 -github.com/davesavic/clink/client.go:50.59,52.4 1 0 -github.com/davesavic/clink/client.go:55.2,58.55 3 18 -github.com/davesavic/clink/client.go:58.55,61.33 2 22 -github.com/davesavic/clink/client.go:61.33,63.4 1 1 -github.com/davesavic/clink/client.go:65.3,65.69 1 21 -github.com/davesavic/clink/client.go:65.69,66.9 1 1 -github.com/davesavic/clink/client.go:69.3,69.29 1 20 -github.com/davesavic/clink/client.go:69.29,70.11 1 5 -github.com/davesavic/clink/client.go:71.60,71.60 0 4 -github.com/davesavic/clink/client.go:72.32,73.36 1 1 -github.com/davesavic/clink/client.go:78.2,78.16 1 16 -github.com/davesavic/clink/client.go:78.16,80.3 1 0 -github.com/davesavic/clink/client.go:82.2,82.18 1 16 -github.com/davesavic/clink/client.go:86.59,88.16 2 1 -github.com/davesavic/clink/client.go:88.16,90.3 1 0 -github.com/davesavic/clink/client.go:91.2,91.18 1 1 -github.com/davesavic/clink/client.go:95.62,97.16 2 1 -github.com/davesavic/clink/client.go:97.16,99.3 1 0 -github.com/davesavic/clink/client.go:100.2,100.18 1 1 -github.com/davesavic/clink/client.go:104.58,106.16 2 1 +github.com/davesavic/clink/client.go:25.40,28.27 2 27 +github.com/davesavic/clink/client.go:28.27,30.3 1 33 +github.com/davesavic/clink/client.go:32.2,32.10 1 27 +github.com/davesavic/clink/client.go:35.30,40.2 1 27 +github.com/davesavic/clink/client.go:45.64,46.36 1 19 +github.com/davesavic/clink/client.go:46.36,48.3 1 2 +github.com/davesavic/clink/client.go:50.2,50.26 1 19 +github.com/davesavic/clink/client.go:50.26,51.59 1 2 +github.com/davesavic/clink/client.go:51.59,53.4 1 0 +github.com/davesavic/clink/client.go:56.2,60.48 4 19 +github.com/davesavic/clink/client.go:60.48,62.17 2 1 +github.com/davesavic/clink/client.go:62.17,64.4 1 0 +github.com/davesavic/clink/client.go:66.3,67.17 2 1 +github.com/davesavic/clink/client.go:67.17,69.4 1 0 +github.com/davesavic/clink/client.go:72.2,72.55 1 19 +github.com/davesavic/clink/client.go:72.55,73.20 1 24 +github.com/davesavic/clink/client.go:73.20,75.4 1 2 +github.com/davesavic/clink/client.go:77.3,79.33 2 24 +github.com/davesavic/clink/client.go:79.33,81.4 1 1 +github.com/davesavic/clink/client.go:83.3,83.69 1 23 +github.com/davesavic/clink/client.go:83.69,84.9 1 1 +github.com/davesavic/clink/client.go:87.3,87.29 1 22 +github.com/davesavic/clink/client.go:87.29,88.11 1 6 +github.com/davesavic/clink/client.go:89.60,89.60 0 5 +github.com/davesavic/clink/client.go:90.32,91.36 1 1 +github.com/davesavic/clink/client.go:96.2,96.16 1 17 +github.com/davesavic/clink/client.go:96.16,98.3 1 0 +github.com/davesavic/clink/client.go:100.2,100.18 1 17 +github.com/davesavic/clink/client.go:104.59,106.16 2 1 github.com/davesavic/clink/client.go:106.16,108.3 1 0 github.com/davesavic/clink/client.go:109.2,109.18 1 1 -github.com/davesavic/clink/client.go:113.75,115.16 2 1 +github.com/davesavic/clink/client.go:113.62,115.16 2 1 github.com/davesavic/clink/client.go:115.16,117.3 1 0 github.com/davesavic/clink/client.go:118.2,118.18 1 1 -github.com/davesavic/clink/client.go:122.74,124.16 2 1 +github.com/davesavic/clink/client.go:122.58,124.16 2 1 github.com/davesavic/clink/client.go:124.16,126.3 1 0 github.com/davesavic/clink/client.go:127.2,127.18 1 1 -github.com/davesavic/clink/client.go:131.76,133.16 2 1 +github.com/davesavic/clink/client.go:131.75,133.16 2 1 github.com/davesavic/clink/client.go:133.16,135.3 1 0 github.com/davesavic/clink/client.go:136.2,136.18 1 1 -github.com/davesavic/clink/client.go:140.61,142.16 2 1 +github.com/davesavic/clink/client.go:140.74,142.16 2 1 github.com/davesavic/clink/client.go:142.16,144.3 1 0 github.com/davesavic/clink/client.go:145.2,145.18 1 1 -github.com/davesavic/clink/client.go:151.45,152.25 1 18 -github.com/davesavic/clink/client.go:152.25,154.3 1 18 -github.com/davesavic/clink/client.go:158.43,159.25 1 2 -github.com/davesavic/clink/client.go:159.25,161.3 1 2 -github.com/davesavic/clink/client.go:165.52,166.25 1 2 -github.com/davesavic/clink/client.go:166.25,167.35 1 2 -github.com/davesavic/clink/client.go:167.35,169.4 1 2 -github.com/davesavic/clink/client.go:174.36,175.25 1 2 -github.com/davesavic/clink/client.go:175.25,178.3 2 2 -github.com/davesavic/clink/client.go:182.54,183.25 1 1 -github.com/davesavic/clink/client.go:183.25,187.3 3 1 -github.com/davesavic/clink/client.go:191.42,192.25 1 1 -github.com/davesavic/clink/client.go:192.25,194.3 1 1 -github.com/davesavic/clink/client.go:198.38,199.25 1 1 -github.com/davesavic/clink/client.go:199.25,201.3 1 1 -github.com/davesavic/clink/client.go:205.95,206.25 1 4 -github.com/davesavic/clink/client.go:206.25,209.3 2 4 -github.com/davesavic/clink/client.go:213.70,214.21 1 7 -github.com/davesavic/clink/client.go:214.21,216.3 1 1 -github.com/davesavic/clink/client.go:218.2,218.26 1 6 -github.com/davesavic/clink/client.go:218.26,220.3 1 1 -github.com/davesavic/clink/client.go:222.2,222.33 1 5 -github.com/davesavic/clink/client.go:222.33,224.3 1 5 -github.com/davesavic/clink/client.go:226.2,226.70 1 5 -github.com/davesavic/clink/client.go:226.70,228.3 1 1 -github.com/davesavic/clink/client.go:230.2,230.12 1 4 +github.com/davesavic/clink/client.go:149.76,151.16 2 1 +github.com/davesavic/clink/client.go:151.16,153.3 1 0 +github.com/davesavic/clink/client.go:154.2,154.18 1 1 +github.com/davesavic/clink/client.go:158.61,160.16 2 1 +github.com/davesavic/clink/client.go:160.16,162.3 1 0 +github.com/davesavic/clink/client.go:163.2,163.18 1 1 +github.com/davesavic/clink/client.go:169.45,170.25 1 19 +github.com/davesavic/clink/client.go:170.25,172.3 1 19 +github.com/davesavic/clink/client.go:176.43,177.25 1 2 +github.com/davesavic/clink/client.go:177.25,179.3 1 2 +github.com/davesavic/clink/client.go:183.52,184.25 1 2 +github.com/davesavic/clink/client.go:184.25,185.35 1 2 +github.com/davesavic/clink/client.go:185.35,187.4 1 2 +github.com/davesavic/clink/client.go:192.36,193.25 1 2 +github.com/davesavic/clink/client.go:193.25,196.3 2 2 +github.com/davesavic/clink/client.go:200.54,201.25 1 1 +github.com/davesavic/clink/client.go:201.25,205.3 3 1 +github.com/davesavic/clink/client.go:209.42,210.25 1 1 +github.com/davesavic/clink/client.go:210.25,212.3 1 1 +github.com/davesavic/clink/client.go:216.38,217.25 1 1 +github.com/davesavic/clink/client.go:217.25,219.3 1 1 +github.com/davesavic/clink/client.go:223.95,224.25 1 5 +github.com/davesavic/clink/client.go:224.25,227.3 2 5 +github.com/davesavic/clink/client.go:231.70,232.21 1 7 +github.com/davesavic/clink/client.go:232.21,234.3 1 1 +github.com/davesavic/clink/client.go:236.2,236.26 1 6 +github.com/davesavic/clink/client.go:236.26,238.3 1 1 +github.com/davesavic/clink/client.go:240.2,240.33 1 5 +github.com/davesavic/clink/client.go:240.33,242.3 1 5 +github.com/davesavic/clink/client.go:244.2,244.70 1 5 +github.com/davesavic/clink/client.go:244.70,246.3 1 1 +github.com/davesavic/clink/client.go:248.2,248.12 1 4 From bef3a979388ac0ae9f4469f4facabd5a6a3c8092 Mon Sep 17 00:00:00 2001 From: davesavic Date: Sun, 7 Jan 2024 14:12:15 +1000 Subject: [PATCH 2/2] Updated README wording --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 0b83d4a..9c75101 100644 --- a/README.md +++ b/README.md @@ -56,7 +56,7 @@ func main() { } ``` -Common *http verbs(HEAD, OPTIONS, GET, HEAD, POST, PATCH, DELETE)* are also supported +*HTTP Methods (HEAD, OPTIONS, GET, HEAD, POST, PATCH, DELETE)* are also supported ```go package main