Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix broken thumbnails for box and dropbox #3460

Merged
merged 1 commit into from
Feb 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 2 additions & 6 deletions packages/@uppy/companion/src/server/controllers/thumbnail.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
91 changes: 58 additions & 33 deletions packages/@uppy/companion/src/server/provider/box/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Murderlon marked this conversation as resolved.
Show resolved Hide resolved
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) {
Expand Down Expand Up @@ -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)

Expand Down
40 changes: 17 additions & 23 deletions packages/@uppy/companion/src/server/provider/dropbox/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)

Expand Down