Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(internal/gensupport): Make SendRequestWithRetry check for canceled contexts twice #1359

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 13 additions & 0 deletions internal/gensupport/send.go
Expand Up @@ -99,6 +99,19 @@ func sendAndRetry(ctx context.Context, client *http.Client, req *http.Request, r
case <-time.After(pause):
}

select {
lhchavez marked this conversation as resolved.
Show resolved Hide resolved
case <-ctx.Done():
// Check for context cancellation once more. If more than one case in a
// select is satisfied at the same time, Go will choose one arbitrarily.
// That can cause an operation to go through even if the context was
// canceled before.
if err == nil {
err = ctx.Err()
}
return resp, err
default:
}

resp, err = client.Do(req.WithContext(ctx))

var status int
Expand Down
22 changes: 22 additions & 0 deletions internal/gensupport/send_test.go
Expand Up @@ -6,6 +6,7 @@ package gensupport

import (
"context"
"errors"
lhchavez marked this conversation as resolved.
Show resolved Hide resolved
"net/http"
"testing"
)
Expand All @@ -29,3 +30,24 @@ func TestSendRequestWithRetry(t *testing.T) {
t.Error("got nil, want error")
}
}

type brokenRoundTripper struct{}

func (t *brokenRoundTripper) RoundTrip(r *http.Request) (*http.Response, error) {
return nil, errors.New("this should not happen")
}

func TestCanceledContextDoesNotPerformRequest(t *testing.T) {
client := http.Client{
Transport: &brokenRoundTripper{},
}
for i := 0; i < 1000; i++ {
req, _ := http.NewRequest("GET", "url", nil)
ctx, cancel := context.WithCancel(context.Background())
cancel()
_, err := SendRequestWithRetry(ctx, &client, req)
if !errors.Is(err, context.Canceled) {
t.Fatalf("got %v, want %v", err, context.Canceled)
}
}
}