Skip to content

Commit

Permalink
fix(storage/chunk/client/aws): have GetObject check for canceled context
Browse files Browse the repository at this point in the history
S3ObjectClient.GetObject incorrectly returned nil, 0, nil when the
provided context is already canceled.
  • Loading branch information
rfratto committed Oct 8, 2024
1 parent 91c7d34 commit fee5b5c
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 1 deletion.
2 changes: 1 addition & 1 deletion pkg/storage/chunk/client/aws/s3_storage_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ func (a *S3ObjectClient) GetObject(ctx context.Context, objectKey string) (io.Re
// Map the key into a bucket
bucket := a.bucketFromKey(objectKey)

var lastErr error
lastErr := ctx.Err()

retries := backoff.New(ctx, a.cfg.BackoffConfig)
for retries.Ongoing() {
Expand Down
25 changes: 25 additions & 0 deletions pkg/storage/chunk/client/aws/s3_storage_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,31 @@ func TestRequestMiddleware(t *testing.T) {
}
}

func TestS3ObjectClient_GetObject_CanceledContext(t *testing.T) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
fmt.Fprintln(w, r.Header.Get("echo-me"))
}))
defer ts.Close()

cfg := S3Config{
Endpoint: ts.URL,
BucketNames: "buck-o",
S3ForcePathStyle: true,
Insecure: true,
AccessKeyID: "key",
SecretAccessKey: flagext.SecretWithValue("secret"),
}

client, err := NewS3ObjectClient(cfg, hedging.Config{})
require.NoError(t, err)

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

_, _, err = client.GetObject(ctx, "key")
require.Error(t, err, "GetObject should fail when given a canceled context")
}

func Test_Hedging(t *testing.T) {
for _, tc := range []struct {
name string
Expand Down

0 comments on commit fee5b5c

Please sign in to comment.