Skip to content

Commit

Permalink
rsstorage: return original resolver err when S3 upload fails during P…
Browse files Browse the repository at this point in the history
…ut()

When the resolver failed before, a generic S3 upload error was returned,
"Something went wrong uploading to S3", which was both hard to error
handle and misleading (because it failed before even attempting to upload).
  • Loading branch information
glin committed Aug 29, 2024
1 parent 133c615 commit 6915532
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 6 deletions.
15 changes: 10 additions & 5 deletions pkg/rsstorage/servers/s3server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,17 +251,17 @@ func (s *StorageServer) Put(resolve types.Resolver, dir, address string) (string
// Resolve (read) the item using the piped writer
var wdir string
var waddress string
var resolverErr error
go func() {
// Make sure an EOF gets sent to the pipe writer. Without this, we end
// up hanging forever while the uploader reads from the pipe.
var err error
wdir, waddress, err = resolve(w)
if err != nil {
w.CloseWithError(err)
wdir, waddress, resolverErr = resolve(w)
if resolverErr != nil {
_ = w.CloseWithError(resolverErr)
return
}
// Close with EOF if successful
w.CloseWithError(io.EOF)
_ = w.CloseWithError(io.EOF)
}()

// Upload to a temporary S3 address using the piped reader
Expand All @@ -274,6 +274,11 @@ func (s *StorageServer) Put(resolve types.Resolver, dir, address string) (string

if err != nil {
cancel()
// If the upload failed because of a resolver error, return the original error
// instead of a generic upload error
if resolverErr != nil {
return "", "", resolverErr
}
return "", "", err
}

Expand Down
4 changes: 3 additions & 1 deletion pkg/rsstorage/servers/s3server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ func (s *fakeS3) Upload(input *s3manager.UploadInput, ctx context.Context, optio
s.address = *input.Key

}
// If there's an upload error, copy anyway to ensure that the resolver runs
_, _ = io.Copy(io.Discard, input.Body)
return s.upload, s.uploadErr
}

Expand Down Expand Up @@ -419,7 +421,7 @@ func (s *S3StorageServerSuite) TestPut(c *check.C) {
c.Assert(err, check.ErrorMatches, "upload error")

// Resolve error
svc.uploadErr = nil
svc.uploadErr = errors.New("upload error (should not be returned)")
resolver = func(w io.Writer) (string, string, error) {
return "", "", errors.New("resolver error")
}
Expand Down

0 comments on commit 6915532

Please sign in to comment.