From 155998a32de274d45bdf89ba05f39a63b9eecf86 Mon Sep 17 00:00:00 2001 From: Umegbewe Nwebedu Date: Mon, 3 Feb 2025 14:09:27 +0100 Subject: [PATCH 1/4] fix: client prompt return on context cancellation Signed-off-by: Umegbewe Nwebedu --- api/client.go | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/api/client.go b/api/client.go index ddbfea099..046bd5ef9 100644 --- a/api/client.go +++ b/api/client.go @@ -132,36 +132,26 @@ func (c *httpClient) Do(ctx context.Context, req *http.Request) (*http.Response, req = req.WithContext(ctx) } resp, err := c.client.Do(req) - defer func() { - if resp != nil { - _, _ = io.Copy(io.Discard, resp.Body) - _ = resp.Body.Close() - } - }() - if err != nil { return nil, nil, err } var body []byte - done := make(chan struct{}) + done := make(chan error, 1) go func() { var buf bytes.Buffer - // TODO(bwplotka): Add LimitReader for too long err messages (e.g. limit by 1KB) - _, err = buf.ReadFrom(resp.Body) + _, err := buf.ReadFrom(resp.Body) body = buf.Bytes() - close(done) + done <- err }() select { case <-ctx.Done(): + resp.Body.Close() <-done - err = resp.Body.Close() - if err == nil { - err = ctx.Err() - } - case <-done: + return resp, nil, ctx.Err() + case err = <-done: + resp.Body.Close() + return resp, body, err } - - return resp, body, err } From 9a59353b1404f7425b6f15c75acc2c52e1aa2236 Mon Sep 17 00:00:00 2001 From: Umegbewe Nwebedu Date: Tue, 29 Apr 2025 13:40:49 +0100 Subject: [PATCH 2/4] test: add context cancellation unit test Signed-off-by: Umegbewe Nwebedu --- api/client_test.go | 47 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/api/client_test.go b/api/client_test.go index 874387868..c8a13b83a 100644 --- a/api/client_test.go +++ b/api/client_test.go @@ -21,6 +21,7 @@ import ( "net/http/httptest" "net/url" "testing" + "time" ) func TestConfig(t *testing.T) { @@ -116,6 +117,52 @@ func TestClientURL(t *testing.T) { } } +func TestDoContextCancellation(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte("partial")) + if f, ok := w.(http.Flusher); ok { + f.Flush() + } + + <-r.Context().Done() + })) + + defer ts.Close() + + client, err := NewClient(Config{ + Address: ts.URL, + }) + if err != nil { + t.Fatalf("failed to create client: %v", err) + } + + req, err := http.NewRequest("GET", ts.URL, nil) + if err != nil { + t.Fatalf("failed to create request: %v", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + + start := time.Now() + resp, body, err := client.Do(ctx, req) + elapsed := time.Since(start) + + if err != context.DeadlineExceeded { + t.Errorf("expected error %v, got: %v", context.DeadlineExceeded, err) + } + if body != nil { + t.Errorf("expected no body due to cancellation, got: %q", string(body)) + } + if elapsed > 200*time.Millisecond { + t.Errorf("Do did not return promptly on cancellation: took %v", elapsed) + } + + if resp != nil && resp.Body != nil { + resp.Body.Close() + } +} + // Serve any http request with a response of N KB of spaces. type serveSpaces struct { sizeKB int From 270a2d3cb61def139bafb4dd6b34241014d9d349 Mon Sep 17 00:00:00 2001 From: Umegbewe Nwebedu Date: Tue, 29 Apr 2025 14:01:36 +0100 Subject: [PATCH 3/4] fix/rid unused package Signed-off-by: Umegbewe Nwebedu --- api/client.go | 1 - 1 file changed, 1 deletion(-) diff --git a/api/client.go b/api/client.go index 046bd5ef9..0e647b675 100644 --- a/api/client.go +++ b/api/client.go @@ -18,7 +18,6 @@ import ( "bytes" "context" "errors" - "io" "net" "net/http" "net/url" From d2b716f1cf3a410080751de34a99c5f869ca253e Mon Sep 17 00:00:00 2001 From: Umegbewe Nwebedu Date: Tue, 29 Apr 2025 14:16:44 +0100 Subject: [PATCH 4/4] fix/lint and formatting Signed-off-by: Umegbewe Nwebedu --- api/client_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/api/client_test.go b/api/client_test.go index c8a13b83a..720084aa0 100644 --- a/api/client_test.go +++ b/api/client_test.go @@ -16,6 +16,7 @@ package api import ( "bytes" "context" + "errors" "fmt" "net/http" "net/http/httptest" @@ -123,10 +124,10 @@ func TestDoContextCancellation(t *testing.T) { if f, ok := w.(http.Flusher); ok { f.Flush() } - + <-r.Context().Done() })) - + defer ts.Close() client, err := NewClient(Config{ @@ -136,7 +137,7 @@ func TestDoContextCancellation(t *testing.T) { t.Fatalf("failed to create client: %v", err) } - req, err := http.NewRequest("GET", ts.URL, nil) + req, err := http.NewRequest(http.MethodGet, ts.URL, nil) if err != nil { t.Fatalf("failed to create request: %v", err) } @@ -148,7 +149,7 @@ func TestDoContextCancellation(t *testing.T) { resp, body, err := client.Do(ctx, req) elapsed := time.Since(start) - if err != context.DeadlineExceeded { + if !errors.Is(err, context.DeadlineExceeded) { t.Errorf("expected error %v, got: %v", context.DeadlineExceeded, err) } if body != nil {