ソースを参照

companion: catch download failures via response status codes (#2223)

* companion: catch download failures via response status codes

* companion: fix test

* companion: refactor provider file download + status code handling
Ifedapo .A. Olarewaju 5 年 前
コミット
97b311324d

+ 1 - 1
packages/@uppy/companion/src/server/Uploader.js

@@ -331,7 +331,7 @@ class Uploader {
 
   getResponse () {
     if (this._errRespMessage) {
-      return { body: { error: this._errRespMessage }, status: 400 }
+      return { body: { message: this._errRespMessage }, status: 400 }
     }
     return { body: { token: this.token }, status: 200 }
   }

+ 1 - 1
packages/@uppy/companion/src/server/controllers/get.js

@@ -20,7 +20,7 @@ function get (req, res, next) {
 
     if (!size) {
       logger.error('unable to determine file size', 'controller.get.provider.size', req.id)
-      return res.status(400).json({ error: 'unable to determine file size' })
+      return res.status(400).json({ message: 'unable to determine file size' })
     }
 
     logger.debug('Instantiating uploader.', null, req.id)

+ 32 - 29
packages/@uppy/companion/src/server/provider/drive/index.js

@@ -112,18 +112,23 @@ class Drive extends Provider {
       .request()
   }
 
-  _getGsuiteFileMeta (id, token, mimeType, onDone) {
-    logger.info(`calling Gsuite file meta for ${id}`, 'provider.drive.export.meta')
-    return this.client
-      .query()
-      .head(`files/${id}/export`)
-      .qs({ supportsAllDrives: true, mimeType })
-      .auth(token)
-      .request(onDone)
+  _waitForFailedResponse (resp) {
+    return new Promise((resolve, reject) => {
+      let data = ''
+      resp.on('data', (chunk) => {
+        data += chunk
+      }).on('end', () => {
+        try {
+          resolve(JSON.parse(data.toString()))
+        } catch (error) {
+          reject(error)
+        }
+      })
+    })
   }
 
   download ({ id, token }, onData) {
-    this.stats({ id, token }, (err, resp, body) => {
+    this.stats({ id, token }, (err, _, body) => {
       if (err) {
         logger.error(err, 'provider.drive.download.stats.error')
         onData(err)
@@ -143,7 +148,18 @@ class Drive extends Provider {
       }
 
       requestStream
-        .on('data', (chunk) => onData(null, chunk))
+        .on('response', (resp) => {
+          if (resp.statusCode !== 200) {
+            this._waitForFailedResponse(resp)
+              .then((jsonResp) => {
+                resp.body = jsonResp
+                onData(this._error(null, resp))
+              })
+              .catch((err) => onData(this._error(err, resp)))
+          } else {
+            resp.on('data', (chunk) => onData(null, chunk))
+          }
+        })
         .on('end', () => onData(null, null))
         .on('error', (err) => {
           logger.error(err, 'provider.drive.download.error')
@@ -168,25 +184,12 @@ class Drive extends Provider {
       }
 
       if (adapter.isGsuiteFile(body.mimeType)) {
-        // Google Docs file sizes can be determined
-        // while Google sheets file sizes can't be determined
-        const googleDocMimeType = 'application/vnd.google-apps.document'
-        if (body.mimeType !== googleDocMimeType) {
-          const maxExportFileSize = 10 * 1024 * 1024 // 10 MB
-          done(null, maxExportFileSize)
-          return
-        }
-
-        this._getGsuiteFileMeta(id, token, adapter.getGsuiteExportType(body.mimeType), (err, resp) => {
-          if (err || resp.statusCode !== 200) {
-            err = this._error(err, resp)
-            logger.error(err, 'provider.drive.docs.size.error')
-            return done(err)
-          }
-
-          const size = resp.headers['content-length']
-          done(null, size ? parseInt(size) : null)
-        })
+        // Not all GSuite file sizes can be predetermined
+        // also for files whose size can be predetermined,
+        // the request to get it can be sometimes expesnive, depending
+        // on the file size. So we default the size to the size export limit
+        const maxExportFileSize = 10 * 1024 * 1024 // 10 MB
+        done(null, maxExportFileSize)
       } else {
         done(null, parseInt(body.size))
       }

+ 7 - 1
packages/@uppy/companion/src/server/provider/dropbox/index.js

@@ -121,7 +121,13 @@ class DropBox extends Provider {
       })
       .auth(token)
       .request()
-      .on('data', (chunk) => onData(null, chunk))
+      .on('response', (resp) => {
+        if (resp.statusCode !== 200) {
+          onData(this._error(null, resp))
+        } else {
+          resp.on('data', (chunk) => onData(null, chunk))
+        }
+      })
       .on('end', () => onData(null, null))
       .on('error', (err) => {
         logger.error(err, 'provider.dropbox.download.error')

+ 14 - 2
packages/@uppy/companion/src/server/provider/facebook/index.js

@@ -80,9 +80,21 @@ class Facebook extends Provider {
       .qs({ fields: 'images' })
       .auth(token)
       .request((err, resp, body) => {
-        if (err) return logger.error(err, 'provider.facebook.download.error')
+        if (err || resp.statusCode !== 200) {
+          err = this._error(err, resp)
+          logger.error(err, 'provider.facebook.download.error')
+          onData(err)
+          return
+        }
+
         request(this._getMediaUrl(body))
-          .on('data', (chunk) => onData(null, chunk))
+          .on('response', (resp) => {
+            if (resp.statusCode !== 200) {
+              onData(this._error(null, resp))
+            } else {
+              resp.on('data', (chunk) => onData(null, chunk))
+            }
+          })
           .on('end', () => onData(null, null))
           .on('error', (err) => {
             logger.error(err, 'provider.facebook.download.url.error')

+ 14 - 2
packages/@uppy/companion/src/server/provider/instagram/graph/index.js

@@ -76,9 +76,21 @@ class Instagram extends Provider {
       .qs({ fields: 'media_url' })
       .auth(token)
       .request((err, resp, body) => {
-        if (err) return logger.error(err, 'provider.instagram.download.error')
+        if (err || resp.statusCode !== 200) {
+          err = this._error(err, resp)
+          logger.error(err, 'provider.instagram.download.error')
+          onData(err)
+          return
+        }
+
         request(body.media_url)
-          .on('data', (chunk) => onData(null, chunk))
+          .on('response', (resp) => {
+            if (resp.statusCode !== 200) {
+              onData(this._error(null, resp))
+            } else {
+              resp.on('data', (chunk) => onData(null, chunk))
+            }
+          })
           .on('end', () => onData(null, null))
           .on('error', (err) => {
             logger.error(err, 'provider.instagram.download.url.error')

+ 14 - 2
packages/@uppy/companion/src/server/provider/instagram/index.js

@@ -77,9 +77,21 @@ class Instagram extends Provider {
       .get(`media/${id}`)
       .auth(token)
       .request((err, resp, body) => {
-        if (err) return logger.error(err, 'provider.instagram.download.error')
+        if (err || resp.statusCode !== 200) {
+          err = this._error(err, resp)
+          logger.error(err, 'provider.instagram.download.error')
+          onData(err)
+          return
+        }
+
         request(this._getMediaUrl(body, query.carousel_id))
-          .on('data', (chunk) => onData(null, chunk))
+          .on('response', (resp) => {
+            if (resp.statusCode !== 200) {
+              onData(this._error(null, resp))
+            } else {
+              resp.on('data', (chunk) => onData(null, chunk))
+            }
+          })
           .on('end', () => onData(null, null))
           .on('error', (err) => {
             logger.error(err, 'provider.instagram.download.url.error')

+ 7 - 1
packages/@uppy/companion/src/server/provider/onedrive/index.js

@@ -70,7 +70,13 @@ class OneDrive extends Provider {
       .get(`${rootPath}/items/${id}/content`)
       .auth(token)
       .request()
-      .on('data', (chunk) => onData(null, chunk))
+      .on('response', (resp) => {
+        if (resp.statusCode !== 200) {
+          onData(this._error(null, resp))
+        } else {
+          resp.on('data', (chunk) => onData(null, chunk))
+        }
+      })
       .on('end', () => onData(null, null))
       .on('error', (err) => {
         logger.error(err, 'provider.onedrive.download.error')

+ 5 - 5
packages/@uppy/companion/test/__tests__/companion.js

@@ -77,7 +77,7 @@ describe('validate upload data', () => {
         protocol: 'tusInvalid'
       })
       .expect(400)
-      .then((res) => expect(res.body.error).toBe('unsupported protocol specified'))
+      .then((res) => expect(res.body.message).toBe('unsupported protocol specified'))
   })
 
   test('invalid upload fieldname gets rejected', () => {
@@ -91,7 +91,7 @@ describe('validate upload data', () => {
         fieldname: 390
       })
       .expect(400)
-      .then((res) => expect(res.body.error).toBe('fieldname must be a string'))
+      .then((res) => expect(res.body.message).toBe('fieldname must be a string'))
   })
 
   test('invalid upload metadata gets rejected', () => {
@@ -105,7 +105,7 @@ describe('validate upload data', () => {
         metadata: 'I am a string instead of object'
       })
       .expect(400)
-      .then((res) => expect(res.body.error).toBe('metadata must be an object'))
+      .then((res) => expect(res.body.message).toBe('metadata must be an object'))
   })
 
   test('invalid upload headers get rejected', () => {
@@ -119,7 +119,7 @@ describe('validate upload data', () => {
         headers: 'I am a string instead of object'
       })
       .expect(400)
-      .then((res) => expect(res.body.error).toBe('headers must be an object'))
+      .then((res) => expect(res.body.message).toBe('headers must be an object'))
   })
 
   test('invalid upload HTTP Method gets rejected', () => {
@@ -133,7 +133,7 @@ describe('validate upload data', () => {
         httpMethod: 'DELETE'
       })
       .expect(400)
-      .then((res) => expect(res.body.error).toBe('unsupported HTTP METHOD specified'))
+      .then((res) => expect(res.body.message).toBe('unsupported HTTP METHOD specified'))
   })
 
   test('valid upload data is allowed - tus', () => {