aws-s3-multipart: fix queueing behaviours (#1855)

* aws-s3-multipart: prevent deadlock in rate limit queue

* utils: fix RateLimitedQueue thrashing when aborting all items

* aws-s3-multipart: only send abort request if upload was already created, fixes #1146

packages/@uppy/aws-s3-multipart/src/MultipartUploader.js

@@ -18,19 +18,32 @@ function remove (arr, el) {
 
 class MultipartUploader {
   constructor (file, options) {
-    this.options = Object.assign({}, defaultOptions, options)
+    this.options = {
+      ...defaultOptions,
+      ...options
+    }
     this.file = file
 
     this.key = this.options.key || null
     this.uploadId = this.options.uploadId || null
     this.parts = this.options.parts || []
 
+    // Do `this.createdPromise.then(OP)` to execute an operation `OP` _only_ if the
+    // upload was created already. That also ensures that the sequencing is right
+    // (so the `OP` definitely happens if the upload is created).
+    //
+    // This mostly exists to make `_abortUpload` work well: only sending the abort request if
+    // the upload was already created, and if the createMultipartUpload request is still in flight,
+    // aborting it immediately after it finishes.
+    this.createdPromise = Promise.reject() // eslint-disable-line prefer-promise-reject-errors
     this.isPaused = false
     this.chunks = null
     this.chunkState = null
     this.uploading = []
 
     this._initChunks()
+
+    this.createdPromise.catch(() => {}) // silence uncaught rejection warning
   }
 
   _initChunks () {
@@ -51,22 +64,21 @@ class MultipartUploader {
   }
 
   _createUpload () {
-    return Promise.resolve().then(() =>
+    this.createdPromise = Promise.resolve().then(() =>
       this.options.createMultipartUpload()
-    ).then((result) => {
+    )
+    return this.createdPromise.then((result) => {
       const valid = typeof result === 'object' && result &&
         typeof result.uploadId === 'string' &&
         typeof result.key === 'string'
       if (!valid) {
         throw new TypeError('AwsS3/Multipart: Got incorrect result from `createMultipartUpload()`, expected an object `{ uploadId, key }`.')
       }
-      return result
-    }).then((result) => {
+
       this.key = result.key
       this.uploadId = result.uploadId
 
       this.options.onStart(result)
-    }).then(() => {
       this._uploadParts()
     }).catch((err) => {
       this._onError(err)
@@ -250,9 +262,13 @@ class MultipartUploader {
     this.uploading.slice().forEach(xhr => {
       xhr.abort()
     })
-    this.options.abortMultipartUpload({
-      key: this.key,
-      uploadId: this.uploadId
+    this.createdPromise.then(() => {
+      this.options.abortMultipartUpload({
+        key: this.key,
+        uploadId: this.uploadId
+      })
+    }, () => {
+      // if the creation failed we do not need to abort
     })
     this.uploading = []
   }

packages/@uppy/aws-s3-multipart/src/index.js

@@ -194,15 +194,11 @@ module.exports = class AwsS3Multipart extends Plugin {
 
       const upload = new Uploader(file.data, {
         // .bind to pass the file object to each handler.
-        createMultipartUpload: this.requests.wrapPromiseFunction(
-          this.opts.createMultipartUpload.bind(this, file)),
-        listParts: this.requests.wrapPromiseFunction(
-          this.opts.listParts.bind(this, file)),
+        createMultipartUpload: this.opts.createMultipartUpload.bind(this, file),
+        listParts: this.opts.listParts.bind(this, file),
         prepareUploadPart: this.opts.prepareUploadPart.bind(this, file),
-        completeMultipartUpload: this.requests.wrapPromiseFunction(
-          this.opts.completeMultipartUpload.bind(this, file)),
-        abortMultipartUpload: this.requests.wrapPromiseFunction(
-          this.opts.abortMultipartUpload.bind(this, file)),
+        completeMultipartUpload: this.opts.completeMultipartUpload.bind(this, file),
+        abortMultipartUpload: this.opts.abortMultipartUpload.bind(this, file),
 
         onStart,
         onProgress,

packages/@uppy/utils/src/RateLimitedQueue.js

@@ -29,18 +29,27 @@ module.exports = class RateLimitedQueue {
         done = true
         this.activeRequests -= 1
         cancelActive()
-        this._next()
+        this._queueNext()
       },
 
       done: () => {
         if (done) return
         done = true
         this.activeRequests -= 1
-        this._next()
+        this._queueNext()
       }
     }
   }
 
+  _queueNext () {
+    // Do it soon but not immediately, this allows clearing out the entire queue synchronously
+    // one by one without continuously _advancing_ it (and starting new tasks before immediately
+    // aborting them)
+    Promise.resolve().then(() => {
+      this._next()
+    })
+  }
+
   _next () {
     if (this.activeRequests >= this.limit) {
       return