Merge pull request #404 from transloadit/feature/successful-failed

Return { successful, failed } object from .upload().

examples/bundled-example/main.js

@@ -72,9 +72,14 @@ const uppy = Uppy({
   // .use(GoldenRetriever, {serviceWorker: true})
   .run()
 
-uppy.on('core:success', (fileList) => {
-  console.log('UPLOAD SUCCESSFUL!!!')
-  console.log(fileList)
+uppy.on('core:complete', ({ successful, failed }) => {
+  if (failed.length === 0) {
+    console.log('UPLOAD SUCCESSFUL!!!')
+  } else {
+    console.warn('UPLOAD FAILED!!!')
+  }
+  console.log('successful files:', successful)
+  console.log('failed files:', failed)
 })
 
 if ('serviceWorker' in navigator) {

src/core/Core.js

@@ -1055,9 +1055,17 @@ class Uppy {
     })
 
     return lastStep.then(() => {
+      const files = fileIDs.map((fileID) => this.getFile(fileID))
+      const successful = files.filter((file) => !file.error)
+      const failed = files.filter((file) => file.error)
+      this.emit('core:complete', { successful, failed })
+
+      // Compatibility with pre-0.21
       this.emit('core:success', fileIDs)
 
       this.removeUpload(uploadID)
+
+      return { successful, failed }
     })
   }
 

src/core/Core.test.js

@@ -591,7 +591,50 @@ describe('src/Core', () => {
     })
   })
 
-  describe('uploading a file', () => {})
+  describe('uploading a file', () => {
+    it('should return a { successful, failed } pair containing file objects', () => {
+      const core = new Core().run()
+      core.addUploader((fileIDs) => Promise.resolve())
+      return Promise.all([
+        core.addFile({ source: 'jest', name: 'foo.jpg', type: 'image/jpeg', data: new Uint8Array() }),
+        core.addFile({ source: 'jest', name: 'bar.jpg', type: 'image/jpeg', data: new Uint8Array() })
+      ]).then(() => {
+        return expect(core.upload()).resolves.toMatchObject({
+          successful: [
+            { name: 'foo.jpg' },
+            { name: 'bar.jpg' }
+          ],
+          failed: []
+        })
+      })
+    })
+
+    it('should return files with errors in the { failed } key', () => {
+      const core = new Core().run()
+      core.addUploader((fileIDs) => {
+        fileIDs.forEach((fileID) => {
+          if (/bar/.test(core.getFile(fileID).name)) {
+            core.emit('core:upload-error', fileID, new Error('This is bar and I do not like bar'))
+          }
+        })
+        return Promise.resolve()
+      })
+
+      return Promise.all([
+        core.addFile({ source: 'jest', name: 'foo.jpg', type: 'image/jpeg', data: new Uint8Array() }),
+        core.addFile({ source: 'jest', name: 'bar.jpg', type: 'image/jpeg', data: new Uint8Array() })
+      ]).then(() => {
+        return expect(core.upload()).resolves.toMatchObject({
+          successful: [
+            { name: 'foo.jpg' }
+          ],
+          failed: [
+            { name: 'bar.jpg', error: 'This is bar and I do not like bar' }
+          ]
+        })
+      })
+    })
+  })
 
   describe('removing a file', () => {
     it('should remove the file', () => {

src/core/Utils.js

@@ -519,13 +519,6 @@ function settle (promises) {
   )
 
   return wait.then(() => {
-    if (rejections.length === promises.length) {
-      // Very ad-hoc multiple-error reporting, should wrap this in a
-      // CombinedError or whatever kind of error class instead.
-      const error = rejections[0]
-      error.errors = rejections
-      return Promise.reject(error)
-    }
     return {
       successful: resolutions,
       failed: rejections

src/core/Utils.test.js

@@ -345,14 +345,15 @@ describe('core/utils', () => {
   })
 
   describe('settle', () => {
-    it('should reject if all input promises reject', () => {
+    it('should resolve even if all input promises reject', () => {
       return expect(
         utils.settle([
           Promise.reject(new Error('oops')),
           Promise.reject(new Error('this went wrong'))
         ])
-      ).rejects.toMatchObject({
-        message: 'oops'
+      ).resolves.toMatchObject({
+        successful: [],
+        failed: [{ message: 'oops' }, { message: 'this went wrong' }]
       })
     })
 

src/plugins/AwsS3/index.js

@@ -60,12 +60,18 @@ module.exports = class AwsS3 extends Plugin {
             value: 1
           })
           return params
+        }).catch((error) => {
+          this.core.emit('core:upload-error', file.id, error)
         })
       })
     ).then((responses) => {
       const updatedFiles = {}
       fileIDs.forEach((id, index) => {
         const file = this.core.getFile(id)
+        if (file.error) {
+          return
+        }
+
         const {
           method = 'post',
           url,

src/plugins/GoldenRetriever/index.js

@@ -213,9 +213,10 @@ module.exports = class GoldenRetriever extends Plugin {
       this.IndexedDBStore.delete(fileID)
     })
 
-    this.core.on('core:success', (fileIDs) => {
+    this.core.on('core:complete', ({ successful }) => {
+      const fileIDs = successful.map((file) => file.id)
       this.deleteBlobs(fileIDs).then(() => {
-        this.core.log(`[GoldenRetriever] removed ${fileIDs.length} files that finished uploading`)
+        this.core.log(`RestoreFiles: removed ${successful.length} files that finished uploading`)
       })
     })
 

src/plugins/Transloadit/index.js

@@ -317,6 +317,9 @@ module.exports = class Transloadit extends Plugin {
   }
 
   prepareUpload (fileIDs, uploadID) {
+    // Only use files without errors
+    fileIDs = fileIDs.filter((file) => !file.error)
+
     fileIDs.forEach((fileID) => {
       this.core.emit('core:preprocess-progress', fileID, {
         mode: 'indeterminate',
@@ -367,6 +370,9 @@ module.exports = class Transloadit extends Plugin {
   }
 
   afterUpload (fileIDs, uploadID) {
+    // Only use files without errors
+    fileIDs = fileIDs.filter((file) => !file.error)
+
     const state = this.getPluginState()
     const assemblyIDs = state.uploadsAssemblies[uploadID]
 
@@ -411,12 +417,7 @@ module.exports = class Transloadit extends Plugin {
           this.core.emit('core:postprocess-complete', file.id)
         })
 
-        finishedAssemblies += 1
-        if (finishedAssemblies === assemblyIDs.length) {
-          // We're done, these listeners can be removed
-          removeListeners()
-          resolve()
-        }
+        checkAllComplete()
       }
 
       const onAssemblyError = (assembly, error) => {
@@ -434,14 +435,7 @@ module.exports = class Transloadit extends Plugin {
           this.core.emit('core:postprocess-complete', file.id)
         })
 
-        // Should we remove the listeners here or should we keep handling finished
-        // assemblies?
-        // Doing this for now so that it's not possible to receive more postprocessing
-        // events once the upload has failed.
-        removeListeners()
-
-        // Reject the `afterUpload()` promise.
-        reject(error)
+        checkAllComplete()
       }
 
       const onImportError = (assembly, fileID, error) => {
@@ -457,6 +451,15 @@ module.exports = class Transloadit extends Plugin {
         onAssemblyError(assembly, error)
       }
 
+      const checkAllComplete = () => {
+        finishedAssemblies += 1
+        if (finishedAssemblies === assemblyIDs.length) {
+          // We're done, these listeners can be removed
+          removeListeners()
+          resolve()
+        }
+      }
+
       const removeListeners = () => {
         this.core.off('transloadit:complete', onAssemblyFinished)
         this.core.off('transloadit:assembly-error', onAssemblyError)

src/plugins/Tus.js

@@ -364,7 +364,9 @@ module.exports = class Tus extends Plugin {
       const current = parseInt(index, 10) + 1
       const total = files.length
 
-      if (!file.isRemote) {
+      if (file.error) {
+        return Promise.reject(new Error(file.error))
+      } else if (!file.isRemote) {
         return this.upload(file, current, total)
       } else {
         return this.uploadRemote(file, current, total)

src/plugins/XHRUpload.js

@@ -252,7 +252,9 @@ module.exports = class XHRUpload extends Plugin {
       const current = parseInt(i, 10) + 1
       const total = files.length
 
-      if (file.isRemote) {
+      if (file.error) {
+        return Promise.reject(new Error(file.error))
+      } else if (file.isRemote) {
         return this.uploadRemote(file, current, total)
       } else {
         return this.upload(file, current, total)

website/src/docs/index.md

@@ -22,8 +22,8 @@ const uppy = Uppy({ autoProceed: false })
   .use(Tus, {endpoint: '//master.tus.io/files/'})
   .run()
  
-uppy.on('core:success', (files) => {
-  console.log(`Upload complete! We’ve uploaded these files: ${files}`)
+uppy.on('core:complete', (result) => {
+  console.log(`Upload complete! We’ve uploaded these files: ${result.successful}`)
 })
 ```
 

website/src/docs/react.md

@@ -27,8 +27,8 @@ const uppy = Uppy({
 
 uppy.use(Tus, { endpoint: '/upload' })
 
-uppy.on('core:success', (fileIDs) => {
-  const url = uppy.getFile(fileIDs[0]).uploadURL
+uppy.on('core:complete', (result) => {
+  const url = result.successful[0].uploadURL
   store.dispatch({
     type: SET_USER_AVATAR_URL,
     payload: { url: url }

website/src/docs/uppy.md

@@ -267,6 +267,25 @@ this.info('Oh my, something good happened!', 'success', 5000)
 
 Start uploading selected files.
 
+Returns a Promise `result` that resolves with an object containing two arrays of uploaded files.
+
+ - `result.successful` - Files that were uploaded successfully.
+ - `result.failed` - Files that did not upload successfully.
+   These file objects will have a `.error` property describing what went wrong.
+
+```js
+uppy.upload().then((result) => {
+  console.info('Successful uploads:', result.successful)
+
+  if (result.failed.length > 0) {
+    console.error('Errors:')
+    result.failed.forEach((file) => {
+      console.error(file.error)
+    })
+  }
+})
+```
+
 ### `uppy.on('event', action)`
 
 Subscribe to an uppy-event. See full list of events below.
@@ -308,12 +327,13 @@ uppy.on('core:upload-success', (fileId, url) => {
 })
 ```
 
-### `core:success`
+### `core:complete`
 
 Fired when all uploads are complete.
+The `result` parameter is an object with arrays of `successful` and `failed` files, just like in [`uppy.upload()`](#uppy-upload)'s return value.
 
 ``` javascript
-uppy.on('core:success', (fileCount) => {
-  console.log(fileCount)
+uppy.on('core:complete', (result) => {
+  console.log(result)
 })
 ```

website/src/examples/dashboard/app.es6

@@ -65,9 +65,9 @@ function uppyInit () {
   })
   uppy.run()
 
-  uppy.on('core:success', (fileList) => {
+  uppy.on('core:complete', (result) => {
     console.log('Yo, uploaded: ')
-    console.log(fileList)
+    console.log(result.successful)
   })
 }
 

website/src/examples/dashboard/index.ejs

@@ -57,7 +57,7 @@ const uppy = Uppy({
 })
 .run()
 
-uppy.on('core:success', (fileList) => {
-  console.log('Successfully uploaded:', fileList)
+uppy.on('core:complete', (result) => {
+  console.log('Successfully uploaded:', result.successful)
 })
 {% endcodeblock %}