bug(lib-storage): single-part uploads not really abortable

EinfachHans commented 1 year ago

Checkboxes for prior research

[X] I've gone through Developer Guide and API reference
[X] I've checked AWS Forums and StackOverflow.
[X] I've searched for previous similar issues and didn't find any solution.

Describe the bug

When uploading a small file (under 5mb) via the lib-storage Upload it is not really "abortable" for me. The abort error is thrown but the file is still uploaded to my bucket.

SDK version number

@aws-sdk/lib-storage@3.395.0, @aws-sdk/client-s3@3.395.0

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

18.16.0

Reproduction Steps

const upload = new Upload({
  client: this.s3,
  params: {
    Bucket: 'some-bucket',
    Key: 'some-key',
    Body: buffer, // file smaller than 5mb
    ContentType: contentType
  }
});

setTimeout(() => {
  upload.abort();
}, 10);

return upload
  .done()
  .then((data: CompleteMultipartUploadCommandOutput) => {
    console.log('Success);
    return data;
  })
  .catch((e) => {
    console.error(`Failed to upload File to S3`, e);
    throw e;
  });

Observed Behavior

I debugged a little bit and mentioned that the code runs into this here:

https://github.com/aws/aws-sdk-js-v3/blob/0a6131e16a8a6578686679704d0da63805b632a7/lib/lib-storage/src/Upload.ts#L321-L323

But at this point, because the the file is uploaded in a single put instead of multipart the, the file is already uploaded completely and visible in my aws console. This results in the error "AbortError" be thrown after the file has been uploaded completly

Expected Behavior

The request should be abortable correctly if not multipart

Possible Solution

No response

Additional Information/Context

No response

yenfryherrerafeliz commented 1 year ago

Hi @EinfachHans, Upload::abort is an async method, which can be confirmed here, and that means that if you want to have it executed intermediately you would need to use await as follow:

setTimeout(async () => {
    await upload.abort()
}, 10)

I also tried it myself and worked fine for me.

Please let me know if that helps!

Thanks!

EinfachHans commented 1 year ago

Hi @yenfryherrerafeliz , thank you for your answer!

Sadly this does not work for me 🤔 Honestly i would be really confused if it had, because even if the method is declared as async here, it doesn't do async stuff, so it doesn't await anything.

yenfryherrerafeliz commented 1 year ago

Hi @EinfachHans, I agree with you. It seems that yesterday when testing, abort got called before the upload started, and therefore the abort worked fine. I tried it today and it did not work. I will mark this issue with a needs-review label so we can address this further.

Thanks!

yenfryherrerafeliz commented 1 year ago

Hi @EinfachHans, I am not sure why, but in my case the issue happens sporadically. Can you please confirm if in your case the issue happens every time you try it?

I used the code below:

import { S3Client } from "@aws-sdk/client-s3";
import { Upload } from "@aws-sdk/lib-storage";

const client = new S3Client({
    region: "us-east-2"
});
const upload = new Upload({
    client: client,
    params: {
        Bucket: process.env.TEST_BUCKET,
        Key: process.env.TEST_KEY,
        Body: "#".repeat(1024 * 1024 * 3),
    }
})
setTimeout(() => {
    upload.abort()
}, 10);

await upload.done();

Thanks!

EinfachHans commented 1 year ago

hi again @yenfryherrerafeliz, it does happen for me all the time but looking into the code i can understand why it might appear sporadically. When uploading a single-part file, the upload function has two lines where the abort state is checked:

The first one is checked before the real single-part put upload happens here. So if the abort function is called before the code enters here the abortion works. The second one is after all of the concurrentUploaders are finished. That's the one i'm running into. And at this point the file is already uploaded completely, because it is single-part and no CompleteMultipartUploadCommand is needed.

So i would say the sporadically could be related to if the setTimeout runs before or between of this two abortion checks

aws / aws-sdk-js-v3