Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Correctly handle errors for prepareUploadParts #3912

Merged
merged 2 commits into from Aug 3, 2022
Merged

Correctly handle errors for prepareUploadParts #3912

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3054f7b
Correctly handle errors for prepareUploadParts
Murderlon Jul 27, 2022
9d9c210
Check value of rejects in test
Murderlon Jul 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 17 additions & 14 deletions packages/@uppy/aws-s3-multipart/src/MultipartUploader.js
View file
Open in desktop
Expand Up @@ -198,19 +198,22 @@ class MultipartUploader {

if (chunkIndexes.length === 0) return

return this.#prepareUploadParts(chunkIndexes).then((result) => {
const { presignedUrls, headers } = result

chunkIndexes.forEach((index) => {
const partNumber = index + 1
const prePreparedPart = { url: presignedUrls[partNumber], headers: headers?.[partNumber] }
this.#uploadPartRetryable(index, prePreparedPart).then(() => {
this.#uploadParts()
}, (err) => {
this.#onError(err)
})
})
})
this.#prepareUploadPartsRetryable(chunkIndexes).then(
({ presignedUrls, headers }) => {
for (const index of chunkIndexes) {
const partNumber = index + 1
const prePreparedPart = {
url: presignedUrls[partNumber],
headers: headers?.[partNumber],
}
this.#uploadPartRetryable(index, prePreparedPart).then(
() => this.#uploadParts(),
(err) => this.#onError(err),
)
}
},
(err) => this.#onError(err),
)
}

#retryable ({ before, attempt, after }) {
Expand Down Expand Up @@ -247,7 +250,7 @@ class MultipartUploader {
})
}

async #prepareUploadParts (chunkIndexes) {
async #prepareUploadPartsRetryable (chunkIndexes) {
chunkIndexes.forEach((i) => {
this.chunkState[i].busy = true
})
Expand Down
93 changes: 62 additions & 31 deletions packages/@uppy/aws-s3-multipart/src/index.test.js
View file
Open in desktop
Expand Up @@ -185,41 +185,41 @@ describe('AwsS3Multipart', () => {
})

describe('MultipartUploader', () => {
let core
let awsS3Multipart
const createMultipartUpload = jest.fn(() => {
return {
uploadId: '6aeb1980f3fc7ce0b5454d25b71992',
key: 'test/upload/multitest.dat',
}
})

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 } })),
const 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 }
})
awsS3Multipart = core.getPlugin('AwsS3Multipart')
})

afterEach(() => jest.clearAllMocks())

it('retries prepareUploadParts when it fails once', async () => {
const core = new Core()
.use(AwsS3Multipart, {
createMultipartUpload,
completeMultipartUpload: jest.fn(async () => ({ location: 'test' })),
abortMultipartUpload: jest.fn(),
prepareUploadParts:
prepareUploadParts
// eslint-disable-next-line prefer-promise-reject-errors
.mockImplementationOnce(() => Promise.reject({ source: { status: 500 } })),
})
const awsS3Multipart = core.getPlugin('AwsS3Multipart')
const fileSize = 5 * MB + 1 * MB

core.addFile({
Expand All @@ -235,6 +235,37 @@ describe('AwsS3Multipart', () => {

expect(awsS3Multipart.opts.prepareUploadParts.mock.calls.length).toEqual(2)
})

it('calls `upload-error` when prepareUploadParts fails after all retries', async () => {
const core = new Core()
.use(AwsS3Multipart, {
retryDelays: [100],
createMultipartUpload,
completeMultipartUpload: jest.fn(async () => ({ location: 'test' })),
abortMultipartUpload: jest.fn(),
prepareUploadParts: prepareUploadParts
// eslint-disable-next-line prefer-promise-reject-errors
.mockImplementation(() => Promise.reject({ source: { status: 500 } })),
})
const awsS3Multipart = core.getPlugin('AwsS3Multipart')
const fileSize = 5 * MB + 1 * MB
const mock = jest.fn()
core.on('upload-error', mock)

core.addFile({
source: 'jest',
name: 'multitest.dat',
type: 'application/octet-stream',
data: new File([new Uint8Array(fileSize)], {
type: 'application/octet-stream',
}),
})

await expect(core.upload()).rejects.toEqual({ source: { status: 500 } })

expect(awsS3Multipart.opts.prepareUploadParts.mock.calls.length).toEqual(2)
expect(mock.mock.calls.length).toEqual(1)
})
})

describe('dynamic companionHeader', () => {
Expand Down