diff --git a/packages/@uppy/aws-s3-multipart/src/MultipartUploader.js b/packages/@uppy/aws-s3-multipart/src/MultipartUploader.js index 7ef7af1db3..a62d42e4c3 100644 --- a/packages/@uppy/aws-s3-multipart/src/MultipartUploader.js +++ b/packages/@uppy/aws-s3-multipart/src/MultipartUploader.js @@ -244,18 +244,20 @@ class MultipartUploader { async #prepareUploadParts (candidates) { this.lockedCandidatesForBatch.push(...candidates) - const result = await this.options.prepareUploadParts({ - key: this.key, - uploadId: this.uploadId, - partNumbers: candidates.map((index) => index + 1), + const result = await this.#retryable({ + attempt: () => this.options.prepareUploadParts({ + key: this.key, + uploadId: this.uploadId, + partNumbers: candidates.map((index) => index + 1), + }), }) - const valid = typeof result?.presignedUrls === 'object' - if (!valid) { + if (typeof result?.presignedUrls !== 'object') { throw new TypeError( 'AwsS3/Multipart: Got incorrect result from `prepareUploadParts()`, expected an object `{ presignedUrls }`.' ) } + return result } diff --git a/packages/@uppy/aws-s3-multipart/src/index.test.js b/packages/@uppy/aws-s3-multipart/src/index.test.js index dd4f151092..b250b1e5f4 100644 --- a/packages/@uppy/aws-s3-multipart/src/index.test.js +++ b/packages/@uppy/aws-s3-multipart/src/index.test.js @@ -50,9 +50,9 @@ describe('AwsS3Multipart', () => { key: 'test/upload/multitest.dat', } }), - completeMultipartUpload: jest.fn(() => Promise.resolve({ location: 'test' })), + completeMultipartUpload: jest.fn(async () => ({ location: 'test' })), abortMultipartUpload: jest.fn(), - prepareUploadParts: jest.fn(() => { + prepareUploadParts: jest.fn(async () => { const presignedUrls = {} const possiblePartNumbers = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] possiblePartNumbers.forEach((partNumber) => { @@ -66,7 +66,7 @@ describe('AwsS3Multipart', () => { awsS3Multipart = core.getPlugin('AwsS3Multipart') }) - it('Calls the prepareUploadParts function totalChunks / limit times', (done) => { + it('Calls the prepareUploadParts function totalChunks / limit times', async () => { const scope = nock( 'https://bucket.s3.us-east-2.amazonaws.com' ).defaultReplyHeaders({ @@ -74,6 +74,10 @@ describe('AwsS3Multipart', () => { 'access-control-allow-origin': '*', 'access-control-expose-headers': 'ETag', }) + // 6MB file will give us 2 chunks, so there will be 2 PUT and 2 OPTIONS + // calls to the presigned URL from 1 prepareUploadParts calls + const fileSize = 5 * MB + 1 * MB + scope .options((uri) => uri.includes('test/upload/multitest.dat')) .reply(200, '') @@ -87,9 +91,6 @@ describe('AwsS3Multipart', () => { .put((uri) => uri.includes('test/upload/multitest.dat')) .reply(200, '', { ETag: 'test2' }) - // 6MB file will give us 2 chunks, so there will be 2 PUT and 2 OPTIONS - // calls to the presigned URL from 1 prepareUploadParts calls - const fileSize = 5 * MB + 1 * MB core.addFile({ source: 'jest', name: 'multitest.dat', @@ -98,16 +99,17 @@ describe('AwsS3Multipart', () => { type: 'application/octet-stream', }), }) - core.upload().then(() => { - expect( - awsS3Multipart.opts.prepareUploadParts.mock.calls.length - ).toEqual(1) - scope.done() - done() - }) + + await core.upload() + + expect( + awsS3Multipart.opts.prepareUploadParts.mock.calls.length + ).toEqual(1) + + scope.done() }) - it('Calls prepareUploadParts with a Math.ceil(limit / 2) minimum, instead of one at a time for the remaining chunks after the first limit batch', (done) => { + it('Calls prepareUploadParts with a Math.ceil(limit / 2) minimum, instead of one at a time for the remaining chunks after the first limit batch', async () => { const scope = nock( 'https://bucket.s3.us-east-2.amazonaws.com' ).defaultReplyHeaders({ @@ -115,6 +117,13 @@ describe('AwsS3Multipart', () => { 'access-control-allow-origin': '*', 'access-control-expose-headers': 'ETag', }) + // 50MB file will give us 10 chunks, so there will be 10 PUT and 10 OPTIONS + // calls to the presigned URL from 3 prepareUploadParts calls + // + // The first prepareUploadParts call will be for 5 parts, the second + // will be for 3 parts, the third will be for 2 parts. + const fileSize = 50 * MB + scope .options((uri) => uri.includes('test/upload/multitest.dat')) .reply(200, '') @@ -123,12 +132,6 @@ describe('AwsS3Multipart', () => { .reply(200, '', { ETag: 'test' }) scope.persist() - // 50MB file will give us 10 chunks, so there will be 10 PUT and 10 OPTIONS - // calls to the presigned URL from 3 prepareUploadParts calls - // - // The first prepareUploadParts call will be for 5 parts, the second - // will be for 3 parts, the third will be for 2 parts. - const fileSize = 50 * MB core.addFile({ source: 'jest', name: 'multitest.dat', @@ -137,28 +140,81 @@ describe('AwsS3Multipart', () => { type: 'application/octet-stream', }), }) - core.upload().then(() => { - expect( - awsS3Multipart.opts.prepareUploadParts.mock.calls.length - ).toEqual(3) - expect(awsS3Multipart.opts.prepareUploadParts.mock.calls[0][1].partNumbers).toEqual([1, 2, 3, 4, 5]) - expect(awsS3Multipart.opts.prepareUploadParts.mock.calls[1][1].partNumbers).toEqual([6, 7, 8]) - expect(awsS3Multipart.opts.prepareUploadParts.mock.calls[2][1].partNumbers).toEqual([9, 10]) - const completeCall = awsS3Multipart.opts.completeMultipartUpload.mock.calls[0][1] - expect(completeCall.parts).toEqual([ - { ETag: 'test', PartNumber: 1 }, - { ETag: 'test', PartNumber: 2 }, - { ETag: 'test', PartNumber: 3 }, - { ETag: 'test', PartNumber: 4 }, - { ETag: 'test', PartNumber: 5 }, - { ETag: 'test', PartNumber: 6 }, - { ETag: 'test', PartNumber: 7 }, - { ETag: 'test', PartNumber: 8 }, - { ETag: 'test', PartNumber: 9 }, - { ETag: 'test', PartNumber: 10 }, - ]) - done() + + await core.upload() + + expect(awsS3Multipart.opts.prepareUploadParts.mock.calls.length).toEqual(3) + expect(awsS3Multipart.opts.prepareUploadParts.mock.calls[0][1].partNumbers).toEqual([1, 2, 3, 4, 5]) + expect(awsS3Multipart.opts.prepareUploadParts.mock.calls[1][1].partNumbers).toEqual([6, 7, 8]) + expect(awsS3Multipart.opts.prepareUploadParts.mock.calls[2][1].partNumbers).toEqual([9, 10]) + + const completeCall = awsS3Multipart.opts.completeMultipartUpload.mock.calls[0][1] + + expect(completeCall.parts).toEqual([ + { ETag: 'test', PartNumber: 1 }, + { ETag: 'test', PartNumber: 2 }, + { ETag: 'test', PartNumber: 3 }, + { ETag: 'test', PartNumber: 4 }, + { ETag: 'test', PartNumber: 5 }, + { ETag: 'test', PartNumber: 6 }, + { ETag: 'test', PartNumber: 7 }, + { ETag: 'test', PartNumber: 8 }, + { ETag: 'test', PartNumber: 9 }, + { ETag: 'test', PartNumber: 10 }, + ]) + }) + }) + + describe('MultipartUploader', () => { + let core + let awsS3Multipart + + beforeEach(() => { + core = new Core() + core.use(AwsS3Multipart, { + createMultipartUpload: jest.fn(() => { + return { + uploadId: '6aeb1980f3fc7ce0b5454d25b71992', + key: 'test/upload/multitest.dat', + } + }), + completeMultipartUpload: jest.fn(async () => ({ location: 'test' })), + abortMultipartUpload: jest.fn(), + prepareUploadParts: jest + .fn(async () => { + const presignedUrls = {} + const possiblePartNumbers = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] + + possiblePartNumbers.forEach((partNumber) => { + presignedUrls[ + partNumber + ] = `https://bucket.s3.us-east-2.amazonaws.com/test/upload/multitest.dat?partNumber=${partNumber}&uploadId=6aeb1980f3fc7ce0b5454d25b71992&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIATEST%2F20210729%2Fus-east-2%2Fs3%2Faws4_request&X-Amz-Date=20210729T014044Z&X-Amz-Expires=600&X-Amz-SignedHeaders=host&X-Amz-Signature=test` + }) + + return { presignedUrls } + }) + // This runs first and only once + // eslint-disable-next-line prefer-promise-reject-errors + .mockImplementationOnce(() => Promise.reject({ source: { status: 500 } })), + }) + awsS3Multipart = core.getPlugin('AwsS3Multipart') + }) + + it('retries prepareUploadParts when it fails once', async () => { + const fileSize = 5 * MB + 1 * MB + + core.addFile({ + source: 'jest', + name: 'multitest.dat', + type: 'application/octet-stream', + data: new File([Buffer.alloc(fileSize)], { + type: 'application/octet-stream', + }), }) + + await core.upload() + + expect(awsS3Multipart.opts.prepareUploadParts.mock.calls.length).toEqual(2) }) }) }) diff --git a/website/src/docs/aws-s3-multipart.md b/website/src/docs/aws-s3-multipart.md index e6399de0de..c556c3cfdd 100644 --- a/website/src/docs/aws-s3-multipart.md +++ b/website/src/docs/aws-s3-multipart.md @@ -47,7 +47,9 @@ For example, with a 50MB file and a `limit` of 5 we end up with 10 chunks. 5 of ### `retryDelays: [0, 1000, 3000, 5000]` -When uploading a chunk to S3 using a presigned URL fails, automatically try again after the millisecond intervals specified in this array. By default, we first retry instantly; if that fails, we retry after 1 second; if that fails, we retry after 3 seconds, etc. +`retryDelays` are the intervals in milliseconds used to retry a failed chunk as well as [`prepareUploadParts`](#prepareUploadParts-file-partData). + +By default, we first retry instantly; if that fails, we retry after 1 second; if that fails, we retry after 3 seconds, etc. Set to `null` to disable automatic retries, and fail instantly if any chunk fails to upload. @@ -109,20 +111,32 @@ A function that generates a batch of signed URLs for the specified part numbers. - `key` - The object key in the S3 bucket. - `partNumbers` - An array of indecies of this part in the file (`PartNumber` in S3 terminology). Note that part numbers are _not_ zero-based. -Return a Promise for an object with keys: - - - `presignedUrls` - A JavaScript object with the part numbers as keys and the presigned URL for each part as the value. An example of what the return value should look like: +`prepareUploadParts` should return a `Promise` with an `Object` with keys: - ```js - // for partNumbers [1, 2, 3] - return { - 1: 'https://bucket.region.amazonaws.com/path/to/file.jpg?partNumber=1&...', - 2: 'https://bucket.region.amazonaws.com/path/to/file.jpg?partNumber=2&...', - 3: 'https://bucket.region.amazonaws.com/path/to/file.jpg?partNumber=3&...', - } - ``` + - `presignedUrls` - A JavaScript object with the part numbers as keys and the presigned URL for each part as the value. - `headers` - **(Optional)** Custom headers that should be sent to the S3 presigned URL. +An example of what the return value should look like: +```json +{ + "presignedUrls": { + "1": "https://bucket.region.amazonaws.com/path/to/file.jpg?partNumber=1&...", + "2": "https://bucket.region.amazonaws.com/path/to/file.jpg?partNumber=2&...", + "3": "https://bucket.region.amazonaws.com/path/to/file.jpg?partNumber=3&..." + }, + "headers": { "some-header": "value" } +} +``` + +If an error occured, reject the `Promise` with an `Object` with the following keys: + + +```json +{ "source": { "status": 500 } } +``` + +`status` is the HTTP code and is required for determining whether to retry the request. `prepareUploadParts` will be retried if the code is `0`, `409`, `423`, or between `500` and `600`. + ### `abortMultipartUpload(file, { uploadId, key })` A function that calls the S3 Multipart API to abort a Multipart upload, and delete all parts that have been uploaded so far. Receives the `file` object from Uppy's state, and an object with keys: