aws / aws-sdk-js-v3

Modularized AWS SDK for JavaScript.
Apache License 2.0
2.98k stars 560 forks source link

Multipart upload doesn't throw or reject when max upload parts exceeded #5964

Closed piurafunk closed 1 month ago

piurafunk commented 3 months ago

Checkboxes for prior research

Describe the bug

Exceeding Upload.MAX_PARTS doesn't throw an exception completely out

SDK version number

@aws-sdk/client-s3@3.549.0

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

v20.12.0

Reproduction Steps

import { S3 } from '@aws-sdk/client-s3'
import { Upload } from '@aws-sdk/lib-storage'

const s3 = new S3()
const bucket = 'bucket' // Update this to your bucket name

// The upload must be more than 5242880000 bytes (5 MiB x 10000 parts)
// To pass this via stdin, you can use the following command:
// $ dd if=/dev/urandom bs=5242880 count=11000 | node repro.mjs

// If you have pipeviewer installed, you can use the following command to see the progress:
// $ dd if=/dev/urandom bs=5242880 count=11000 | pv -s $((5242880 * 11000)) | node repro.mjs
const upload = new Upload({
    params: {
        Bucket: bucket,
        Key: 'bigfile.txt',
        Body: process.stdin,
    },
    client: s3,
    queueSize: 50,
})

await upload.done()

Observed Behavior

aws --profile sso-staging s3api get-object-attributes --bucket <bucket> --key bigfile.txt --object-attributes ObjectSize
{
    "LastModified": "2024-04-04T20:46:13+00:00",
    "VersionId": "lAYUcGXinSavYWPxPLsbfTFRWoD_UT5s",
    "ObjectSize": 52428800000
}

The object size is actually exactly 5 MiB x 10,000 parts: 52428800000 bytes. Notably, it also correctly completes the multi-part upload, so it really does look like it completes.

I also tested with await upload.done().catch(err => {throw err}) in case of promise being rejected; it has the same problem.

Expected Behavior

https://github.com/aws/aws-sdk-js-v3/blob/v3.549.0/lib/lib-storage/src/Upload.ts#L195 should throw an exception out when the parts max is exceeded.

Possible Solution

I'm not a node expert, but I suspect it has something to do with throwing an error inside an async function.

Additional Information/Context

No response

RanVaknin commented 2 months ago

Hi @piurafunk ,

Thanks for your patience. Im able to reproduce the reported behavior. It's not clear to me at this point why an error is not thrown by the SDK. Will review this and let you know.

Thanks, Ran~

kuhe commented 1 month ago

fix included in https://github.com/aws/aws-sdk-js-v3/pull/6112

kuhe commented 1 month ago

should be released in https://github.com/aws/aws-sdk-js-v3/releases/tag/v3.582.0 tomorrow

github-actions[bot] commented 2 weeks ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.