mirror of
https://github.com/transloadit/uppy.git
synced 2026-07-18 00:55:35 +00:00
wip fix edgecase pause just after createMultipart request is fired
This commit is contained in:
parent
de7d9c2302
commit
6223bc98ee
2 changed files with 192 additions and 4 deletions
|
|
@ -80,6 +80,10 @@ export default class S3Uploader<M extends Meta, B extends Body> {
|
|||
#uploadId?: string
|
||||
#uploadHasStarted: boolean = false
|
||||
#abortController: AbortController | undefined
|
||||
// Both pause() and abort() abort the same signal; this flag lets the
|
||||
// signal-less create checkpoint tell a resumable pause apart from a
|
||||
// terminal cancel.
|
||||
#paused: boolean = false
|
||||
|
||||
constructor(options: S3UploaderOptions<M, B>) {
|
||||
if (options.file.data == null) {
|
||||
|
|
@ -181,6 +185,8 @@ export default class S3Uploader<M extends Meta, B extends Body> {
|
|||
}
|
||||
|
||||
async start(): Promise<void> {
|
||||
// A (re)start clears any prior pause.
|
||||
this.#paused = false
|
||||
// Abort any pending operations (if not already aborted)
|
||||
this.#abortController?.abort()
|
||||
// Always create a fresh AbortController (also for resume)
|
||||
|
|
@ -198,6 +204,7 @@ export default class S3Uploader<M extends Meta, B extends Body> {
|
|||
}
|
||||
|
||||
pause(): void {
|
||||
this.#paused = true
|
||||
this.#abortController?.abort()
|
||||
}
|
||||
|
||||
|
|
@ -206,6 +213,8 @@ export default class S3Uploader<M extends Meta, B extends Body> {
|
|||
* @param opts - `abortInS3`: Whether to also abort the multipart upload in S3. Default: true. Set to false to keep the multipart upload in S3 active, allowing for manual cleanup later and preventing accidental data loss if the user later tries to resume the upload.
|
||||
*/
|
||||
abort(opts?: { abortInS3?: boolean }): void {
|
||||
// aboring request
|
||||
this.#paused = false
|
||||
this.#abortController?.abort()
|
||||
// Clean up event listeners
|
||||
this.#eventManager.remove()
|
||||
|
|
@ -296,13 +305,22 @@ export default class S3Uploader<M extends Meta, B extends Body> {
|
|||
this.#key = key
|
||||
this.#uploadId = uploadId
|
||||
|
||||
// The file may have been removed/cancelled while createMultipartUpload was
|
||||
// in flight. We deliberately don't pass the abort signal into the create
|
||||
// The file may have been paused/removed/cancelled while createMultipartUpload
|
||||
// was in flight. We deliberately don't pass the abort signal into the create
|
||||
// request: if it were cancelled mid-flight, S3 might still create the upload
|
||||
// while we never receive the uploadId — an orphan we couldn't clean up.
|
||||
// Instead we let create finish, then abort the upload in S3 so it isn't left
|
||||
// behind, and skip persisting resume state for a cancelled upload.
|
||||
// So we let create finish, then act on *why* the signal was aborted:
|
||||
if (this.#abortController?.signal.aborted) {
|
||||
if (this.#paused) {
|
||||
// Paused, not cancelled: keep the S3 upload alive and persist resume
|
||||
// state so start()/Golden Retriever can continue from the parts.
|
||||
this.#options.uppy.setFileState(this.#options.file.id, {
|
||||
s3Multipart: { uploadId, key },
|
||||
})
|
||||
return
|
||||
}
|
||||
// Cancelled: abort the now-created upload in S3 so it isn't left behind,
|
||||
// and skip persisting resume state.
|
||||
this.abort()
|
||||
return
|
||||
}
|
||||
|
|
|
|||
170
packages/@uppy/aws-s3/tests/pause-during-create.test.ts
Normal file
170
packages/@uppy/aws-s3/tests/pause-during-create.test.ts
Normal file
|
|
@ -0,0 +1,170 @@
|
|||
import { HttpResponse, http } from 'msw'
|
||||
import { setupServer } from 'msw/node'
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest'
|
||||
import 'whatwg-fetch'
|
||||
import Core from '@uppy/core'
|
||||
import AwsS3 from '../src/index.js'
|
||||
|
||||
const MB = 1024 * 1024
|
||||
const s3Url = 'https://test-bucket.s3.us-east-1.amazonaws.com/:key'
|
||||
const server = setupServer()
|
||||
|
||||
const createMultipartXml = (uploadId: string, key: string) =>
|
||||
`<?xml version="1.0" encoding="UTF-8"?><InitiateMultipartUploadResult><UploadId>${uploadId}</UploadId><Key>${key}</Key></InitiateMultipartUploadResult>`
|
||||
|
||||
describe('pause during createMultipartUpload', () => {
|
||||
beforeAll(() => server.listen({ onUnhandledRequest: 'error' }))
|
||||
afterEach(() => server.resetHandlers())
|
||||
afterAll(() => server.close())
|
||||
|
||||
it('must NOT abort the S3 upload when merely PAUSED (only cancel should)', async () => {
|
||||
const operations: string[] = []
|
||||
let markCreateStarted!: () => void
|
||||
const createStarted = new Promise<void>((r) => {
|
||||
markCreateStarted = r
|
||||
})
|
||||
let releaseCreate!: () => void
|
||||
const createGate = new Promise<void>((r) => {
|
||||
releaseCreate = r
|
||||
})
|
||||
|
||||
const signRequest = vi.fn(async (req: any) => {
|
||||
const params = new URLSearchParams()
|
||||
if (req.uploadId) params.set('uploadId', req.uploadId)
|
||||
if (req.partNumber) params.set('partNumber', String(req.partNumber))
|
||||
params.set('method', req.method)
|
||||
return {
|
||||
url: `https://test-bucket.s3.us-east-1.amazonaws.com/${req.key}?${params}`,
|
||||
}
|
||||
})
|
||||
|
||||
server.use(
|
||||
http.post(s3Url, async ({ request }) => {
|
||||
const hasUploadId = new URL(request.url).searchParams.has('uploadId')
|
||||
if (!hasUploadId) {
|
||||
operations.push('createMultipart')
|
||||
markCreateStarted()
|
||||
await createGate // hang create until the test releases it
|
||||
return new HttpResponse(createMultipartXml('upload-1', 'test-key'), {
|
||||
status: 200,
|
||||
headers: { 'Content-Type': 'application/xml' },
|
||||
})
|
||||
}
|
||||
operations.push('completeMultipart')
|
||||
return new HttpResponse('', { status: 200 })
|
||||
}),
|
||||
http.put(s3Url, () => {
|
||||
operations.push('uploadPart')
|
||||
return new HttpResponse('', { status: 200, headers: { ETag: '"e"' } })
|
||||
}),
|
||||
http.get(s3Url, () => {
|
||||
operations.push('listParts')
|
||||
return new HttpResponse(
|
||||
'<?xml version="1.0"?><ListPartsResult></ListPartsResult>',
|
||||
{ status: 200, headers: { 'Content-Type': 'application/xml' } },
|
||||
)
|
||||
}),
|
||||
http.delete(s3Url, () => {
|
||||
operations.push('abortMultipart')
|
||||
return new HttpResponse('', { status: 204 })
|
||||
}),
|
||||
)
|
||||
|
||||
const core = new Core().use(AwsS3, {
|
||||
s3Endpoint: 'https://test-bucket.s3.us-east-1.amazonaws.com',
|
||||
region: 'us-east-1',
|
||||
signRequest,
|
||||
shouldUseMultipart: true,
|
||||
})
|
||||
core.addFile({
|
||||
source: 'test',
|
||||
name: 'big.bin',
|
||||
type: 'application/octet-stream',
|
||||
data: new File([new Uint8Array(6 * MB)], 'big.bin'),
|
||||
})
|
||||
const fileId = Object.keys(core.getState().files)[0]
|
||||
|
||||
const uploadPromise = core.upload()
|
||||
uploadPromise?.catch(() => {}) // may hang/reject on the bug — don't fail on it
|
||||
|
||||
await createStarted // createMultipartUpload is now in flight
|
||||
core.pauseResume(fileId) // PAUSE (not remove) during the create window
|
||||
releaseCreate() // let create finish so S3 returns the uploadId
|
||||
|
||||
await new Promise((r) => setTimeout(r, 50)) // let the post-create check run
|
||||
|
||||
// A pause must keep the upload alive & resumable — it must NOT be deleted in S3.
|
||||
expect(operations).not.toContain('abortMultipart')
|
||||
// ...and resume state must be persisted so it can actually resume.
|
||||
expect(core.getFile(fileId)?.s3Multipart).toBeDefined()
|
||||
|
||||
core.cancelAll()
|
||||
})
|
||||
|
||||
it('cancel overrides a prior pause: pause then remove during create still aborts in S3', async () => {
|
||||
const operations: string[] = []
|
||||
let markCreateStarted!: () => void
|
||||
const createStarted = new Promise<void>((r) => {
|
||||
markCreateStarted = r
|
||||
})
|
||||
let releaseCreate!: () => void
|
||||
const createGate = new Promise<void>((r) => {
|
||||
releaseCreate = r
|
||||
})
|
||||
|
||||
const signRequest = vi.fn(async (req: any) => {
|
||||
const params = new URLSearchParams()
|
||||
if (req.uploadId) params.set('uploadId', req.uploadId)
|
||||
params.set('method', req.method)
|
||||
return {
|
||||
url: `https://test-bucket.s3.us-east-1.amazonaws.com/${req.key}?${params}`,
|
||||
}
|
||||
})
|
||||
|
||||
server.use(
|
||||
http.post(s3Url, async ({ request }) => {
|
||||
const hasUploadId = new URL(request.url).searchParams.has('uploadId')
|
||||
if (!hasUploadId) {
|
||||
operations.push('createMultipart')
|
||||
markCreateStarted()
|
||||
await createGate
|
||||
return new HttpResponse(createMultipartXml('upload-2', 'test-key'), {
|
||||
status: 200,
|
||||
headers: { 'Content-Type': 'application/xml' },
|
||||
})
|
||||
}
|
||||
return new HttpResponse('', { status: 200 })
|
||||
}),
|
||||
http.delete(s3Url, () => {
|
||||
operations.push('abortMultipart')
|
||||
return new HttpResponse('', { status: 204 })
|
||||
}),
|
||||
)
|
||||
|
||||
const core = new Core().use(AwsS3, {
|
||||
s3Endpoint: 'https://test-bucket.s3.us-east-1.amazonaws.com',
|
||||
region: 'us-east-1',
|
||||
signRequest,
|
||||
shouldUseMultipart: true,
|
||||
})
|
||||
core.addFile({
|
||||
source: 'test',
|
||||
name: 'big.bin',
|
||||
type: 'application/octet-stream',
|
||||
data: new File([new Uint8Array(6 * MB)], 'big.bin'),
|
||||
})
|
||||
const fileId = Object.keys(core.getState().files)[0]
|
||||
|
||||
const uploadPromise = core.upload()
|
||||
uploadPromise?.catch(() => {})
|
||||
|
||||
await createStarted
|
||||
core.pauseResume(fileId) // pause first...
|
||||
core.removeFile(fileId) // ...then cancel, both inside the create window
|
||||
releaseCreate()
|
||||
await uploadPromise
|
||||
|
||||
// The cancel must win: the now-orphaned S3 upload gets aborted.
|
||||
await vi.waitFor(() => expect(operations).toContain('abortMultipart'))
|
||||
})
|
||||
})
|
||||
Loading…
Add table
Add a link
Reference in a new issue