S3Manager swallows AbortMultipartUpload Errors

[shawnHartsell]

Documentation

• [X] I've gone though the API reference
• [X] I've checked AWS Forums and StackOverflow for answers
• [X] I've searched for previous similar issues and didn't find any solution

Describe the bug

In https://github.com/aws/aws-sdk-go-v2/blob/main/feature/s3/manager/upload.go#L763, I noticed that if the call to AbortMultipartUpload fails the error is swallowed.

Expected behavior

The error returned by Upload should contain enough context to let callers know that this happened as well the required data (upload id?) so they can take an appropriate response.

Current behavior

Currently an error is returned by Upload, but callers do not know that the uploaded parts have not been cleaned up

Steps to Reproduce

The only way to reproduce the issue would be through an integration test as well as mock the call to AbortMultipartUpload to make it fail.

Possible Solution

I haven't dived too deeply into the implementation, but a backwards compatible fix could be to introduce an error type that could be wrapped and returned via Upload. That would allow callers to check for the error by using errors.As

AWS Go SDK version used

v1.16.0

Compiler and Version used

go version go1.17.3 darwin/amd64

Operating System and version

MacOS 12.2.1

[vudh1]

Hi @shawnHartsell thanks for reaching out. Can you provide sample of reproducible code to help us investigate this behavior?

[github-actions[bot]]

This issue has not received a response in 1 week. If you want to keep this issue open, please just leave a comment below and auto-close will be canceled.

[shawnHartsell]

Hi @shawnHartsell thanks for reaching out. Can you provide sample of reproducible code to help us investigate this behavior?

The tricky bit is that this isn't reproducible by client code. However, we could simulate the error using mocks. For example we could write a unit test that does the following:

1. Preform an upload with the upload manager that results in a multi-part upload. I think we could use a 10MB file using the default configuration values.

2. While uploading the 2nd part, force it to return an error

3. When the SDK calls AbortMultipartUpload (i.e. the code I linked) force that to return an error

4. The error returned to the client does not include context that the AbortMultipartUpload call failed (b/c it's swallowed)

[vudh1]

Do you still have the test code you mentioned above?

[shawnHartsell]

Do you still have the test code you mentioned above?

Unfortunately I don't. I discovered the issue while debugging into the library on a separate issue.