Merge pull request #612 from transloadit/fix/response-errors

Revamp XHR response handling

src/core/Core.js

@@ -436,7 +436,7 @@ class Uppy {
     })
 
     this._calculateTotalProgress()
-    this.emit('file-removed', fileID)
+    this.emit('file-removed', removedFile)
 
     // Clean up object URLs.
     if (removedFile.preview && Utils.isObjectURL(removedFile.preview)) {

src/core/Core.test.js

@@ -717,10 +717,11 @@ describe('src/Core', () => {
             totalProgress: 50
           })
 
+          const file = core.getFile(fileId)
           core.removeFile(fileId)
 
           expect(Object.keys(core.state.files).length).toEqual(0)
-          expect(fileRemovedEventMock.mock.calls[0][0]).toEqual(fileId)
+          expect(fileRemovedEventMock.mock.calls[0][0]).toEqual(file)
           expect(core.state.totalProgress).toEqual(0)
         })
     })

src/plugins/Tus.js

@@ -130,7 +130,7 @@ module.exports = class Tus extends Plugin {
 
       optsTus.onError = (err) => {
         this.uppy.log(err)
-        this.uppy.emit('upload-error', file.id, err)
+        this.uppy.emit('upload-error', file, err)
         err.message = `Failed because: ${err.message}`
 
         this.resetUploaderReferences(file.id)
@@ -148,7 +148,7 @@ module.exports = class Tus extends Plugin {
       }
 
       optsTus.onSuccess = () => {
-        this.uppy.emit('upload-success', file.id, upload, upload.url)
+        this.uppy.emit('upload-success', file, upload, upload.url)
 
         if (upload.url) {
           this.uppy.log('Download ' + upload.file.name + ' from ' + upload.url)
@@ -300,12 +300,12 @@ module.exports = class Tus extends Plugin {
       socket.on('progress', (progressData) => emitSocketProgress(this, progressData, file))
 
       socket.on('error', (errData) => {
-        this.uppy.emit('upload-error', file.id, new Error(errData.error))
+        this.uppy.emit('upload-error', file, new Error(errData.error))
         reject(new Error(errData.error))
       })
 
       socket.on('success', (data) => {
-        this.uppy.emit('upload-success', file.id, data, data.url)
+        this.uppy.emit('upload-success', file, data, data.url)
         this.resetUploaderReferences(file.id)
         resolve()
       })

src/plugins/XHRUpload.js

@@ -9,6 +9,20 @@ const {
   limitPromises
 } = require('../core/Utils')
 
+function buildResponseError (xhr, error) {
+  // No error message
+  if (!error) error = new Error('Upload error')
+  // Got an error message string
+  if (typeof error === 'string') error = new Error(error)
+  // Got something else
+  if (!(error instanceof Error)) {
+    error = Object.assign(new Error('Upload error'), { data: error })
+  }
+
+  error.request = xhr
+  return error
+}
+
 module.exports = class XHRUpload extends Plugin {
   constructor (uppy, opts) {
     super(uppy, opts)
@@ -170,7 +184,7 @@ module.exports = class XHRUpload extends Plugin {
 
       const timer = this.createProgressTimeout(opts.timeout, (error) => {
         xhr.abort()
-        this.uppy.emit('upload-error', file.id, error)
+        this.uppy.emit('upload-error', file, error)
         reject(error)
       })
 
@@ -202,10 +216,18 @@ module.exports = class XHRUpload extends Plugin {
         timer.done()
 
         if (ev.target.status >= 200 && ev.target.status < 300) {
-          const resp = opts.getResponseData(xhr.responseText, xhr)
-          const uploadURL = resp[opts.responseUrlFieldName]
+          const body = opts.getResponseData(xhr.responseText, xhr)
+          const uploadURL = body[opts.responseUrlFieldName]
+
+          const response = {
+            status: ev.target.status,
+            body,
+            uploadURL
+          }
 
-          this.uppy.emit('upload-success', file.id, resp, uploadURL)
+          this.uppy.setFileState(file.id, { response })
+
+          this.uppy.emit('upload-success', file, body, uploadURL)
 
           if (uploadURL) {
             this.uppy.log(`Download ${file.name} from ${file.uploadURL}`)
@@ -213,9 +235,17 @@ module.exports = class XHRUpload extends Plugin {
 
           return resolve(file)
         } else {
-          const error = opts.getResponseError(xhr.responseText, xhr) || new Error('Upload error')
-          error.request = xhr
-          this.uppy.emit('upload-error', file.id, error)
+          const body = opts.getResponseData(xhr)
+          const error = buildResponseError(xhr, opts.getResponseError(xhr.responseText, xhr))
+
+          const response = {
+            status: ev.target.status,
+            body
+          }
+
+          this.uppy.setFileState(file.id, { response })
+
+          this.uppy.emit('upload-error', file, error)
           return reject(error)
         }
       })
@@ -224,8 +254,8 @@ module.exports = class XHRUpload extends Plugin {
         this.uppy.log(`[XHRUpload] ${id} errored`)
         timer.done()
 
-        const error = opts.getResponseError(xhr.responseText, xhr) || new Error('Upload error')
-        this.uppy.emit('upload-error', file.id, error)
+        const error = buildResponseError(xhr, opts.getResponseError(xhr.responseText, xhr))
+        this.uppy.emit('upload-error', file, error)
         return reject(error)
       })
 
@@ -294,7 +324,7 @@ module.exports = class XHRUpload extends Plugin {
           socket.on('success', (data) => {
             const resp = opts.getResponseData(data.response.responseText, data.response)
             const uploadURL = resp[opts.responseUrlFieldName]
-            this.uppy.emit('upload-success', file.id, resp, uploadURL)
+            this.uppy.emit('upload-success', file, resp, uploadURL)
             socket.close()
             return resolve()
           })
@@ -302,7 +332,7 @@ module.exports = class XHRUpload extends Plugin {
           socket.on('error', (errData) => {
             const resp = errData.response
             const error = resp ? opts.getResponseError(resp.responseText, resp) : new Error(errData.error)
-            this.uppy.emit('upload-error', file.id, error)
+            this.uppy.emit('upload-error', file, error)
             reject(new Error(errData.error))
           })
         })
@@ -331,7 +361,7 @@ module.exports = class XHRUpload extends Plugin {
 
       const emitError = (error) => {
         files.forEach((file) => {
-          this.uppy.emit('upload-error', file.id, error)
+          this.uppy.emit('upload-error', file, error)
         })
       }
 
@@ -361,7 +391,7 @@ module.exports = class XHRUpload extends Plugin {
         if (ev.target.status >= 200 && ev.target.status < 300) {
           const resp = this.opts.getResponseData(xhr.responseText, xhr)
           files.forEach((file) => {
-            this.uppy.emit('upload-success', file.id, resp)
+            this.uppy.emit('upload-success', file, resp)
           })
           return resolve()
         }

website/src/docs/uppy.md

@@ -333,8 +333,8 @@ uppy.on('file-added', (file) => {
 Fired each time file is removed.
 
 ```javascript
-uppy.on('file-removed', (fileID) => {
-  console.log('Removed file', fileID)
+uppy.on('file-removed', (file) => {
+  console.log('Removed file', file)
 })
 ```
 

website/src/docs/xhrupload.md

@@ -72,11 +72,16 @@ uppy.setFileState(otherFileID, {
 
 ### `getResponseData(xhr)`
 
-When an upload has completed, Uppy will extract response data from the upload endpoint and send it back in the `upload-success` event:
+When an upload has completed, Uppy will extract response data from the upload endpoint. This response data will be available on the file's `.response` property, and be emitted in the `upload-success` event:
 
 ```js
-uppy.on('upload-success', (fileId, resp, uploadURL) => {
-  // do something with resp
+uppy.getFile(fileID).response
+// { status: HTTP status code,
+//   body: extracted response data }
+
+uppy.on('upload-success', (fileID, body) => {
+  // do something with extracted response data
+  // (`body` is equivalent to `uppy.getFile(fileID).response.body`)
 })
 ```