Skip to content

Commit

Permalink
Merge #43789
Browse files Browse the repository at this point in the history
43789: importccl: Correctly handle cancelled context when importing r=miretskiy a=miretskiy

When calling sendRequest with the cancelled context, ensure
that we return appropriate error to the caller.

Release note (bug fix): handle cancelled context in sendRequest

Fixes #43783 

Co-authored-by: Yevgeniy Miretskiy <yevgeniy@cockroachlabs.com>
  • Loading branch information
craig[bot] and Yevgeniy Miretskiy committed Jan 7, 2020
2 parents 4adfb5e + 0986b76 commit 7855cbd
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 1 deletion.
6 changes: 5 additions & 1 deletion pkg/storage/cloud/external_storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,10 @@ func checkHTTPContentRangeHeader(h string, pos int64) error {
func (r *resumingHTTPReader) sendRequest(
reqHeaders map[string]string,
) (resp *http.Response, err error) {
// Initialize err to the context.Canceled: if our context is canceled, we will
// never enter the loop below; in this case we want to return "nil, canceled"
err = context.Canceled

for attempt, retries := 0,
retry.StartWithCtx(r.ctx, httpRetryOptions); retries.Next(); attempt++ {
resp, err = r.client.req(r.ctx, "GET", r.url, nil, reqHeaders)
Expand All @@ -570,7 +574,7 @@ func (r *resumingHTTPReader) sendRequest(
log.Errorf(r.ctx, "HTTP:Req error: err=%s (attempt %d)", err, attempt)

if _, ok := err.(*retryableHTTPError); !ok {
break
return
}
}

Expand Down
19 changes: 19 additions & 0 deletions pkg/storage/cloud/external_storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -878,3 +878,22 @@ func TestHttpGet(t *testing.T) {
})
}
}

func TestHttpGetWithCancelledContext(t *testing.T) {
defer leaktest.AfterTest(t)()

s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
defer s.Close()

store, err := makeHTTPStorage(s.URL, testSettings)
require.NoError(t, err)
defer func() {
require.NoError(t, store.Close())
}()

ctx, cancel := context.WithCancel(context.Background())
cancel()

_, err = store.ReadFile(ctx, "/something")
require.Error(t, context.Canceled, err)
}

0 comments on commit 7855cbd

Please sign in to comment.