Skip to content

Commit

Permalink
external: improve S3 error message (#59327) (#59749)
Browse files Browse the repository at this point in the history
close #59326
  • Loading branch information
ti-chi-bot authored Feb 27, 2025
1 parent e34e953 commit db55e5e
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 4 deletions.
10 changes: 8 additions & 2 deletions br/pkg/storage/ks3.go
Original file line number Diff line number Diff line change
Expand Up @@ -502,8 +502,14 @@ func (rs *KS3Storage) open(
}

if startOffset != r.Start || (endOffset != 0 && endOffset != r.End+1) {
return nil, r, errors.Annotatef(berrors.ErrStorageUnknown, "open file '%s' failed, expected range: %s, got: %v",
path, *rangeOffset, result.ContentRange)
rangeStr := "<empty>"
if result.ContentRange != nil {
rangeStr = *result.ContentRange
}
return nil, r, errors.Annotatef(
berrors.ErrStorageUnknown,
"open file '%s' failed, expected range: %s, got: %s",
path, *rangeOffset, rangeStr)
}

return result.Body, r, nil
Expand Down
9 changes: 7 additions & 2 deletions br/pkg/storage/s3.go
Original file line number Diff line number Diff line change
Expand Up @@ -846,8 +846,13 @@ func (rs *S3Storage) open(
}

if startOffset != r.Start || (endOffset != 0 && endOffset != r.End+1) {
return nil, r, errors.Annotatef(berrors.ErrStorageUnknown, "open file '%s' failed, expected range: %s, got: %v",
path, *rangeOffset, result.ContentRange)
rangeStr := "<empty>"
if result.ContentRange != nil {
rangeStr = *result.ContentRange
}
return nil, r, errors.Annotatef(berrors.ErrStorageUnknown,
"open file '%s' failed, expected range: %s, got: %s",
path, *rangeOffset, rangeStr)
}

return result.Body, r, nil
Expand Down
27 changes: 27 additions & 0 deletions br/pkg/storage/s3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1421,3 +1421,30 @@ func TestS3ReadFileRetryable(t *testing.T) {
require.Error(t, err)
require.True(t, strings.Contains(err.Error(), errMsg))
}

func TestOpenRangeMismatchErrorMsg(t *testing.T) {
s := createS3Suite(t)
ctx := aws.BackgroundContext()
start, end := int64(10), int64(30)

s.s3.EXPECT().
GetObjectWithContext(ctx, gomock.Any()).
DoAndReturn(func(context.Context, *s3.GetObjectInput, ...request.Option) (*s3.GetObjectOutput, error) {
return &s3.GetObjectOutput{
ContentRange: aws.String("bytes 10-20/20"),
}, nil
})
reader, err := s.storage.Open(ctx, "test", &ReaderOption{StartOffset: &start, EndOffset: &end})
require.ErrorContains(t, err, "expected range: bytes=10-29, got: bytes 10-20/20")
require.Nil(t, reader)

s.s3.EXPECT().
GetObjectWithContext(ctx, gomock.Any()).
DoAndReturn(func(context.Context, *s3.GetObjectInput, ...request.Option) (*s3.GetObjectOutput, error) {
return &s3.GetObjectOutput{}, nil
})
reader, err = s.storage.Open(ctx, "test", &ReaderOption{StartOffset: &start, EndOffset: &end})
// other function will throw error
require.ErrorContains(t, err, "ContentRange is empty")
require.Nil(t, reader)
}

0 comments on commit db55e5e

Please sign in to comment.