Browse Source

fix broken thumbnails for box and dropbox (#3460)

those are the only two providers who use companion-proxied thumbnails
dropbox had been broken after #3159
box did never work before because it just showed a placeholder (see box api)
also upgraded dropbox thumbnail api to v2
and rewrite to async/await
Mikael Finstad 3 years ago
parent
commit
18b23e3871

+ 2 - 6
packages/@uppy/companion/src/server/controllers/thumbnail.js

@@ -10,12 +10,8 @@ async function thumbnail (req, res, next) {
   const { provider } = req.companion
 
   try {
-    const response = await provider.thumbnail({ id, token })
-    if (response) {
-      response.pipe(res)
-    } else {
-      res.sendStatus(404)
-    }
+    const { stream } = await provider.thumbnail({ id, token })
+    stream.pipe(res)
   } catch (err) {
     if (err.isAuthError) res.sendStatus(401)
     else next(err)

+ 58 - 33
packages/@uppy/companion/src/server/provider/box/index.js

@@ -81,39 +81,65 @@ class Box extends Provider {
     }
   }
 
-  _thumbnail ({ id, token }, done) {
-    return this.client
-      .get(`files/${id}/thumbnail.png`)
-      .qs({ max_height: BOX_THUMBNAIL_SIZE, max_width: BOX_THUMBNAIL_SIZE })
-      .auth(token)
-      .request()
-      .on('response', (resp) => {
-        // box generates a thumbnail on first request
-        // so they return a placeholder in the header while that's happening
-        if (resp.statusCode === 202 && resp.headers.location) {
-          return this.client.get(resp.headers.location)
-            .request()
-            .on('response', (placeholderResp) => {
-              if (placeholderResp.statusCode !== 200) {
-                const err = this._error(null, placeholderResp)
-                logger.error(err, 'provider.box.thumbnail.error')
-                return done(err)
-              }
-
-              done(null, placeholderResp)
-            })
-        }
+  async thumbnail ({ id, token }) {
+    const maxRetryTime = 10
+    const extension = 'jpg' // set to png to more easily reproduce http 202 retry-after
+    let remainingRetryTime = maxRetryTime
 
-        if (resp.statusCode !== 200) {
-          const err = this._error(null, resp)
-          logger.error(err, 'provider.box.thumbnail.error')
-          return done(err)
-        }
-        done(null, resp)
-      })
-      .on('error', (err) => {
-        logger.error(err, 'provider.box.thumbnail.error')
-      })
+    const tryGetThumbnail = async () => {
+      const req = this.client
+        .get(`files/${id}/thumbnail.${extension}`)
+        .qs({ max_height: BOX_THUMBNAIL_SIZE, max_width: BOX_THUMBNAIL_SIZE })
+        .auth(token)
+        .request()
+
+      // See also requestStream
+      const resp = await new Promise((resolve, reject) => (
+        req
+          .on('response', (response) => {
+            // Don't allow any more data to flow yet.
+            // https://github.com/request/request/issues/1990#issuecomment-184712275
+            response.pause()
+            resolve(response)
+          })
+          .on('error', reject)
+      ))
+
+      if (resp.statusCode === 200) {
+        return { stream: resp }
+      }
+
+      req.abort() // Or we will leak memory (the stream is paused and we're not using this response stream anymore)
+
+      // From box API docs:
+      // Sometimes generating a thumbnail can take a few seconds.
+      // In these situations the API returns a Location-header pointing to a placeholder graphic
+      // for this file type.
+      // The placeholder graphic can be used in a user interface until the thumbnail generation has completed.
+      // The Retry-After-header indicates when to the thumbnail will be ready.
+      // At that time, retry this endpoint to retrieve the thumbnail.
+      //
+      // This can be reproduced more easily by changing extension to png and trying on a newly uploaded image
+      const retryAfter = parseInt(resp.headers['retry-after'], 10)
+      if (!Number.isNaN(retryAfter)) {
+        const retryInSec = Math.min(remainingRetryTime, retryAfter)
+        if (retryInSec <= 0) throw new ProviderApiError('Timed out waiting for thumbnail', 504)
+        logger.debug(`Need to retry box thumbnail in ${retryInSec} sec`)
+        remainingRetryTime -= retryInSec
+        await new Promise((resolve) => setTimeout(resolve, retryInSec * 1000))
+        return tryGetThumbnail()
+      }
+
+      // we have an error status code, throw
+      throw this._error(null, resp)
+    }
+
+    try {
+      return await tryGetThumbnail()
+    } catch (err) {
+      logger.error(err, 'provider.box.thumbnail.error')
+      throw err
+    }
   }
 
   _size ({ id, token }, done) {
@@ -189,7 +215,6 @@ class Box extends Provider {
 Box.version = 2
 
 Box.prototype.list = promisify(Box.prototype._list)
-Box.prototype.thumbnail = promisify(Box.prototype._thumbnail)
 Box.prototype.size = promisify(Box.prototype._size)
 Box.prototype.logout = promisify(Box.prototype._logout)
 

+ 17 - 23
packages/@uppy/companion/src/server/provider/dropbox/index.js

@@ -141,28 +141,23 @@ class DropBox extends Provider {
     }
   }
 
-  _thumbnail ({ id, token }, done) {
-    return this.client
-      .post('https://content.dropboxapi.com/2/files/get_thumbnail')
-      .options({
-        version: '2',
-        headers: {
-          'Dropbox-API-Arg': httpHeaderSafeJson({ path: `${id}`, size: 'w256h256' }),
-        },
-      })
-      .auth(token)
-      .request()
-      .on('response', (resp) => {
-        if (resp.statusCode !== 200) {
-          const err = this._error(null, resp)
-          logger.error(err, 'provider.dropbox.thumbnail.error')
-          return done(err)
-        }
-        done(null, resp)
-      })
-      .on('error', (err) => {
-        logger.error(err, 'provider.dropbox.thumbnail.error')
-      })
+  async thumbnail ({ id, token }) {
+    try {
+      const req = this.client
+        .post('https://content.dropboxapi.com/2/files/get_thumbnail_v2')
+        .options({
+          headers: {
+            'Dropbox-API-Arg': httpHeaderSafeJson({ resource: { '.tag': 'path', path: `${id}` }, size: 'w256h256' }),
+          },
+        })
+        .auth(token)
+        .request()
+
+      return await requestStream(req, (resp) => this._error(null, resp))
+    } catch (err) {
+      logger.error(err, 'provider.dropbox.thumbnail.error')
+      throw err
+    }
   }
 
   _size ({ id, token }, done) {
@@ -232,7 +227,6 @@ class DropBox extends Provider {
 DropBox.version = 2
 
 DropBox.prototype.list = promisify(DropBox.prototype._list)
-DropBox.prototype.thumbnail = promisify(DropBox.prototype._thumbnail)
 DropBox.prototype.size = promisify(DropBox.prototype._size)
 DropBox.prototype.logout = promisify(DropBox.prototype._logout)