core, transloadit: Allow new uploads when retrying; improve error han…

…dling (#1960)

* Set `allowNewUpload: true` when an error occurs to allow retryAll

* Add assembly error message and assembly_id

* Pass debug option to Robodog

* Add forceAllowNewUpload to use in retry and retryAll, improve calls to _showOrLogErrorAndThrow

* Capitalize Create Assembly message

* don’t throw error from 'upload-error' event,  improved error message structure

* handle errors better

* check if error.details exists

* Doc tweaks: added error.assembly

packages/@uppy/core/src/index.js

@@ -464,17 +464,32 @@ class Uppy {
     }
   }
 
-  _showOrLogErrorAndThrow (err, { showInformer = true, file = null } = {}) {
+  /**
+   * Logs an error, sets Informer message, then throws the error.
+   * Emits a 'restriction-failed' event if it’s a restriction error
+   *
+   * @param {object | string} err — Error object or plain string message
+   * @param {object} [options]
+   * @param {boolean} [options.showInformer=true] — Sometimes developer might want to show Informer manually
+   * @param {object} [options.file=null] — File object used to emit the restriction error
+   * @param {boolean} [options.throwErr=true] — Errors shouldn’t be thrown, for example, in `upload-error` event
+   * @private
+   */
+  _showOrLogErrorAndThrow (err, { showInformer = true, file = null, throwErr = true } = {}) {
     const message = typeof err === 'object' ? err.message : err
     const details = (typeof err === 'object' && err.details) ? err.details : ''
 
     // Restriction errors should be logged, but not as errors,
     // as they are expected and shown in the UI.
+    let logMessageWithDetails = message
+    if (details) {
+      logMessageWithDetails += ' ' + details
+    }
     if (err.isRestriction) {
-      this.log(`${message} ${details}`)
+      this.log(logMessageWithDetails)
       this.emit('restriction-failed', file, err)
     } else {
-      this.log(`${message} ${details}`, 'error')
+      this.log(logMessageWithDetails, 'error')
     }
 
     // Sometimes informer has to be shown manually by the developer,
@@ -483,7 +498,9 @@ class Uppy {
       this.info({ message: message, details: details }, 'error', 5000)
     }
 
-    throw (typeof err === 'object' ? err : new Error(err))
+    if (throwErr) {
+      throw (typeof err === 'object' ? err : new Error(err))
+    }
   }
 
   _assertNewUploadAllowed (file) {
@@ -816,7 +833,9 @@ class Uppy {
 
     this.emit('retry-all', filesToRetry)
 
-    const uploadID = this._createUpload(filesToRetry)
+    const uploadID = this._createUpload(filesToRetry, {
+      forceAllowNewUpload: true // create new upload even if allowNewUpload: false
+    })
     return this._runUpload(uploadID)
   }
 
@@ -844,7 +863,9 @@ class Uppy {
 
     this.emit('upload-retry', fileID)
 
-    const uploadID = this._createUpload([fileID])
+    const uploadID = this._createUpload([fileID], {
+      forceAllowNewUpload: true // create new upload even if allowNewUpload: false
+    })
     return this._runUpload(uploadID)
   }
 
@@ -937,22 +958,50 @@ class Uppy {
    */
   _addListeners () {
     this.on('error', (error) => {
-      this.setState({ error: error.message || 'Unknown error' })
+      let errorMsg = 'Unknown error'
+      if (error.message) {
+        errorMsg = error.message
+      }
+
+      if (error.details) {
+        errorMsg += ' ' + error.details
+      }
+
+      this.setState({ error: errorMsg })
     })
 
     this.on('upload-error', (file, error, response) => {
+      let errorMsg = 'Unknown error'
+      if (error.message) {
+        errorMsg = error.message
+      }
+
+      if (error.details) {
+        errorMsg += ' ' + error.details
+      }
+
       this.setFileState(file.id, {
-        error: error.message || 'Unknown error',
+        error: errorMsg,
         response
       })
 
       this.setState({ error: error.message })
 
-      let message = this.i18n('failedToUpload', { file: file.name })
       if (typeof error === 'object' && error.message) {
-        message = { message: message, details: error.message }
+        const newError = new Error(error.message)
+        newError.details = error.message
+        if (error.details) {
+          newError.details += ' ' + error.details
+        }
+        newError.message = this.i18n('failedToUpload', { file: file.name })
+        this._showOrLogErrorAndThrow(newError, {
+          throwErr: false
+        })
+      } else {
+        this._showOrLogErrorAndThrow(error, {
+          throwErr: false
+        })
       }
-      this.info(message, 'error', 5000)
     })
 
     this.on('upload', () => {
@@ -1289,9 +1338,13 @@ class Uppy {
    * @param {Array<string>} fileIDs File IDs to include in this upload.
    * @returns {string} ID of this upload.
    */
-  _createUpload (fileIDs) {
+  _createUpload (fileIDs, opts = {}) {
+    const {
+      forceAllowNewUpload = false // uppy.retryAll sets this to true — when retrying we want to ignore `allowNewUpload: false`
+    } = opts
+
     const { allowNewUpload, currentUploads } = this.getState()
-    if (!allowNewUpload) {
+    if (!allowNewUpload && !forceAllowNewUpload) {
       throw new Error('Cannot create a new upload: already uploading.')
     }
 

packages/@uppy/robodog/src/createUppy.js

@@ -32,7 +32,8 @@ const uppyOptionNames = [
   'restrictions',
   'meta',
   'onBeforeFileAdded',
-  'onBeforeUpload'
+  'onBeforeUpload',
+  'debug'
 ]
 function createUppy (opts, overrides = {}) {
   const uppyOptions = {}

packages/@uppy/status-bar/src/StatusBar.js

@@ -105,11 +105,8 @@ module.exports = (props) => {
     uploadState !== statusBarStates.STATE_WAITING &&
     uploadState !== statusBarStates.STATE_COMPLETE
   const showPauseResumeBtn = resumableUploads && !hidePauseResumeButton &&
-    uploadState !== statusBarStates.STATE_WAITING &&
-    uploadState !== statusBarStates.STATE_PREPROCESSING &&
-    uploadState !== statusBarStates.STATE_POSTPROCESSING &&
-    uploadState !== statusBarStates.STATE_ERROR &&
-    uploadState !== statusBarStates.STATE_COMPLETE
+    uploadState === statusBarStates.STATE_UPLOADING
+
   const showRetryBtn = error && !hideRetryButton
 
   const progressClassNames = `uppy-StatusBar-progress
@@ -170,7 +167,9 @@ const RetryBtn = (props) => {
   return (
     <button
       type="button"
-      class="uppy-u-reset uppy-c-btn uppy-StatusBar-actionBtn uppy-StatusBar-actionBtn--retry" aria-label={props.i18n('retryUpload')} onclick={props.retryAll}
+      class="uppy-u-reset uppy-c-btn uppy-StatusBar-actionBtn uppy-StatusBar-actionBtn--retry"
+      aria-label={props.i18n('retryUpload')}
+      onclick={props.retryAll}
       data-uppy-super-focusable
     >
       <svg aria-hidden="true" focusable="false" class="UppyIcon" width="8" height="10" viewBox="0 0 8 10">
@@ -235,7 +234,7 @@ const PauseResumeButton = (props) => {
 
 const LoadingSpinner = () => {
   return (
-    <svg aria-hidden="true" focusable="false" class="uppy-StatusBar-spinner" width="14" height="14">
+    <svg class="uppy-StatusBar-spinner" aria-hidden="true" focusable="false" width="14" height="14">
       <path d="M13.983 6.547c-.12-2.509-1.64-4.893-3.939-5.936-2.48-1.127-5.488-.656-7.556 1.094C.524 3.367-.398 6.048.162 8.562c.556 2.495 2.46 4.52 4.94 5.183 2.932.784 5.61-.602 7.256-3.015-1.493 1.993-3.745 3.309-6.298 2.868-2.514-.434-4.578-2.349-5.153-4.84a6.226 6.226 0 0 1 2.98-6.778C6.34.586 9.74 1.1 11.373 3.493c.407.596.693 1.282.842 1.988.127.598.073 1.197.161 1.794.078.525.543 1.257 1.15.864.525-.341.49-1.05.456-1.592-.007-.15.02.3 0 0" fill-rule="evenodd" />
     </svg>
   )
@@ -384,9 +383,6 @@ const ProgressBarError = ({ error, retryAll, hideRetryButton, i18n }) => {
           {i18n('uploadFailed')}
         </div>
       </div>
-      {/* {!hideRetryButton &&
-        <span class="uppy-StatusBar-contentPadding">{i18n('pleasePressRetry')}</span>
-      } */}
       <span
         class="uppy-StatusBar-details"
         aria-label={error}

packages/@uppy/status-bar/src/index.js

@@ -106,10 +106,8 @@ module.exports = class StatusBar extends Plugin {
   }
 
   startUpload = () => {
-    return this.uppy.upload().catch((err) => {
-      if (!err.isRestriction) {
-        this.uppy.log(err.stack || err.message || err)
-      }
+    return this.uppy.upload().catch(() => {
+      // Error logged in Core
     })
   }
 
@@ -199,8 +197,7 @@ module.exports = class StatusBar extends Plugin {
       completeFiles.length === Object.keys(files).length &&
       processingFiles.length === 0
 
-    const isAllErrored = isUploadStarted &&
-      erroredFiles.length === uploadStartedFiles.length
+    const isAllErrored = error && erroredFiles.length === filesArray.length
 
     const isAllPaused = inProgressFiles.length !== 0 &&
       pausedFiles.length === inProgressFiles.length

packages/@uppy/transloadit/src/Client.js

@@ -45,8 +45,11 @@ module.exports = class Client {
     }).then((response) => response.json()).then((assembly) => {
       if (assembly.error) {
         const error = new Error(assembly.error)
-        error.message = assembly.error
-        error.details = assembly.reason
+        error.details = assembly.message
+        error.assembly = assembly
+        if (assembly.assembly_id) {
+          error.details += ' ' + `Assembly ID: ${assembly.assembly_id}`
+        }
         throw error
       }
 

packages/@uppy/transloadit/src/index.js

@@ -198,7 +198,7 @@ module.exports = class Transloadit extends Plugin {
   }
 
   _createAssembly (fileIDs, uploadID, options) {
-    this.uppy.log('[Transloadit] create Assembly')
+    this.uppy.log('[Transloadit] Create Assembly')
 
     return this.client.createAssembly({
       params: options.params,
@@ -246,7 +246,6 @@ module.exports = class Transloadit extends Plugin {
       return assembly
     }).catch((err) => {
       err.message = `${this.i18n('creatingAssemblyFailed')}: ${err.message}`
-
       // Reject the promise.
       throw err
     })
@@ -699,9 +698,7 @@ module.exports = class Transloadit extends Plugin {
     })
   }
 
-  _onError (err, uploadID) {
-    this.uppy.log(`[Transloadit] _onError in upload ${uploadID}`)
-    this.uppy.log(err)
+  _onError (err = null, uploadID) {
     const state = this.getPluginState()
     const assemblyIDs = state.uploadsAssemblies[uploadID]
 

website/src/docs/robodog-form.md

@@ -207,6 +207,10 @@ Notice how the form is submitted to the inexistant `/uploads` route once all tra
     })
     .on('error', (error) => {
       console.log('>> Assembly got an error:', error);
+      if (error.assembly) {
+        console.log(`>> Assembly ID ${error.assembly.assembly_id} failed!`);
+        console.log(error.assembly);
+      }
     });
     </script>
   </body>

website/src/docs/transloadit.md

@@ -130,7 +130,6 @@ uppy.use(Transloadit, {
 })
 ```
 
-
 ### `waitForEncoding`
 
 Configures whether or not to wait for all Assemblies to complete before completing the upload.
@@ -287,7 +286,8 @@ If an error occurs when an Assembly has already started, you can find the Assemb
 ```js
 uppy.on('error', (error) => {
   if (error.assembly) {
-    console.log(`${error.assembly.assembly_id} failed!`)
+    console.log(`Assembly ID ${error.assembly.assembly_id} failed!`)
+    console.log(error.assembly)
   }
 })
 ```