@uppy/aws-s3-multipart: fix pause/resume (#4523)

When pausing the upload, we don't want to remove the cached `uploadId`, otherwise Uppy needs to re-upload the file from the begininng, even if it has already uploaded some chunks.
transloadit · Jun 27, 2023 · b6a6f83 · b6a6f83
1 parent 692481a
commit b6a6f83
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 6 deletions.
diff --git a/e2e/cypress/integration/dashboard-aws-multipart.spec.ts b/e2e/cypress/integration/dashboard-aws-multipart.spec.ts
@@ -34,8 +34,9 @@ describe('Dashboard with @uppy/aws-s3-multipart', () => {
 
     cy.intercept('POST', '/s3/multipart', { statusCode: 200, times: 1, body: JSON.stringify({ key:'mocked-key-attempt1', uploadId:'mocked-uploadId-attempt1' }) }).as('createMultipartUpload-attempt1')
     cy.intercept('GET', '/s3/multipart/mocked-uploadId-attempt1/1?key=mocked-key-attempt1', { forceNetworkError: true }).as('signPart-fails')
+    cy.intercept('DELETE', '/s3/multipart/mocked-uploadId-attempt1?key=mocked-key-attempt1', { statusCode: 204 }).as('abortAttempt-1')
     cy.get('.uppy-StatusBar-actions > .uppy-c-btn').click()
-    cy.wait(['@createMultipartUpload-attempt1', '@signPart-fails'])
+    cy.wait(['@createMultipartUpload-attempt1', '@signPart-fails', '@abortAttempt-1'])
     cy.get('.uppy-StatusBar-statusPrimary').should('contain', 'Upload failed')
 
     cy.intercept('POST', '/s3/multipart', { statusCode: 200, times: 1, body: JSON.stringify({ key:'mocked-key-attempt2', uploadId:'mocked-uploadId-attempt2' }) }).as('createMultipartUpload-attempt2')
@@ -46,9 +47,10 @@ describe('Dashboard with @uppy/aws-s3-multipart', () => {
       },
       body: JSON.stringify({ url:'/put-fail', expires:8 }),
     }).as('signPart-toFail')
+    cy.intercept('DELETE', '/s3/multipart/mocked-uploadId-attempt2?key=mocked-key-attempt2', { statusCode: 204 }).as('abortAttempt-2')
     cy.intercept('PUT', '/put-fail', { forceNetworkError: true }).as('put-fails')
     cy.get('.uppy-StatusBar-actions > .uppy-c-btn').click()
-    cy.wait(['@createMultipartUpload-attempt2', '@signPart-toFail', ...Array(5).fill('@put-fails')], { timeout: 10_000 })
+    cy.wait(['@createMultipartUpload-attempt2', '@signPart-toFail', ...Array(5).fill('@put-fails'), '@abortAttempt-2'], { timeout: 10_000 })
     cy.get('.uppy-StatusBar-statusPrimary').should('contain', 'Upload failed')
 
     cy.intercept('GET', '/s3/multipart/mocked-uploadId-attempt2/1?key=mocked-key-attempt2', {
@@ -66,7 +68,7 @@ describe('Dashboard with @uppy/aws-s3-multipart', () => {
     }).as('put-attempt2')
     cy.intercept('POST', '/s3/multipart/mocked-uploadId-attempt2/complete?key=mocked-key-attempt2', { forceNetworkError: true }).as('completeMultipartUpload-fails')
     cy.get('.uppy-StatusBar-actions > .uppy-c-btn').click()
-    cy.wait(['@createMultipartUpload-attempt2', '@signPart-attempt2', '@put-attempt2', '@completeMultipartUpload-fails'])
+    cy.wait(['@createMultipartUpload-attempt2', '@signPart-attempt2', '@put-attempt2', '@completeMultipartUpload-fails', '@abortAttempt-2'])
     cy.get('.uppy-StatusBar-statusPrimary').should('contain', 'Upload failed')
 
     cy.intercept('POST', '/s3/multipart', { statusCode: 200, times: 1, body: JSON.stringify({ key:'mocked-key-attempt3', uploadId:'mocked-uploadId-attempt3' }) }).as('createMultipartUpload-attempt3')

diff --git a/packages/@uppy/aws-s3-multipart/src/MultipartUploader.js b/packages/@uppy/aws-s3-multipart/src/MultipartUploader.js
@@ -24,7 +24,7 @@ function ensureInt (value) {
   throw new TypeError('Expected a number')
 }
 
-const pausingUploadReason = Symbol('pausing upload, not an actual error')
+export const pausingUploadReason = Symbol('pausing upload, not an actual error')
 
 class MultipartUploader {
   #abortController = new AbortController()

diff --git a/packages/@uppy/aws-s3-multipart/src/index.js b/packages/@uppy/aws-s3-multipart/src/index.js
@@ -7,7 +7,7 @@ import { RateLimitedQueue } from '@uppy/utils/lib/RateLimitedQueue'
 import { filterNonFailedFiles, filterFilesToEmitUploadStarted } from '@uppy/utils/lib/fileFilters'
 import { createAbortError } from '@uppy/utils/lib/AbortController'
 import packageJson from '../package.json'
-import MultipartUploader from './MultipartUploader.js'
+import MultipartUploader, { pausingUploadReason } from './MultipartUploader.js'
 
 function assertServerError (res) {
   if (res && res.error) {
@@ -201,6 +201,9 @@ class HTTPCommunicationQueue {
       // need to send the abortMultipartUpload request.
       return
     }
+    // Remove the cache entry right away for follow-up requests do not try to
+    // use the soon-to-be aborted chached values.
+    this.#cache.delete(file.data)
     let awaitedResult
     try {
       awaitedResult = await result
@@ -248,7 +251,9 @@ class HTTPCommunicationQueue {
       throwIfAborted(signal)
       return await this.#sendCompletionRequest(file, { key, uploadId, parts, signal }).abortOn(signal)
     } catch (err) {
-      this.#cache.delete(file.data)
+      if (err?.cause !== pausingUploadReason && err?.name !== 'AbortError') {
+        this.abortFileUpload(file).catch(() => {})
+      }
       throw err
     }
   }