Merge pull request #604 from transloadit/chore/add-file-rejections

`addFile()` rejections revamp

src/core/Core.js

@@ -288,10 +288,8 @@ class Uppy {
   _checkMinNumberOfFiles () {
     const {minNumberOfFiles} = this.opts.restrictions
     if (Object.keys(this.getState().files).length < minNumberOfFiles) {
-      this.info(`${this.i18n('youHaveToAtLeastSelectX', { smart_count: minNumberOfFiles })}`, 'error', 5000)
-      return false
+      throw new Error(`${this.i18n('youHaveToAtLeastSelectX', { smart_count: minNumberOfFiles })}`)
     }
-    return true
   }
 
   /**
@@ -299,7 +297,6 @@ class Uppy {
   * maxNumberOfFiles and allowedFileTypes.
   *
   * @param {object} file object to check
-  * @return {boolean}
   * @private
   */
   _checkRestrictions (file) {
@@ -307,8 +304,7 @@ class Uppy {
 
     if (maxNumberOfFiles) {
       if (Object.keys(this.getState().files).length + 1 > maxNumberOfFiles) {
-        this.info(`${this.i18n('youCanOnlyUploadX', { smart_count: maxNumberOfFiles })}`, 'error', 5000)
-        return false
+        throw new Error(`${this.i18n('youCanOnlyUploadX', { smart_count: maxNumberOfFiles })}`)
       }
     }
 
@@ -320,19 +316,15 @@ class Uppy {
 
       if (!isCorrectFileType) {
         const allowedFileTypesString = allowedFileTypes.join(', ')
-        this.info(`${this.i18n('youCanOnlyUploadFileTypes')} ${allowedFileTypesString}`, 'error', 5000)
-        return false
+        throw new Error(`${this.i18n('youCanOnlyUploadFileTypes')} ${allowedFileTypesString}`)
       }
     }
 
     if (maxFileSize) {
       if (file.data.size > maxFileSize) {
-        this.info(`${this.i18n('exceedsSize')} ${prettyBytes(maxFileSize)}`, 'error', 5000)
-        return false
+        throw new Error(`${this.i18n('exceedsSize')} ${prettyBytes(maxFileSize)}`)
       }
     }
-
-    return true
   }
 
   /**
@@ -343,17 +335,12 @@ class Uppy {
   * @param {object} file object to add
   */
   addFile (file) {
-    // Wrap this in a Promise `.then()` handler so errors will reject the Promise
-    // instead of throwing.
-    const beforeFileAdded = Promise.resolve()
+    return Promise.resolve()
+      // Wrap this in a Promise `.then()` handler so errors will reject the Promise
+      // instead of throwing.
       .then(() => this.opts.onBeforeFileAdded(file, this.getState().files))
-
-    return beforeFileAdded.catch((err) => {
-      const message = typeof err === 'object' ? err.message : err
-      this.info(message, 'error', 5000)
-      return Promise.reject(new Error(`onBeforeFileAdded: ${message}`))
-    }).then(() => {
-      return Utils.getFileType(file).then((fileType) => {
+      .then(() => Utils.getFileType(file))
+      .then((fileType) => {
         const updatedFiles = Object.assign({}, this.getState().files)
         let fileName
         if (file.name) {
@@ -392,10 +379,7 @@ class Uppy {
           preview: file.preview
         }
 
-        const isFileAllowed = this._checkRestrictions(newFile)
-        if (!isFileAllowed) {
-          return Promise.reject(new Error('File not allowed'))
-        }
+        this._checkRestrictions(newFile)
 
         updatedFiles[fileID] = newFile
         this.setState({files: updatedFiles})
@@ -412,7 +396,11 @@ class Uppy {
           }, 4)
         }
       })
-    })
+      .catch((err) => {
+        const message = typeof err === 'object' ? err.message : err
+        this.info(message, 'error', 5000)
+        return Promise.reject(typeof err === 'object' ? err : new Error(err))
+      })
   }
 
   removeFile (fileID) {
@@ -1100,35 +1088,31 @@ class Uppy {
       this.log('No uploader type plugins are used', 'warning')
     }
 
-    const isMinNumberOfFilesReached = this._checkMinNumberOfFiles()
-    if (!isMinNumberOfFilesReached) {
-      return Promise.reject(new Error('Minimum number of files has not been reached'))
-    }
-
-    const beforeUpload = Promise.resolve()
+    return Promise.resolve()
       .then(() => this.opts.onBeforeUpload(this.getState().files))
+      .then(() => this._checkMinNumberOfFiles())
+      .then(() => {
+        const { currentUploads } = this.getState()
+        // get a list of files that are currently assigned to uploads
+        const currentlyUploadingFiles = Object.keys(currentUploads).reduce((prev, curr) => prev.concat(currentUploads[curr].fileIDs), [])
 
-    return beforeUpload.catch((err) => {
-      const message = typeof err === 'object' ? err.message : err
-      this.info(message, 'error', 5000)
-      return Promise.reject(new Error(`onBeforeUpload: ${message}`))
-    }).then(() => {
-      const { currentUploads } = this.getState()
-      // get a list of files that are currently assigned to uploads
-      const currentlyUploadingFiles = Object.keys(currentUploads).reduce((prev, curr) => prev.concat(currentUploads[curr].fileIDs), [])
+        const waitingFileIDs = []
+        Object.keys(this.getState().files).forEach((fileID) => {
+          const file = this.getFile(fileID)
+          // if the file hasn't started uploading and hasn't already been assigned to an upload..
+          if ((!file.progress.uploadStarted) && (currentlyUploadingFiles.indexOf(fileID) === -1)) {
+            waitingFileIDs.push(file.id)
+          }
+        })
 
-      const waitingFileIDs = []
-      Object.keys(this.getState().files).forEach((fileID) => {
-        const file = this.getFile(fileID)
-        // if the file hasn't started uploading and hasn't already been assigned to an upload..
-        if ((!file.progress.uploadStarted) && (currentlyUploadingFiles.indexOf(fileID) === -1)) {
-          waitingFileIDs.push(file.id)
-        }
+        const uploadID = this._createUpload(waitingFileIDs)
+        return this._runUpload(uploadID)
+      })
+      .catch((err) => {
+        const message = typeof err === 'object' ? err.message : err
+        this.info(message, 'error', 5000)
+        return Promise.reject(typeof err === 'object' ? err : new Error(err))
       })
-
-      const uploadID = this._createUpload(waitingFileIDs)
-      return this._runUpload(uploadID)
-    })
   }
 }
 

src/core/Core.test.js

@@ -610,7 +610,7 @@ describe('src/Core', () => {
           throw new Error('File was allowed through')
         })
         .catch(e => {
-          expect(e.message).toEqual('File not allowed')
+          expect(e.message).toEqual('You can only upload: image/gif')
         })
     })
 
@@ -625,7 +625,7 @@ describe('src/Core', () => {
         name: 'foo.jpg',
         type: 'image/jpeg',
         data: null
-      })).rejects.toMatchObject(new Error('onBeforeFileAdded: a plain string'))
+      })).rejects.toMatchObject(new Error('a plain string'))
     })
   })
 
@@ -955,7 +955,7 @@ describe('src/Core', () => {
         name: 'foo2.jpg',
         type: 'image/jpeg',
         data: utils.dataURItoFile(sampleImageDataURI, {})
-      })).rejects.toMatchObject(new Error('File not allowed')).then(() => {
+      })).rejects.toMatchObject(new Error('You can only upload 1 file')).then(() => {
         expect(core.state.info.message).toEqual('You can only upload 1 file')
       })
     })
@@ -975,7 +975,7 @@ describe('src/Core', () => {
         name: 'foo2.jpg',
         type: 'image/jpeg',
         data: utils.dataURItoFile(sampleImageDataURI, {})
-      })).rejects.toMatchObject(new Error('File not allowed')).then(() => {
+      })).rejects.toMatchObject(new Error('You can only upload: image/gif, image/png')).then(() => {
         expect(core.state.info.message).toEqual('You can only upload: image/gif, image/png')
       })
     })
@@ -993,7 +993,7 @@ describe('src/Core', () => {
         name: 'foo.jpg',
         type: 'image/jpeg',
         data: utils.dataURItoFile(sampleImageDataURI, {})
-      })).rejects.toMatchObject(new Error('File not allowed')).then(() => {
+      })).rejects.toMatchObject(new Error('This file exceeds maximum allowed size of 1.2 KB')).then(() => {
         expect(core.state.info.message).toEqual('This file exceeds maximum allowed size of 1.2 KB')
       })
     })

src/plugins/Dashboard/index.js

@@ -277,6 +277,8 @@ module.exports = class Dashboard extends Plugin {
         name: file.name,
         type: file.type,
         data: blob
+      }).catch(() => {
+        // Ignore
       })
     })
   }
@@ -291,6 +293,8 @@ module.exports = class Dashboard extends Plugin {
         name: file.name,
         type: file.type,
         data: file
+      }).catch(() => {
+        // Ignore
       })
     })
   }
@@ -360,6 +364,8 @@ module.exports = class Dashboard extends Plugin {
         name: file.name,
         type: file.type,
         data: file
+      }).catch(() => {
+        // Ignore
       })
     })
   }

src/plugins/DragDrop/index.js

@@ -83,6 +83,8 @@ module.exports = class DragDrop extends Plugin {
         name: file.name,
         type: file.type,
         data: file
+      }).catch(() => {
+        // Ignore
       })
     })
   }
@@ -98,6 +100,8 @@ module.exports = class DragDrop extends Plugin {
         name: file.name,
         type: file.type,
         data: file
+      }).catch(() => {
+        // Ignore
       })
     })
   }

src/plugins/Dummy.js

@@ -35,7 +35,9 @@ module.exports = class Dummy extends Plugin {
       data: blob
     }
     this.props.log('Adding fake file blob')
-    this.props.addFile(file)
+    this.props.addFile(file).catch(() => {
+      // Ignore
+    })
   }
 
   render (state) {

src/plugins/FileInput.js

@@ -51,6 +51,8 @@ module.exports = class FileInput extends Plugin {
         name: file.name,
         type: file.type,
         data: file
+      }).catch(() => {
+        // Ignore
       })
     })
   }

src/plugins/Provider/view/index.js

@@ -169,7 +169,9 @@ module.exports = class View {
         tagFile.preview = this.plugin.getItemThumbnailUrl(file)
       }
       this.plugin.uppy.log('Adding remote file')
-      this.plugin.uppy.addFile(tagFile)
+      this.plugin.uppy.addFile(tagFile).catch(() => {
+        // Ignore
+      })
       if (!isCheckbox) {
         this.donePicking()
       }

src/plugins/Transloadit/index.test.js

@@ -168,6 +168,6 @@ describe('Transloadit', () => {
       }
     })
 
-    return expect(uppy.upload()).rejects.toBe('short-circuited')
+    return expect(uppy.upload()).rejects.toEqual(new Error('short-circuited'))
   })
 })

src/plugins/Webcam/index.js

@@ -170,7 +170,7 @@ module.exports = class Webcam extends Plugin {
       })
       return this.getVideo()
     })
-    .then(this.uppy.addFile)
+    .then((file) => this.uppy.addFile(file))
     .then(() => {
       this.recordingChunks = null
       this.recorder = null
@@ -233,9 +233,11 @@ module.exports = class Webcam extends Plugin {
       return this.getImage()
     }).then((tagFile) => {
       this.captureInProgress = false
-      this.uppy.addFile(tagFile)
       const dashboard = this.uppy.getPlugin('Dashboard')
       if (dashboard) dashboard.hideAllPanels()
+      return this.uppy.addFile(tagFile).catch(() => {
+        // Ignore
+      })
     }, (error) => {
       this.captureInProgress = false
       throw error