From 27c64c7aa642ce97b4d4dfad95467e73bc5c03bf Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Mon, 26 Jul 2021 17:10:39 +0700 Subject: [PATCH 01/46] rewrite to async/await --- .../companion/src/server/controllers/url.js | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/packages/@uppy/companion/src/server/controllers/url.js b/packages/@uppy/companion/src/server/controllers/url.js index 186610dd23..143be786ce 100644 --- a/packages/@uppy/companion/src/server/controllers/url.js +++ b/packages/@uppy/companion/src/server/controllers/url.js @@ -43,15 +43,18 @@ const meta = (req, res) => { * @param {object} res expressJS response object */ const get = (req, res) => { - logger.debug('URL file import handler running', null, req.id) - const { debug } = req.companion.options - if (!validateURL(req.body.url, debug)) { - logger.debug('Invalid request body detected. Exiting url import handler.', null, req.id) - return res.status(400).json({ error: 'Invalid request body' }) - } + (async () => { + try { + logger.debug('URL file import handler running', null, req.id) + const { debug } = req.companion.options + if (!validateURL(req.body.url, debug)) { + logger.debug('Invalid request body detected. Exiting url import handler.', null, req.id) + res.status(400).json({ error: 'Invalid request body' }) + return + } + + const { size } = await reqUtil.getURLMeta(req.body.url, !debug) - reqUtil.getURLMeta(req.body.url, !debug) - .then(({ size }) => { // @ts-ignore logger.debug('Instantiating uploader.', null, req.id) const uploader = new Uploader(Uploader.reqToOptions(req, size)) @@ -70,11 +73,12 @@ const get = (req, res) => { const response = uploader.getResponse() res.status(response.status).json(response.body) - }).catch((err) => { + } catch (err) { logger.error(err, 'controller.url.get.error', req.id) // @todo send more meaningful error message and status code to client if possible - return res.status(err.status || 500).json({ message: 'failed to fetch URL metadata' }) - }) + res.status(err.status || 500).json({ message: 'failed to fetch URL metadata' }) + } + })() } /** From ce4e8f32db093723e8fecbde7fc7f35a1f4bfd54 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Mon, 26 Jul 2021 17:11:27 +0700 Subject: [PATCH 02/46] Only fetch size (HEAD) if needed #3034 --- packages/@uppy/companion/src/server/controllers/url.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/@uppy/companion/src/server/controllers/url.js b/packages/@uppy/companion/src/server/controllers/url.js index 143be786ce..0b3e280d4a 100644 --- a/packages/@uppy/companion/src/server/controllers/url.js +++ b/packages/@uppy/companion/src/server/controllers/url.js @@ -53,7 +53,13 @@ const get = (req, res) => { return } - const { size } = await reqUtil.getURLMeta(req.body.url, !debug) + // If we already have size, no need to get it again. + // See https://github.com/transloadit/uppy/issues/3034 + let size = req.body && req.body.size + if (!size) { + const urlMeta = await reqUtil.getURLMeta(req.body.url, !debug) + size = urlMeta.size + } // @ts-ignore logger.debug('Instantiating uploader.', null, req.id) From b9d2091705439cd458fecc61dd47c6ca1bb87a3c Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Thu, 29 Jul 2021 20:36:49 +0700 Subject: [PATCH 03/46] Update packages/@uppy/companion/src/server/controllers/url.js Co-authored-by: Antoine du Hamel --- packages/@uppy/companion/src/server/controllers/url.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@uppy/companion/src/server/controllers/url.js b/packages/@uppy/companion/src/server/controllers/url.js index 0b3e280d4a..c1bf3125b8 100644 --- a/packages/@uppy/companion/src/server/controllers/url.js +++ b/packages/@uppy/companion/src/server/controllers/url.js @@ -55,7 +55,7 @@ const get = (req, res) => { // If we already have size, no need to get it again. // See https://github.com/transloadit/uppy/issues/3034 - let size = req.body && req.body.size + let { size } = req.body if (!size) { const urlMeta = await reqUtil.getURLMeta(req.body.url, !debug) size = urlMeta.size From 0b6c4882fa852afc04a09586d62798dedfc582d2 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Mon, 30 Aug 2021 21:02:51 +0700 Subject: [PATCH 04/46] Change HEAD to GET in getURLMeta and abort request immediately upon response headers received https://github.com/transloadit/uppy/issues/3034#issuecomment-908059234 --- .../@uppy/companion/src/server/controllers/url.js | 8 +------- .../@uppy/companion/src/server/helpers/request.js | 13 ++++++++----- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/packages/@uppy/companion/src/server/controllers/url.js b/packages/@uppy/companion/src/server/controllers/url.js index c1bf3125b8..143be786ce 100644 --- a/packages/@uppy/companion/src/server/controllers/url.js +++ b/packages/@uppy/companion/src/server/controllers/url.js @@ -53,13 +53,7 @@ const get = (req, res) => { return } - // If we already have size, no need to get it again. - // See https://github.com/transloadit/uppy/issues/3034 - let { size } = req.body - if (!size) { - const urlMeta = await reqUtil.getURLMeta(req.body.url, !debug) - size = urlMeta.size - } + const { size } = await reqUtil.getURLMeta(req.body.url, !debug) // @ts-ignore logger.debug('Instantiating uploader.', null, req.id) diff --git a/packages/@uppy/companion/src/server/helpers/request.js b/packages/@uppy/companion/src/server/helpers/request.js index 3d7e32d9ce..781f0e7d9e 100644 --- a/packages/@uppy/companion/src/server/helpers/request.js +++ b/packages/@uppy/companion/src/server/helpers/request.js @@ -174,18 +174,21 @@ exports.getURLMeta = (url, blockLocalIPs = false) => { return new Promise((resolve, reject) => { const opts = { uri: url, - method: 'HEAD', + method: 'GET', followRedirect: exports.getRedirectEvaluator(url, blockLocalIPs), agentClass: exports.getProtectedHttpAgent((new URL(url)).protocol, blockLocalIPs), } - request(opts, (err, response) => { - if (err || response.statusCode >= 300) { + const req = request(opts, (err) => { + if (err) reject(err) + }) + req.on('response', (response) => { + if (response.statusCode >= 300) { // @todo possibly set a status code in the error object to get a more helpful // hint at what the cause of error is. - err = err || new Error(`URL server responded with status: ${response.statusCode}`) - reject(err) + reject(new Error(`URL server responded with status: ${response.statusCode}`)) } else { + req.abort() // No need to get the rest of the response, as we only want header resolve({ type: response.headers['content-type'], size: parseInt(response.headers['content-length']), From 77c4f25dfc95647f11ff202257d454c408ac34f3 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Mon, 30 Aug 2021 21:04:18 +0700 Subject: [PATCH 05/46] fix lint --- .../@uppy/companion/src/server/helpers/request.js | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/@uppy/companion/src/server/helpers/request.js b/packages/@uppy/companion/src/server/helpers/request.js index 781f0e7d9e..6180cc63d6 100644 --- a/packages/@uppy/companion/src/server/helpers/request.js +++ b/packages/@uppy/companion/src/server/helpers/request.js @@ -141,25 +141,23 @@ function dnsLookup (hostname, options, callback) { class HttpAgent extends http.Agent { createConnection (options, callback) { - options.lookup = dnsLookup if (isIPAddress(options.host) && isPrivateIP(options.host)) { callback(new Error(FORBIDDEN_IP_ADDRESS)) - return + return undefined } // @ts-ignore - return super.createConnection(options, callback) + return super.createConnection({ ...options, lookup: dnsLookup }, callback) } } class HttpsAgent extends https.Agent { createConnection (options, callback) { - options.lookup = dnsLookup if (isIPAddress(options.host) && isPrivateIP(options.host)) { callback(new Error(FORBIDDEN_IP_ADDRESS)) - return + return undefined } // @ts-ignore - return super.createConnection(options, callback) + return super.createConnection({ ...options, lookup: dnsLookup }, callback) } } @@ -167,7 +165,7 @@ class HttpsAgent extends https.Agent { * Gets the size and content type of a url's content * * @param {string} url - * @param {boolean=} blockLocalIPs + * @param {boolean} blockLocalIPs * @returns {Promise<{type: string, size: number}>} */ exports.getURLMeta = (url, blockLocalIPs = false) => { @@ -191,7 +189,7 @@ exports.getURLMeta = (url, blockLocalIPs = false) => { req.abort() // No need to get the rest of the response, as we only want header resolve({ type: response.headers['content-type'], - size: parseInt(response.headers['content-length']), + size: parseInt(response.headers['content-length'], 10), }) } }) From 4226c95a078b5e459507ee7efa0715ae510f8e9b Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Mon, 30 Aug 2021 21:04:53 +0700 Subject: [PATCH 06/46] fix lint --- .../companion/src/server/helpers/request.js | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/packages/@uppy/companion/src/server/helpers/request.js b/packages/@uppy/companion/src/server/helpers/request.js index 6180cc63d6..da312cd962 100644 --- a/packages/@uppy/companion/src/server/helpers/request.js +++ b/packages/@uppy/companion/src/server/helpers/request.js @@ -106,20 +106,6 @@ module.exports.getRedirectEvaluator = (rawRequestURL, blockPrivateIPs) => { } } -/** - * Returns http Agent that will prevent requests to private IPs (to preven SSRF) - * - * @param {string} protocol http or http: or https: or https protocol needed for the request - * @param {boolean} blockPrivateIPs if set to false, this protection will be disabled - */ -module.exports.getProtectedHttpAgent = (protocol, blockPrivateIPs) => { - if (blockPrivateIPs) { - return protocol.startsWith('https') ? HttpsAgent : HttpAgent - } - - return protocol.startsWith('https') ? https.Agent : http.Agent -} - function dnsLookup (hostname, options, callback) { dns.lookup(hostname, options, (err, addresses, maybeFamily) => { if (err) { @@ -161,6 +147,20 @@ class HttpsAgent extends https.Agent { } } +/** + * Returns http Agent that will prevent requests to private IPs (to preven SSRF) + * + * @param {string} protocol http or http: or https: or https protocol needed for the request + * @param {boolean} blockPrivateIPs if set to false, this protection will be disabled + */ +module.exports.getProtectedHttpAgent = (protocol, blockPrivateIPs) => { + if (blockPrivateIPs) { + return protocol.startsWith('https') ? HttpsAgent : HttpAgent + } + + return protocol.startsWith('https') ? https.Agent : http.Agent +} + /** * Gets the size and content type of a url's content * From cf494e5eee6234397a0804406d68f0976eafe63e Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Mon, 30 Aug 2021 21:20:38 +0700 Subject: [PATCH 07/46] cut off length of file names or else we get "MetadataTooLarge: Your metadata headers exceed the maximum allowed metadata size" in tus / S3 --- packages/@uppy/companion/src/server/Uploader.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/@uppy/companion/src/server/Uploader.js b/packages/@uppy/companion/src/server/Uploader.js index dde26c8566..a5a37fc9b1 100644 --- a/packages/@uppy/companion/src/server/Uploader.js +++ b/packages/@uppy/companion/src/server/Uploader.js @@ -16,6 +16,9 @@ const logger = require('./logger') const headerSanitize = require('./header-blacklist') const redis = require('./redis') +// Need to limit length or we can get +// "MetadataTooLarge: Your metadata headers exceed the maximum allowed metadata size" in tus / S3 +const MAX_FILENAME_LENGTH = 500 const DEFAULT_FIELD_NAME = 'files[]' const PROTOCOLS = Object.freeze({ multipart: 'multipart', @@ -58,7 +61,9 @@ class Uploader { this.path = `${this.options.pathPrefix}/${Uploader.FILE_NAME_PREFIX}-${this.token}` this.options.metadata = this.options.metadata || {} this.options.fieldname = this.options.fieldname || DEFAULT_FIELD_NAME - this.uploadFileName = this.options.metadata.name || path.basename(this.path) + this.uploadFileName = this.options.metadata.name + ? this.options.metadata.name.substring(0, MAX_FILENAME_LENGTH) + : path.basename(this.path) this.streamsEnded = false this.uploadStopped = false this.writeStream = fs.createWriteStream(this.path, { mode: 0o666 }) // no executable files From 4aabeea6ea430fa2ea2bc3e3d0eaf7da498ccd2d Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Mon, 30 Aug 2021 21:34:37 +0700 Subject: [PATCH 08/46] try to fix flaky test --- packages/@uppy/utils/src/delay.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@uppy/utils/src/delay.test.js b/packages/@uppy/utils/src/delay.test.js index 4ed769fa62..61afab4b57 100644 --- a/packages/@uppy/utils/src/delay.test.js +++ b/packages/@uppy/utils/src/delay.test.js @@ -30,7 +30,7 @@ describe('delay', () => { // should have rejected before the timer is done const time = Date.now() - start - expect(time).toBeGreaterThanOrEqual(50) - expect(time).toBeLessThan(100) + expect(time).toBeGreaterThanOrEqual(30) + expect(time).toBeLessThan(70) }) }) From 11429a7f2469469c5f0f9528da39694ea227e3b0 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Tue, 31 Aug 2021 16:38:38 +0700 Subject: [PATCH 09/46] remove iife and cleanup code a bit --- .../companion/src/server/controllers/url.js | 108 +++++++++--------- 1 file changed, 54 insertions(+), 54 deletions(-) diff --git a/packages/@uppy/companion/src/server/controllers/url.js b/packages/@uppy/companion/src/server/controllers/url.js index 143be786ce..72fb375c0c 100644 --- a/packages/@uppy/companion/src/server/controllers/url.js +++ b/packages/@uppy/companion/src/server/controllers/url.js @@ -2,15 +2,14 @@ const router = require('express').Router const request = require('request') const { URL } = require('url') const validator = require('validator') + const Uploader = require('../Uploader') -const reqUtil = require('../helpers/request') +const { getURLMeta, getRedirectEvaluator, getProtectedHttpAgent } = require('../helpers/request') const logger = require('../logger') -module.exports = () => { - return router() - .post('/meta', meta) - .post('/get', get) -} +module.exports = () => router() + .post('/meta', meta) + .post('/get', get) /** * Fteches the size and content type of a URL @@ -18,21 +17,22 @@ module.exports = () => { * @param {object} req expressJS request object * @param {object} res expressJS response object */ -const meta = (req, res) => { - logger.debug('URL file import handler running', null, req.id) - const { debug } = req.companion.options - if (!validateURL(req.body.url, debug)) { - logger.debug('Invalid request body detected. Exiting url meta handler.', null, req.id) - return res.status(400).json({ error: 'Invalid request body' }) - } +const meta = async (req, res) => { + try { + logger.debug('URL file import handler running', null, req.id) + const { debug } = req.companion.options + if (!validateURL(req.body.url, debug)) { + logger.debug('Invalid request body detected. Exiting url meta handler.', null, req.id) + return res.status(400).json({ error: 'Invalid request body' }) + } - reqUtil.getURLMeta(req.body.url, !debug) - .then((meta) => res.json(meta)) - .catch((err) => { - logger.error(err, 'controller.url.meta.error', req.id) - // @todo send more meaningful error message and status code to client if possible - return res.status(err.status || 500).json({ message: 'failed to fetch URL metadata' }) - }) + const urlMeta = await getURLMeta(req.body.url, !debug) + res.json(urlMeta) + } catch (err) { + logger.error(err, 'controller.url.meta.error', req.id) + // @todo send more meaningful error message and status code to client if possible + return res.status(err.status || 500).json({ message: 'failed to fetch URL metadata' }) + } } /** @@ -42,43 +42,43 @@ const meta = (req, res) => { * @param {object} req expressJS request object * @param {object} res expressJS response object */ -const get = (req, res) => { - (async () => { - try { - logger.debug('URL file import handler running', null, req.id) - const { debug } = req.companion.options - if (!validateURL(req.body.url, debug)) { - logger.debug('Invalid request body detected. Exiting url import handler.', null, req.id) - res.status(400).json({ error: 'Invalid request body' }) - return - } - - const { size } = await reqUtil.getURLMeta(req.body.url, !debug) - - // @ts-ignore - logger.debug('Instantiating uploader.', null, req.id) - const uploader = new Uploader(Uploader.reqToOptions(req, size)) +const get = async (req, res) => { + try { + logger.debug('URL file import handler running', null, req.id) + const { debug } = req.companion.options + if (!validateURL(req.body.url, debug)) { + logger.debug('Invalid request body detected. Exiting url import handler.', null, req.id) + res.status(400).json({ error: 'Invalid request body' }) + return + } - if (uploader.hasError()) { - const response = uploader.getResponse() - res.status(response.status).json(response.body) - return - } + const { size } = await getURLMeta(req.body.url, !debug) - logger.debug('Waiting for socket connection before beginning remote download.', null, req.id) - uploader.onSocketReady(() => { - logger.debug('Socket connection received. Starting remote download.', null, req.id) - downloadURL(req.body.url, uploader.handleChunk.bind(uploader), !debug, req.id) - }) + // @ts-ignore + logger.debug('Instantiating uploader.', null, req.id) + const uploader = new Uploader(Uploader.reqToOptions(req, size)) + if (uploader.hasError()) { const response = uploader.getResponse() res.status(response.status).json(response.body) - } catch (err) { - logger.error(err, 'controller.url.get.error', req.id) - // @todo send more meaningful error message and status code to client if possible - res.status(err.status || 500).json({ message: 'failed to fetch URL metadata' }) + return } - })() + + logger.debug('Waiting for socket connection before beginning remote download.', null, req.id) + uploader.onSocketReady(() => { + logger.debug('Socket connection received. Starting remote download.', null, req.id) + downloadURL(req.body.url, uploader.handleChunk.bind(uploader), !debug, req.id) + }) + + const response = uploader.getResponse() + + // NOTE: Uploader will continue running after the http request is responded + res.status(response.status).json(response.body) + } catch (err) { + logger.error(err, 'controller.url.get.error', req.id) + // @todo send more meaningful error message and status code to client if possible + res.status(err.status || 500).json({ message: 'failed to fetch URL metadata' }) + } } /** @@ -117,14 +117,14 @@ const validateURL = (url, debug) => { * @param {string} url * @param {downloadCallback} onDataChunk * @param {boolean} blockLocalIPs - * @param {string=} traceId + * @param {string} traceId */ const downloadURL = (url, onDataChunk, blockLocalIPs, traceId) => { const opts = { uri: url, method: 'GET', - followRedirect: reqUtil.getRedirectEvaluator(url, blockLocalIPs), - agentClass: reqUtil.getProtectedHttpAgent((new URL(url)).protocol, blockLocalIPs), + followRedirect: getRedirectEvaluator(url, blockLocalIPs), + agentClass: getProtectedHttpAgent((new URL(url)).protocol, blockLocalIPs), } request(opts) From 0ba83499482f7352780dcb8bf35224bc9de1044f Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Tue, 31 Aug 2021 16:39:58 +0700 Subject: [PATCH 10/46] fix lint by reordering code --- .../companion/src/server/controllers/url.js | 130 +++++++++--------- 1 file changed, 65 insertions(+), 65 deletions(-) diff --git a/packages/@uppy/companion/src/server/controllers/url.js b/packages/@uppy/companion/src/server/controllers/url.js index 72fb375c0c..b5c33ee315 100644 --- a/packages/@uppy/companion/src/server/controllers/url.js +++ b/packages/@uppy/companion/src/server/controllers/url.js @@ -7,9 +7,67 @@ const Uploader = require('../Uploader') const { getURLMeta, getRedirectEvaluator, getProtectedHttpAgent } = require('../helpers/request') const logger = require('../logger') -module.exports = () => router() - .post('/meta', meta) - .post('/get', get) +/** + * Validates that the download URL is secure + * + * @param {string} url the url to validate + * @param {boolean} debug whether the server is running in debug mode + */ +const validateURL = (url, debug) => { + if (!url) { + return false + } + + const validURLOpts = { + protocols: ['http', 'https'], + require_protocol: true, + require_tld: !debug, + } + if (!validator.isURL(url, validURLOpts)) { + return false + } + + return true +} + +/** + * @callback downloadCallback + * @param {Error} err + * @param {string | Buffer | Buffer[]} chunk + */ + +/** + * Downloads the content in the specified url, and passes the data + * to the callback chunk by chunk. + * + * @param {string} url + * @param {downloadCallback} onDataChunk + * @param {boolean} blockLocalIPs + * @param {string} traceId + */ +const downloadURL = (url, onDataChunk, blockLocalIPs, traceId) => { + const opts = { + uri: url, + method: 'GET', + followRedirect: getRedirectEvaluator(url, blockLocalIPs), + agentClass: getProtectedHttpAgent((new URL(url)).protocol, blockLocalIPs), + } + + request(opts) + .on('response', (resp) => { + if (resp.statusCode >= 300) { + const err = new Error(`URL server responded with status: ${resp.statusCode}`) + onDataChunk(err, null) + } else { + resp.on('data', (chunk) => onDataChunk(null, chunk)) + } + }) + .on('end', () => onDataChunk(null, null)) + .on('error', (err) => { + logger.error(err, 'controller.url.download.error', traceId) + onDataChunk(err, null) + }) +} /** * Fteches the size and content type of a URL @@ -27,7 +85,7 @@ const meta = async (req, res) => { } const urlMeta = await getURLMeta(req.body.url, !debug) - res.json(urlMeta) + return res.json(urlMeta) } catch (err) { logger.error(err, 'controller.url.meta.error', req.id) // @todo send more meaningful error message and status code to client if possible @@ -81,64 +139,6 @@ const get = async (req, res) => { } } -/** - * Validates that the download URL is secure - * - * @param {string} url the url to validate - * @param {boolean} debug whether the server is running in debug mode - */ -const validateURL = (url, debug) => { - if (!url) { - return false - } - - const validURLOpts = { - protocols: ['http', 'https'], - require_protocol: true, - require_tld: !debug, - } - if (!validator.isURL(url, validURLOpts)) { - return false - } - - return true -} - -/** - * @callback downloadCallback - * @param {Error} err - * @param {string | Buffer | Buffer[]} chunk - */ - -/** - * Downloads the content in the specified url, and passes the data - * to the callback chunk by chunk. - * - * @param {string} url - * @param {downloadCallback} onDataChunk - * @param {boolean} blockLocalIPs - * @param {string} traceId - */ -const downloadURL = (url, onDataChunk, blockLocalIPs, traceId) => { - const opts = { - uri: url, - method: 'GET', - followRedirect: getRedirectEvaluator(url, blockLocalIPs), - agentClass: getProtectedHttpAgent((new URL(url)).protocol, blockLocalIPs), - } - - request(opts) - .on('response', (resp) => { - if (resp.statusCode >= 300) { - const err = new Error(`URL server responded with status: ${resp.statusCode}`) - onDataChunk(err, null) - } else { - resp.on('data', (chunk) => onDataChunk(null, chunk)) - } - }) - .on('end', () => onDataChunk(null, null)) - .on('error', (err) => { - logger.error(err, 'controller.url.download.error', traceId) - onDataChunk(err, null) - }) -} +module.exports = () => router() + .post('/meta', meta) + .post('/get', get) From 55370c5e59326a1bdceb86c30420fd10362a88bc Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Tue, 31 Aug 2021 17:14:30 +0700 Subject: [PATCH 11/46] rename Uploader to MultipartUploader --- packages/@uppy/aws-s3-multipart/src/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@uppy/aws-s3-multipart/src/index.js b/packages/@uppy/aws-s3-multipart/src/index.js index 8fffa1511d..d8af8c0651 100644 --- a/packages/@uppy/aws-s3-multipart/src/index.js +++ b/packages/@uppy/aws-s3-multipart/src/index.js @@ -4,7 +4,7 @@ const EventTracker = require('@uppy/utils/lib/EventTracker') const emitSocketProgress = require('@uppy/utils/lib/emitSocketProgress') const getSocketHost = require('@uppy/utils/lib/getSocketHost') const { RateLimitedQueue } = require('@uppy/utils/lib/RateLimitedQueue') -const Uploader = require('./MultipartUploader') +const MultipartUploader = require('./MultipartUploader') function assertServerError (res) { if (res && res.error) { @@ -187,7 +187,7 @@ module.exports = class AwsS3Multipart extends BasePlugin { this.uppy.emit('s3-multipart:part-uploaded', cFile, part) } - const upload = new Uploader(file.data, { + const upload = new MultipartUploader(file.data, { // .bind to pass the file object to each handler. createMultipartUpload: this.opts.createMultipartUpload.bind(this, file), listParts: this.opts.listParts.bind(this, file), From f4062880525e4fba6b31dda706f77fdf74527433 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Thu, 2 Sep 2021 14:04:41 +0700 Subject: [PATCH 12/46] Rewrite Uploader to use fs-capacitor #3098 This allows for upload to start almost immediately without having to first download the file. And it allows for uploading bigger files, because transloadit assembly will not timeout, as it will get upload progress events all the time. No longer need for illusive progress. Also fix eslint warnings and simplify logic Still TODO: TUS pause/resume has a bug: https://github.com/tus/tus-js-client/issues/275 --- package-lock.json | 95 ++-- packages/@uppy/companion/package.json | 1 + .../@uppy/companion/src/server/Uploader.js | 432 +++++++++--------- .../companion/src/server/controllers/get.js | 6 +- .../companion/src/server/controllers/url.js | 6 +- .../companion/test/__tests__/uploader.js | 7 +- 6 files changed, 272 insertions(+), 275 deletions(-) diff --git a/package-lock.json b/package-lock.json index 03f3949b0f..682765ba74 100644 --- a/package-lock.json +++ b/package-lock.json @@ -34916,6 +34916,14 @@ "safe-buffer": "~5.1.0" } }, + "node_modules/fs-capacitor": { + "version": "7.0.1", + "resolved": "https://registry.npmjs.org/fs-capacitor/-/fs-capacitor-7.0.1.tgz", + "integrity": "sha512-YjxAAorsFA/pK3PTLuYJO+FlZ7wvGTIwGPbpGzVAJ+DUp6uof0zZjG6dcYsrGX864BMeUCj9R6lmkYZ5uY5lWQ==", + "engines": { + "node": ">=12" + } + }, "node_modules/fs-constants": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/fs-constants/-/fs-constants-1.0.0.tgz", @@ -78484,7 +78492,7 @@ } }, "packages/@uppy/angular": { - "version": "0.2.0", + "version": "0.2.1", "dependencies": { "@angular/animations": "~12.1.0", "@angular/common": "~12.1.0", @@ -78572,7 +78580,7 @@ "integrity": "sha512-N82ooyxVNm6h1riLCoyS9e3fuJ3AMG2zIZs2Gd1ATcSFjSA23Q0fzjjZeh0jbJvWVDZ0cJT8yaNNaaXHzueNjg==" }, "packages/@uppy/aws-s3": { - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "dependencies": { "@uppy/companion-client": "file:../companion-client", @@ -78588,7 +78596,7 @@ } }, "packages/@uppy/aws-s3-multipart": { - "version": "2.0.0", + "version": "2.0.2", "license": "MIT", "dependencies": { "@uppy/companion-client": "file:../companion-client", @@ -78614,7 +78622,7 @@ } }, "packages/@uppy/box": { - "version": "1.0.0", + "version": "1.0.1", "license": "MIT", "dependencies": { "@uppy/companion-client": "file:../companion-client", @@ -78627,7 +78635,7 @@ } }, "packages/@uppy/companion": { - "version": "3.0.0", + "version": "3.0.1", "license": "ISC", "dependencies": { "@purest/providers": "1.0.1", @@ -78645,6 +78653,7 @@ "express-prom-bundle": "6.3.0", "express-request-id": "1.4.1", "express-session": "1.17.1", + "fs-capacitor": "^7.0.1", "grant": "4.7.0", "helmet": "^4.6.0", "ip-address": "6.2.0", @@ -78942,7 +78951,7 @@ } }, "packages/@uppy/core": { - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "dependencies": { "@transloadit/prettier-bytes": "0.0.7", @@ -78967,7 +78976,7 @@ } }, "packages/@uppy/dashboard": { - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "dependencies": { "@transloadit/prettier-bytes": "0.0.7", @@ -78984,7 +78993,7 @@ "preact": "^10.5.13" }, "devDependencies": { - "@uppy/google-drive": "2.0.0", + "@uppy/google-drive": "2.0.1", "@uppy/status-bar": "*", "resize-observer-polyfill": "^1.5.0" }, @@ -79004,7 +79013,7 @@ } }, "packages/@uppy/drag-drop": { - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "dependencies": { "@uppy/utils": "file:../utils", @@ -79015,7 +79024,7 @@ } }, "packages/@uppy/drop-target": { - "version": "1.0.0", + "version": "1.0.1", "license": "MIT", "dependencies": { "@uppy/utils": "file:../utils" @@ -79025,7 +79034,7 @@ } }, "packages/@uppy/dropbox": { - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "dependencies": { "@uppy/companion-client": "file:../companion-client", @@ -79038,7 +79047,7 @@ } }, "packages/@uppy/facebook": { - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "dependencies": { "@uppy/companion-client": "file:../companion-client", @@ -79051,7 +79060,7 @@ } }, "packages/@uppy/file-input": { - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "dependencies": { "@uppy/utils": "file:../utils", @@ -79062,7 +79071,7 @@ } }, "packages/@uppy/form": { - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "dependencies": { "@uppy/utils": "file:../utils", @@ -79073,7 +79082,7 @@ } }, "packages/@uppy/golden-retriever": { - "version": "2.0.0", + "version": "2.0.2", "license": "MIT", "dependencies": { "@transloadit/prettier-bytes": "0.0.7", @@ -79085,7 +79094,7 @@ } }, "packages/@uppy/google-drive": { - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "dependencies": { "@uppy/companion-client": "file:../companion-client", @@ -79098,7 +79107,7 @@ } }, "packages/@uppy/image-editor": { - "version": "1.0.0", + "version": "1.0.1", "license": "MIT", "dependencies": { "@uppy/utils": "file:../utils", @@ -79110,7 +79119,7 @@ } }, "packages/@uppy/informer": { - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "dependencies": { "@uppy/utils": "file:../utils", @@ -79121,7 +79130,7 @@ } }, "packages/@uppy/instagram": { - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "dependencies": { "@uppy/companion-client": "file:../companion-client", @@ -79138,7 +79147,7 @@ "license": "MIT" }, "packages/@uppy/onedrive": { - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "dependencies": { "@uppy/companion-client": "file:../companion-client", @@ -79151,7 +79160,7 @@ } }, "packages/@uppy/progress-bar": { - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "dependencies": { "@uppy/utils": "file:../utils", @@ -79162,7 +79171,7 @@ } }, "packages/@uppy/provider-views": { - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "dependencies": { "@uppy/utils": "file:../utils", @@ -79174,7 +79183,7 @@ } }, "packages/@uppy/react": { - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "dependencies": { "@uppy/dashboard": "file:../dashboard", @@ -79198,7 +79207,7 @@ } }, "packages/@uppy/react-native": { - "version": "0.2.0", + "version": "0.2.1", "license": "MIT", "dependencies": { "@uppy/instagram": "file:../instagram", @@ -79214,14 +79223,14 @@ } }, "packages/@uppy/redux-dev-tools": { - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "peerDependencies": { "@uppy/core": "^2.0.0" } }, "packages/@uppy/robodog": { - "version": "2.0.0", + "version": "2.0.2", "license": "MIT", "dependencies": { "@uppy/core": "file:../core", @@ -79247,7 +79256,7 @@ } }, "packages/@uppy/screen-capture": { - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "dependencies": { "@uppy/utils": "file:../utils", @@ -79258,7 +79267,7 @@ } }, "packages/@uppy/status-bar": { - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "dependencies": { "@transloadit/prettier-bytes": "0.0.7", @@ -79316,7 +79325,7 @@ } }, "packages/@uppy/svelte": { - "version": "1.0.0", + "version": "1.0.1", "dependencies": { "@uppy/dashboard": "file:../dashboard", "@uppy/drag-drop": "file:../drag-drop", @@ -79358,7 +79367,7 @@ } }, "packages/@uppy/thumbnail-generator": { - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "dependencies": { "@uppy/utils": "file:../utils", @@ -79372,7 +79381,7 @@ } }, "packages/@uppy/transloadit": { - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "dependencies": { "@uppy/companion-client": "file:../companion-client", @@ -79390,7 +79399,7 @@ } }, "packages/@uppy/tus": { - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "dependencies": { "@uppy/companion-client": "file:../companion-client", @@ -79402,7 +79411,7 @@ } }, "packages/@uppy/unsplash": { - "version": "1.0.0", + "version": "1.0.1", "license": "MIT", "dependencies": { "@uppy/companion-client": "file:../companion-client", @@ -79415,7 +79424,7 @@ } }, "packages/@uppy/url": { - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "dependencies": { "@uppy/companion-client": "file:../companion-client", @@ -79434,7 +79443,7 @@ } }, "packages/@uppy/vue": { - "version": "0.3.0", + "version": "0.3.1", "dependencies": { "@uppy/dashboard": "file:../dashboard", "@uppy/drag-drop": "file:../drag-drop", @@ -79451,7 +79460,7 @@ } }, "packages/@uppy/webcam": { - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "dependencies": { "@uppy/utils": "file:../utils", @@ -79462,7 +79471,7 @@ } }, "packages/@uppy/xhr-upload": { - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "dependencies": { "@uppy/companion-client": "file:../companion-client", @@ -79488,7 +79497,7 @@ } }, "packages/@uppy/zoom": { - "version": "1.0.0", + "version": "1.0.1", "license": "MIT", "dependencies": { "@uppy/companion-client": "file:../companion-client", @@ -79501,7 +79510,7 @@ } }, "packages/uppy": { - "version": "2.0.0", + "version": "2.0.2", "license": "MIT", "dependencies": { "@uppy/aws-s3": "file:../@uppy/aws-s3", @@ -92768,6 +92777,7 @@ "express-prom-bundle": "6.3.0", "express-request-id": "1.4.1", "express-session": "1.17.1", + "fs-capacitor": "*", "grant": "4.7.0", "helmet": "^4.6.0", "ip-address": "6.2.0", @@ -93021,7 +93031,7 @@ "version": "file:packages/@uppy/dashboard", "requires": { "@transloadit/prettier-bytes": "0.0.7", - "@uppy/google-drive": "2.0.0", + "@uppy/google-drive": "2.0.1", "@uppy/informer": "file:../informer", "@uppy/provider-views": "file:../provider-views", "@uppy/status-bar": "*", @@ -109269,6 +109279,11 @@ "from2": "^2.0.3" } }, + "fs-capacitor": { + "version": "7.0.1", + "resolved": "https://registry.npmjs.org/fs-capacitor/-/fs-capacitor-7.0.1.tgz", + "integrity": "sha512-YjxAAorsFA/pK3PTLuYJO+FlZ7wvGTIwGPbpGzVAJ+DUp6uof0zZjG6dcYsrGX864BMeUCj9R6lmkYZ5uY5lWQ==" + }, "fs-constants": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/fs-constants/-/fs-constants-1.0.0.tgz", diff --git a/packages/@uppy/companion/package.json b/packages/@uppy/companion/package.json index 829e797f5f..351cede41c 100644 --- a/packages/@uppy/companion/package.json +++ b/packages/@uppy/companion/package.json @@ -45,6 +45,7 @@ "express-prom-bundle": "6.3.0", "express-request-id": "1.4.1", "express-session": "1.17.1", + "fs-capacitor": "^7.0.1", "grant": "4.7.0", "helmet": "^4.6.0", "ip-address": "6.2.0", diff --git a/packages/@uppy/companion/src/server/Uploader.js b/packages/@uppy/companion/src/server/Uploader.js index a5a37fc9b1..cf310f2b81 100644 --- a/packages/@uppy/companion/src/server/Uploader.js +++ b/packages/@uppy/companion/src/server/Uploader.js @@ -1,10 +1,10 @@ -const fs = require('fs') -const path = require('path') const tus = require('tus-js-client') const uuid = require('uuid') const isObject = require('isobject') +// @ts-ignore const validator = require('validator') const request = require('request') + /** @type {any} */ // @ts-ignore - typescript resolves this this to a hoisted version of // serialize-error that ships with a declaration file, we are using a version @@ -26,6 +26,8 @@ const PROTOCOLS = Object.freeze({ tus: 'tus', }) +class AbortError extends Error {} + class Uploader { /** * Uploads file to destination based on the supplied protocol (tus, s3-multipart, multipart) @@ -34,19 +36,19 @@ class Uploader { * * @typedef {object} UploaderOptions * @property {string} endpoint - * @property {string=} uploadUrl + * @property {string} uploadUrl * @property {string} protocol * @property {number} size - * @property {string=} fieldname + * @property {string} fieldname * @property {string} pathPrefix - * @property {any=} s3 + * @property {any} s3 * @property {any} metadata * @property {any} companionOptions - * @property {any=} storage - * @property {any=} headers - * @property {string=} httpMethod - * @property {boolean=} useFormData - * @property {number=} chunkSize + * @property {any} storage + * @property {any} headers + * @property {string} httpMethod + * @property {boolean} useFormData + * @property {number} chunkSize * * @param {UploaderOptions} options */ @@ -58,16 +60,14 @@ class Uploader { this.options = options this.token = uuid.v4() - this.path = `${this.options.pathPrefix}/${Uploader.FILE_NAME_PREFIX}-${this.token}` this.options.metadata = this.options.metadata || {} this.options.fieldname = this.options.fieldname || DEFAULT_FIELD_NAME this.uploadFileName = this.options.metadata.name ? this.options.metadata.name.substring(0, MAX_FILENAME_LENGTH) - : path.basename(this.path) - this.streamsEnded = false + : `${Uploader.FILE_NAME_PREFIX}-${this.token}` this.uploadStopped = false - this.writeStream = fs.createWriteStream(this.path, { mode: 0o666 }) // no executable files - .on('error', (err) => logger.error(`${err}`, 'uploader.write.error', this.shortToken)) + this.bytesWritten = 0 + /** @type {number} */ this.emittedProgress = 0 this.storage = options.storage @@ -75,6 +75,7 @@ class Uploader { if (this.options.protocol === PROTOCOLS.tus) { emitter().on(`pause:${this.token}`, () => { + logger.debug('Received from client: pause', 'uploader', this.shortToken) this._paused = true if (this.tus) { this.tus.abort() @@ -82,23 +83,72 @@ class Uploader { }) emitter().on(`resume:${this.token}`, () => { + logger.debug('Received from client: cancel', 'uploader', this.shortToken) this._paused = false if (this.tus) { this.tus.start() } }) + } - emitter().on(`cancel:${this.token}`, () => { - this._paused = true - if (this.tus) { - const shouldTerminate = !!this.tus.url - this.tus.abort(shouldTerminate).catch(() => {}) - } - this.cleanUp() - }) + emitter().on(`cancel:${this.token}`, () => { + logger.debug('Received from client: cancel', 'uploader', this.shortToken) + this._paused = true + if (this.tus) { + const shouldTerminate = !!this.tus.url + this.tus.abort(shouldTerminate).catch(() => {}) + } + this.capacitor.destroy(new AbortError()) + }) + } + + async _uploadByProtocol (readStream) { + // @todo a default protocol should not be set. We should ensure that the user specifies their protocol. + const protocol = this.options.protocol || PROTOCOLS.multipart + + switch (protocol) { + case PROTOCOLS.multipart: + return this.uploadMultipart(readStream) + case PROTOCOLS.s3Multipart: + return this.uploadS3Multipart(readStream) + case PROTOCOLS.tus: + return this.uploadTus(readStream) + default: + throw new Error('Invalid protocol') + } + } + + async _startUpload (readStream) { + try { + const { url, extraData } = await Promise.race([ + this._uploadByProtocol(readStream), + // If we don't handle stream errors, we get unhandled error in node. + new Promise((resolve, reject) => this.capacitor.on('error', reject)), + new Promise((resolve, reject) => readStream.on('error', reject)), + ]) + this.emitSuccess(url, extraData) + } catch (err) { + if (!(err instanceof AbortError)) { + // console.log(err) + logger.error(err, 'uploader.error', this.shortToken) + this.emitError(err, err.extraData) + } + } finally { + this.cleanUp() } } + async initCapacitor () { + if (this.capacitor) throw new Error('Already initialized capacitor') + // Because it's an ESM so we cannot require + const { WriteStream } = await import('fs-capacitor') + + this.capacitor = new WriteStream({ tmpdir: () => this.options.pathPrefix }) + const readStream = this.capacitor.createReadStream() + + this._startUpload(readStream) + } + /** * returns a substring of the token. Used as traceId for logging * we avoid using the entire token because this is meant to be a short term @@ -136,13 +186,6 @@ class Uploader { } } - /** - * the number of bytes written into the streams - */ - get bytesWritten () { - return this.writeStream.bytesWritten - } - /** * Validate the options passed down to the uplaoder * @@ -236,21 +279,17 @@ class Uploader { return Uploader.shortenToken(this.token) } - /** - * - * @param {Function} callback - */ - onSocketReady (callback) { - emitter().once(`connection:${this.token}`, () => callback()) + async awaitReady () { logger.debug('waiting for connection', 'uploader.socket.wait', this.shortToken) + await new Promise((resolve) => emitter().once(`connection:${this.token}`, resolve)) + await this.initCapacitor() } cleanUp () { - fs.unlink(this.path, (err) => { - if (err) { - logger.error(`cleanup failed for: ${this.path} err: ${err}`, 'uploader.cleanup.error') - } - }) + if (this.uploadStopped) return + logger.debug('cleanup', this.shortToken) + this.capacitor.destroy() + emitter().removeAllListeners(`pause:${this.token}`) emitter().removeAllListeners(`resume:${this.token}`) emitter().removeAllListeners(`cancel:${this.token}`) @@ -264,59 +303,28 @@ class Uploader { */ handleChunk (err, chunk) { if (this.uploadStopped) { + logger.debug('Received chunk after upload stopped', 'uploader.download', this.shortToken) return } if (err) { logger.error(err, 'uploader.download.error', this.shortToken) - this.emitError(err) - this.cleanUp() + this.capacitor.destroy(err) return } - // @todo a default protocol should not be set. We should ensure that the user specifies their protocol. - const protocol = this.options.protocol || PROTOCOLS.multipart - - // The download has completed; close the file and start an upload if necessary. if (chunk === null) { - this.writeStream.on('finish', () => { - this.streamsEnded = true - switch (protocol) { - case PROTOCOLS.multipart: - if (this.options.endpoint) { - this.uploadMultipart() - } - break - case PROTOCOLS.s3Multipart: - if (!this.s3Upload) { - this.uploadS3Multipart() - } else { - logger.warn('handleChunk() called multiple times', 'uploader.s3.duplicate', this.shortToken) - } - break - case PROTOCOLS.tus: - if (!this.tus) { - this.uploadTus() - } else { - logger.warn('handleChunk() called multiple times', 'uploader.tus.duplicate', this.shortToken) - } - break - } - }) - - return this.endStreams() + // Finished! End the write stream so that the read streams will also finish. + this.capacitor.end() + return } - this.writeStream.write(chunk, () => { + this.capacitor.write(chunk, () => { + this.bytesWritten += chunk.length logger.debug(`${this.bytesWritten} bytes`, 'uploader.download.progress', this.shortToken) - return this.emitIllusiveProgress() }) } - endStreams () { - this.writeStream.end() - } - getResponse () { if (this._errRespMessage) { return { body: { message: this._errRespMessage }, status: 400 } @@ -333,46 +341,15 @@ class Uploader { this.storage.set(`${Uploader.STORAGE_PREFIX}:${this.token}`, jsonStringify(state)) } - /** - * This method emits upload progress but also creates an "upload progress" illusion - * for the waiting period while only download is happening. Hence, it combines both - * download and upload into an upload progress. - * - * @see emitProgress - * @param {number=} bytesUploaded the bytes actually Uploaded so far - */ - emitIllusiveProgress (bytesUploaded = 0) { - if (this._paused) { - return - } - - let bytesTotal = this.streamsEnded ? this.bytesWritten : this.options.size - if (!this.streamsEnded) { - bytesTotal = Math.max(bytesTotal, this.bytesWritten) - } - // for a 10MB file, 10MB of download will account for 5MB upload progress - // and 10MB of actual upload will account for the other 5MB upload progress. - const illusiveBytesUploaded = (this.bytesWritten / 2) + (bytesUploaded / 2) - - logger.debug( - `${bytesUploaded} ${illusiveBytesUploaded} ${bytesTotal}`, - 'uploader.illusive.progress', - this.shortToken - ) - this.emitProgress(illusiveBytesUploaded, bytesTotal) - } - /** * * @param {number} bytesUploaded - * @param {number | null} bytesTotal + * @param {number | null} bytesTotalIn */ - emitProgress (bytesUploaded, bytesTotal) { - bytesTotal = bytesTotal || this.options.size - if (this.tus && this.tus.options.uploadLengthDeferred && this.streamsEnded) { - bytesTotal = this.bytesWritten - } - const percentage = (bytesUploaded / bytesTotal * 100) + onProgress (bytesUploaded, bytesTotalIn) { + const bytesTotal = bytesTotalIn || this.options.size + + const percentage = Math.min(Math.max(0, ((bytesUploaded / bytesTotal) * 100)), 100) const formatPercentage = percentage.toFixed(2) logger.debug( `${bytesUploaded} ${bytesTotal} ${formatPercentage}%`, @@ -380,6 +357,10 @@ class Uploader { this.shortToken ) + if (this._paused || this.uploadStopped) { + return + } + const dataToEmit = { action: 'progress', payload: { progress: formatPercentage, bytesUploaded, bytesTotal }, @@ -411,7 +392,7 @@ class Uploader { /** * * @param {Error} err - * @param {object=} extraData + * @param {object} extraData */ emitError (err, extraData = {}) { const serializedErr = serializeError(err) @@ -428,151 +409,145 @@ class Uploader { /** * start the tus upload */ - uploadTus () { - const file = fs.createReadStream(this.path) + async uploadTus (stream) { const uploader = this - this.tus = new tus.Upload(file, { - endpoint: this.options.endpoint, - uploadUrl: this.options.uploadUrl, - uploadLengthDeferred: false, - retryDelays: [0, 1000, 3000, 5000], - uploadSize: this.bytesWritten, - chunkSize: this.options.chunkSize || Infinity, - headers: headerSanitize(this.options.headers), - addRequestId: true, - metadata: { - // file name and type as required by the tusd tus server - // https://github.com/tus/tusd/blob/5b376141903c1fd64480c06dde3dfe61d191e53d/unrouted_handler.go#L614-L646 - filename: this.uploadFileName, - filetype: this.options.metadata.type, - ...this.options.metadata, - }, - /** - * - * @param {Error} error - */ - onError (error) { - logger.error(error, 'uploader.tus.error') - // deleting tus originalRequest field because it uses the same http-agent - // as companion, and this agent may contain sensitive request details (e.g headers) - // previously made to providers. Deleting the field would prevent it from getting leaked - // to the frontend etc. - // @ts-ignore - delete error.originalRequest + return new Promise((resolve, reject) => { + this.tus = new tus.Upload(stream, { + endpoint: this.options.endpoint, + uploadUrl: this.options.uploadUrl, + uploadLengthDeferred: false, + retryDelays: [0, 1000, 3000, 5000], + uploadSize: this.options.size, + chunkSize: this.options.chunkSize || 50e6, + headers: headerSanitize(this.options.headers), + addRequestId: true, + metadata: { + // file name and type as required by the tusd tus server + // https://github.com/tus/tusd/blob/5b376141903c1fd64480c06dde3dfe61d191e53d/unrouted_handler.go#L614-L646 + filename: this.uploadFileName, + filetype: this.options.metadata.type, + ...this.options.metadata, + }, + /** + * + * @param {Error} error + */ + onError (error) { + logger.error(error, 'uploader.tus.error') + // deleting tus originalRequest field because it uses the same http-agent + // as companion, and this agent may contain sensitive request details (e.g headers) + // previously made to providers. Deleting the field would prevent it from getting leaked + // to the frontend etc. + // @ts-ignore + delete error.originalRequest + // @ts-ignore + delete error.originalResponse + reject(error) + }, + /** + * + * @param {number} bytesUploaded + * @param {number} bytesTotal + */ // @ts-ignore - delete error.originalResponse - uploader.emitError(error) - }, - /** - * - * @param {number} bytesUploaded - * @param {number} bytesTotal - */ - onProgress (bytesUploaded, bytesTotal) { // eslint-disable-line no-unused-vars - uploader.emitIllusiveProgress(bytesUploaded) - }, - onSuccess () { - uploader.emitSuccess(uploader.tus.url) - uploader.cleanUp() - }, - }) + onProgress (bytesUploaded, bytesTotal) { // eslint-disable-line no-unused-vars + uploader.onProgress(bytesUploaded, bytesTotal) + }, + onSuccess () { + resolve({ url: uploader.tus.url }) + }, + }) - if (!this._paused) { - this.tus.start() - } + if (!this._paused) { + this.tus.start() + } + }) } - uploadMultipart () { - const file = fs.createReadStream(this.path) + async uploadMultipart (stream) { + if (!this.options.endpoint) { + throw new Error('No multipart endpoint set') + } // upload progress let bytesUploaded = 0 - file.on('data', (data) => { + stream.on('data', (data) => { bytesUploaded += data.length - this.emitIllusiveProgress(bytesUploaded) + this.onProgress(bytesUploaded, undefined) }) const httpMethod = (this.options.httpMethod || '').toLowerCase() === 'put' ? 'put' : 'post' const headers = headerSanitize(this.options.headers) const reqOptions = { url: this.options.endpoint, headers, encoding: null } - const httpRequest = request[httpMethod] + const runRequest = request[httpMethod] + if (this.options.useFormData) { reqOptions.formData = { - ...this.options.metadata, [this.options.fieldname]: { - value: file, + value: stream, options: { filename: this.uploadFileName, contentType: this.options.metadata.type, + knownLength: this.options.size, }, }, } - - httpRequest(reqOptions, (error, response, body) => { - this._onMultipartComplete(error, response, body, bytesUploaded) - }) } else { - reqOptions.headers['content-length'] = this.bytesWritten - reqOptions.body = file - httpRequest(reqOptions, (error, response, body) => { - this._onMultipartComplete(error, response, body, bytesUploaded) - }) + reqOptions.headers['content-length'] = this.options.size + reqOptions.body = stream } - } - _onMultipartComplete (error, response, body, bytesUploaded) { - if (error) { - logger.error(error, 'upload.multipart.error') - this.emitError(error) - return - } - const { headers } = response + const { response, body } = await new Promise((resolve, reject) => { + runRequest(reqOptions, (error, response2, body2) => { + if (error) { + logger.error(error, 'upload.multipart.error') + reject(error) + return + } + + resolve({ response: response2, body: body2 }) + }) + }) + // remove browser forbidden headers - delete headers['set-cookie'] - delete headers['set-cookie2'] + delete response.headers['set-cookie'] + delete response.headers['set-cookie2'] const respObj = { responseText: body.toString(), status: response.statusCode, statusText: response.statusMessage, - headers, + headers: response.headers, } if (response.statusCode >= 400) { logger.error(`upload failed with status: ${response.statusCode}`, 'upload.multipart.error') - this.emitError(new Error(response.statusMessage), respObj) - } else if (bytesUploaded !== this.bytesWritten && bytesUploaded !== this.options.size) { - const errMsg = `uploaded only ${bytesUploaded} of ${this.bytesWritten} with status: ${response.statusCode}` + const err = new Error(response.statusMessage) + // @ts-ignore + err.extraData = respObj + throw err + } + + if (bytesUploaded !== this.options.size) { + const errMsg = `uploaded only ${bytesUploaded} of ${this.options.size} with status: ${response.statusCode}` logger.error(errMsg, 'upload.multipart.mismatch.error') - this.emitError(new Error(errMsg)) - } else { - this.emitSuccess(null, { response: respObj, bytesUploaded }) + throw new Error(errMsg) } - this.cleanUp() + return { url: null, extraData: { response: respObj, bytesUploaded } } } /** * Upload the file to S3 using a Multipart upload. */ - uploadS3Multipart () { - const file = fs.createReadStream(this.path) - - return this._uploadS3MultipartStream(file) - } - - /** - * Upload a stream to S3. - */ - _uploadS3MultipartStream (stream) { + async uploadS3Multipart (stream) { if (!this.options.s3) { - this.emitError(new Error('The S3 client is not configured on this companion instance.')) - return + throw new Error('The S3 client is not configured on this companion instance.') } - const filename = this.options.metadata.name || path.basename(this.path) + const filename = this.uploadFileName const { client, options } = this.options.s3 const upload = client.upload({ @@ -584,28 +559,29 @@ class Uploader { Body: stream, }) - this.s3Upload = upload - upload.on('httpUploadProgress', ({ loaded, total }) => { - this.emitProgress(loaded, total) + this.onProgress(loaded, total) }) - upload.send((error, data) => { - this.s3Upload = null - if (error) { - this.emitError(error) - } else { - const url = data && data.Location ? data.Location : null - this.emitSuccess(url, { - response: { - responseText: JSON.stringify(data), - headers: { - 'content-type': 'application/json', + return new Promise((resolve, reject) => { + upload.send((error, data) => { + if (error) { + reject(error) + return + } + + resolve({ + url: data && data.Location ? data.Location : null, + extraData: { + response: { + responseText: JSON.stringify(data), + headers: { + 'content-type': 'application/json', + }, }, }, }) - } - this.cleanUp() + }) }) } } diff --git a/packages/@uppy/companion/src/server/controllers/get.js b/packages/@uppy/companion/src/server/controllers/get.js index 41dbb24c66..f383f3e347 100644 --- a/packages/@uppy/companion/src/server/controllers/get.js +++ b/packages/@uppy/companion/src/server/controllers/get.js @@ -34,11 +34,11 @@ function get (req, res, next) { // wait till the client has connected to the socket, before starting // the download, so that the client can receive all download/upload progress. logger.debug('Waiting for socket connection before beginning remote download.', null, req.id) - // waiting for socketReady. - uploader.onSocketReady(() => { + uploader.awaitReady().then(() => { logger.debug('Socket connection received. Starting remote download.', null, req.id) provider.download({ id, token, query: req.query }, uploader.handleChunk.bind(uploader)) - }) + }).catch((err2) => logger.error(err2, req.id)) + const response = uploader.getResponse() res.status(response.status).json(response.body) }) diff --git a/packages/@uppy/companion/src/server/controllers/url.js b/packages/@uppy/companion/src/server/controllers/url.js index b5c33ee315..44db6b7831 100644 --- a/packages/@uppy/companion/src/server/controllers/url.js +++ b/packages/@uppy/companion/src/server/controllers/url.js @@ -53,6 +53,8 @@ const downloadURL = (url, onDataChunk, blockLocalIPs, traceId) => { agentClass: getProtectedHttpAgent((new URL(url)).protocol, blockLocalIPs), } + // return onDataChunk(new Error('test error')) + request(opts) .on('response', (resp) => { if (resp.statusCode >= 300) { @@ -123,10 +125,10 @@ const get = async (req, res) => { } logger.debug('Waiting for socket connection before beginning remote download.', null, req.id) - uploader.onSocketReady(() => { + uploader.awaitReady().then(() => { logger.debug('Socket connection received. Starting remote download.', null, req.id) downloadURL(req.body.url, uploader.handleChunk.bind(uploader), !debug, req.id) - }) + }).catch((err2) => logger.error(err2, req.id)) const response = uploader.getResponse() diff --git a/packages/@uppy/companion/test/__tests__/uploader.js b/packages/@uppy/companion/test/__tests__/uploader.js index 364b0c4b92..b496627b1b 100644 --- a/packages/@uppy/companion/test/__tests__/uploader.js +++ b/packages/@uppy/companion/test/__tests__/uploader.js @@ -9,6 +9,9 @@ const standalone = require('../../src/standalone') describe('uploader with tus protocol', () => { test('upload functions with tus protocol', () => { + if (true) { + throw new Error('TODO this test hangs') + } const fileContent = Buffer.from('Some file content') const { companionOptions } = standalone() const opts = { @@ -26,13 +29,13 @@ describe('uploader with tus protocol', () => { return new Promise((resolve) => { // validate that the test is resolved on socket connection - uploader.onSocketReady(() => { + uploader.awaitReady().then(() => { const fileInfo = fs.statSync(uploader.path) expect(fileInfo.isFile()).toBe(true) expect(fileInfo.size).toBe(0) uploader.handleChunk(null, fileContent) uploader.handleChunk(null, null) - }) + }).catch((err2) => console.error(err2)) let progressReceived = 0 // emulate socket connection From 7a08af4e5f63ccd9fee481a9bf5c7c1593db9c66 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Thu, 2 Sep 2021 14:06:23 +0700 Subject: [PATCH 13/46] add comment in dev Dashboard and pull out variable --- examples/dev/Dashboard.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/examples/dev/Dashboard.js b/examples/dev/Dashboard.js index 4a6d54261d..160b8da7f2 100644 --- a/examples/dev/Dashboard.js +++ b/examples/dev/Dashboard.js @@ -29,6 +29,8 @@ const DropTarget = require('@uppy/drop-target/src') const UPLOADER = 'tus' // const UPLOADER = 's3' // const UPLOADER = 's3-multipart' +// xhr will use protocol 'multipart' in companion, if used with a remote service, e.g. google drive. +// If local upload will use browser XHR // const UPLOADER = 'xhr' // const UPLOADER = 'transloadit' // const UPLOADER = 'transloadit-s3' @@ -44,6 +46,7 @@ const XHR_ENDPOINT = 'https://xhr-server.herokuapp.com/upload' const TRANSLOADIT_KEY = '...' const TRANSLOADIT_TEMPLATE = '...' +const TRANSLOADIT_SERVICE_URL = 'https://api2.transloadit.com' // DEV CONFIG: enable or disable Golden Retriever @@ -109,6 +112,7 @@ module.exports = () => { break case 'transloadit': uppyDashboard.use(Transloadit, { + service: TRANSLOADIT_SERVICE_URL, waitForEncoding: true, params: { auth: { key: TRANSLOADIT_KEY }, From 2d6efdf7e2c8b3a43b283b2c86c748a2cc8cf6fe Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Thu, 2 Sep 2021 14:14:25 +0700 Subject: [PATCH 14/46] fix a bug where remote xhr upload would ignore progress events in the UI --- packages/@uppy/xhr-upload/src/index.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/@uppy/xhr-upload/src/index.js b/packages/@uppy/xhr-upload/src/index.js index e51da28662..f10eaedbe8 100644 --- a/packages/@uppy/xhr-upload/src/index.js +++ b/packages/@uppy/xhr-upload/src/index.js @@ -354,6 +354,8 @@ module.exports = class XHRUpload extends BasePlugin { uploadRemote (file) { const opts = this.getOptions(file) return new Promise((resolve, reject) => { + this.uppy.emit('upload-started', file) + const fields = {} const metaFields = Array.isArray(opts.metaFields) ? opts.metaFields From 1a77274824788e8411ba23257e9a814e514a6d88 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Thu, 2 Sep 2021 14:22:13 +0700 Subject: [PATCH 15/46] fix bug where s3 multipart cancel wasn't working --- packages/@uppy/aws-s3-multipart/src/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@uppy/aws-s3-multipart/src/index.js b/packages/@uppy/aws-s3-multipart/src/index.js index d8af8c0651..682f528412 100644 --- a/packages/@uppy/aws-s3-multipart/src/index.js +++ b/packages/@uppy/aws-s3-multipart/src/index.js @@ -320,7 +320,7 @@ module.exports = class AwsS3Multipart extends BasePlugin { this.onFileRemove(file.id, () => { queuedRequest.abort() - socket.send('pause', {}) + socket.send('cancel', {}) this.resetUploaderReferences(file.id, { abort: true }) resolve(`upload ${file.id} was removed`) }) @@ -348,7 +348,7 @@ module.exports = class AwsS3Multipart extends BasePlugin { this.onCancelAll(file.id, () => { queuedRequest.abort() - socket.send('pause', {}) + socket.send('cancel', {}) this.resetUploaderReferences(file.id) resolve(`upload ${file.id} was canceled`) }) From 1b763bee94c80c5dd029579270562a8e38c1585c Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Thu, 2 Sep 2021 18:36:56 +0700 Subject: [PATCH 16/46] fix also cancel for xhr --- packages/@uppy/xhr-upload/src/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@uppy/xhr-upload/src/index.js b/packages/@uppy/xhr-upload/src/index.js index f10eaedbe8..3328c2b456 100644 --- a/packages/@uppy/xhr-upload/src/index.js +++ b/packages/@uppy/xhr-upload/src/index.js @@ -384,13 +384,13 @@ module.exports = class XHRUpload extends BasePlugin { this.uploaderEvents[file.id] = new EventTracker(this.uppy) this.onFileRemove(file.id, () => { - socket.send('pause', {}) + socket.send('cancel', {}) queuedRequest.abort() resolve(`upload ${file.id} was removed`) }) this.onCancelAll(file.id, () => { - socket.send('pause', {}) + socket.send('cancel', {}) queuedRequest.abort() resolve(`upload ${file.id} was canceled`) }) From cdb62397a73ea2426a44f7eabedc7b59daa07036 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Fri, 3 Sep 2021 14:26:00 +0700 Subject: [PATCH 17/46] Rewrite providers to use streams This removes the need for disk space as data will be buffered in memory and backpressure will be respected https://github.com/transloadit/uppy/issues/3098#issuecomment-907763809 All providers "download" methods will now return a { stream } which can be consumed by uploader. Also: - Remove capacitor (no longer needed) - Change Provider/SearchProvider API to async (Breaking change for custom companion providers) - Fix the case with unknown length streams (zoom / google drive). Need to be downloaded first - rewrite controllers deauth-callback, thumbnail, list, logout to async - getURLMeta: make sure size is never NaN (NaN gets converted to null in JSON.stringify when sent to client but not when used in backend) - fix purest mock (it wasn't returning statusCode on('response')) - add missing http mock for "request" for THUMBNAIL_URL and http://url.myendpoint.com/file (these request errors were never caught by tests previously) - "upload functions with tus protocol" test: move filename checking to new test where size is null. Fix broken expects - fix some lint --- .../custom-provider/server/customprovider.js | 72 +++++---- package-lock.json | 72 ++++++--- packages/@uppy/companion/package.json | 3 +- .../@uppy/companion/src/server/Uploader.js | 147 ++++++++++-------- .../src/server/controllers/deauth-callback.js | 22 +-- .../companion/src/server/controllers/get.js | 50 +++--- .../companion/src/server/controllers/list.js | 21 +-- .../src/server/controllers/logout.js | 34 ++-- .../src/server/controllers/thumbnail.js | 15 +- .../companion/src/server/controllers/url.js | 65 +++++--- .../companion/src/server/helpers/request.js | 5 +- .../companion/src/server/helpers/utils.js | 22 +++ .../companion/src/server/provider/Provider.js | 20 +-- .../src/server/provider/SearchProvider.js | 12 +- .../src/server/provider/box/index.js | 57 ++++--- .../src/server/provider/drive/index.js | 124 ++++++--------- .../src/server/provider/dropbox/index.js | 69 ++++---- .../companion/src/server/provider/error.js | 2 + .../src/server/provider/facebook/index.js | 83 +++++----- .../server/provider/instagram/graph/index.js | 79 +++++----- .../src/server/provider/onedrive/index.js | 59 +++---- .../src/server/provider/unsplash/index.js | 75 +++++---- .../src/server/provider/zoom/index.js | 114 ++++++++------ .../@uppy/companion/test/__mocks__/purest.js | 21 ++- .../companion/test/__tests__/providers.js | 13 ++ .../companion/test/__tests__/uploader.js | 102 +++++++++--- .../@uppy/companion/test/__tests__/url.js | 14 +- .../@uppy/companion/test/fixtures/facebook.js | 3 - 28 files changed, 804 insertions(+), 571 deletions(-) diff --git a/examples/custom-provider/server/customprovider.js b/examples/custom-provider/server/customprovider.js index d1255dabac..4c71e00ced 100644 --- a/examples/custom-provider/server/customprovider.js +++ b/examples/custom-provider/server/customprovider.js @@ -36,7 +36,8 @@ class MyCustomProvider { this.authProvider = 'myunsplash' } - list ({ token, directory }, done) { + // eslint-disable-next-line class-methods-use-this + async list ({ token, directory }) { const path = directory ? `/${directory}/photos` : '' const options = { url: `${BASE_URL}/collections${path}`, @@ -47,18 +48,20 @@ class MyCustomProvider { }, } - request(options, (err, resp, body) => { - if (err) { - console.log(err) - done(err) - return - } + return new Promise((resolve, reject) => ( + request(options, (err, resp, body) => { + if (err) { + console.log(err) + reject(err) + return + } - done(null, adaptData(body)) - }) + resolve(adaptData(body)) + }))) } - download ({ id, token }, onData) { + // eslint-disable-next-line class-methods-use-this + async download ({ id, token }) { const options = { url: `${BASE_URL}/photos/${id}`, method: 'GET', @@ -68,21 +71,27 @@ class MyCustomProvider { }, } - request(options, (err, resp, body) => { - if (err) { - console.log(err) - return - } - - const url = body.links.download - request.get(url) - .on('data', (chunk) => onData(null, chunk)) - .on('end', () => onData(null, null)) - .on('error', (err) => console.log(err)) - }) + const resp = await new Promise((resolve, reject) => ( + request(options) + .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) { + throw new Error(`HTTP response ${resp.statusCode}`) + } + + // The returned stream will be consumed and uploaded from the current position + return { stream: resp } } - size ({ id, token }, done) { + // eslint-disable-next-line class-methods-use-this + async size ({ id, token }) { const options = { url: `${BASE_URL}/photos/${id}`, method: 'GET', @@ -92,15 +101,16 @@ class MyCustomProvider { }, } - request(options, (err, resp, body) => { - if (err) { - console.log(err) - done(err) - return - } + return new Promise((resolve, reject) => ( + request(options, (err, resp, body) => { + if (err) { + console.log(err) + reject(err) + return + } - done(null, body.width * body.height) - }) + resolve(body.size) + }))) } } diff --git a/package-lock.json b/package-lock.json index 682765ba74..b1bb277e7a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -34916,14 +34916,6 @@ "safe-buffer": "~5.1.0" } }, - "node_modules/fs-capacitor": { - "version": "7.0.1", - "resolved": "https://registry.npmjs.org/fs-capacitor/-/fs-capacitor-7.0.1.tgz", - "integrity": "sha512-YjxAAorsFA/pK3PTLuYJO+FlZ7wvGTIwGPbpGzVAJ+DUp6uof0zZjG6dcYsrGX864BMeUCj9R6lmkYZ5uY5lWQ==", - "engines": { - "node": ">=12" - } - }, "node_modules/fs-constants": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/fs-constants/-/fs-constants-1.0.0.tgz", @@ -48416,9 +48408,9 @@ "dev": true }, "node_modules/nock": { - "version": "13.1.1", - "resolved": "https://registry.npmjs.org/nock/-/nock-13.1.1.tgz", - "integrity": "sha512-YKTR9MjfK3kS9/l4nuTxyYm30cgOExRHzkLNhL8nhEUyU4f8Za/dRxOqjhVT1vGs0svWo3dDnJTUX1qxYeWy5w==", + "version": "13.1.3", + "resolved": "https://registry.npmjs.org/nock/-/nock-13.1.3.tgz", + "integrity": "sha512-YKj0rKQWMGiiIO+Y65Ut8OEgYM3PplLU2+GAhnPmqZdBd6z5IskgdBqWmjzA6lH3RF0S2a3wiAlrMOF5Iv2Jeg==", "dev": true, "dependencies": { "debug": "^4.1.0", @@ -78653,7 +78645,6 @@ "express-prom-bundle": "6.3.0", "express-request-id": "1.4.1", "express-session": "1.17.1", - "fs-capacitor": "^7.0.1", "grant": "4.7.0", "helmet": "^4.6.0", "ip-address": "6.2.0", @@ -78699,6 +78690,8 @@ "@types/uuid": "3.4.7", "@types/webpack": "^5.28.0", "@types/ws": "6.0.4", + "into-stream": "^6.0.0", + "nock": "^13.1.3", "supertest": "3.4.2", "typescript": "~4.3" }, @@ -78875,6 +78868,22 @@ "node": ">=4" } }, + "packages/@uppy/companion/node_modules/into-stream": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/into-stream/-/into-stream-6.0.0.tgz", + "integrity": "sha512-XHbaOAvP+uFKUFsOgoNPRjLkwB+I22JFPFe5OjTkQ0nwgj6+pSjb4NmB6VMxaPshLiOf+zcpOCBQuLwC1KHhZA==", + "dev": true, + "dependencies": { + "from2": "^2.3.0", + "p-is-promise": "^3.0.0" + }, + "engines": { + "node": ">=10" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "packages/@uppy/companion/node_modules/mime-db": { "version": "1.42.0", "resolved": "https://registry.npmjs.org/mime-db/-/mime-db-1.42.0.tgz", @@ -78894,6 +78903,15 @@ "node": ">= 0.6" } }, + "packages/@uppy/companion/node_modules/p-is-promise": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/p-is-promise/-/p-is-promise-3.0.0.tgz", + "integrity": "sha512-Wo8VsW4IRQSKVXsJCn7TomUaVtyfjVDn3nUP7kE967BQk0CwFpdbZs0X0uk5sW9mkBa9eNM7hCMaG93WUAwxYQ==", + "dev": true, + "engines": { + "node": ">=8" + } + }, "packages/@uppy/companion/node_modules/safe-buffer": { "version": "5.2.0", "resolved": "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.2.0.tgz", @@ -92777,9 +92795,9 @@ "express-prom-bundle": "6.3.0", "express-request-id": "1.4.1", "express-session": "1.17.1", - "fs-capacitor": "*", "grant": "4.7.0", "helmet": "^4.6.0", + "into-stream": "^6.0.0", "ip-address": "6.2.0", "isobject": "3.0.1", "jsonwebtoken": "8.5.1", @@ -92789,6 +92807,7 @@ "moment-timezone": "^0.5.31", "morgan": "1.10.0", "ms": "2.1.2", + "nock": "*", "node-redis-pubsub": "4.0.0", "node-schedule": "1.3.2", "prom-client": "12.0.0", @@ -92941,6 +92960,16 @@ "resolved": "https://registry.npmjs.org/has-flag/-/has-flag-3.0.0.tgz", "integrity": "sha1-tdRU3CGZriJWmfNGfloH87lVuv0=" }, + "into-stream": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/into-stream/-/into-stream-6.0.0.tgz", + "integrity": "sha512-XHbaOAvP+uFKUFsOgoNPRjLkwB+I22JFPFe5OjTkQ0nwgj6+pSjb4NmB6VMxaPshLiOf+zcpOCBQuLwC1KHhZA==", + "dev": true, + "requires": { + "from2": "^2.3.0", + "p-is-promise": "^3.0.0" + } + }, "mime-db": { "version": "1.42.0", "resolved": "https://registry.npmjs.org/mime-db/-/mime-db-1.42.0.tgz", @@ -92954,6 +92983,12 @@ "mime-db": "1.42.0" } }, + "p-is-promise": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/p-is-promise/-/p-is-promise-3.0.0.tgz", + "integrity": "sha512-Wo8VsW4IRQSKVXsJCn7TomUaVtyfjVDn3nUP7kE967BQk0CwFpdbZs0X0uk5sW9mkBa9eNM7hCMaG93WUAwxYQ==", + "dev": true + }, "safe-buffer": { "version": "5.2.0", "resolved": "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.2.0.tgz", @@ -109279,11 +109314,6 @@ "from2": "^2.0.3" } }, - "fs-capacitor": { - "version": "7.0.1", - "resolved": "https://registry.npmjs.org/fs-capacitor/-/fs-capacitor-7.0.1.tgz", - "integrity": "sha512-YjxAAorsFA/pK3PTLuYJO+FlZ7wvGTIwGPbpGzVAJ+DUp6uof0zZjG6dcYsrGX864BMeUCj9R6lmkYZ5uY5lWQ==" - }, "fs-constants": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/fs-constants/-/fs-constants-1.0.0.tgz", @@ -120232,9 +120262,9 @@ } }, "nock": { - "version": "13.1.1", - "resolved": "https://registry.npmjs.org/nock/-/nock-13.1.1.tgz", - "integrity": "sha512-YKTR9MjfK3kS9/l4nuTxyYm30cgOExRHzkLNhL8nhEUyU4f8Za/dRxOqjhVT1vGs0svWo3dDnJTUX1qxYeWy5w==", + "version": "13.1.3", + "resolved": "https://registry.npmjs.org/nock/-/nock-13.1.3.tgz", + "integrity": "sha512-YKj0rKQWMGiiIO+Y65Ut8OEgYM3PplLU2+GAhnPmqZdBd6z5IskgdBqWmjzA6lH3RF0S2a3wiAlrMOF5Iv2Jeg==", "dev": true, "requires": { "debug": "^4.1.0", diff --git a/packages/@uppy/companion/package.json b/packages/@uppy/companion/package.json index 351cede41c..9391030f6d 100644 --- a/packages/@uppy/companion/package.json +++ b/packages/@uppy/companion/package.json @@ -45,7 +45,6 @@ "express-prom-bundle": "6.3.0", "express-request-id": "1.4.1", "express-session": "1.17.1", - "fs-capacitor": "^7.0.1", "grant": "4.7.0", "helmet": "^4.6.0", "ip-address": "6.2.0", @@ -88,6 +87,8 @@ "@types/uuid": "3.4.7", "@types/webpack": "^5.28.0", "@types/ws": "6.0.4", + "into-stream": "^6.0.0", + "nock": "^13.1.3", "supertest": "3.4.2", "typescript": "~4.3" }, diff --git a/packages/@uppy/companion/src/server/Uploader.js b/packages/@uppy/companion/src/server/Uploader.js index cf310f2b81..258aad8020 100644 --- a/packages/@uppy/companion/src/server/Uploader.js +++ b/packages/@uppy/companion/src/server/Uploader.js @@ -4,6 +4,16 @@ const isObject = require('isobject') // @ts-ignore const validator = require('validator') const request = require('request') +// eslint-disable-next-line no-unused-vars +const { Readable, pipeline: pipelineCb } = require('stream') +const { join } = require('path') +const fs = require('fs') +const { promisify } = require('util') + +const pipeline = promisify(pipelineCb) + +const { createReadStream, createWriteStream } = fs +const { stat, unlink } = fs.promises /** @type {any} */ // @ts-ignore - typescript resolves this this to a hoisted version of @@ -62,17 +72,20 @@ class Uploader { this.token = uuid.v4() this.options.metadata = this.options.metadata || {} this.options.fieldname = this.options.fieldname || DEFAULT_FIELD_NAME + this.uploadSize = options.size this.uploadFileName = this.options.metadata.name ? this.options.metadata.name.substring(0, MAX_FILENAME_LENGTH) : `${Uploader.FILE_NAME_PREFIX}-${this.token}` + this.uploadStopped = false - this.bytesWritten = 0 /** @type {number} */ this.emittedProgress = 0 this.storage = options.storage this._paused = false + this.readStream = null + if (this.options.protocol === PROTOCOLS.tus) { emitter().on(`pause:${this.token}`, () => { logger.debug('Received from client: pause', 'uploader', this.shortToken) @@ -83,7 +96,7 @@ class Uploader { }) emitter().on(`resume:${this.token}`, () => { - logger.debug('Received from client: cancel', 'uploader', this.shortToken) + logger.debug('Received from client: resume', 'uploader', this.shortToken) this._paused = false if (this.tus) { this.tus.start() @@ -98,57 +111,81 @@ class Uploader { const shouldTerminate = !!this.tus.url this.tus.abort(shouldTerminate).catch(() => {}) } - this.capacitor.destroy(new AbortError()) + this.uploadStopped = true + if (this.readStream) this.readStream.destroy(new AbortError()) }) } - async _uploadByProtocol (readStream) { + async _uploadByProtocol () { // @todo a default protocol should not be set. We should ensure that the user specifies their protocol. const protocol = this.options.protocol || PROTOCOLS.multipart switch (protocol) { case PROTOCOLS.multipart: - return this.uploadMultipart(readStream) + return this.uploadMultipart(this.readStream) case PROTOCOLS.s3Multipart: - return this.uploadS3Multipart(readStream) + return this.uploadS3Multipart(this.readStream) case PROTOCOLS.tus: - return this.uploadTus(readStream) + return this.uploadTus(this.readStream) default: throw new Error('Invalid protocol') } } - async _startUpload (readStream) { + // Some streams need to be downloaded entirely first, because we don't know their size from the provider + // This is true for zoom and drive (exported files) or some URL downloads. + // The stream will then typically come from a "Transfer-Encoding: chunked" response + async _prepareUnknownLengthStream (stream) { + this.tmpPath = join(this.options.pathPrefix, this.uploadFileName) + + logger.debug('fully downloading file', 'uploader.download', this.shortToken) + // TODO limit to 10MB? (which is max for google drive and zoom export) + // TODO progress? Maybe OK with no progress because files are small + const writeStream = createWriteStream(this.tmpPath) + await pipeline(stream, writeStream) + logger.debug('fully downloaded file', 'uploader.download', this.shortToken) + const { size } = await stat(this.tmpPath) + this.uploadSize = size + + const readStream = createReadStream(this.tmpPath) + this.readStream = readStream + } + + /** + * + * @param {Readable} stream + */ + async uploadStream (stream) { try { + if (this.uploadStopped) throw new Error('Cannot upload stream after upload stopped') + if (this.readStream) throw new Error('Already uploading') + + this.readStream = stream + if (!this.uploadSize) { + logger.debug('unable to determine file size, need to download the whole file first', 'controller.get.provider.size', this.shortToken) + await this._prepareUnknownLengthStream(this.readStream) + } + if (this.uploadStopped) return + const { url, extraData } = await Promise.race([ - this._uploadByProtocol(readStream), + this._uploadByProtocol(), // If we don't handle stream errors, we get unhandled error in node. - new Promise((resolve, reject) => this.capacitor.on('error', reject)), - new Promise((resolve, reject) => readStream.on('error', reject)), + new Promise((resolve, reject) => this.readStream.on('error', reject)), ]) this.emitSuccess(url, extraData) } catch (err) { - if (!(err instanceof AbortError)) { - // console.log(err) - logger.error(err, 'uploader.error', this.shortToken) - this.emitError(err, err.extraData) + if (err instanceof AbortError) { + logger.error('Aborted upload', 'uploader.aborted', this.shortToken) + return } + // console.log(err) + logger.error(err, 'uploader.error', this.shortToken) + this.emitError(err) } finally { this.cleanUp() } } - async initCapacitor () { - if (this.capacitor) throw new Error('Already initialized capacitor') - // Because it's an ESM so we cannot require - const { WriteStream } = await import('fs-capacitor') - - this.capacitor = new WriteStream({ tmpdir: () => this.options.pathPrefix }) - const readStream = this.capacitor.createReadStream() - - this._startUpload(readStream) - } - /** * returns a substring of the token. Used as traceId for logging * we avoid using the entire token because this is meant to be a short term @@ -280,49 +317,21 @@ class Uploader { } async awaitReady () { - logger.debug('waiting for connection', 'uploader.socket.wait', this.shortToken) + // TODO timeout after a while? Else we could leak emitters + logger.debug('waiting for socket connection', 'uploader.socket.wait', this.shortToken) await new Promise((resolve) => emitter().once(`connection:${this.token}`, resolve)) - await this.initCapacitor() + logger.debug('socket connection received', 'uploader.socket.wait', this.shortToken) } cleanUp () { - if (this.uploadStopped) return logger.debug('cleanup', this.shortToken) - this.capacitor.destroy() + if (this.readStream && !this.readStream.destroyed) this.readStream.destroy() + + if (this.tmpPath) unlink(this.tmpPath).catch(() => {}) emitter().removeAllListeners(`pause:${this.token}`) emitter().removeAllListeners(`resume:${this.token}`) emitter().removeAllListeners(`cancel:${this.token}`) - this.uploadStopped = true - } - - /** - * - * @param {Error} err - * @param {string | Buffer | Buffer[]} chunk - */ - handleChunk (err, chunk) { - if (this.uploadStopped) { - logger.debug('Received chunk after upload stopped', 'uploader.download', this.shortToken) - return - } - - if (err) { - logger.error(err, 'uploader.download.error', this.shortToken) - this.capacitor.destroy(err) - return - } - - if (chunk === null) { - // Finished! End the write stream so that the read streams will also finish. - this.capacitor.end() - return - } - - this.capacitor.write(chunk, () => { - this.bytesWritten += chunk.length - logger.debug(`${this.bytesWritten} bytes`, 'uploader.download.progress', this.shortToken) - }) } getResponse () { @@ -347,7 +356,7 @@ class Uploader { * @param {number | null} bytesTotalIn */ onProgress (bytesUploaded, bytesTotalIn) { - const bytesTotal = bytesTotalIn || this.options.size + const bytesTotal = bytesTotalIn || this.uploadSize const percentage = Math.min(Math.max(0, ((bytesUploaded / bytesTotal) * 100)), 100) const formatPercentage = percentage.toFixed(2) @@ -392,15 +401,15 @@ class Uploader { /** * * @param {Error} err - * @param {object} extraData */ - emitError (err, extraData = {}) { + emitError (err) { const serializedErr = serializeError(err) // delete stack to avoid sending server info to client delete serializedErr.stack const dataToEmit = { action: 'error', - payload: Object.assign(extraData, { error: serializedErr }), + // @ts-ignore + payload: Object.assign(err.extraData || {}, { error: serializedErr }), } this.saveState(dataToEmit) emitter().emit(this.token, dataToEmit) @@ -418,7 +427,7 @@ class Uploader { uploadUrl: this.options.uploadUrl, uploadLengthDeferred: false, retryDelays: [0, 1000, 3000, 5000], - uploadSize: this.options.size, + uploadSize: this.uploadSize, chunkSize: this.options.chunkSize || 50e6, headers: headerSanitize(this.options.headers), addRequestId: true, @@ -490,12 +499,12 @@ class Uploader { options: { filename: this.uploadFileName, contentType: this.options.metadata.type, - knownLength: this.options.size, + knownLength: this.uploadSize, }, }, } } else { - reqOptions.headers['content-length'] = this.options.size + reqOptions.headers['content-length'] = this.uploadSize reqOptions.body = stream } @@ -530,8 +539,8 @@ class Uploader { throw err } - if (bytesUploaded !== this.options.size) { - const errMsg = `uploaded only ${bytesUploaded} of ${this.options.size} with status: ${response.statusCode}` + if (bytesUploaded !== this.uploadSize) { + const errMsg = `uploaded only ${bytesUploaded} of ${this.uploadSize} with status: ${response.statusCode}` logger.error(errMsg, 'upload.multipart.mismatch.error') throw new Error(errMsg) } diff --git a/packages/@uppy/companion/src/server/controllers/deauth-callback.js b/packages/@uppy/companion/src/server/controllers/deauth-callback.js index 8037d7fcd4..0157b90673 100644 --- a/packages/@uppy/companion/src/server/controllers/deauth-callback.js +++ b/packages/@uppy/companion/src/server/controllers/deauth-callback.js @@ -1,20 +1,22 @@ const { errorToResponse } = require('../provider/error') -function deauthCallback ({ body, companion, headers }, res, next) { +async function deauthCallback ({ body, companion, headers }, res, next) { // we need the provider instance to decide status codes because // this endpoint does not cater to a uniform client. // It doesn't respond to Uppy client like other endpoints. // Instead it responds to the providers themselves. - companion.provider.deauthorizationCallback({ companion, body, headers }, (err, data, status) => { - if (err) { - const errResp = errorToResponse(err) - if (errResp) { - return res.status(errResp.code).json({ message: errResp.message }) - } - return next(err) + try { + const { data, status } = await companion.provider.deauthorizationCallback({ companion, body, headers }) + res.status(status || 200).json(data) + return + } catch (err) { + const errResp = errorToResponse(err) + if (errResp) { + res.status(errResp.code).json({ message: errResp.message }) + return } - return res.status(status || 200).json(data) - }) + next(err) + } } module.exports = deauthCallback diff --git a/packages/@uppy/companion/src/server/controllers/get.js b/packages/@uppy/companion/src/server/controllers/get.js index f383f3e347..a8e0db58bb 100644 --- a/packages/@uppy/companion/src/server/controllers/get.js +++ b/packages/@uppy/companion/src/server/controllers/get.js @@ -2,25 +2,13 @@ const Uploader = require('../Uploader') const logger = require('../logger') const { errorToResponse } = require('../provider/error') -function get (req, res, next) { +async function get (req, res) { const { id } = req.params const token = req.companion.providerToken const { provider } = req.companion - // get the file size before proceeding - provider.size({ id, token, query: req.query }, (err, size) => { - if (err) { - const errResp = errorToResponse(err) - if (errResp) { - return res.status(errResp.code).json({ message: errResp.message }) - } - return next(err) - } - - if (!size) { - logger.error('unable to determine file size', 'controller.get.provider.size', req.id) - return res.status(400).json({ message: 'unable to determine file size' }) - } + try { + const size = await provider.size({ id, token, query: req.query }) logger.debug('Instantiating uploader.', null, req.id) const uploader = new Uploader(Uploader.reqToOptions(req, size)) @@ -31,17 +19,33 @@ function get (req, res, next) { return } - // wait till the client has connected to the socket, before starting - // the download, so that the client can receive all download/upload progress. - logger.debug('Waiting for socket connection before beginning remote download.', null, req.id) - uploader.awaitReady().then(() => { - logger.debug('Socket connection received. Starting remote download.', null, req.id) - provider.download({ id, token, query: req.query }, uploader.handleChunk.bind(uploader)) - }).catch((err2) => logger.error(err2, req.id)) + const { stream } = await provider.download({ id, token, query: req.query }) + + // "Forking" off the upload operation to background, so we can return the http request: + ;(async () => { + // wait till the client has connected to the socket, before starting + // the download, so that the client can receive all download/upload progress. + logger.debug('Waiting for socket connection before beginning remote download/upload.', null, req.id) + await uploader.awaitReady() + logger.debug('Socket connection received. Starting remote download/upload.', null, req.id) + uploader.uploadStream(stream) + })() + + // Respond the request + // NOTE: Uploader will continue running after the http request is responded const response = uploader.getResponse() res.status(response.status).json(response.body) - }) + } catch (err) { + const errResp = errorToResponse(err) + if (errResp) { + res.status(errResp.code).json({ message: errResp.message }) + return + } + + logger.error(err, 'controller.get.error', req.id) + res.status(400).json({ message: 'Failed to download file' }) + } } module.exports = get diff --git a/packages/@uppy/companion/src/server/controllers/list.js b/packages/@uppy/companion/src/server/controllers/list.js index 2d096f6675..f539442548 100644 --- a/packages/@uppy/companion/src/server/controllers/list.js +++ b/packages/@uppy/companion/src/server/controllers/list.js @@ -1,18 +1,19 @@ const { errorToResponse } = require('../provider/error') -function list ({ query, params, companion }, res, next) { +async function list ({ query, params, companion }, res, next) { const token = companion.providerToken - companion.provider.list({ companion, token, directory: params.id, query }, (err, data) => { - if (err) { - const errResp = errorToResponse(err) - if (errResp) { - return res.status(errResp.code).json({ message: errResp.message }) - } - return next(err) + try { + const data = await companion.provider.list({ companion, token, directory: params.id, query }) + res.json(data) + } catch (err) { + const errResp = errorToResponse(err) + if (errResp) { + res.status(errResp.code).json({ message: errResp.message }) + return } - return res.json(data) - }) + next(err) + } } module.exports = list diff --git a/packages/@uppy/companion/src/server/controllers/logout.js b/packages/@uppy/companion/src/server/controllers/logout.js index 7a67b68dce..db9bf9a9bd 100644 --- a/packages/@uppy/companion/src/server/controllers/logout.js +++ b/packages/@uppy/companion/src/server/controllers/logout.js @@ -6,7 +6,7 @@ const { errorToResponse } = require('../provider/error') * @param {object} req * @param {object} res */ -function logout (req, res, next) { +async function logout (req, res, next) { const cleanSession = () => { if (req.session.grant) { req.session.grant.state = null @@ -16,24 +16,26 @@ function logout (req, res, next) { const { providerName } = req.params const { companion } = req const token = companion.providerTokens ? companion.providerTokens[providerName] : null - if (token) { - companion.provider.logout({ token, companion }, (err, data) => { - if (err) { - const errResp = errorToResponse(err) - if (errResp) { - return res.status(errResp.code).json({ message: errResp.message }) - } - return next(err) - } - delete companion.providerTokens[providerName] - tokenService.removeFromCookies(res, companion.options, companion.provider.authProvider) - cleanSession() - res.json({ ok: true, ...data }) - }) - } else { + if (!token) { cleanSession() res.json({ ok: true, revoked: false }) + return + } + + try { + const data = await companion.provider.logout({ token, companion }) + delete companion.providerTokens[providerName] + tokenService.removeFromCookies(res, companion.options, companion.provider.authProvider) + cleanSession() + res.json({ ok: true, ...data }) + } catch (err) { + const errResp = errorToResponse(err) + if (errResp) { + res.status(errResp.code).json({ message: errResp.message }) + return + } + next(err) } } diff --git a/packages/@uppy/companion/src/server/controllers/thumbnail.js b/packages/@uppy/companion/src/server/controllers/thumbnail.js index 7fa670b5cc..1fe4d95343 100644 --- a/packages/@uppy/companion/src/server/controllers/thumbnail.js +++ b/packages/@uppy/companion/src/server/controllers/thumbnail.js @@ -3,22 +3,23 @@ * @param {object} req * @param {object} res */ -function thumbnail (req, res, next) { +async function thumbnail (req, res, next) { const { providerName } = req.params const { id } = req.params const token = req.companion.providerTokens[providerName] const { provider } = req.companion - provider.thumbnail({ id, token }, (err, response) => { - if (err) { - if (err.isAuthError) res.sendStatus(401) - else next(err) - } else if (response) { + try { + const response = await provider.thumbnail({ id, token }) + if (response) { response.pipe(res) } else { res.sendStatus(404) } - }) + } catch (err) { + if (err.isAuthError) res.sendStatus(401) + else next(err) + } } module.exports = thumbnail diff --git a/packages/@uppy/companion/src/server/controllers/url.js b/packages/@uppy/companion/src/server/controllers/url.js index 44db6b7831..e93c52f225 100644 --- a/packages/@uppy/companion/src/server/controllers/url.js +++ b/packages/@uppy/companion/src/server/controllers/url.js @@ -6,6 +6,7 @@ const validator = require('validator') const Uploader = require('../Uploader') const { getURLMeta, getRedirectEvaluator, getProtectedHttpAgent } = require('../helpers/request') const logger = require('../logger') +const { errorToResponse } = require('../provider/error') /** * Validates that the download URL is secure @@ -41,11 +42,11 @@ const validateURL = (url, debug) => { * to the callback chunk by chunk. * * @param {string} url - * @param {downloadCallback} onDataChunk * @param {boolean} blockLocalIPs * @param {string} traceId + * @returns {Promise} */ -const downloadURL = (url, onDataChunk, blockLocalIPs, traceId) => { +const downloadURL = async (url, blockLocalIPs, traceId) => { const opts = { uri: url, method: 'GET', @@ -55,20 +56,24 @@ const downloadURL = (url, onDataChunk, blockLocalIPs, traceId) => { // return onDataChunk(new Error('test error')) - request(opts) - .on('response', (resp) => { - if (resp.statusCode >= 300) { - const err = new Error(`URL server responded with status: ${resp.statusCode}`) - onDataChunk(err, null) - } else { - resp.on('data', (chunk) => onDataChunk(null, chunk)) - } - }) - .on('end', () => onDataChunk(null, null)) - .on('error', (err) => { - logger.error(err, 'controller.url.download.error', traceId) - onDataChunk(err, null) - }) + return new Promise((resolve, reject) => { + request(opts) + .on('response', (resp) => { + if (resp.statusCode >= 300) { + reject(new Error(`URL server responded with status: ${resp.statusCode}`)) + return + } + + // Don't allow any more data to flow yet. + // https://github.com/request/request/issues/1990#issuecomment-184712275 + resp.pause() + resolve(resp) + }) + .on('error', (err) => { + logger.error(err, 'controller.url.download.error', traceId) + reject(err) + }) + }) } /** @@ -114,7 +119,6 @@ const get = async (req, res) => { const { size } = await getURLMeta(req.body.url, !debug) - // @ts-ignore logger.debug('Instantiating uploader.', null, req.id) const uploader = new Uploader(Uploader.reqToOptions(req, size)) @@ -124,18 +128,31 @@ const get = async (req, res) => { return } - logger.debug('Waiting for socket connection before beginning remote download.', null, req.id) - uploader.awaitReady().then(() => { - logger.debug('Socket connection received. Starting remote download.', null, req.id) - downloadURL(req.body.url, uploader.handleChunk.bind(uploader), !debug, req.id) - }).catch((err2) => logger.error(err2, req.id)) + const stream = await downloadURL(req.body.url, !debug, req.id) - const response = uploader.getResponse() + // "Forking" off the upload operation to background, so we can return the http request: + ;(async () => { + // wait till the client has connected to the socket, before starting + // the download, so that the client can receive all download/upload progress. + logger.debug('Waiting for socket connection before beginning remote download/upload.', null, req.id) + await uploader.awaitReady() + logger.debug('Socket connection received. Starting remote download/upload.', null, req.id) + + uploader.uploadStream(stream) + })() + // Respond the request // NOTE: Uploader will continue running after the http request is responded + const response = uploader.getResponse() res.status(response.status).json(response.body) } catch (err) { - logger.error(err, 'controller.url.get.error', req.id) + const errResp = errorToResponse(err) + if (errResp) { + res.status(errResp.code).json({ message: errResp.message }) + return + } + + logger.error(err, 'controller.url.error', req.id) // @todo send more meaningful error message and status code to client if possible res.status(err.status || 500).json({ message: 'failed to fetch URL metadata' }) } diff --git a/packages/@uppy/companion/src/server/helpers/request.js b/packages/@uppy/companion/src/server/helpers/request.js index da312cd962..aa0b83565c 100644 --- a/packages/@uppy/companion/src/server/helpers/request.js +++ b/packages/@uppy/companion/src/server/helpers/request.js @@ -187,9 +187,12 @@ exports.getURLMeta = (url, blockLocalIPs = false) => { reject(new Error(`URL server responded with status: ${response.statusCode}`)) } else { req.abort() // No need to get the rest of the response, as we only want header + + // Can be undefined for unknown length URLs, e.g. transfer-encoding: chunked + const contentLength = parseInt(response.headers['content-length'], 10) resolve({ type: response.headers['content-type'], - size: parseInt(response.headers['content-length'], 10), + size: Number.isNaN(contentLength) ? null : contentLength, }) } }) diff --git a/packages/@uppy/companion/src/server/helpers/utils.js b/packages/@uppy/companion/src/server/helpers/utils.js index 12390ff33e..3fc69aeff2 100644 --- a/packages/@uppy/companion/src/server/helpers/utils.js +++ b/packages/@uppy/companion/src/server/helpers/utils.js @@ -130,3 +130,25 @@ module.exports.decrypt = (encrypted, secret) => { decrypted += decipher.final('utf8') return decrypted } + +// This is a helper that will wait for the headers of a request, +// then it will pause the response, so that the stream is ready to be attached/piped in the uploader. +// If we don't pause it will lose some data. +module.exports.requestStream = async (req, convertResponseToError) => { + 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) { + throw await convertResponseToError(resp) + } + + return { stream: resp } +} diff --git a/packages/@uppy/companion/src/server/provider/Provider.js b/packages/@uppy/companion/src/server/provider/Provider.js index ccf10516d0..373b1d403a 100644 --- a/packages/@uppy/companion/src/server/provider/Provider.js +++ b/packages/@uppy/companion/src/server/provider/Provider.js @@ -22,9 +22,9 @@ class Provider { * list the files and folders in the provider account * * @param {object} options - * @param {Function} cb + * @returns {Promise} */ - list (options, cb) { // eslint-disable-line no-unused-vars + async list (options) { // eslint-disable-line no-unused-vars throw new Error('method not implemented') } @@ -32,9 +32,9 @@ class Provider { * download a certain file from the provider account * * @param {object} options - * @param {Function} cb + * @returns {Promise} */ - download (options, cb) { // eslint-disable-line no-unused-vars + async download (options) { // eslint-disable-line no-unused-vars throw new Error('method not implemented') } @@ -42,9 +42,9 @@ class Provider { * return a thumbnail for a provider file * * @param {object} options - * @param {Function} cb + * @returns {Promise} */ - thumbnail (options, cb) { // eslint-disable-line no-unused-vars + async thumbnail (options) { // eslint-disable-line no-unused-vars throw new Error('method not implemented') } @@ -52,9 +52,9 @@ class Provider { * get the size of a certain file in the provider account * * @param {object} options - * @param {Function} cb + * @returns {Promise} */ - size (options, cb) { // eslint-disable-line no-unused-vars + async size (options) { // eslint-disable-line no-unused-vars throw new Error('method not implemented') } @@ -62,9 +62,9 @@ class Provider { * handle deauthorization notification from oauth providers * * @param {object} options - * @param {Function} cb + * @returns {Promise} */ - deauthorizationCallback (options, cb) { // eslint-disable-line no-unused-vars + async deauthorizationCallback (options) { // eslint-disable-line no-unused-vars // @todo consider doing something like cb(new NotImplementedError()) instead throw new Error('method not implemented') } diff --git a/packages/@uppy/companion/src/server/provider/SearchProvider.js b/packages/@uppy/companion/src/server/provider/SearchProvider.js index b2ea5e576f..c3a833417b 100644 --- a/packages/@uppy/companion/src/server/provider/SearchProvider.js +++ b/packages/@uppy/companion/src/server/provider/SearchProvider.js @@ -6,9 +6,9 @@ class SearchProvider { * list the files available based on the search query * * @param {object} options - * @param {Function} cb + * @returns {Promise} */ - list (options, cb) { // eslint-disable-line no-unused-vars + async list (options) { // eslint-disable-line no-unused-vars throw new Error('method not implemented') } @@ -16,9 +16,9 @@ class SearchProvider { * download a certain file from the provider files * * @param {object} options - * @param {Function} cb + * @returns {Promise} */ - download (options, cb) { // eslint-disable-line no-unused-vars + async download (options) { // eslint-disable-line no-unused-vars throw new Error('method not implemented') } @@ -26,9 +26,9 @@ class SearchProvider { * get the size of a certain file in the provider files * * @param {object} options - * @param {Function} cb + * @returns {Promise} */ - size (options, cb) { // eslint-disable-line no-unused-vars + async size (options) { // eslint-disable-line no-unused-vars throw new Error('method not implemented') } } diff --git a/packages/@uppy/companion/src/server/provider/box/index.js b/packages/@uppy/companion/src/server/provider/box/index.js index 2d5b4c7c7f..513d1d840d 100644 --- a/packages/@uppy/companion/src/server/provider/box/index.js +++ b/packages/@uppy/companion/src/server/provider/box/index.js @@ -2,9 +2,12 @@ const Provider = require('../Provider') const request = require('request') const purest = require('purest')({ request }) +const { promisify } = require('util') + const logger = require('../../logger') const adapter = require('./adapter') const { ProviderApiError, ProviderAuthError } = require('../error') +const { requestStream } = require('../../helpers/utils') const BOX_FILES_FIELDS = 'id,modified_at,name,permissions,size,type' const BOX_THUMBNAIL_SIZE = 256 @@ -35,13 +38,17 @@ class Box extends Provider { .request(done) } + async list (options) { + return promisify(this._list.bind(this))(options) + } + /** * Lists files and folders from Box API * * @param {object} options * @param {Function} done */ - list ({ directory, token, query, companion }, done) { + _list ({ directory, token, query, companion }, done) { const rootFolderID = '0' const path = `folders/${directory || rootFolderID}/items` @@ -66,26 +73,24 @@ class Box extends Provider { }) } - download ({ id, token }, onData) { - return this.client - .get(`files/${id}/content`) - .auth(token) - .request() - .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.box.download.error') - onData(err) - }) + async download ({ id, token }) { + try { + const req = this.client + .get(`files/${id}/content`) + .auth(token) + .request() + return await requestStream(req, async (res) => this._error(null, res)) + } catch (err) { + logger.error(err, 'provider.box.download.error') + throw err + } + } + + async thumbnail (options) { + return promisify(this._thumbnail.bind(this))(options) } - thumbnail ({ id, token }, done) { + _thumbnail ({ id, token }, done) { return this.client .get(`files/${id}/thumbnail.png`) .qs({ max_height: BOX_THUMBNAIL_SIZE, max_width: BOX_THUMBNAIL_SIZE }) @@ -120,7 +125,11 @@ class Box extends Provider { }) } - size ({ id, token }, done) { + async size (options) { + return promisify(this._size.bind(this))(options) + } + + _size ({ id, token }, done) { return this.client .get(`files/${id}`) .auth(token) @@ -130,11 +139,15 @@ class Box extends Provider { logger.error(err, 'provider.box.size.error') return done(err) } - done(null, parseInt(body.size)) + done(null, parseInt(body.size, 10)) }) } - logout ({ companion, token }, done) { + async logout (options) { + return promisify(this._logout.bind(this))(options) + } + + _logout ({ companion, token }, done) { const { key, secret } = companion.options.providerOptions.box return this.client diff --git a/packages/@uppy/companion/src/server/provider/drive/index.js b/packages/@uppy/companion/src/server/provider/drive/index.js index ef8d8231cc..d7967a896a 100644 --- a/packages/@uppy/companion/src/server/provider/drive/index.js +++ b/packages/@uppy/companion/src/server/provider/drive/index.js @@ -1,12 +1,13 @@ /* eslint-disable no-underscore-dangle */ -const { callbackify } = require('util') const request = require('request') const purest = require('purest')({ request }) +const { promisify } = require('util') const Provider = require('../Provider') const logger = require('../../logger') const adapter = require('./adapter') const { ProviderApiError, ProviderAuthError } = require('../error') +const { requestStream } = require('../../helpers/utils') const DRIVE_FILE_FIELDS = 'kind,id,imageMediaMetadata,name,mimeType,ownedByMe,permissions(role,emailAddress),size,modifiedTime,iconLink,thumbnailLink,teamDriveId,videoMediaMetadata,shortcutDetails(targetId,targetMimeType)' const DRIVE_FILES_FIELDS = `kind,nextPageToken,incompleteSearch,files(${DRIVE_FILE_FIELDS})` @@ -20,19 +21,15 @@ function sortByName (first, second) { return first.name.localeCompare(second.name) } -function waitForFailedResponse (resp) { - return new Promise((resolve, reject) => { +async function waitForFailedResponse (resp) { + const buf = await new Promise((resolve) => { let data = '' resp.on('data', (chunk) => { data += chunk - }).on('end', () => { - try { - resolve(JSON.parse(data.toString())) - } catch (error) { - reject(error) - } - }) + }).on('end', () => resolve(data)) + resp.resume() }) + return JSON.parse(buf.toString()) } function adaptData (listFilesResp, sharedDrivesResp, directory, query, showSharedWithMe) { @@ -104,7 +101,7 @@ class Drive extends Provider { return 'google' } - async _list (options) { + async list (options) { const directory = options.directory || 'root' const query = options.query || {} @@ -180,12 +177,7 @@ class Drive extends Provider { ) } - list (options, done) { - // @ts-ignore - callbackify(this._list.bind(this))(options, done) - } - - async stats ({ id, token }) { + async _stats ({ id, token }) { const getStats = async (statsOfId) => new Promise((resolve, reject) => { this.client .query() @@ -217,79 +209,59 @@ class Drive extends Provider { .request() } - download ({ id: idIn, token }, onData) { - this.stats({ id: idIn, token }) - .then(({ mimeType, id }) => { - let requestStream - if (adapter.isGsuiteFile(mimeType)) { - requestStream = this._exportGsuiteFile(id, token, adapter.getGsuiteExportType(mimeType)) - } else { - requestStream = this.client - .query() - .get(`files/${encodeURIComponent(id)}`) - .qs({ alt: 'media', supportsAllDrives: true }) - .auth(token) - .request() - } + async download ({ id: idIn, token }) { + try { + const { mimeType, id } = await this._stats({ id: idIn, token }) - requestStream - .on('response', (resp) => { - if (resp.statusCode !== 200) { - waitForFailedResponse(resp) - .then((jsonResp) => { - onData(this._error(null, { ...resp, body: jsonResp })) - }) - .catch((err2) => onData(this._error(err2, resp))) - } else { - resp.on('data', (chunk) => onData(null, chunk)) - } - }) - .on('end', () => onData(null, null)) - .on('error', (err2) => { - logger.error(err2, 'provider.drive.download.error') - onData(err2) - }) - }) - .catch((err) => { - logger.error(err, 'provider.drive.download.stats.error') - onData(err) + const req = adapter.isGsuiteFile(mimeType) + ? this._exportGsuiteFile(id, token, adapter.getGsuiteExportType(mimeType)) + : this.client + .query() + .get(`files/${encodeURIComponent(id)}`) + .qs({ alt: 'media', supportsAllDrives: true }) + .auth(token) + .request() + + return await requestStream(req, async (res) => { + try { + const jsonResp = await waitForFailedResponse(res) + return this._error(null, { ...res, body: jsonResp }) + } catch (err2) { + return this._error(err2, res) + } }) + } catch (err) { + logger.error(err, 'provider.drive.download.error') + throw err + } } // eslint-disable-next-line class-methods-use-this - thumbnail (_, done) { + async thumbnail () { // not implementing this because a public thumbnail from googledrive will be used instead - const err = new Error('call to thumbnail is not implemented') - logger.error(err, 'provider.drive.thumbnail.error') - return done(err) + logger.error('call to thumbnail is not implemented', 'provider.drive.thumbnail.error') + throw new Error('call to thumbnail is not implemented') } - async _size ({ id, token }) { + async size ({ id, token }) { try { - const body = await this.stats({ id, token }) - - if (adapter.isGsuiteFile(body.mimeType)) { - // 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 - return maxExportFileSize + const { mimeType, size } = await this._stats({ id, token }) + + if (adapter.isGsuiteFile(mimeType)) { + // GSuite file sizes cannot be predetermined (but are max 10MB) + // e.g. Transfer-Encoding: chunked + return undefined } - return parseInt(body.size, 10) + + return parseInt(size, 10) } catch (err) { logger.error(err, 'provider.drive.size.error') throw err } } - size (options, done) { - // @ts-ignore - callbackify(this._size.bind(this))(options, done) - } - - logout ({ token }, done) { - return this.client + _logout ({ token }, done) { + this.client .get('https://accounts.google.com/o/oauth2/revoke') .qs({ token }) .request((err, resp) => { @@ -302,6 +274,10 @@ class Drive extends Provider { }) } + async logout (options) { + return promisify(this._logout.bind(this))(options) + } + _error (err, resp) { if (resp) { const fallbackMessage = `request to ${this.authProvider} returned ${resp.statusCode}` diff --git a/packages/@uppy/companion/src/server/provider/dropbox/index.js b/packages/@uppy/companion/src/server/provider/dropbox/index.js index 051ff7914f..9a18381be0 100644 --- a/packages/@uppy/companion/src/server/provider/dropbox/index.js +++ b/packages/@uppy/companion/src/server/provider/dropbox/index.js @@ -2,9 +2,12 @@ const Provider = require('../Provider') const request = require('request') const purest = require('purest')({ request }) +const { promisify } = require('util') + const logger = require('../../logger') const adapter = require('./adapter') const { ProviderApiError, ProviderAuthError } = require('../error') +const { requestStream } = require('../../helpers/utils') // From https://www.dropbox.com/developers/reference/json-encoding: // @@ -45,6 +48,10 @@ class DropBox extends Provider { .request(done) } + async list (options) { + return promisify(this._list.bind(this))(options) + } + /** * Makes 2 requests in parallel - 1. to get files, 2. to get user email * it then waits till both requests are done before proceeding with the callback @@ -52,7 +59,7 @@ class DropBox extends Provider { * @param {object} options * @param {Function} done */ - list (options, done) { + _list (options, done) { let userInfoDone = false let statsDone = false let userInfo @@ -119,32 +126,30 @@ class DropBox extends Provider { .request(done) } - download ({ id, token }, onData) { - return this.client - .post('https://content.dropboxapi.com/2/files/download') - .options({ - version: '2', - headers: { - 'Dropbox-API-Arg': httpHeaderSafeJson({ path: `${id}` }), - }, - }) - .auth(token) - .request() - .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') - onData(err) - }) + async download ({ id, token }) { + try { + const req = this.client + .post('https://content.dropboxapi.com/2/files/download') + .options({ + version: '2', + headers: { + 'Dropbox-API-Arg': httpHeaderSafeJson({ path: `${id}` }), + }, + }) + .auth(token) + + return await requestStream(req, async (res) => this._error(null, res)) + } catch (err) { + logger.error(err, 'provider.dropbox.download.error') + throw err + } } - thumbnail ({ id, token }, done) { + async thumbnail (options) { + return promisify(this._thumbnail.bind(this))(options) + } + + _thumbnail ({ id, token }, done) { return this.client .post('https://content.dropboxapi.com/2/files/get_thumbnail') .options({ @@ -168,7 +173,11 @@ class DropBox extends Provider { }) } - size ({ id, token }, done) { + async size (options) { + return promisify(this._size.bind(this))(options) + } + + _size ({ id, token }, done) { return this.client .post('files/get_metadata') .options({ version: '2' }) @@ -180,11 +189,15 @@ class DropBox extends Provider { logger.error(err, 'provider.dropbox.size.error') return done(err) } - done(null, parseInt(body.size)) + done(null, parseInt(body.size, 10)) }) } - logout ({ token }, done) { + async logout (options) { + return promisify(this._logout.bind(this))(options) + } + + _logout ({ token }, done) { return this.client .post('auth/token/revoke') .options({ version: '2' }) diff --git a/packages/@uppy/companion/src/server/provider/error.js b/packages/@uppy/companion/src/server/provider/error.js index c68542da2f..716c30fa02 100644 --- a/packages/@uppy/companion/src/server/provider/error.js +++ b/packages/@uppy/companion/src/server/provider/error.js @@ -48,6 +48,8 @@ function errorToResponse (err) { return { code: 424, message: err.message } } } + + return undefined } module.exports = { ProviderAuthError, ProviderApiError, errorToResponse } diff --git a/packages/@uppy/companion/src/server/provider/facebook/index.js b/packages/@uppy/companion/src/server/provider/facebook/index.js index 6552748854..9b7ada872f 100644 --- a/packages/@uppy/companion/src/server/provider/facebook/index.js +++ b/packages/@uppy/companion/src/server/provider/facebook/index.js @@ -2,10 +2,13 @@ const Provider = require('../Provider') const request = require('request') const purest = require('purest')({ request }) +const { promisify } = require('util') + const { getURLMeta } = require('../../helpers/request') const logger = require('../../logger') const adapter = require('./adapter') const { ProviderApiError, ProviderAuthError } = require('../error') +const { requestStream } = require('../../helpers/utils') /** * Adapter for API https://developers.facebook.com/docs/graph-api/using-graph-api/ @@ -24,7 +27,11 @@ class Facebook extends Provider { return 'facebook' } - list ({ directory, token, query = { cursor: null } }, done) { + async list (options) { + return promisify(this._list.bind(this))(options) + } + + _list ({ directory, token, query = { cursor: null } }, done) { const qs = { fields: 'name,cover_photo,created_time,type', } @@ -79,43 +86,45 @@ class Facebook extends Provider { return sortedImages[sortedImages.length - 1].source } - download ({ id, token }, onData) { - return this.client - .get(`https://graph.facebook.com/${id}`) - .qs({ fields: 'images' }) - .auth(token) - .request((err, resp, body) => { - 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('response', (resp) => { - if (resp.statusCode !== 200) { - onData(this._error(null, resp)) - } else { - resp.on('data', (chunk) => onData(null, chunk)) + async download ({ id, token }) { + try { + const body1 = await new Promise((resolve, reject) => ( + this.client + .get(`https://graph.facebook.com/${id}`) + .qs({ fields: 'images' }) + .auth(token) + .request((err, resp, body) => { + if (err || resp.statusCode !== 200) { + err = this._error(err, resp) + logger.error(err, 'provider.facebook.download.error') + reject(err) + return } + resolve(body) }) - .on('end', () => onData(null, null)) - .on('error', (err) => { - logger.error(err, 'provider.facebook.download.url.error') - onData(err) - }) - }) + )) + + const url = this._getMediaUrl(body1) + const req = request(url) + return await requestStream(req, async (res) => this._error(null, res)) + } catch (err) { + logger.error(err, 'provider.facebook.download.url.error') + throw err + } } - thumbnail (_, done) { + // eslint-disable-next-line class-methods-use-this + async thumbnail () { // not implementing this because a public thumbnail from facebook will be used instead - const err = new Error('call to thumbnail is not implemented') - logger.error(err, 'provider.facebook.thumbnail.error') - return done(err) + logger.error('call to thumbnail is not implemented', 'provider.facebook.thumbnail.error') + throw new Error('call to thumbnail is not implemented') + } + + async size (options) { + return promisify(this._size.bind(this))(options) } - size ({ id, token }, done) { + _size ({ id, token }, done) { return this.client .get(`https://graph.facebook.com/${id}`) .qs({ fields: 'images' }) @@ -129,14 +138,18 @@ class Facebook extends Provider { getURLMeta(this._getMediaUrl(body)) .then(({ size }) => done(null, size)) - .catch((err) => { - logger.error(err, 'provider.facebook.size.error') - done() + .catch((err2) => { + logger.error(err2, 'provider.facebook.size.error') + done(err2) }) }) } - logout ({ token }, done) { + async logout (options) { + return promisify(this._logout.bind(this))(options) + } + + _logout ({ token }, done) { return this.client .delete('me/permissions') .auth(token) diff --git a/packages/@uppy/companion/src/server/provider/instagram/graph/index.js b/packages/@uppy/companion/src/server/provider/instagram/graph/index.js index 22d06c1eb9..c0f4826770 100644 --- a/packages/@uppy/companion/src/server/provider/instagram/graph/index.js +++ b/packages/@uppy/companion/src/server/provider/instagram/graph/index.js @@ -2,10 +2,13 @@ const Provider = require('../../Provider') const request = require('request') const purest = require('purest')({ request }) +const { promisify } = require('util') + const { getURLMeta } = require('../../../helpers/request') const logger = require('../../../logger') const adapter = require('./adapter') const { ProviderApiError, ProviderAuthError } = require('../../error') +const { requestStream } = require('../../../helpers/utils') /** * Adapter for API https://developers.facebook.com/docs/instagram-api/overview @@ -31,7 +34,11 @@ class Instagram extends Provider { return 'instagram' } - list ({ directory, token, query = { cursor: null } }, done) { + async list (options) { + return promisify(this._list.bind(this))(options) + } + + _list ({ directory, token, query = { cursor: null } }, done) { const qs = { fields: 'id,media_type,thumbnail_url,media_url,timestamp,children{media_type,media_url,thumbnail_url,timestamp}', } @@ -72,43 +79,43 @@ class Instagram extends Provider { }) } - download ({ id, token }, onData) { - return this.client - .get(`https://graph.instagram.com/${id}`) - .qs({ fields: 'media_url' }) - .auth(token) - .request((err, resp, body) => { - 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('response', (resp) => { - if (resp.statusCode !== 200) { - onData(this._error(null, resp)) - } else { - resp.on('data', (chunk) => onData(null, chunk)) + async download ({ id, token }) { + try { + const body1 = await new Promise((resolve, reject) => ( + this.client + .get(`https://graph.instagram.com/${id}`) + .qs({ fields: 'media_url' }) + .auth(token) + .request((err, resp, body) => { + if (err || resp.statusCode !== 200) { + err = this._error(err, resp) + logger.error(err, 'provider.instagram.download.error') + reject(err) + return } + resolve(body) }) - .on('end', () => onData(null, null)) - .on('error', (err) => { - logger.error(err, 'provider.instagram.download.url.error') - onData(err) - }) - }) + )) + + const req = request(body1.media_url) + return await requestStream(req, async (res) => this._error(null, res)) + } catch (err) { + logger.error(err, 'provider.instagram.download.url.error') + throw err + } } - thumbnail (_, done) { + async thumbnail () { // not implementing this because a public thumbnail from instagram will be used instead - const err = new Error('call to thumbnail is not implemented') - logger.error(err, 'provider.instagram.thumbnail.error') - return done(err) + logger.error('call to thumbnail is not implemented', 'provider.instagram.thumbnail.error') + throw new Error('call to thumbnail is not implemented') + } + + async size (options) { + return promisify(this._size.bind(this))(options) } - size ({ id, token }, done) { + _size ({ id, token }, done) { return this.client .get(`https://graph.instagram.com/${id}`) .qs({ fields: 'media_url' }) @@ -122,16 +129,16 @@ class Instagram extends Provider { getURLMeta(body.media_url) .then(({ size }) => done(null, size)) - .catch((err) => { - logger.error(err, 'provider.instagram.size.error') - done() + .catch((err2) => { + logger.error(err2, 'provider.instagram.size.error') + done(err2) }) }) } - logout (_, done) { + async logout () { // access revoke is not supported by Instagram's API - done(null, { revoked: false, manual_revoke_url: 'https://www.instagram.com/accounts/manage_access/' }) + return { revoked: false, manual_revoke_url: 'https://www.instagram.com/accounts/manage_access/' } } adaptData (res, username, directory, currentQuery) { diff --git a/packages/@uppy/companion/src/server/provider/onedrive/index.js b/packages/@uppy/companion/src/server/provider/onedrive/index.js index 69b32ec740..ce93c98c58 100644 --- a/packages/@uppy/companion/src/server/provider/onedrive/index.js +++ b/packages/@uppy/companion/src/server/provider/onedrive/index.js @@ -2,9 +2,12 @@ const Provider = require('../Provider') const request = require('request') const purest = require('purest')({ request }) +const { promisify } = require('util') + const logger = require('../../logger') const adapter = require('./adapter') const { ProviderApiError, ProviderAuthError } = require('../error') +const { requestStream } = require('../../helpers/utils') /** * Adapter for API https://docs.microsoft.com/en-us/onedrive/developer/rest-api/ @@ -30,6 +33,10 @@ class OneDrive extends Provider { .request(done) } + async list (options) { + return promisify(this._list.bind(this))(options) + } + /** * Makes 2 requests in parallel - 1. to get files, 2. to get user email * it then waits till both requests are done before proceeding with the callback @@ -37,7 +44,7 @@ class OneDrive extends Provider { * @param {object} options * @param {Function} done */ - list ({ directory, query, token }, done) { + _list ({ directory, query, token }, done) { const path = directory ? `items/${directory}` : 'root' const rootPath = query.driveId ? `/drives/${query.driveId}` : '/me/drive' const qs = { $expand: 'thumbnails' } @@ -66,34 +73,33 @@ class OneDrive extends Provider { }) } - download ({ id, token, query }, onData) { - const rootPath = query.driveId ? `/drives/${query.driveId}` : '/me/drive' - return this.client - .get(`${rootPath}/items/${id}/content`) - .auth(token) - .request() - .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') - onData(err) - }) + async download ({ id, token, query }) { + try { + const rootPath = query.driveId ? `/drives/${query.driveId}` : '/me/drive' + + const req = this.client + .get(`${rootPath}/items/${id}/content`) + .auth(token) + .request() + + return await requestStream(req, async (res) => this._error(null, res)) + } catch (err) { + logger.error(err, 'provider.onedrive.download.error') + throw err + } } - thumbnail (_, done) { + async thumbnail () { // not implementing this because a public thumbnail from onedrive will be used instead - const err = new Error('call to thumbnail is not implemented') - logger.error(err, 'provider.onedrive.thumbnail.error') - return done(err) + logger.error('call to thumbnail is not implemented', 'provider.onedrive.thumbnail.error') + throw new Error('call to thumbnail is not implemented') + } + + async size (options) { + return promisify(this._size.bind(this))(options) } - size ({ id, query, token }, done) { + _size ({ id, query, token }, done) { const rootPath = query.driveId ? `/drives/${query.driveId}` : '/me/drive' return this.client .get(`${rootPath}/items/${id}`) @@ -108,9 +114,8 @@ class OneDrive extends Provider { }) } - logout (_, done) { - // access revoke is not supported by Microsoft/OneDrive's API - done(null, { revoked: false, manual_revoke_url: 'https://account.live.com/consent/Manage' }) + async logout () { + return { revoked: false, manual_revoke_url: 'https://account.live.com/consent/Manage' } } adaptData (res, username) { diff --git a/packages/@uppy/companion/src/server/provider/unsplash/index.js b/packages/@uppy/companion/src/server/provider/unsplash/index.js index b0dadd916c..21f9997bf8 100644 --- a/packages/@uppy/companion/src/server/provider/unsplash/index.js +++ b/packages/@uppy/companion/src/server/provider/unsplash/index.js @@ -1,9 +1,12 @@ const request = require('request') +const { promisify } = require('util') + const SearchProvider = require('../SearchProvider') const { getURLMeta } = require('../../helpers/request') const logger = require('../../logger') const adapter = require('./adapter') const { ProviderApiError } = require('../error') +const { requestStream } = require('../../helpers/utils') const BASE_URL = 'https://api.unsplash.com' @@ -11,7 +14,11 @@ const BASE_URL = 'https://api.unsplash.com' * Adapter for API https://api.unsplash.com */ class Unsplash extends SearchProvider { - list ({ token, query = { cursor: null, q: null } }, done) { + async list (options) { + return promisify(this._list.bind(this))(options) + } + + _list ({ token, query = { cursor: null, q: null } }, done) { const reqOpts = { url: `${BASE_URL}/search/photos`, method: 'GET', @@ -39,42 +46,42 @@ class Unsplash extends SearchProvider { }) } - download ({ id, token }, onData) { - const reqOpts = { - url: `${BASE_URL}/photos/${id}`, - method: 'GET', - json: true, - headers: { - Authorization: `Client-ID ${token}`, - }, - } - - request(reqOpts, (err, resp, body) => { - if (err || resp.statusCode !== 200) { - err = this._error(err, resp) - logger.error(err, 'provider.unsplash.download.error') - onData(err) - return + async download ({ id, token }) { + try { + const reqOpts = { + url: `${BASE_URL}/photos/${id}`, + method: 'GET', + json: true, + headers: { + Authorization: `Client-ID ${token}`, + }, } - const url = body.links.download - request.get(url) - .on('response', (resp) => { - if (resp.statusCode !== 200) { - onData(this._error(null, resp)) - } else { - resp.on('data', (chunk) => onData(null, chunk)) + const url = await new Promise((resolve, reject) => ( + request(reqOpts, (err, resp, body) => { + if (err || resp.statusCode !== 200) { + err = this._error(err, resp) + logger.error(err, 'provider.unsplash.download.error') + reject(err) + return } + resolve(body.links.download) }) - .on('end', () => onData(null, null)) - .on('error', (err) => { - logger.error(err, 'provider.unsplash.download.url.error') - onData(err) - }) - }) + )) + + const req = request.get(url) + return await requestStream(req, async (res) => this._error(null, res)) + } catch (err) { + logger.error(err, 'provider.unsplash.download.url.error') + throw err + } + } + + async size (options) { + return promisify(this._size.bind(this))(options) } - size ({ id, token }, done) { + _size ({ id, token }, done) { const reqOpts = { url: `${BASE_URL}/photos/${id}`, method: 'GET', @@ -94,9 +101,9 @@ class Unsplash extends SearchProvider { getURLMeta(body.links.download) .then(({ size }) => done(null, size)) - .catch((err) => { - logger.error(err, 'provider.unsplash.size.error') - done() + .catch((err2) => { + logger.error(err2, 'provider.unsplash.size.error') + done(err2) }) }) } diff --git a/packages/@uppy/companion/src/server/provider/zoom/index.js b/packages/@uppy/companion/src/server/provider/zoom/index.js index ce6fffbffe..06165db398 100644 --- a/packages/@uppy/companion/src/server/provider/zoom/index.js +++ b/packages/@uppy/companion/src/server/provider/zoom/index.js @@ -1,11 +1,13 @@ const Provider = require('../Provider') +const { promisify } = require('util') const request = require('request') const moment = require('moment-timezone') const purest = require('purest')({ request }) const logger = require('../../logger') const adapter = require('./adapter') const { ProviderApiError, ProviderAuthError } = require('../error') +const { requestStream } = require('../../helpers/utils') const BASE_URL = 'https://zoom.us/v2' const GET_LIST_PATH = '/users/me/recordings' @@ -31,7 +33,11 @@ class Zoom extends Provider { return 'zoom' } - list (options, done) { + async list (options) { + return promisify(this._list.bind(this))(options) + } + + _list (options, done) { /* - returns list of months by default - drill down for specific files in each month @@ -102,54 +108,54 @@ class Zoom extends Provider { .request(done) } - download ({ id, token, query }, done) { - // meeting id + file id required - // cc files don't have an ID or size - const meetingId = id - const fileId = query.recordingId - const { recordingStart } = query - const GET_MEETING_FILES = `/meetings/${encodeURIComponent(meetingId)}/recordings` + async download ({ id, token, query }) { + try { + // meeting id + file id required + // cc files don't have an ID or size + const meetingId = id + const fileId = query.recordingId + const { recordingStart } = query + const GET_MEETING_FILES = `/meetings/${encodeURIComponent(meetingId)}/recordings` + + const downloadUrl = await new Promise((resolve, reject) => { + this.client + .get(`${BASE_URL}${GET_MEETING_FILES}`) + .auth(token) + .request((err, resp) => { + if (err || resp.statusCode !== 200) { + const error = this._error(null, resp) + reject(error) + return + } + const file = resp + .body + .recording_files + .find(file => fileId === file.id || (file.file_type === fileId && file.recording_start === recordingStart)) + if (!file || !file.download_url) { + const error = this._error(null, resp) + reject(error) + return + } + resolve(file.download_url) + }) + }) - const downloadUrlPromise = new Promise((resolve) => { - this.client - .get(`${BASE_URL}${GET_MEETING_FILES}`) - .auth(token) - .request((err, resp) => { - if (err || resp.statusCode !== 200) { - return this._downloadError(resp, done) - } - const file = resp - .body - .recording_files - .find(file => fileId === file.id || (file.file_type === fileId && file.recording_start === recordingStart)) - if (!file || !file.download_url) { - return this._downloadError(resp, done) - } - resolve(file.download_url) - }) - }) - downloadUrlPromise.then((downloadUrl) => { - this.client + const req = this.client .get(`${downloadUrl}?access_token=${token}`) .request() - .on('response', (resp) => { - if (resp.statusCode !== 200) { - done(this._error(null, resp)) - } else { - resp.on('data', (chunk) => done(null, chunk)) - } - }) - .on('end', () => { - done(null, null) - }) - .on('error', (err) => { - logger.error(err, 'provider.zoom.download.error') - done(err) - }) - }) + + return await requestStream(req, async (res) => this._error(null, res)) + } catch (err) { + logger.error(err, 'provider.zoom.download.error') + throw err + } + } + + async size (options) { + return promisify(this._size.bind(this))(options) } - size ({ id, token, query }, done) { + _size ({ id, token, query }, done) { const meetingId = id const fileId = query.recordingId const { recordingStart } = query @@ -170,8 +176,7 @@ class Zoom extends Provider { if (!file) { return this._downloadError(resp, done) } - const maxExportFileSize = 10 * 1024 * 1024 // 10MB - done(null, file.file_size || maxExportFileSize) + done(null, file.file_size) // May be undefined. }) } @@ -254,7 +259,11 @@ class Zoom extends Provider { return data } - logout ({ companion, token }, done) { + async logout (options) { + return promisify(this._logout.bind(this))(options) + } + + _logout ({ companion, token }, done) { companion.getProviderCredentials().then(({ key, secret }) => { const encodedAuth = Buffer.from(`${key}:${secret}`, 'binary').toString('base64') return this.client @@ -276,15 +285,20 @@ class Zoom extends Provider { }).catch((err) => done(err)) } - deauthorizationCallback ({ companion, body, headers }, done) { + async deauthorizationCallback (options) { + return promisify(this._deauthorizationCallback.bind(this))(options) + } + + _deauthorizationCallback ({ companion, body, headers }, done) { if (!body || body.event !== DEAUTH_EVENT_NAME) { - return done(null, {}, 400) + done(null, { data: {}, status: 400 }) + return } companion.getProviderCredentials().then(({ verificationToken, key, secret }) => { const tokenSupplied = headers.authorization if (!tokenSupplied || verificationToken !== tokenSupplied) { - return done(null, {}, 400) + return done(null, { data: {}, status: 400 }) } const encodedAuth = Buffer.from(`${key}:${secret}`, 'binary').toString('base64') diff --git a/packages/@uppy/companion/test/__mocks__/purest.js b/packages/@uppy/companion/test/__mocks__/purest.js index ec3b770273..89102ea5a5 100644 --- a/packages/@uppy/companion/test/__mocks__/purest.js +++ b/packages/@uppy/companion/test/__mocks__/purest.js @@ -1,5 +1,6 @@ const fs = require('fs') const qs = require('querystring') + const fixtures = require('../fixtures').providers function has (object, property) { @@ -33,6 +34,14 @@ class MockPurest { return this } + _getStatusCode () { + const { validators } = fixtures[this.opts.providerName] + if (validators && validators[this._requestUrl]) { + return validators[this._requestUrl](this._requestOptions) ? 200 : 400 + } + return 200 + } + request (done) { if (typeof done === 'function') { const { responses } = fixtures[this.opts.providerName] @@ -40,14 +49,10 @@ class MockPurest { const endpointResponses = responses[url] || responses[this._requestUrl] if (endpointResponses == null || !has(endpointResponses, this._method)) { done(new Error(`No fixture for ${this._method} ${url}`)) - return + return this } - let statusCode = 200 - const { validators } = fixtures[this.opts.providerName] - if (validators && validators[this._requestUrl]) { - statusCode = validators[this._requestUrl](this._requestOptions) ? 200 : 400 - } + const statusCode = this._getStatusCode() const body = statusCode === 200 ? endpointResponses[this._method] : {} done(null, { body, statusCode }, body) @@ -58,7 +63,9 @@ class MockPurest { on (evt, cb) { if (evt === 'response') { - cb(fs.createReadStream('./README.md')) + const stream = fs.createReadStream('./README.md') + stream.statusCode = this._getStatusCode() + cb(stream) } return this } diff --git a/packages/@uppy/companion/test/__tests__/providers.js b/packages/@uppy/companion/test/__tests__/providers.js index 9e4ab149bc..57479bcc9a 100644 --- a/packages/@uppy/companion/test/__tests__/providers.js +++ b/packages/@uppy/companion/test/__tests__/providers.js @@ -10,9 +10,12 @@ jest.mock('../../src/server/helpers/request', () => { jest.mock('../../src/server/helpers/oauth-state', () => require('../mockoauthstate')()) const request = require('supertest') +const nock = require('nock') + const fixtures = require('../fixtures') const tokenService = require('../../src/server/helpers/jwt') const { getServer } = require('../mockserver') +const defaults = require('../fixtures/constants') const authServer = getServer() const OAUTH_STATE = 'some-cool-nice-encrytpion' @@ -37,6 +40,16 @@ const thisOrThat = (value1, value2) => { return value2 } +beforeAll(() => { + const url = new URL(defaults.THUMBNAIL_URL) + nock(url.origin).get(url.pathname).reply(200, () => '').persist() +}) + +afterAll(() => { + nock.cleanAll() + nock.restore() +}) + describe('set i-am header', () => { test.each(providerNames)('set i-am header in response (%s)', (providerName) => { const providerFixtures = fixtures.providers[providerName].expects diff --git a/packages/@uppy/companion/test/__tests__/uploader.js b/packages/@uppy/companion/test/__tests__/uploader.js index b496627b1b..e30b3467b2 100644 --- a/packages/@uppy/companion/test/__tests__/uploader.js +++ b/packages/@uppy/companion/test/__tests__/uploader.js @@ -2,18 +2,19 @@ jest.mock('tus-js-client') +const intoStream = require('into-stream') const fs = require('fs') + const Uploader = require('../../src/server/Uploader') const socketClient = require('../mocksocket') const standalone = require('../../src/standalone') +const { companionOptions } = standalone() + describe('uploader with tus protocol', () => { - test('upload functions with tus protocol', () => { - if (true) { - throw new Error('TODO this test hangs') - } + test('upload functions with tus protocol', async () => { const fileContent = Buffer.from('Some file content') - const { companionOptions } = standalone() + const stream = intoStream(fileContent) const opts = { companionOptions, endpoint: 'http://url.myendpoint.com/files', @@ -27,37 +28,88 @@ describe('uploader with tus protocol', () => { expect(uploader.hasError()).toBe(false) expect(uploadToken).toBeTruthy() - return new Promise((resolve) => { + return new Promise((resolve, reject) => { + // validate that the test is resolved on socket connection + uploader.awaitReady().then(() => { + uploader.uploadStream(stream).then(() => resolve()) + }) + + let progressReceived = 0 + // emulate socket connection + socketClient.connect(uploadToken) + socketClient.onProgress(uploadToken, (message) => { + progressReceived = message.payload.bytesUploaded + try { + expect(message.payload.bytesTotal).toBe(fileContent.length) + } catch (err) { + reject(err) + } + }) + socketClient.onUploadSuccess(uploadToken, (message) => { + try { + expect(progressReceived).toBe(fileContent.length) + // see __mocks__/tus-js-client.js + expect(message.payload.url).toBe('https://tus.endpoint/files/foo-bar') + } catch (err) { + reject(err) + } + }) + }) + }) + + test('upload functions with tus protocol without size', async () => { + const fileContent = Buffer.alloc(1e6) + const stream = intoStream(fileContent) + const opts = { + companionOptions, + endpoint: 'http://url.myendpoint.com/files', + protocol: 'tus', + size: null, + pathPrefix: companionOptions.filePath, + } + + const uploader = new Uploader(opts) + const uploadToken = uploader.token + expect(uploader.hasError()).toBe(false) + expect(uploadToken).toBeTruthy() + + return new Promise((resolve, reject) => { // validate that the test is resolved on socket connection uploader.awaitReady().then(() => { - const fileInfo = fs.statSync(uploader.path) - expect(fileInfo.isFile()).toBe(true) - expect(fileInfo.size).toBe(0) - uploader.handleChunk(null, fileContent) - uploader.handleChunk(null, null) - }).catch((err2) => console.error(err2)) + uploader.uploadStream(stream).then(() => { + try { + expect(fs.existsSync(uploader.path)).toBe(false) + resolve() + } catch (err) { + reject(err) + } + }) + }) let progressReceived = 0 // emulate socket connection socketClient.connect(uploadToken) socketClient.onProgress(uploadToken, (message) => { // validate that the file has been downloaded and saved into the file path - const fileInfo = fs.statSync(uploader.path) - expect(fileInfo.isFile()).toBe(true) - expect(fileInfo.size).toBe(fileContent.length) + try { + const fileInfo = fs.statSync(uploader.tmpPath) + expect(fileInfo.isFile()).toBe(true) + expect(fileInfo.size).toBe(fileContent.length) - progressReceived = message.payload.bytesUploaded - expect(message.payload.bytesTotal).toBe(fileContent.length) + progressReceived = message.payload.bytesUploaded + expect(message.payload.bytesTotal).toBe(fileContent.length) + } catch (err) { + reject(err) + } }) socketClient.onUploadSuccess(uploadToken, (message) => { - expect(progressReceived).toBe(fileContent.length) - // see __mocks__/tus-js-client.js - expect(message.payload.url).toBe('https://tus.endpoint/files/foo-bar') - setTimeout(() => { - // check that file has been cleaned up - expect(fs.existsSync(uploader.path)).toBe(false) - resolve() - }, 100) + try { + expect(progressReceived).toBe(fileContent.length) + // see __mocks__/tus-js-client.js + expect(message.payload.url).toBe('https://tus.endpoint/files/foo-bar') + } catch (err) { + reject(err) + } }) }) }) diff --git a/packages/@uppy/companion/test/__tests__/url.js b/packages/@uppy/companion/test/__tests__/url.js index a3c1d0d8a3..201518cdad 100644 --- a/packages/@uppy/companion/test/__tests__/url.js +++ b/packages/@uppy/companion/test/__tests__/url.js @@ -1,8 +1,12 @@ /* global jest:false, test:false, expect:false, describe:false */ +const nock = require('nock') +const request = require('supertest') + jest.mock('tus-js-client') jest.mock('../../src/server/helpers/request', () => { return { + ...jest.requireActual('../../src/server/helpers/request'), getURLMeta: () => { return Promise.resolve({ size: 7580, type: 'image/jpg' }) }, @@ -11,7 +15,15 @@ jest.mock('../../src/server/helpers/request', () => { const { getServer } = require('../mockserver') const mockServer = getServer() -const request = require('supertest') + +beforeAll(() => { + nock('http://url.myendpoint.com').get('/files').reply(200, () => '') +}) + +afterAll(() => { + nock.cleanAll() + nock.restore() +}) const invalids = [ // no url at all or unsupported protocol diff --git a/packages/@uppy/companion/test/fixtures/facebook.js b/packages/@uppy/companion/test/fixtures/facebook.js index 20b09bcd71..ff2ebf159f 100644 --- a/packages/@uppy/companion/test/fixtures/facebook.js +++ b/packages/@uppy/companion/test/fixtures/facebook.js @@ -43,9 +43,6 @@ module.exports.responses = { id: defaults.ITEM_ID, }, }, - [defaults.THUMBNAIL_URL]: { - get: {}, - }, } module.exports.expects = { From 50cd8c309b559ce7164d64e050c3c4609fcb8b9c Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Tue, 7 Sep 2021 15:53:08 +0700 Subject: [PATCH 18/46] Implement streamingUpload flag COMPANION_STREAMING_UPLOAD Default to false due to backward compatibility If set to true, will start to upload files at the same time as dowlnoading them, by piping the streams - Also implement progress for downloading too - and fix progress duplication logic - fix test that assumed file was fully downloaded after first progress event --- packages/@uppy/companion/KUBERNETES.md | 1 + packages/@uppy/companion/env_example | 1 + packages/@uppy/companion/src/companion.js | 1 + .../@uppy/companion/src/server/Uploader.js | 86 ++++++++++++------- .../@uppy/companion/src/standalone/helper.js | 1 + .../companion/test/__tests__/uploader.js | 12 +-- test/endtoend/utils.js | 1 + website/src/docs/companion.md | 10 ++- 8 files changed, 77 insertions(+), 36 deletions(-) diff --git a/packages/@uppy/companion/KUBERNETES.md b/packages/@uppy/companion/KUBERNETES.md index 7853c330b5..c076328358 100644 --- a/packages/@uppy/companion/KUBERNETES.md +++ b/packages/@uppy/companion/KUBERNETES.md @@ -26,6 +26,7 @@ data: COMPANION_DOMAIN: "YOUR SERVER DOMAIN" COMPANION_DOMAINS: "sub1.domain.com,sub2.domain.com,sub3.domain.com" COMPANION_PROTOCOL: "YOUR SERVER PROTOCOL" + COMPANION_STREAMING_UPLOAD: true COMPANION_REDIS_URL: redis://:superSecretPassword@uppy-redis.uppy.svc.cluster.local:6379 COMPANION_SECRET: "shh!Issa Secret!" COMPANION_DROPBOX_KEY: "YOUR DROPBOX KEY" diff --git a/packages/@uppy/companion/env_example b/packages/@uppy/companion/env_example index a9ecf32257..1bddc277c4 100644 --- a/packages/@uppy/companion/env_example +++ b/packages/@uppy/companion/env_example @@ -4,6 +4,7 @@ COMPANION_DOMAIN=uppy.xxxx.com COMPANION_SELF_ENDPOINT=uppy.xxxx.com COMPANION_HIDE_METRICS=false COMPANION_HIDE_WELCOME=false +COMPANION_STREAMING_UPLOAD=true COMPANION_PROTOCOL=https COMPANION_DATADIR=/mnt/uppy-server-data diff --git a/packages/@uppy/companion/src/companion.js b/packages/@uppy/companion/src/companion.js index 0e636c2319..1a33753b20 100644 --- a/packages/@uppy/companion/src/companion.js +++ b/packages/@uppy/companion/src/companion.js @@ -39,6 +39,7 @@ const defaultOptions = { }, debug: true, logClientVersion: true, + streamingUpload: false, } // make the errors available publicly for custom providers diff --git a/packages/@uppy/companion/src/server/Uploader.js b/packages/@uppy/companion/src/server/Uploader.js index 258aad8020..137891d7b3 100644 --- a/packages/@uppy/companion/src/server/Uploader.js +++ b/packages/@uppy/companion/src/server/Uploader.js @@ -72,18 +72,19 @@ class Uploader { this.token = uuid.v4() this.options.metadata = this.options.metadata || {} this.options.fieldname = this.options.fieldname || DEFAULT_FIELD_NAME - this.uploadSize = options.size + this.size = options.size this.uploadFileName = this.options.metadata.name ? this.options.metadata.name.substring(0, MAX_FILENAME_LENGTH) : `${Uploader.FILE_NAME_PREFIX}-${this.token}` this.uploadStopped = false - /** @type {number} */ - this.emittedProgress = 0 + this.emittedProgress = {} this.storage = options.storage this._paused = false + this.downloadedBytes = 0 + this.readStream = null if (this.options.protocol === PROTOCOLS.tus) { @@ -132,23 +133,33 @@ class Uploader { } } - // Some streams need to be downloaded entirely first, because we don't know their size from the provider - // This is true for zoom and drive (exported files) or some URL downloads. - // The stream will then typically come from a "Transfer-Encoding: chunked" response - async _prepareUnknownLengthStream (stream) { + async _downloadStreamAsFile (stream) { this.tmpPath = join(this.options.pathPrefix, this.uploadFileName) logger.debug('fully downloading file', 'uploader.download', this.shortToken) // TODO limit to 10MB? (which is max for google drive and zoom export) - // TODO progress? Maybe OK with no progress because files are small const writeStream = createWriteStream(this.tmpPath) + + const onData = (chunk) => { + this.downloadedBytes += chunk.length + this.onProgress(0, undefined) + } + + stream.on('data', onData) + await pipeline(stream, writeStream) - logger.debug('fully downloaded file', 'uploader.download', this.shortToken) + logger.debug('finished fully downloading file', 'uploader.download', this.shortToken) + const { size } = await stat(this.tmpPath) - this.uploadSize = size - const readStream = createReadStream(this.tmpPath) - this.readStream = readStream + this.size = size + + const fileStream = createReadStream(this.tmpPath) + this.readStream = fileStream + } + + _needDownloadFirst () { + return !this.options.size || !this.options.companionOptions.streamingUpload } /** @@ -161,9 +172,12 @@ class Uploader { if (this.readStream) throw new Error('Already uploading') this.readStream = stream - if (!this.uploadSize) { - logger.debug('unable to determine file size, need to download the whole file first', 'controller.get.provider.size', this.shortToken) - await this._prepareUnknownLengthStream(this.readStream) + if (this._needDownloadFirst()) { + logger.debug('need to download the whole file first', 'controller.get.provider.size', this.shortToken) + // Some streams need to be downloaded entirely first, because we don't know their size from the provider + // This is true for zoom and drive (exported files) or some URL downloads. + // The stream will then typically come from a "Transfer-Encoding: chunked" response + await this._downloadStreamAsFile(this.readStream) } if (this.uploadStopped) return @@ -356,13 +370,23 @@ class Uploader { * @param {number | null} bytesTotalIn */ onProgress (bytesUploaded, bytesTotalIn) { - const bytesTotal = bytesTotalIn || this.uploadSize + const bytesTotal = bytesTotalIn || this.size || 0 + + // If fully downloading before uploading, combine downloaded and uploaded bytes + // This will make sure that the user sees half of the progress before upload starts (while downloading) + let combinedBytes = bytesUploaded + if (this._needDownloadFirst()) { + combinedBytes = Math.floor((combinedBytes + (this.downloadedBytes || 0)) / 2) + } + + // Prevent divide by zero + let percentage = 0 + if (bytesTotal > 0) percentage = Math.min(Math.max(0, ((combinedBytes / bytesTotal) * 100)), 100) - const percentage = Math.min(Math.max(0, ((bytesUploaded / bytesTotal) * 100)), 100) - const formatPercentage = percentage.toFixed(2) + const formattedPercentage = percentage.toFixed(2) logger.debug( - `${bytesUploaded} ${bytesTotal} ${formatPercentage}%`, - 'uploader.upload.progress', + `${combinedBytes} ${bytesTotal} ${formattedPercentage}%`, + 'uploader.total.progress', this.shortToken ) @@ -370,16 +394,20 @@ class Uploader { return } + const payload = { progress: formattedPercentage, bytesUploaded: combinedBytes, bytesTotal } const dataToEmit = { action: 'progress', - payload: { progress: formatPercentage, bytesUploaded, bytesTotal }, + payload, } this.saveState(dataToEmit) + const isEqual = (p1, p2) => (p1.progress === p2.progress + && p1.bytesUploaded === p2.bytesUploaded + && p1.bytesTotal === p2.bytesTotal) + // avoid flooding the client with progress events. - const roundedPercentage = Math.floor(percentage) - if (this.emittedProgress !== roundedPercentage) { - this.emittedProgress = roundedPercentage + if (!isEqual(this.emittedProgress, payload)) { + this.emittedProgress = payload emitter().emit(this.token, dataToEmit) } } @@ -427,7 +455,7 @@ class Uploader { uploadUrl: this.options.uploadUrl, uploadLengthDeferred: false, retryDelays: [0, 1000, 3000, 5000], - uploadSize: this.uploadSize, + uploadSize: this.size, chunkSize: this.options.chunkSize || 50e6, headers: headerSanitize(this.options.headers), addRequestId: true, @@ -499,12 +527,12 @@ class Uploader { options: { filename: this.uploadFileName, contentType: this.options.metadata.type, - knownLength: this.uploadSize, + knownLength: this.size, }, }, } } else { - reqOptions.headers['content-length'] = this.uploadSize + reqOptions.headers['content-length'] = this.size reqOptions.body = stream } @@ -539,8 +567,8 @@ class Uploader { throw err } - if (bytesUploaded !== this.uploadSize) { - const errMsg = `uploaded only ${bytesUploaded} of ${this.uploadSize} with status: ${response.statusCode}` + if (bytesUploaded !== this.size) { + const errMsg = `uploaded only ${bytesUploaded} of ${this.size} with status: ${response.statusCode}` logger.error(errMsg, 'upload.multipart.mismatch.error') throw new Error(errMsg) } diff --git a/packages/@uppy/companion/src/standalone/helper.js b/packages/@uppy/companion/src/standalone/helper.js index 0494344c36..4d3f6a67df 100644 --- a/packages/@uppy/companion/src/standalone/helper.js +++ b/packages/@uppy/companion/src/standalone/helper.js @@ -102,6 +102,7 @@ const getConfigFromEnv = () => { // cookieDomain is kind of a hack to support distributed systems. This should be improved but we never got so far. cookieDomain: process.env.COMPANION_COOKIE_DOMAIN, multipleInstances: true, + streamingUpload: process.env.COMPANION_STREAMING_UPLOAD === 'true', } } diff --git a/packages/@uppy/companion/test/__tests__/uploader.js b/packages/@uppy/companion/test/__tests__/uploader.js index e30b3467b2..d7f7c02617 100644 --- a/packages/@uppy/companion/test/__tests__/uploader.js +++ b/packages/@uppy/companion/test/__tests__/uploader.js @@ -92,12 +92,14 @@ describe('uploader with tus protocol', () => { socketClient.onProgress(uploadToken, (message) => { // validate that the file has been downloaded and saved into the file path try { - const fileInfo = fs.statSync(uploader.tmpPath) - expect(fileInfo.isFile()).toBe(true) - expect(fileInfo.size).toBe(fileContent.length) - progressReceived = message.payload.bytesUploaded - expect(message.payload.bytesTotal).toBe(fileContent.length) + + if (progressReceived === fileContent.length) { + const fileInfo = fs.statSync(uploader.tmpPath) + expect(fileInfo.isFile()).toBe(true) + expect(fileInfo.size).toBe(fileContent.length) + expect(message.payload.bytesTotal).toBe(fileContent.length) + } } catch (err) { reject(err) } diff --git a/test/endtoend/utils.js b/test/endtoend/utils.js index 05b1619557..720ed2c1dd 100644 --- a/test/endtoend/utils.js +++ b/test/endtoend/utils.js @@ -76,6 +76,7 @@ class CompanionService { COMPANION_DOMAIN: 'localhost:3030', COMPANION_PROTOCOL: 'http', COMPANION_PORT: 3030, + COMPANION_STREAMING_UPLOAD: true, COMPANION_SECRET: process.env.TEST_COMPANION_SECRET, COMPANION_DROPBOX_KEY: process.env.TEST_COMPANION_DROPBOX_KEY, COMPANION_DROPBOX_SECRET: process.env.TEST_COMPANION_DROPBOX_SECRET, diff --git a/website/src/docs/companion.md b/website/src/docs/companion.md index 3820eb1fb1..8610b8d403 100644 --- a/website/src/docs/companion.md +++ b/website/src/docs/companion.md @@ -36,7 +36,7 @@ Install from NPM: npm install @uppy/companion ``` -If you don't have a Node.js project with a `package.json` you might want to install/run Companion globally like so: `[sudo] npm install -g @uppy/companion@2.x`. +If you don't have a Node.js project with a `package.json` you might want to install/run Companion globally like so: `npm install -g @uppy/companion`. ### Prerequisite @@ -48,7 +48,7 @@ Unfortunately, Windows is not a supported platform right now. It may work, and w Companion may either be used as a pluggable express app, which you plug into your already existing server, or it may simply be run as a standalone server: -### Plugging into an already existing server +### Plugging into an existing express server To plug Companion into an existing server, call its `.app` method, passing in an [options](#Options) object as a parameter. This returns a server instance that you can mount on a subpath in your Express or app. @@ -238,6 +238,9 @@ export COMPANION_SELF_ENDPOINT="THIS SHOULD BE SAME AS YOUR DOMAIN + PATH" # comma-separated URLs # corresponds to the uploadUrls option export COMPANION_UPLOAD_URLS="http://tusd.tusdemo.net/files/,https://tusd.tusdemo.net/files/" + +# corresponds to the streamingUpload option +export COMPANION_STREAMING_UPLOAD=true ``` See [env.example.sh](https://github.com/transloadit/uppy/blob/master/env.example.sh) for an example configuration script. @@ -288,6 +291,7 @@ const options = { uploadUrls: ['https://myuploadurl.com', 'http://myuploadurl2.com'], debug: true, metrics: false, + streamingUpload: true, } ``` @@ -324,6 +328,8 @@ const options = { 13. **metrics(optional)** - A boolean flag to tell Companion whether or not to provide an endpoint `/metrics` with Prometheus metrics. +14. **streamingUpload(optional)** - A boolean flag to tell Companion whether or not to enable streaming uploads. If enabled, it will lead to **faster uploads* because companion will start uploading at the same time as downloading using `stream.pipe`. If `false`, files will be fully downloaded first, then uploaded. Defaults to `false`. + ### Provider Redirect URIs When generating your provider API keys on their corresponding developer platforms (e.g [Google Developer Console](https://console.developers.google.com/)), you'd need to provide a `redirect URI` for the OAuth authorization process. In general the redirect URI for each provider takes the format: From bd255aa31efcf5aab135c51ecf985ba210c8b3a0 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Tue, 7 Sep 2021 17:27:07 +0700 Subject: [PATCH 19/46] rearrange validation logic --- .../@uppy/companion/src/server/Uploader.js | 44 ++++++++++--------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/packages/@uppy/companion/src/server/Uploader.js b/packages/@uppy/companion/src/server/Uploader.js index 137891d7b3..12e4ed4738 100644 --- a/packages/@uppy/companion/src/server/Uploader.js +++ b/packages/@uppy/companion/src/server/Uploader.js @@ -286,13 +286,29 @@ class Uploader { // s3 uploads don't require upload destination // validation, because the destination is determined // by the server's s3 config - if (options.protocol === PROTOCOLS.s3Multipart) { - return true - } + if (options.protocol !== PROTOCOLS.s3Multipart) { + if (!options.endpoint && !options.uploadUrl) { + this._errRespMessage = 'no destination specified' + return false + } - if (!options.endpoint && !options.uploadUrl) { - this._errRespMessage = 'no destination specified' - return false + const validateUrl = (url) => { + const validatorOpts = { require_protocol: true, require_tld: !options.companionOptions.debug } + if (url && !validator.isURL(url, validatorOpts)) { + this._errRespMessage = 'invalid destination url' + return false + } + + const allowedUrls = options.companionOptions.uploadUrls + if (allowedUrls && url && !hasMatch(url, allowedUrls)) { + this._errRespMessage = 'upload destination does not match any allowed destinations' + return false + } + + return true + } + + if (![options.endpoint, options.uploadUrl].every(validateUrl)) return false } if (options.chunkSize != null && typeof options.chunkSize !== 'number') { @@ -300,21 +316,7 @@ class Uploader { return false } - const validatorOpts = { require_protocol: true, require_tld: !options.companionOptions.debug } - return [options.endpoint, options.uploadUrl].every((url) => { - if (url && !validator.isURL(url, validatorOpts)) { - this._errRespMessage = 'invalid destination url' - return false - } - - const allowedUrls = options.companionOptions.uploadUrls - if (allowedUrls && url && !hasMatch(url, allowedUrls)) { - this._errRespMessage = 'upload destination does not match any allowed destinations' - return false - } - - return true - }) + return true } hasError () { From e756b0aa0463c085f0267c8fdf643d4a4dc52883 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Tue, 7 Sep 2021 17:27:49 +0700 Subject: [PATCH 20/46] add COMPANION_STREAMING_UPLOAD to env.test.sh too --- packages/@uppy/companion/env.test.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/@uppy/companion/env.test.sh b/packages/@uppy/companion/env.test.sh index 02f94c9ef3..840e39a181 100644 --- a/packages/@uppy/companion/env.test.sh +++ b/packages/@uppy/companion/env.test.sh @@ -5,6 +5,8 @@ export COMPANION_SELF_ENDPOINT="localhost:3020" export COMPANION_HIDE_METRICS="false" export COMPANION_HIDE_WELCOME="false" +export COMPANION_STREAMING_UPLOAD="true" + export COMPANION_PROTOCOL="http" export COMPANION_DATADIR="./test/output" export COMPANION_SECRET="secret" From 91134725a510ec32419ab72c336ec4cd7e3ee944 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Tue, 7 Sep 2021 17:36:30 +0700 Subject: [PATCH 21/46] implement maxFileSize option in companion for both unknown length and known length downloads --- .../@uppy/companion/src/server/Uploader.js | 19 +++++-- .../@uppy/companion/src/standalone/helper.js | 1 + .../companion/test/__tests__/uploader.js | 52 +++++++++++++++++++ packages/@uppy/companion/test/mocksocket.js | 8 +++ website/src/docs/companion.md | 6 +++ 5 files changed, 83 insertions(+), 3 deletions(-) diff --git a/packages/@uppy/companion/src/server/Uploader.js b/packages/@uppy/companion/src/server/Uploader.js index 12e4ed4738..e86619328d 100644 --- a/packages/@uppy/companion/src/server/Uploader.js +++ b/packages/@uppy/companion/src/server/Uploader.js @@ -36,6 +36,10 @@ const PROTOCOLS = Object.freeze({ tus: 'tus', }) +function exceedsMaxFileSize (maxFileSize, size) { + return maxFileSize && size && size > maxFileSize +} + class AbortError extends Error {} class Uploader { @@ -112,11 +116,15 @@ class Uploader { const shouldTerminate = !!this.tus.url this.tus.abort(shouldTerminate).catch(() => {}) } - this.uploadStopped = true - if (this.readStream) this.readStream.destroy(new AbortError()) + this.abortReadStream(new AbortError()) }) } + abortReadStream (err) { + this.uploadStopped = true + if (this.readStream) this.readStream.destroy(err) + } + async _uploadByProtocol () { // @todo a default protocol should not be set. We should ensure that the user specifies their protocol. const protocol = this.options.protocol || PROTOCOLS.multipart @@ -137,11 +145,11 @@ class Uploader { this.tmpPath = join(this.options.pathPrefix, this.uploadFileName) logger.debug('fully downloading file', 'uploader.download', this.shortToken) - // TODO limit to 10MB? (which is max for google drive and zoom export) const writeStream = createWriteStream(this.tmpPath) const onData = (chunk) => { this.downloadedBytes += chunk.length + if (exceedsMaxFileSize(this.options.companionOptions.maxFileSize, this.downloadedBytes)) this.abortReadStream(new Error('maxFileSize exceeded')) this.onProgress(0, undefined) } @@ -258,6 +266,11 @@ class Uploader { } } + if (exceedsMaxFileSize(options.companionOptions.maxFileSize, options.size)) { + this._errRespMessage = 'maxFileSize exceeded' + return false + } + // validate fieldname if (options.fieldname && typeof options.fieldname !== 'string') { this._errRespMessage = 'fieldname must be a string' diff --git a/packages/@uppy/companion/src/standalone/helper.js b/packages/@uppy/companion/src/standalone/helper.js index 4d3f6a67df..484e0a8874 100644 --- a/packages/@uppy/companion/src/standalone/helper.js +++ b/packages/@uppy/companion/src/standalone/helper.js @@ -103,6 +103,7 @@ const getConfigFromEnv = () => { cookieDomain: process.env.COMPANION_COOKIE_DOMAIN, multipleInstances: true, streamingUpload: process.env.COMPANION_STREAMING_UPLOAD === 'true', + maxFileSize: process.env.COMPANION_MAX_FILE_SIZE ? parseInt(process.env.COMPANION_MAX_FILE_SIZE, 10) : undefined, } } diff --git a/packages/@uppy/companion/test/__tests__/uploader.js b/packages/@uppy/companion/test/__tests__/uploader.js index d7f7c02617..514a124046 100644 --- a/packages/@uppy/companion/test/__tests__/uploader.js +++ b/packages/@uppy/companion/test/__tests__/uploader.js @@ -115,4 +115,56 @@ describe('uploader with tus protocol', () => { }) }) }) + + test('uploader respects maxFileSize', async () => { + const opts = { + endpoint: 'http://url.myendpoint.com/files', + companionOptions: { ...companionOptions, maxFileSize: 100 }, + size: 101, + } + + const uploader = new Uploader(opts) + expect(uploader.hasError()).toBe(true) + }) + + test('uploader respects maxFileSize correctly', async () => { + const opts = { + endpoint: 'http://url.myendpoint.com/files', + companionOptions: { ...companionOptions, maxFileSize: 100 }, + size: 99, + } + + const uploader = new Uploader(opts) + expect(uploader.hasError()).toBe(false) + }) + + test('uploader respects maxFileSize with unknown size', async () => { + const fileContent = Buffer.alloc(10000) + const stream = intoStream(fileContent) + const opts = { + companionOptions: { ...companionOptions, maxFileSize: 1000 }, + endpoint: 'http://url.myendpoint.com/files', + protocol: 'tus', + size: null, + pathPrefix: companionOptions.filePath, + } + + const uploader = new Uploader(opts) + const uploadToken = uploader.token + expect(uploader.hasError()).toBe(false) + + return new Promise((resolve, reject) => { + // validate that the test is resolved on socket connection + uploader.awaitReady().then(uploader.uploadStream(stream)) + socketClient.connect(uploadToken) + socketClient.onUploadError(uploadToken, (message) => { + try { + expect(message).toMatchObject({ payload: { error: { message: 'maxFileSize exceeded' } } }) + resolve() + } catch (err) { + reject(err) + } + }) + }) + }) }) diff --git a/packages/@uppy/companion/test/mocksocket.js b/packages/@uppy/companion/test/mocksocket.js index c9ffe52f4c..dd2cf53aab 100644 --- a/packages/@uppy/companion/test/mocksocket.js +++ b/packages/@uppy/companion/test/mocksocket.js @@ -19,3 +19,11 @@ module.exports.onUploadSuccess = (uploadToken, cb) => { } }) } + +module.exports.onUploadError = (uploadToken, cb) => { + emitter().on(uploadToken, (message) => { + if (message.action === 'error') { + cb(message) + } + }) +} diff --git a/website/src/docs/companion.md b/website/src/docs/companion.md index 8610b8d403..54b35efd3c 100644 --- a/website/src/docs/companion.md +++ b/website/src/docs/companion.md @@ -241,6 +241,9 @@ export COMPANION_UPLOAD_URLS="http://tusd.tusdemo.net/files/,https://tusd.tusdem # corresponds to the streamingUpload option export COMPANION_STREAMING_UPLOAD=true + +# corresponds to the maxFileSize option +export COMPANION_MAX_FILE_SIZE="100000000" ``` See [env.example.sh](https://github.com/transloadit/uppy/blob/master/env.example.sh) for an example configuration script. @@ -292,6 +295,7 @@ const options = { debug: true, metrics: false, streamingUpload: true, + maxFileSize: 100000000, } ``` @@ -330,6 +334,8 @@ const options = { 14. **streamingUpload(optional)** - A boolean flag to tell Companion whether or not to enable streaming uploads. If enabled, it will lead to **faster uploads* because companion will start uploading at the same time as downloading using `stream.pipe`. If `false`, files will be fully downloaded first, then uploaded. Defaults to `false`. +15. **maxFileSize(optional)** - If this value is set, companion will limit the maximum file size to process. If unset, it will process files without any size limit (this is the default). + ### Provider Redirect URIs When generating your provider API keys on their corresponding developer platforms (e.g [Google Developer Console](https://console.developers.google.com/)), you'd need to provide a `redirect URI` for the OAuth authorization process. In general the redirect URI for each provider takes the format: From 919ec1853d3f9a0c597f68365bd6f4fd66888325 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Wed, 8 Sep 2021 13:17:15 +0700 Subject: [PATCH 22/46] fix bug --- packages/@uppy/companion/src/server/Uploader.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/@uppy/companion/src/server/Uploader.js b/packages/@uppy/companion/src/server/Uploader.js index e86619328d..067aad36b8 100644 --- a/packages/@uppy/companion/src/server/Uploader.js +++ b/packages/@uppy/companion/src/server/Uploader.js @@ -74,12 +74,13 @@ class Uploader { this.options = options this.token = uuid.v4() + this.fileName = `${Uploader.FILE_NAME_PREFIX}-${this.token}` this.options.metadata = this.options.metadata || {} this.options.fieldname = this.options.fieldname || DEFAULT_FIELD_NAME this.size = options.size this.uploadFileName = this.options.metadata.name ? this.options.metadata.name.substring(0, MAX_FILENAME_LENGTH) - : `${Uploader.FILE_NAME_PREFIX}-${this.token}` + : this.fileName this.uploadStopped = false @@ -142,7 +143,7 @@ class Uploader { } async _downloadStreamAsFile (stream) { - this.tmpPath = join(this.options.pathPrefix, this.uploadFileName) + this.tmpPath = join(this.options.pathPrefix, this.fileName) logger.debug('fully downloading file', 'uploader.download', this.shortToken) const writeStream = createWriteStream(this.tmpPath) From eecdba9bacc997d41fa169fa1baf00e892ec864c Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Wed, 8 Sep 2021 17:50:49 +0700 Subject: [PATCH 23/46] fix memory leak when non 200 status streams were being kept --- .../custom-provider/server/customprovider.js | 17 ++++++++++------- .../companion/src/server/controllers/url.js | 3 ++- .../@uppy/companion/src/server/helpers/utils.js | 1 + 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/examples/custom-provider/server/customprovider.js b/examples/custom-provider/server/customprovider.js index 4c71e00ced..2ae0ac2c3a 100644 --- a/examples/custom-provider/server/customprovider.js +++ b/examples/custom-provider/server/customprovider.js @@ -71,20 +71,23 @@ class MyCustomProvider { }, } - const resp = await new Promise((resolve, reject) => ( - request(options) + const resp = await new Promise((resolve, reject) => { + const req = request(options) .on('response', (response) => { // Don't allow any more data to flow yet. // https://github.com/request/request/issues/1990#issuecomment-184712275 response.pause() + + if (resp.statusCode !== 200) { + req.abort() // Or we will leak memory + reject(new Error(`HTTP response ${resp.statusCode}`)) + return + } + resolve(response) }) .on('error', reject) - )) - - if (resp.statusCode !== 200) { - throw new Error(`HTTP response ${resp.statusCode}`) - } + }) // The returned stream will be consumed and uploaded from the current position return { stream: resp } diff --git a/packages/@uppy/companion/src/server/controllers/url.js b/packages/@uppy/companion/src/server/controllers/url.js index e93c52f225..2f9bc70639 100644 --- a/packages/@uppy/companion/src/server/controllers/url.js +++ b/packages/@uppy/companion/src/server/controllers/url.js @@ -57,9 +57,10 @@ const downloadURL = async (url, blockLocalIPs, traceId) => { // return onDataChunk(new Error('test error')) return new Promise((resolve, reject) => { - request(opts) + const req = request(opts) .on('response', (resp) => { if (resp.statusCode >= 300) { + req.abort() // No need to keep request reject(new Error(`URL server responded with status: ${resp.statusCode}`)) return } diff --git a/packages/@uppy/companion/src/server/helpers/utils.js b/packages/@uppy/companion/src/server/helpers/utils.js index 3fc69aeff2..fe3f5daa2e 100644 --- a/packages/@uppy/companion/src/server/helpers/utils.js +++ b/packages/@uppy/companion/src/server/helpers/utils.js @@ -147,6 +147,7 @@ module.exports.requestStream = async (req, convertResponseToError) => { )) if (resp.statusCode !== 200) { + req.abort() // Or we will leak memory (the stream is paused) throw await convertResponseToError(resp) } From bc62a69916704c0ab10a168bc459b3650034a158 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Wed, 8 Sep 2021 17:51:07 +0700 Subject: [PATCH 24/46] fix lint --- examples/custom-provider/server/index.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/examples/custom-provider/server/index.js b/examples/custom-provider/server/index.js index ce4eb7d93c..552118638c 100644 --- a/examples/custom-provider/server/index.js +++ b/examples/custom-provider/server/index.js @@ -4,6 +4,7 @@ const express = require('express') const bodyParser = require('body-parser') const session = require('express-session') const uppy = require('../../../packages/@uppy/companion') +const MyCustomProvider = require('./customprovider') const app = express() @@ -54,8 +55,8 @@ const uppyOptions = { key: 'your unsplash key here', secret: 'your unsplash secret here', }, - // you provider module - module: require('./customprovider'), + // you provider class/module: + module: MyCustomProvider, }, }, server: { From a16f34c82213db64083e27c9efe45001ace75f2d Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Wed, 8 Sep 2021 17:57:17 +0700 Subject: [PATCH 25/46] Add backward-compatibility for companion providers Implement a new static field "version" on providers, which when not set to 2, will cause a compatibility layer to be added for supporting old callback style provider api also fix some eslint and rename some vars --- .../custom-provider/server/customprovider.js | 2 + .../companion/src/server/provider/Provider.js | 2 + .../src/server/provider/ProviderCompat.js | 52 ++++++++++++++ .../src/server/provider/SearchProvider.js | 2 + .../src/server/provider/box/index.js | 2 + .../src/server/provider/drive/index.js | 2 + .../src/server/provider/dropbox/index.js | 2 + .../src/server/provider/facebook/index.js | 2 + .../companion/src/server/provider/index.js | 72 +++++++++++-------- .../server/provider/instagram/graph/index.js | 2 + .../src/server/provider/onedrive/index.js | 2 + .../src/server/provider/unsplash/index.js | 2 + .../src/server/provider/zoom/index.js | 2 + .../companion/test/__tests__/providers.js | 2 +- 14 files changed, 116 insertions(+), 32 deletions(-) create mode 100644 packages/@uppy/companion/src/server/provider/ProviderCompat.js diff --git a/examples/custom-provider/server/customprovider.js b/examples/custom-provider/server/customprovider.js index 2ae0ac2c3a..b7f0bd6562 100644 --- a/examples/custom-provider/server/customprovider.js +++ b/examples/custom-provider/server/customprovider.js @@ -32,6 +32,8 @@ function adaptData (res) { * an example of a custom provider module. It implements @uppy/companion's Provider interface */ class MyCustomProvider { + static version = 2 + constructor () { this.authProvider = 'myunsplash' } diff --git a/packages/@uppy/companion/src/server/provider/Provider.js b/packages/@uppy/companion/src/server/provider/Provider.js index 373b1d403a..09abb8a52d 100644 --- a/packages/@uppy/companion/src/server/provider/Provider.js +++ b/packages/@uppy/companion/src/server/provider/Provider.js @@ -2,6 +2,8 @@ * Provider interface defines the specifications of any provider implementation */ class Provider { + static version = 1 + /** * * @param {object} options diff --git a/packages/@uppy/companion/src/server/provider/ProviderCompat.js b/packages/@uppy/companion/src/server/provider/ProviderCompat.js new file mode 100644 index 0000000000..a17c31683c --- /dev/null +++ b/packages/@uppy/companion/src/server/provider/ProviderCompat.js @@ -0,0 +1,52 @@ +const { promisify } = require('util') + +const Stream = require('stream') + +/** + * Backward compatibility layer for old provider API using callbacks and onData cb + * + * @returns {any} + */ +const wrapLegacyProvider = (legacyProvider) => { + class CompatProvider extends legacyProvider { + constructor (...args) { + super(...args) + + this.list = promisify((options, cb) => super.list(options, cb)) + this.size = promisify((options, cb) => super.size(options, cb)) + this.thumbnail = promisify((options, cb) => super.thumbnail(options, cb)) + this.deauthorizationCallback = promisify((options, cb) => super.deauthorizationCallback(options, cb)) + this.logout = promisify((options, cb) => super.logout(options, cb)) + + this.download = async (options) => { + let stream + + return new Promise((resolve, reject) => { + super.download(options, (err, chunk) => { + if (err) { + if (stream && !stream.destroyed) stream.destroy(err) + reject(err) + return + } + + // Initialize on first chunk + if (chunk != null && !stream) { + stream = new Stream.PassThrough() + // stream.on('end', () => console.log('stream end')) + stream.pause() + stream.push(chunk) + resolve({ stream }) + return + } + + stream.push(chunk) + }) + }) + } + } + } + + return CompatProvider +} + +module.exports = { wrapLegacyProvider } diff --git a/packages/@uppy/companion/src/server/provider/SearchProvider.js b/packages/@uppy/companion/src/server/provider/SearchProvider.js index c3a833417b..0356efc9b1 100644 --- a/packages/@uppy/companion/src/server/provider/SearchProvider.js +++ b/packages/@uppy/companion/src/server/provider/SearchProvider.js @@ -2,6 +2,8 @@ * SearchProvider interface defines the specifications of any Search provider implementation */ class SearchProvider { + static version = 1 + /** * list the files available based on the search query * diff --git a/packages/@uppy/companion/src/server/provider/box/index.js b/packages/@uppy/companion/src/server/provider/box/index.js index 513d1d840d..c9c5ebf9bd 100644 --- a/packages/@uppy/companion/src/server/provider/box/index.js +++ b/packages/@uppy/companion/src/server/provider/box/index.js @@ -16,6 +16,8 @@ const BOX_THUMBNAIL_SIZE = 256 * Adapter for API https://developer.box.com/reference/ */ class Box extends Provider { + static version = 2 + constructor (options) { super(options) this.authProvider = Box.authProvider diff --git a/packages/@uppy/companion/src/server/provider/drive/index.js b/packages/@uppy/companion/src/server/provider/drive/index.js index d7967a896a..2fe2870b4d 100644 --- a/packages/@uppy/companion/src/server/provider/drive/index.js +++ b/packages/@uppy/companion/src/server/provider/drive/index.js @@ -85,6 +85,8 @@ function adaptData (listFilesResp, sharedDrivesResp, directory, query, showShare * Adapter for API https://developers.google.com/drive/api/v3/ */ class Drive extends Provider { + static version = 2 + constructor (options) { super(options) this.authProvider = Drive.authProvider diff --git a/packages/@uppy/companion/src/server/provider/dropbox/index.js b/packages/@uppy/companion/src/server/provider/dropbox/index.js index 9a18381be0..0829742fc2 100644 --- a/packages/@uppy/companion/src/server/provider/dropbox/index.js +++ b/packages/@uppy/companion/src/server/provider/dropbox/index.js @@ -25,6 +25,8 @@ function httpHeaderSafeJson (v) { * Adapter for API https://www.dropbox.com/developers/documentation/http/documentation */ class DropBox extends Provider { + static version = 2 + constructor (options) { super(options) this.authProvider = DropBox.authProvider diff --git a/packages/@uppy/companion/src/server/provider/facebook/index.js b/packages/@uppy/companion/src/server/provider/facebook/index.js index 9b7ada872f..8a04e0492f 100644 --- a/packages/@uppy/companion/src/server/provider/facebook/index.js +++ b/packages/@uppy/companion/src/server/provider/facebook/index.js @@ -14,6 +14,8 @@ const { requestStream } = require('../../helpers/utils') * Adapter for API https://developers.facebook.com/docs/graph-api/using-graph-api/ */ class Facebook extends Provider { + static version = 2 + constructor (options) { super(options) this.authProvider = Facebook.authProvider diff --git a/packages/@uppy/companion/src/server/provider/index.js b/packages/@uppy/companion/src/server/provider/index.js index 8cf9d66209..3c9221955b 100644 --- a/packages/@uppy/companion/src/server/provider/index.js +++ b/packages/@uppy/companion/src/server/provider/index.js @@ -2,7 +2,7 @@ * @module provider */ // @ts-ignore -const config = require('@purest/providers') +const purestConfig = require('@purest/providers') const dropbox = require('./dropbox') const box = require('./box') const drive = require('./drive') @@ -18,9 +18,10 @@ const { getCredentialsResolver } = require('./credentials') const Provider = require('./Provider') // eslint-disable-next-line const SearchProvider = require('./SearchProvider') +const { wrapLegacyProvider } = require('./ProviderCompat') // leave here for now until Purest Providers gets updated with Zoom provider -config.zoom = { +purestConfig.zoom = { 'https://zoom.us/': { __domain: { auth: { @@ -44,6 +45,25 @@ config.zoom = { }, } +/** + * + * @param {{server: object}} options + */ +const validOptions = (options) => { + return options.server.host && options.server.protocol +} + +/** + * + * @param {string} name of the provider + * @param {{server: object, providerOptions: object}} options + * @returns {string} the authProvider for this provider + */ +const providerNameToAuthName = (name, options) => { // eslint-disable-line no-unused-vars + const providers = exports.getDefaultProviders() + return (providers[name] || {}).authProvider +} + /** * adds the desired provider module to the request object, * based on the providerName parameter specified @@ -60,8 +80,14 @@ module.exports.getProviderMiddleware = (providers, needsProviderCredentials) => * @param {string} providerName */ const middleware = (req, res, next, providerName) => { - if (providers[providerName] && validOptions(req.companion.options)) { - req.companion.provider = new providers[providerName]({ providerName, config }) + let ProviderClass = providers[providerName] + if (ProviderClass && validOptions(req.companion.options)) { + // TODO remove this legacy provider compatibility when we release a new major + // @ts-ignore + if (ProviderClass.version !== 2) ProviderClass = wrapLegacyProvider(ProviderClass) + + req.companion.provider = new ProviderClass({ providerName, config: purestConfig }) + if (needsProviderCredentials) { req.companion.getProviderCredentials = getCredentialsResolver(providerName, req.companion.options, req) } @@ -100,14 +126,17 @@ module.exports.getSearchProviders = () => { */ module.exports.addCustomProviders = (customProviders, providers, grantConfig) => { Object.keys(customProviders).forEach((providerName) => { - providers[providerName] = customProviders[providerName].module - const providerConfig = { ...customProviders[providerName].config } - // todo: consider setting these options from a universal point also used - // by official providers. It'll prevent these from getting left out if the - // requirement changes. - providerConfig.callback = `/${providerName}/callback` - providerConfig.transport = 'session' - grantConfig[providerName] = providerConfig + const customProvider = customProviders[providerName] + + providers[providerName] = customProvider.module + grantConfig[providerName] = { + ...customProvider.config, + // todo: consider setting these options from a universal point also used + // by official providers. It'll prevent these from getting left out if the + // requirement changes. + callback: `/${providerName}/callback`, + transport: 'session', + } }) } @@ -164,22 +193,3 @@ module.exports.addProviderOptions = (companionOptions, grantConfig) => { } }) } - -/** - * - * @param {string} name of the provider - * @param {{server: object, providerOptions: object}} options - * @returns {string} the authProvider for this provider - */ -const providerNameToAuthName = (name, options) => { // eslint-disable-line no-unused-vars - const providers = exports.getDefaultProviders() - return (providers[name] || {}).authProvider -} - -/** - * - * @param {{server: object}} options - */ -const validOptions = (options) => { - return options.server.host && options.server.protocol -} diff --git a/packages/@uppy/companion/src/server/provider/instagram/graph/index.js b/packages/@uppy/companion/src/server/provider/instagram/graph/index.js index c0f4826770..8aad02b165 100644 --- a/packages/@uppy/companion/src/server/provider/instagram/graph/index.js +++ b/packages/@uppy/companion/src/server/provider/instagram/graph/index.js @@ -14,6 +14,8 @@ const { requestStream } = require('../../../helpers/utils') * Adapter for API https://developers.facebook.com/docs/instagram-api/overview */ class Instagram extends Provider { + static version = 2 + constructor (options) { super(options) this.authProvider = Instagram.authProvider diff --git a/packages/@uppy/companion/src/server/provider/onedrive/index.js b/packages/@uppy/companion/src/server/provider/onedrive/index.js index ce93c98c58..95d7391abc 100644 --- a/packages/@uppy/companion/src/server/provider/onedrive/index.js +++ b/packages/@uppy/companion/src/server/provider/onedrive/index.js @@ -13,6 +13,8 @@ const { requestStream } = require('../../helpers/utils') * Adapter for API https://docs.microsoft.com/en-us/onedrive/developer/rest-api/ */ class OneDrive extends Provider { + static version = 2 + constructor (options) { super(options) this.authProvider = OneDrive.authProvider diff --git a/packages/@uppy/companion/src/server/provider/unsplash/index.js b/packages/@uppy/companion/src/server/provider/unsplash/index.js index 21f9997bf8..2218057cf4 100644 --- a/packages/@uppy/companion/src/server/provider/unsplash/index.js +++ b/packages/@uppy/companion/src/server/provider/unsplash/index.js @@ -14,6 +14,8 @@ const BASE_URL = 'https://api.unsplash.com' * Adapter for API https://api.unsplash.com */ class Unsplash extends SearchProvider { + static version = 2 + async list (options) { return promisify(this._list.bind(this))(options) } diff --git a/packages/@uppy/companion/src/server/provider/zoom/index.js b/packages/@uppy/companion/src/server/provider/zoom/index.js index 06165db398..a69f7e4244 100644 --- a/packages/@uppy/companion/src/server/provider/zoom/index.js +++ b/packages/@uppy/companion/src/server/provider/zoom/index.js @@ -20,6 +20,8 @@ const DEAUTH_EVENT_NAME = 'app_deauthorized' * Adapter for API https://marketplace.zoom.us/docs/api-reference/zoom-api */ class Zoom extends Provider { + static version = 2 + constructor (options) { super(options) this.authProvider = Zoom.authProvider diff --git a/packages/@uppy/companion/test/__tests__/providers.js b/packages/@uppy/companion/test/__tests__/providers.js index 57479bcc9a..dcfc4f14dc 100644 --- a/packages/@uppy/companion/test/__tests__/providers.js +++ b/packages/@uppy/companion/test/__tests__/providers.js @@ -96,7 +96,7 @@ describe('list provider files', () => { }) }) -describe('download provdier file', () => { +describe('download provider file', () => { test.each(providerNames)('specified file gets downloaded from %s', (providerName) => { const providerFixtures = fixtures.providers[providerName].expects return request(authServer) From 313d75b61cbccb15d6d21127265d339e59adbe4a Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Wed, 8 Sep 2021 17:58:25 +0700 Subject: [PATCH 26/46] document new provider API --- website/src/docs/companion.md | 43 +++++++++++++++++------------------ 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/website/src/docs/companion.md b/website/src/docs/companion.md index 54b35efd3c..1751694eda 100644 --- a/website/src/docs/companion.md +++ b/website/src/docs/companion.md @@ -332,7 +332,7 @@ const options = { 13. **metrics(optional)** - A boolean flag to tell Companion whether or not to provide an endpoint `/metrics` with Prometheus metrics. -14. **streamingUpload(optional)** - A boolean flag to tell Companion whether or not to enable streaming uploads. If enabled, it will lead to **faster uploads* because companion will start uploading at the same time as downloading using `stream.pipe`. If `false`, files will be fully downloaded first, then uploaded. Defaults to `false`. +14. **streamingUpload(optional)** - A boolean flag to tell Companion whether or not to enable streaming uploads. If enabled, it will lead to **faster uploads* because companion will start uploading at the same time as downloading using `stream.pipe`. If `false`, files will be fully downloaded first, then uploaded. Defaults to `false`. Do not set to `true` if you have any custom Companion providers that do not use the new async/stream API. 15. **maxFileSize(optional)** - If this value is set, companion will limit the maximum file size to process. If unset, it will process files without any size limit (this is the default). @@ -443,27 +443,26 @@ uppy.app(options) The `customProviders` option should be an object containing each custom provider. Each custom provider would, in turn, be an object with two keys, `config` and `module`. The `config` option would contain Oauth API settings, while the `module` would point to the provider module. -To work well with Companion, the **Module** must be a class with the following methods. - -1. `list (options, done)` - lists JSON data of user files (e.g. list of all the files in a particular directory). - - `options` - is an object containing the following attributes - - token - authorization token (retrieved from oauth process) to send along with your request - - directory - the `id/name` of the directory from which data is to be retrieved. This may be ignored if it doesn't apply to your provider - - query - expressjs query params object received by the server (just in case there is some data you need in there). - - `done (err, data)` - the callback that should be called when the request to your provider is made. As the signature indicates, the following data should be passed along to the callback `err`, and [`data`](#list-data). -2. `download (options, onData)` - downloads a particular file from the provider. - - `options` - is an object containing the following attributes: - - token - authorization token (retrieved from oauth process) to send along with your request. - - id - ID of the file being downloaded. - - query - expressjs query params object received by the server (just in case there is some data you need in there). - - `onData (err, chunk)` - a callback that should be called with each data chunk received as download is happening. The `err` argument is an error that should be passed if an error occurs during download. It should be `null` if there's no error. Once the download is completed and there are no more chunks to receive, `onData` should be called with `null` values like so `onData(null, null)` -3. `size (options, done)` - returns the byte size of the file that needs to be downloaded. - - `options` - is an object containing the following attributes: - - token - authorization token (retrieved from oauth process) to send along with your request. - - id - ID of the file being downloaded. - - `done (err, size)` - the callback that should be called after the request to your provider is completed. As the signature indicates, the following data should be passed along to the callback `err`, and `size` (number). - -The class must also have an `authProvider` string (lowercased) field which typically indicates the name of the provider (e.g "dropbox"). +To work well with Companion, the `module` must be a `class` with the following methods. Note that the methods must be `async`, return a `Promise` or reject with an `Error`): + +1. `async list ({ token, directory, query })` - Returns a object containing a list of user files (e.g. list of all the files in a particular directory). See [example returned list data structure](#list-data). + - `token` - authorization token (retrieved from oauth process) to send along with your request + - `directory` - the id/name of the directory from which data is to be retrieved. This may be ignored if it doesn't apply to your provider + - `query` - expressjs query params object received by the server (just in case there is some data you need in there). +2. `async download ({ token, id, query })` - Downloads a particular file from the provider. Returns an object with a single property `{ stream }` - a [`stream.Readable`](https://nodejs.org/api/stream.html#stream_class_stream_readable), which will be read from and uploaded to the destination. To prevent memory leaks, make sure you release your stream if you reject this method with an error. + - `token` - authorization token (retrieved from oauth process) to send along with your request. + - `id` - ID of the file being downloaded. + - `query` - expressjs query params object received by the server (just in case there is some data you need in there). +3. `async size ({ token, id, query })` - Returns the byte size of the file that needs to be downloaded as a `Number`. If the size of the object is not known, `null` may be returned. + - `token` - authorization token (retrieved from oauth process) to send along with your request. + - `id` - ID of the file being downloaded. + - `query` - expressjs query params object received by the server (just in case there is some data you need in there). + +The class must also have: +- A unique `authProvider` string property - a lowercased value which typically indicates the name of the provider (e.g "dropbox"). +- A `static` property `static version = 2`, which is the current version of the Companion Provider API. + +See also [example code with a custom provider](https://github.com/transloadit/uppy/blob/main/examples/custom-provider/server). #### list data From 65ac8c77657dc84e4c4aaf1af0eb2cf3768b8b24 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Wed, 8 Sep 2021 18:08:19 +0700 Subject: [PATCH 27/46] remove static as it doesn't work on node 10 --- packages/@uppy/companion/src/server/provider/Provider.js | 4 ++-- .../@uppy/companion/src/server/provider/SearchProvider.js | 4 ++-- packages/@uppy/companion/src/server/provider/box/index.js | 4 ++-- packages/@uppy/companion/src/server/provider/drive/index.js | 4 ++-- packages/@uppy/companion/src/server/provider/dropbox/index.js | 4 ++-- .../@uppy/companion/src/server/provider/facebook/index.js | 4 ++-- .../companion/src/server/provider/instagram/graph/index.js | 4 ++-- .../@uppy/companion/src/server/provider/onedrive/index.js | 4 ++-- .../@uppy/companion/src/server/provider/unsplash/index.js | 4 ++-- packages/@uppy/companion/src/server/provider/zoom/index.js | 4 ++-- 10 files changed, 20 insertions(+), 20 deletions(-) diff --git a/packages/@uppy/companion/src/server/provider/Provider.js b/packages/@uppy/companion/src/server/provider/Provider.js index 09abb8a52d..58daf68e2d 100644 --- a/packages/@uppy/companion/src/server/provider/Provider.js +++ b/packages/@uppy/companion/src/server/provider/Provider.js @@ -2,8 +2,6 @@ * Provider interface defines the specifications of any provider implementation */ class Provider { - static version = 1 - /** * * @param {object} options @@ -79,4 +77,6 @@ class Provider { } } +Provider.version = 1 + module.exports = Provider diff --git a/packages/@uppy/companion/src/server/provider/SearchProvider.js b/packages/@uppy/companion/src/server/provider/SearchProvider.js index 0356efc9b1..28b78b7752 100644 --- a/packages/@uppy/companion/src/server/provider/SearchProvider.js +++ b/packages/@uppy/companion/src/server/provider/SearchProvider.js @@ -2,8 +2,6 @@ * SearchProvider interface defines the specifications of any Search provider implementation */ class SearchProvider { - static version = 1 - /** * list the files available based on the search query * @@ -35,4 +33,6 @@ class SearchProvider { } } +SearchProvider.version = 1 + module.exports = SearchProvider diff --git a/packages/@uppy/companion/src/server/provider/box/index.js b/packages/@uppy/companion/src/server/provider/box/index.js index c9c5ebf9bd..109f05868d 100644 --- a/packages/@uppy/companion/src/server/provider/box/index.js +++ b/packages/@uppy/companion/src/server/provider/box/index.js @@ -16,8 +16,6 @@ const BOX_THUMBNAIL_SIZE = 256 * Adapter for API https://developer.box.com/reference/ */ class Box extends Provider { - static version = 2 - constructor (options) { super(options) this.authProvider = Box.authProvider @@ -205,4 +203,6 @@ class Box extends Provider { } } +Box.version = 2 + module.exports = Box diff --git a/packages/@uppy/companion/src/server/provider/drive/index.js b/packages/@uppy/companion/src/server/provider/drive/index.js index 2fe2870b4d..f603aefc8e 100644 --- a/packages/@uppy/companion/src/server/provider/drive/index.js +++ b/packages/@uppy/companion/src/server/provider/drive/index.js @@ -85,8 +85,6 @@ function adaptData (listFilesResp, sharedDrivesResp, directory, query, showShare * Adapter for API https://developers.google.com/drive/api/v3/ */ class Drive extends Provider { - static version = 2 - constructor (options) { super(options) this.authProvider = Drive.authProvider @@ -290,4 +288,6 @@ class Drive extends Provider { } } +Drive.version = 2 + module.exports = Drive diff --git a/packages/@uppy/companion/src/server/provider/dropbox/index.js b/packages/@uppy/companion/src/server/provider/dropbox/index.js index 0829742fc2..abb20af68b 100644 --- a/packages/@uppy/companion/src/server/provider/dropbox/index.js +++ b/packages/@uppy/companion/src/server/provider/dropbox/index.js @@ -25,8 +25,6 @@ function httpHeaderSafeJson (v) { * Adapter for API https://www.dropbox.com/developers/documentation/http/documentation */ class DropBox extends Provider { - static version = 2 - constructor (options) { super(options) this.authProvider = DropBox.authProvider @@ -247,4 +245,6 @@ class DropBox extends Provider { } } +DropBox.version = 2 + module.exports = DropBox diff --git a/packages/@uppy/companion/src/server/provider/facebook/index.js b/packages/@uppy/companion/src/server/provider/facebook/index.js index 8a04e0492f..5f446b1907 100644 --- a/packages/@uppy/companion/src/server/provider/facebook/index.js +++ b/packages/@uppy/companion/src/server/provider/facebook/index.js @@ -14,8 +14,6 @@ const { requestStream } = require('../../helpers/utils') * Adapter for API https://developers.facebook.com/docs/graph-api/using-graph-api/ */ class Facebook extends Provider { - static version = 2 - constructor (options) { super(options) this.authProvider = Facebook.authProvider @@ -202,4 +200,6 @@ class Facebook extends Provider { } } +Facebook.version = 2 + module.exports = Facebook diff --git a/packages/@uppy/companion/src/server/provider/instagram/graph/index.js b/packages/@uppy/companion/src/server/provider/instagram/graph/index.js index 8aad02b165..a0917156a8 100644 --- a/packages/@uppy/companion/src/server/provider/instagram/graph/index.js +++ b/packages/@uppy/companion/src/server/provider/instagram/graph/index.js @@ -14,8 +14,6 @@ const { requestStream } = require('../../../helpers/utils') * Adapter for API https://developers.facebook.com/docs/instagram-api/overview */ class Instagram extends Provider { - static version = 2 - constructor (options) { super(options) this.authProvider = Instagram.authProvider @@ -180,4 +178,6 @@ class Instagram extends Provider { } } +Instagram.version = 2 + module.exports = Instagram diff --git a/packages/@uppy/companion/src/server/provider/onedrive/index.js b/packages/@uppy/companion/src/server/provider/onedrive/index.js index 95d7391abc..fc19b28c3b 100644 --- a/packages/@uppy/companion/src/server/provider/onedrive/index.js +++ b/packages/@uppy/companion/src/server/provider/onedrive/index.js @@ -13,8 +13,6 @@ const { requestStream } = require('../../helpers/utils') * Adapter for API https://docs.microsoft.com/en-us/onedrive/developer/rest-api/ */ class OneDrive extends Provider { - static version = 2 - constructor (options) { super(options) this.authProvider = OneDrive.authProvider @@ -153,4 +151,6 @@ class OneDrive extends Provider { } } +OneDrive.version = 2 + module.exports = OneDrive diff --git a/packages/@uppy/companion/src/server/provider/unsplash/index.js b/packages/@uppy/companion/src/server/provider/unsplash/index.js index 2218057cf4..7639974357 100644 --- a/packages/@uppy/companion/src/server/provider/unsplash/index.js +++ b/packages/@uppy/companion/src/server/provider/unsplash/index.js @@ -14,8 +14,6 @@ const BASE_URL = 'https://api.unsplash.com' * Adapter for API https://api.unsplash.com */ class Unsplash extends SearchProvider { - static version = 2 - async list (options) { return promisify(this._list.bind(this))(options) } @@ -149,4 +147,6 @@ class Unsplash extends SearchProvider { } } +Unsplash.version = 2 + module.exports = Unsplash diff --git a/packages/@uppy/companion/src/server/provider/zoom/index.js b/packages/@uppy/companion/src/server/provider/zoom/index.js index a69f7e4244..91bd16c4eb 100644 --- a/packages/@uppy/companion/src/server/provider/zoom/index.js +++ b/packages/@uppy/companion/src/server/provider/zoom/index.js @@ -20,8 +20,6 @@ const DEAUTH_EVENT_NAME = 'app_deauthorized' * Adapter for API https://marketplace.zoom.us/docs/api-reference/zoom-api */ class Zoom extends Provider { - static version = 2 - constructor (options) { super(options) this.authProvider = Zoom.authProvider @@ -357,4 +355,6 @@ class Zoom extends Provider { } } +Zoom.version = 2 + module.exports = Zoom From c8ff49c3d17f8dee47fe6d0c74e0b8b36499a2a3 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Wed, 8 Sep 2021 18:24:01 +0700 Subject: [PATCH 28/46] try to fix build issue --- .../@uppy/companion/src/server/provider/ProviderCompat.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/@uppy/companion/src/server/provider/ProviderCompat.js b/packages/@uppy/companion/src/server/provider/ProviderCompat.js index a17c31683c..c57532a24a 100644 --- a/packages/@uppy/companion/src/server/provider/ProviderCompat.js +++ b/packages/@uppy/companion/src/server/provider/ProviderCompat.js @@ -18,11 +18,13 @@ const wrapLegacyProvider = (legacyProvider) => { this.deauthorizationCallback = promisify((options, cb) => super.deauthorizationCallback(options, cb)) this.logout = promisify((options, cb) => super.logout(options, cb)) + const superDownload = super.download + this.download = async (options) => { let stream return new Promise((resolve, reject) => { - super.download(options, (err, chunk) => { + superDownload(options, (err, chunk) => { if (err) { if (stream && !stream.destroyed) stream.destroy(err) reject(err) From 735bde175cee547feaef56d6ff7b3bb3bd267935 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Wed, 8 Sep 2021 20:12:02 +0700 Subject: [PATCH 29/46] degrade to node 14 in github actions due to hitting this error: https://github.com/nodejs/node/issues/40030 https://github.com/transloadit/uppy/pull/3159/checks?check_run_id=3544858518 --- .github/workflows/cdn.yml | 2 +- .github/workflows/ci.yml | 4 ++-- .github/workflows/end-to-end.yml | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/cdn.yml b/.github/workflows/cdn.yml index 3fdc55d4d7..2c02dd41ab 100644 --- a/.github/workflows/cdn.yml +++ b/.github/workflows/cdn.yml @@ -14,7 +14,7 @@ jobs: - name: Install Node.js uses: actions/setup-node@v2-beta with: - node-version: 16.x + node-version: 14.x - name: Install npm 7 run: npm install --global npm@7 - name: Install dependencies diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7dd0fb188e..ad0f4eeb9e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,7 +7,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - node-version: [12.x, 14.x, 16.x] + node-version: [12.x, 14.x] steps: - name: Checkout sources uses: actions/checkout@v2 @@ -65,7 +65,7 @@ jobs: - name: Install Node.js uses: actions/setup-node@v2-beta with: - node-version: 16.x + node-version: 14.x - name: Install npm 7 run: npm install --global npm@7 - name: Install dependencies diff --git a/.github/workflows/end-to-end.yml b/.github/workflows/end-to-end.yml index 5ad2797c2a..8371c18668 100644 --- a/.github/workflows/end-to-end.yml +++ b/.github/workflows/end-to-end.yml @@ -17,7 +17,7 @@ jobs: - name: Install Node.js uses: actions/setup-node@v2-beta with: - node-version: 16.x + node-version: 14.x - name: Install npm 7 run: npm install --global npm@7 - name: Install dependencies From a1bbb62fb06d1ca9fcbd4ea93e5d9e8b52a6902f Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Wed, 8 Sep 2021 23:16:35 +0700 Subject: [PATCH 30/46] pull out duplicated logic into reusable function --- .../companion/src/server/controllers/get.js | 45 ++++---------- .../companion/src/server/controllers/url.js | 59 ++++++------------- .../companion/src/server/helpers/upload.js | 46 +++++++++++++++ 3 files changed, 73 insertions(+), 77 deletions(-) create mode 100644 packages/@uppy/companion/src/server/helpers/upload.js diff --git a/packages/@uppy/companion/src/server/controllers/get.js b/packages/@uppy/companion/src/server/controllers/get.js index a8e0db58bb..df11673151 100644 --- a/packages/@uppy/companion/src/server/controllers/get.js +++ b/packages/@uppy/companion/src/server/controllers/get.js @@ -1,51 +1,26 @@ -const Uploader = require('../Uploader') const logger = require('../logger') -const { errorToResponse } = require('../provider/error') +const { startDownUpload } = require('../helpers/upload') async function get (req, res) { const { id } = req.params const token = req.companion.providerToken const { provider } = req.companion - try { - const size = await provider.size({ id, token, query: req.query }) - - logger.debug('Instantiating uploader.', null, req.id) - const uploader = new Uploader(Uploader.reqToOptions(req, size)) - - if (uploader.hasError()) { - const response = uploader.getResponse() - res.status(response.status).json(response.body) - return - } + async function getSize () { + return provider.size({ id, token, query: req.query }) + } + async function download () { const { stream } = await provider.download({ id, token, query: req.query }) + return stream + } - // "Forking" off the upload operation to background, so we can return the http request: - ;(async () => { - // wait till the client has connected to the socket, before starting - // the download, so that the client can receive all download/upload progress. - logger.debug('Waiting for socket connection before beginning remote download/upload.', null, req.id) - await uploader.awaitReady() - logger.debug('Socket connection received. Starting remote download/upload.', null, req.id) - - uploader.uploadStream(stream) - })() - - // Respond the request - // NOTE: Uploader will continue running after the http request is responded - const response = uploader.getResponse() - res.status(response.status).json(response.body) - } catch (err) { - const errResp = errorToResponse(err) - if (errResp) { - res.status(errResp.code).json({ message: errResp.message }) - return - } - + function onUnhandledError (err) { logger.error(err, 'controller.get.error', req.id) res.status(400).json({ message: 'Failed to download file' }) } + + startDownUpload({ req, res, getSize, download, onUnhandledError }) } module.exports = get diff --git a/packages/@uppy/companion/src/server/controllers/url.js b/packages/@uppy/companion/src/server/controllers/url.js index 2f9bc70639..0e589c80ba 100644 --- a/packages/@uppy/companion/src/server/controllers/url.js +++ b/packages/@uppy/companion/src/server/controllers/url.js @@ -3,10 +3,9 @@ const request = require('request') const { URL } = require('url') const validator = require('validator') -const Uploader = require('../Uploader') +const { startDownUpload } = require('../helpers/upload') const { getURLMeta, getRedirectEvaluator, getProtectedHttpAgent } = require('../helpers/request') const logger = require('../logger') -const { errorToResponse } = require('../provider/error') /** * Validates that the download URL is secure @@ -109,54 +108,30 @@ const meta = async (req, res) => { * @param {object} res expressJS response object */ const get = async (req, res) => { - try { - logger.debug('URL file import handler running', null, req.id) - const { debug } = req.companion.options - if (!validateURL(req.body.url, debug)) { - logger.debug('Invalid request body detected. Exiting url import handler.', null, req.id) - res.status(400).json({ error: 'Invalid request body' }) - return - } + logger.debug('URL file import handler running', null, req.id) + const { debug } = req.companion.options + if (!validateURL(req.body.url, debug)) { + logger.debug('Invalid request body detected. Exiting url import handler.', null, req.id) + res.status(400).json({ error: 'Invalid request body' }) + return + } + async function getSize () { const { size } = await getURLMeta(req.body.url, !debug) + return size + } - logger.debug('Instantiating uploader.', null, req.id) - const uploader = new Uploader(Uploader.reqToOptions(req, size)) - - if (uploader.hasError()) { - const response = uploader.getResponse() - res.status(response.status).json(response.body) - return - } - - const stream = await downloadURL(req.body.url, !debug, req.id) - - // "Forking" off the upload operation to background, so we can return the http request: - ;(async () => { - // wait till the client has connected to the socket, before starting - // the download, so that the client can receive all download/upload progress. - logger.debug('Waiting for socket connection before beginning remote download/upload.', null, req.id) - await uploader.awaitReady() - logger.debug('Socket connection received. Starting remote download/upload.', null, req.id) - - uploader.uploadStream(stream) - })() - - // Respond the request - // NOTE: Uploader will continue running after the http request is responded - const response = uploader.getResponse() - res.status(response.status).json(response.body) - } catch (err) { - const errResp = errorToResponse(err) - if (errResp) { - res.status(errResp.code).json({ message: errResp.message }) - return - } + async function download () { + return downloadURL(req.body.url, !debug, req.id) + } + function onUnhandledError (err) { logger.error(err, 'controller.url.error', req.id) // @todo send more meaningful error message and status code to client if possible res.status(err.status || 500).json({ message: 'failed to fetch URL metadata' }) } + + startDownUpload({ req, res, getSize, download, onUnhandledError }) } module.exports = () => router() diff --git a/packages/@uppy/companion/src/server/helpers/upload.js b/packages/@uppy/companion/src/server/helpers/upload.js new file mode 100644 index 0000000000..c6d03c0090 --- /dev/null +++ b/packages/@uppy/companion/src/server/helpers/upload.js @@ -0,0 +1,46 @@ +const Uploader = require('../Uploader') +const logger = require('../logger') +const { errorToResponse } = require('../provider/error') + +async function startDownUpload ({ req, res, getSize, download, onUnhandledError }) { + try { + const size = await getSize() + + logger.debug('Instantiating uploader.', null, req.id) + const uploader = new Uploader(Uploader.reqToOptions(req, size)) + + if (uploader.hasError()) { + const response = uploader.getResponse() + res.status(response.status).json(response.body) + return + } + + const stream = await download() + + // "Forking" off the upload operation to background, so we can return the http request: + ;(async () => { + // wait till the client has connected to the socket, before starting + // the download, so that the client can receive all download/upload progress. + logger.debug('Waiting for socket connection before beginning remote download/upload.', null, req.id) + await uploader.awaitReady() + logger.debug('Socket connection received. Starting remote download/upload.', null, req.id) + + uploader.uploadStream(stream) + })() + + // Respond the request + // NOTE: Uploader will continue running after the http request is responded + const response = uploader.getResponse() + res.status(response.status).json(response.body) + } catch (err) { + const errResp = errorToResponse(err) + if (errResp) { + res.status(errResp.code).json({ message: errResp.message }) + return + } + + onUnhandledError(err) + } +} + +module.exports = { startDownUpload } From 6a6b697c08e1defb8484a224121d667458309dbe Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Thu, 9 Sep 2021 17:08:29 +0700 Subject: [PATCH 31/46] fix lint --- examples/dev/Dashboard.js | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/dev/Dashboard.js b/examples/dev/Dashboard.js index 160b8da7f2..0798c15e3b 100644 --- a/examples/dev/Dashboard.js +++ b/examples/dev/Dashboard.js @@ -145,6 +145,7 @@ module.exports = () => { bundle: true, }) break + default: } if (RESTORE) { From 42c2b3d29bc969f1e426be45786416f6c3bc7ddd Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Sat, 23 Oct 2021 12:56:10 +0700 Subject: [PATCH 32/46] make methods private --- packages/@uppy/companion/src/server/Uploader.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/@uppy/companion/src/server/Uploader.js b/packages/@uppy/companion/src/server/Uploader.js index 067aad36b8..505d848fbf 100644 --- a/packages/@uppy/companion/src/server/Uploader.js +++ b/packages/@uppy/companion/src/server/Uploader.js @@ -132,11 +132,11 @@ class Uploader { switch (protocol) { case PROTOCOLS.multipart: - return this.uploadMultipart(this.readStream) + return this._uploadMultipart(this.readStream) case PROTOCOLS.s3Multipart: - return this.uploadS3Multipart(this.readStream) + return this._uploadS3Multipart(this.readStream) case PROTOCOLS.tus: - return this.uploadTus(this.readStream) + return this._uploadTus(this.readStream) default: throw new Error('Invalid protocol') } @@ -462,7 +462,7 @@ class Uploader { /** * start the tus upload */ - async uploadTus (stream) { + async _uploadTus (stream) { const uploader = this return new Promise((resolve, reject) => { @@ -518,7 +518,7 @@ class Uploader { }) } - async uploadMultipart (stream) { + async _uploadMultipart (stream) { if (!this.options.endpoint) { throw new Error('No multipart endpoint set') } @@ -595,7 +595,7 @@ class Uploader { /** * Upload the file to S3 using a Multipart upload. */ - async uploadS3Multipart (stream) { + async _uploadS3Multipart (stream) { if (!this.options.s3) { throw new Error('The S3 client is not configured on this companion instance.') } From e322b232cc3b3bf5f9af0d9206e42a7ced1817a9 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Sat, 23 Oct 2021 21:12:30 +0700 Subject: [PATCH 33/46] re-add unsplash download_location request got lost in merge --- .../companion/src/server/provider/unsplash/index.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/@uppy/companion/src/server/provider/unsplash/index.js b/packages/@uppy/companion/src/server/provider/unsplash/index.js index 98ebd4d122..40476e3d44 100644 --- a/packages/@uppy/companion/src/server/provider/unsplash/index.js +++ b/packages/@uppy/companion/src/server/provider/unsplash/index.js @@ -101,7 +101,14 @@ class Unsplash extends SearchProvider { // To attribute the author of the image, we call the `download_location` // endpoint to increment the download count on Unsplash. // https://help.unsplash.com/en/articles/2511258-guideline-triggering-a-download - request({ ...reqOpts, url: body.links.download_location }) + request({ ...reqOpts, url: body.links.download_location }, (err, resp) => { + if (err || resp.statusCode !== 200) { + const err2 = this.error(err, resp) + logger.error(err2, 'provider.unsplash.download.location.error') + } else { + // console.log('download_location complete') + } + }) return stream } catch (err) { From 478359319fb8d64507bf357b45ba78e56fa17cd4 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Sat, 23 Oct 2021 21:13:07 +0700 Subject: [PATCH 34/46] add try/catch as suggested https://github.com/transloadit/uppy/pull/3159#discussion_r727149263 --- .../companion/src/server/helpers/upload.js | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/@uppy/companion/src/server/helpers/upload.js b/packages/@uppy/companion/src/server/helpers/upload.js index c6d03c0090..9cd0f7a0a5 100644 --- a/packages/@uppy/companion/src/server/helpers/upload.js +++ b/packages/@uppy/companion/src/server/helpers/upload.js @@ -19,13 +19,17 @@ async function startDownUpload ({ req, res, getSize, download, onUnhandledError // "Forking" off the upload operation to background, so we can return the http request: ;(async () => { - // wait till the client has connected to the socket, before starting - // the download, so that the client can receive all download/upload progress. - logger.debug('Waiting for socket connection before beginning remote download/upload.', null, req.id) - await uploader.awaitReady() - logger.debug('Socket connection received. Starting remote download/upload.', null, req.id) - - uploader.uploadStream(stream) + try { + // wait till the client has connected to the socket, before starting + // the download, so that the client can receive all download/upload progress. + logger.debug('Waiting for socket connection before beginning remote download/upload.', null, req.id) + await uploader.awaitReady() + logger.debug('Socket connection received. Starting remote download/upload.', null, req.id) + + await uploader.uploadStream(stream) + } catch (err) { + logger.error(err) + } })() // Respond the request From 670ca8fc21aafd87299b01437a16bbbc412584b6 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Sat, 23 Oct 2021 22:04:29 +0700 Subject: [PATCH 35/46] Only set default chunkSize if needed for being more compliant with previous behavior when streamingUpload = false --- packages/@uppy/companion/src/server/Uploader.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/@uppy/companion/src/server/Uploader.js b/packages/@uppy/companion/src/server/Uploader.js index 6707d8ebb1..325e9a88fd 100644 --- a/packages/@uppy/companion/src/server/Uploader.js +++ b/packages/@uppy/companion/src/server/Uploader.js @@ -12,7 +12,7 @@ const { promisify } = require('util') const pipeline = promisify(pipelineCb) -const { createReadStream, createWriteStream } = fs +const { createReadStream, createWriteStream, ReadStream } = fs const { stat, unlink } = fs.promises /** @type {any} */ @@ -465,6 +465,11 @@ class Uploader { async _uploadTus (stream) { const uploader = this + const isFileStream = stream instanceof ReadStream + // chunkSize needs to be a finite value if the stream is not a file stream (fs.createReadStream) + // https://github.com/tus/tus-js-client/blob/4479b78032937ac14da9b0542e489ac6fe7e0bc7/lib/node/fileReader.js#L50 + const chunkSize = this.options.chunkSize || (isFileStream ? Infinity : 50e6) + return new Promise((resolve, reject) => { this.tus = new tus.Upload(stream, { endpoint: this.options.endpoint, @@ -472,7 +477,7 @@ class Uploader { uploadLengthDeferred: false, retryDelays: [0, 1000, 3000, 5000], uploadSize: this.size, - chunkSize: this.options.chunkSize || 50e6, + chunkSize, headers: headerSanitize(this.options.headers), addRequestId: true, metadata: { From 1e28ef2a59f6ba4589ebdd0c5058d0586137f893 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Sat, 23 Oct 2021 22:19:27 +0700 Subject: [PATCH 36/46] Improve flaky test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Trying to fix this error: FAIL packages/@uppy/utils/src/delay.test.js ● delay › should reject when signal is aborted expect(received).toBeLessThan(expected) Expected: < 70 Received: 107 32 | const time = Date.now() - start 33 | expect(time).toBeGreaterThanOrEqual(30) > 34 | expect(time).toBeLessThan(70) | ^ 35 | }) 36 | }) 37 | at Object. (packages/@uppy/utils/src/delay.test.js:34:18) https://github.com/transloadit/uppy/runs/3984613454?check_suite_focus=true --- packages/@uppy/utils/src/delay.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@uppy/utils/src/delay.test.js b/packages/@uppy/utils/src/delay.test.js index 61afab4b57..059f6160eb 100644 --- a/packages/@uppy/utils/src/delay.test.js +++ b/packages/@uppy/utils/src/delay.test.js @@ -22,7 +22,7 @@ describe('delay', () => { it('should reject when signal is aborted', async () => { const controller = new AbortController() const start = Date.now() - const testDelay = delay(100, { signal: controller.signal }) + const testDelay = delay(1000, { signal: controller.signal }) await Promise.all([ delay(50).then(() => controller.abort()), expect(testDelay).rejects.toHaveProperty('name', 'AbortError'), @@ -31,6 +31,6 @@ describe('delay', () => { // should have rejected before the timer is done const time = Date.now() - start expect(time).toBeGreaterThanOrEqual(30) - expect(time).toBeLessThan(70) + expect(time).toBeLessThan(900) }) }) From d0ff2ccc2921525cfe6e8072fae2fca14777fef2 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Sun, 24 Oct 2021 21:11:17 +0700 Subject: [PATCH 37/46] Apply suggestions from code review Co-authored-by: Antoine du Hamel --- packages/@uppy/companion/src/server/Uploader.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/@uppy/companion/src/server/Uploader.js b/packages/@uppy/companion/src/server/Uploader.js index 325e9a88fd..61e79c62ae 100644 --- a/packages/@uppy/companion/src/server/Uploader.js +++ b/packages/@uppy/companion/src/server/Uploader.js @@ -10,6 +10,7 @@ const { join } = require('path') const fs = require('fs') const { promisify } = require('util') +// TODO move to `require('streams/promises').pipeline` when dropping support for Node.js 14.x. const pipeline = promisify(pipelineCb) const { createReadStream, createWriteStream, ReadStream } = fs @@ -461,6 +462,7 @@ class Uploader { /** * start the tus upload + * @param {any} stream */ async _uploadTus (stream) { const uploader = this From 9a6af7991b459fe5b3ec697e7ab5f99d61c66f21 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Wed, 27 Oct 2021 14:36:37 +0700 Subject: [PATCH 38/46] fix review feedback & lint --- .../@uppy/companion/src/server/Uploader.js | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/@uppy/companion/src/server/Uploader.js b/packages/@uppy/companion/src/server/Uploader.js index 61e79c62ae..08e7db4e9d 100644 --- a/packages/@uppy/companion/src/server/Uploader.js +++ b/packages/@uppy/companion/src/server/Uploader.js @@ -51,19 +51,19 @@ class Uploader { * * @typedef {object} UploaderOptions * @property {string} endpoint - * @property {string} uploadUrl + * @property {string} [uploadUrl] * @property {string} protocol - * @property {number} size - * @property {string} fieldname + * @property {number} [size] + * @property {string} [fieldname] * @property {string} pathPrefix - * @property {any} s3 + * @property {any} [s3] * @property {any} metadata * @property {any} companionOptions - * @property {any} storage - * @property {any} headers - * @property {string} httpMethod - * @property {boolean} useFormData - * @property {number} chunkSize + * @property {any} [storage] + * @property {any} [headers] + * @property {string} [httpMethod] + * @property {boolean} [useFormData] + * @property {number} [chunkSize] * * @param {UploaderOptions} options */ @@ -383,8 +383,8 @@ class Uploader { /** * - * @param {number} bytesUploaded - * @param {number | null} bytesTotalIn + * @param {number} [bytesUploaded] + * @param {number | null} [bytesTotalIn] */ onProgress (bytesUploaded, bytesTotalIn) { const bytesTotal = bytesTotalIn || this.size || 0 @@ -462,6 +462,7 @@ class Uploader { /** * start the tus upload + * * @param {any} stream */ async _uploadTus (stream) { @@ -507,11 +508,10 @@ class Uploader { }, /** * - * @param {number} bytesUploaded - * @param {number} bytesTotal + * @param {number} [bytesUploaded] + * @param {number} [bytesTotal] */ - // @ts-ignore - onProgress (bytesUploaded, bytesTotal) { // eslint-disable-line no-unused-vars + onProgress (bytesUploaded, bytesTotal) { uploader.onProgress(bytesUploaded, bytesTotal) }, onSuccess () { From 4eff3564c0ba895e1d016d29c090b1e72210b762 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Wed, 27 Oct 2021 23:09:56 +0700 Subject: [PATCH 39/46] Apply suggestions from code review Co-authored-by: Merlijn Vos --- packages/@uppy/companion/src/server/provider/unsplash/index.js | 2 -- website/src/docs/companion.md | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/@uppy/companion/src/server/provider/unsplash/index.js b/packages/@uppy/companion/src/server/provider/unsplash/index.js index 40476e3d44..0e96d5f818 100644 --- a/packages/@uppy/companion/src/server/provider/unsplash/index.js +++ b/packages/@uppy/companion/src/server/provider/unsplash/index.js @@ -105,8 +105,6 @@ class Unsplash extends SearchProvider { if (err || resp.statusCode !== 200) { const err2 = this.error(err, resp) logger.error(err2, 'provider.unsplash.download.location.error') - } else { - // console.log('download_location complete') } }) diff --git a/website/src/docs/companion.md b/website/src/docs/companion.md index 70cb438003..23e0993d1c 100644 --- a/website/src/docs/companion.md +++ b/website/src/docs/companion.md @@ -336,7 +336,7 @@ const options = { 13. **metrics(optional)** - A boolean flag to tell Companion whether to provide an endpoint `/metrics` with Prometheus metrics. -14. **streamingUpload(optional)** - A boolean flag to tell Companion whether to enable streaming uploads. If enabled, it will lead to _faster uploads_ because companion will start uploading at the same time as downloading using `stream.pipe`. If `false`, files will be fully downloaded first, then uploaded. Defaults to `false`. Do not set to `true` if you have any custom Companion providers that do not use the new async/stream API. +14. **streamingUpload(optional)** - A boolean flag to tell Companion whether to enable streaming uploads. If enabled, it will lead to _faster uploads_ because companion will start uploading at the same time as downloading using `stream.pipe`. If `false`, files will be fully downloaded first, then uploaded. Defaults to `false`. Do **not** set it to `true` if you have a [custom Companion provider](#adding-custom-providers) that does not use the new async/stream API. 15. **maxFileSize(optional)** - If this value is set, companion will limit the maximum file size to process. If unset, it will process files without any size limit (this is the default). From ecb86dab6b020661c20d1c3699fabcca38f11d4a Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Thu, 28 Oct 2021 22:04:34 +0700 Subject: [PATCH 40/46] remove unneeded ts-ignore --- packages/@uppy/companion/src/server/Uploader.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/@uppy/companion/src/server/Uploader.js b/packages/@uppy/companion/src/server/Uploader.js index 08e7db4e9d..260ef689cd 100644 --- a/packages/@uppy/companion/src/server/Uploader.js +++ b/packages/@uppy/companion/src/server/Uploader.js @@ -1,7 +1,6 @@ const tus = require('tus-js-client') const uuid = require('uuid') const isObject = require('isobject') -// @ts-ignore const validator = require('validator') const request = require('request') // eslint-disable-next-line no-unused-vars From b775d4a73ead91ac49aa821e84c667ad4cfb43dc Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Thu, 28 Oct 2021 22:13:01 +0700 Subject: [PATCH 41/46] Update packages/@uppy/companion/src/server/controllers/url.js Co-authored-by: Antoine du Hamel --- packages/@uppy/companion/src/server/controllers/url.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/@uppy/companion/src/server/controllers/url.js b/packages/@uppy/companion/src/server/controllers/url.js index 0e589c80ba..54ed4e4dee 100644 --- a/packages/@uppy/companion/src/server/controllers/url.js +++ b/packages/@uppy/companion/src/server/controllers/url.js @@ -53,7 +53,6 @@ const downloadURL = async (url, blockLocalIPs, traceId) => { agentClass: getProtectedHttpAgent((new URL(url)).protocol, blockLocalIPs), } - // return onDataChunk(new Error('test error')) return new Promise((resolve, reject) => { const req = request(opts) From 259e49394c7b9290fd0b012470714cb5fc821f4e Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Thu, 28 Oct 2021 22:16:21 +0700 Subject: [PATCH 42/46] Update packages/@uppy/companion/src/server/Uploader.js Co-authored-by: Antoine du Hamel --- packages/@uppy/companion/src/server/Uploader.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@uppy/companion/src/server/Uploader.js b/packages/@uppy/companion/src/server/Uploader.js index 260ef689cd..a5266dc446 100644 --- a/packages/@uppy/companion/src/server/Uploader.js +++ b/packages/@uppy/companion/src/server/Uploader.js @@ -385,7 +385,7 @@ class Uploader { * @param {number} [bytesUploaded] * @param {number | null} [bytesTotalIn] */ - onProgress (bytesUploaded, bytesTotalIn) { + onProgress (bytesUploaded = 0, bytesTotalIn = 0) { const bytesTotal = bytesTotalIn || this.size || 0 // If fully downloading before uploading, combine downloaded and uploaded bytes From 21b9d641d71bb351ae0d834ca23d43ec47d8b9a8 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Thu, 28 Oct 2021 22:18:11 +0700 Subject: [PATCH 43/46] reduce nesting --- .../companion/src/server/helpers/upload.js | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/packages/@uppy/companion/src/server/helpers/upload.js b/packages/@uppy/companion/src/server/helpers/upload.js index 9cd0f7a0a5..5e2a45c162 100644 --- a/packages/@uppy/companion/src/server/helpers/upload.js +++ b/packages/@uppy/companion/src/server/helpers/upload.js @@ -19,18 +19,14 @@ async function startDownUpload ({ req, res, getSize, download, onUnhandledError // "Forking" off the upload operation to background, so we can return the http request: ;(async () => { - try { - // wait till the client has connected to the socket, before starting - // the download, so that the client can receive all download/upload progress. - logger.debug('Waiting for socket connection before beginning remote download/upload.', null, req.id) - await uploader.awaitReady() - logger.debug('Socket connection received. Starting remote download/upload.', null, req.id) - - await uploader.uploadStream(stream) - } catch (err) { - logger.error(err) - } - })() + // wait till the client has connected to the socket, before starting + // the download, so that the client can receive all download/upload progress. + logger.debug('Waiting for socket connection before beginning remote download/upload.', null, req.id) + await uploader.awaitReady() + logger.debug('Socket connection received. Starting remote download/upload.', null, req.id) + + await uploader.uploadStream(stream) + })().catch((err) => logger.error(err)) // Respond the request // NOTE: Uploader will continue running after the http request is responded From 2338d2f776b4aacc54d68d538531f68d77d173df Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Thu, 28 Oct 2021 22:20:22 +0700 Subject: [PATCH 44/46] fix lint --- packages/@uppy/companion/src/server/controllers/url.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/@uppy/companion/src/server/controllers/url.js b/packages/@uppy/companion/src/server/controllers/url.js index 54ed4e4dee..b7bc41c5fe 100644 --- a/packages/@uppy/companion/src/server/controllers/url.js +++ b/packages/@uppy/companion/src/server/controllers/url.js @@ -53,7 +53,6 @@ const downloadURL = async (url, blockLocalIPs, traceId) => { agentClass: getProtectedHttpAgent((new URL(url)).protocol, blockLocalIPs), } - return new Promise((resolve, reject) => { const req = request(opts) .on('response', (resp) => { From 55a754a125a7acb41be4252e2b1704c2594b7f0a Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Thu, 28 Oct 2021 22:33:14 +0700 Subject: [PATCH 45/46] optimize promisify https://github.com/transloadit/uppy/pull/3159#discussion_r738160576 --- .../src/server/provider/box/index.js | 24 +++++------------- .../src/server/provider/drive/index.js | 6 ++--- .../src/server/provider/dropbox/index.js | 24 +++++------------- .../src/server/provider/facebook/index.js | 19 ++++---------- .../server/provider/instagram/graph/index.js | 14 +++-------- .../src/server/provider/onedrive/index.js | 14 +++-------- .../src/server/provider/unsplash/index.js | 11 +++----- .../src/server/provider/zoom/index.js | 25 ++++++------------- 8 files changed, 37 insertions(+), 100 deletions(-) diff --git a/packages/@uppy/companion/src/server/provider/box/index.js b/packages/@uppy/companion/src/server/provider/box/index.js index 109f05868d..8edd2dc37b 100644 --- a/packages/@uppy/companion/src/server/provider/box/index.js +++ b/packages/@uppy/companion/src/server/provider/box/index.js @@ -1,9 +1,8 @@ -const Provider = require('../Provider') - const request = require('request') const purest = require('purest')({ request }) const { promisify } = require('util') +const Provider = require('../Provider') const logger = require('../../logger') const adapter = require('./adapter') const { ProviderApiError, ProviderAuthError } = require('../error') @@ -38,10 +37,6 @@ class Box extends Provider { .request(done) } - async list (options) { - return promisify(this._list.bind(this))(options) - } - /** * Lists files and folders from Box API * @@ -86,10 +81,6 @@ class Box extends Provider { } } - async thumbnail (options) { - return promisify(this._thumbnail.bind(this))(options) - } - _thumbnail ({ id, token }, done) { return this.client .get(`files/${id}/thumbnail.png`) @@ -125,10 +116,6 @@ class Box extends Provider { }) } - async size (options) { - return promisify(this._size.bind(this))(options) - } - _size ({ id, token }, done) { return this.client .get(`files/${id}`) @@ -143,10 +130,6 @@ class Box extends Provider { }) } - async logout (options) { - return promisify(this._logout.bind(this))(options) - } - _logout ({ companion, token }, done) { const { key, secret } = companion.options.providerOptions.box @@ -205,4 +188,9 @@ 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) + module.exports = Box diff --git a/packages/@uppy/companion/src/server/provider/drive/index.js b/packages/@uppy/companion/src/server/provider/drive/index.js index f603aefc8e..fc412d36da 100644 --- a/packages/@uppy/companion/src/server/provider/drive/index.js +++ b/packages/@uppy/companion/src/server/provider/drive/index.js @@ -274,10 +274,6 @@ class Drive extends Provider { }) } - async logout (options) { - return promisify(this._logout.bind(this))(options) - } - _error (err, resp) { if (resp) { const fallbackMessage = `request to ${this.authProvider} returned ${resp.statusCode}` @@ -290,4 +286,6 @@ class Drive extends Provider { Drive.version = 2 +Drive.prototype.logout = promisify(Drive.prototype._logout) + module.exports = Drive diff --git a/packages/@uppy/companion/src/server/provider/dropbox/index.js b/packages/@uppy/companion/src/server/provider/dropbox/index.js index abb20af68b..5fea7f00a7 100644 --- a/packages/@uppy/companion/src/server/provider/dropbox/index.js +++ b/packages/@uppy/companion/src/server/provider/dropbox/index.js @@ -1,9 +1,8 @@ -const Provider = require('../Provider') - const request = require('request') const purest = require('purest')({ request }) const { promisify } = require('util') +const Provider = require('../Provider') const logger = require('../../logger') const adapter = require('./adapter') const { ProviderApiError, ProviderAuthError } = require('../error') @@ -48,10 +47,6 @@ class DropBox extends Provider { .request(done) } - async list (options) { - return promisify(this._list.bind(this))(options) - } - /** * Makes 2 requests in parallel - 1. to get files, 2. to get user email * it then waits till both requests are done before proceeding with the callback @@ -145,10 +140,6 @@ class DropBox extends Provider { } } - async thumbnail (options) { - return promisify(this._thumbnail.bind(this))(options) - } - _thumbnail ({ id, token }, done) { return this.client .post('https://content.dropboxapi.com/2/files/get_thumbnail') @@ -173,10 +164,6 @@ class DropBox extends Provider { }) } - async size (options) { - return promisify(this._size.bind(this))(options) - } - _size ({ id, token }, done) { return this.client .post('files/get_metadata') @@ -193,10 +180,6 @@ class DropBox extends Provider { }) } - async logout (options) { - return promisify(this._logout.bind(this))(options) - } - _logout ({ token }, done) { return this.client .post('auth/token/revoke') @@ -247,4 +230,9 @@ 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) + module.exports = DropBox diff --git a/packages/@uppy/companion/src/server/provider/facebook/index.js b/packages/@uppy/companion/src/server/provider/facebook/index.js index 5f446b1907..6e684dbdbc 100644 --- a/packages/@uppy/companion/src/server/provider/facebook/index.js +++ b/packages/@uppy/companion/src/server/provider/facebook/index.js @@ -1,9 +1,8 @@ -const Provider = require('../Provider') - const request = require('request') const purest = require('purest')({ request }) const { promisify } = require('util') +const Provider = require('../Provider') const { getURLMeta } = require('../../helpers/request') const logger = require('../../logger') const adapter = require('./adapter') @@ -27,10 +26,6 @@ class Facebook extends Provider { return 'facebook' } - async list (options) { - return promisify(this._list.bind(this))(options) - } - _list ({ directory, token, query = { cursor: null } }, done) { const qs = { fields: 'name,cover_photo,created_time,type', @@ -120,10 +115,6 @@ class Facebook extends Provider { throw new Error('call to thumbnail is not implemented') } - async size (options) { - return promisify(this._size.bind(this))(options) - } - _size ({ id, token }, done) { return this.client .get(`https://graph.facebook.com/${id}`) @@ -145,10 +136,6 @@ class Facebook extends Provider { }) } - async logout (options) { - return promisify(this._logout.bind(this))(options) - } - _logout ({ token }, done) { return this.client .delete('me/permissions') @@ -202,4 +189,8 @@ class Facebook extends Provider { Facebook.version = 2 +Facebook.prototype.list = promisify(Facebook.prototype._list) +Facebook.prototype.size = promisify(Facebook.prototype._size) +Facebook.prototype.logout = promisify(Facebook.prototype._logout) + module.exports = Facebook diff --git a/packages/@uppy/companion/src/server/provider/instagram/graph/index.js b/packages/@uppy/companion/src/server/provider/instagram/graph/index.js index a0917156a8..79b380d5cd 100644 --- a/packages/@uppy/companion/src/server/provider/instagram/graph/index.js +++ b/packages/@uppy/companion/src/server/provider/instagram/graph/index.js @@ -1,9 +1,8 @@ -const Provider = require('../../Provider') - const request = require('request') const purest = require('purest')({ request }) const { promisify } = require('util') +const Provider = require('../../Provider') const { getURLMeta } = require('../../../helpers/request') const logger = require('../../../logger') const adapter = require('./adapter') @@ -34,10 +33,6 @@ class Instagram extends Provider { return 'instagram' } - async list (options) { - return promisify(this._list.bind(this))(options) - } - _list ({ directory, token, query = { cursor: null } }, done) { const qs = { fields: 'id,media_type,thumbnail_url,media_url,timestamp,children{media_type,media_url,thumbnail_url,timestamp}', @@ -111,10 +106,6 @@ class Instagram extends Provider { throw new Error('call to thumbnail is not implemented') } - async size (options) { - return promisify(this._size.bind(this))(options) - } - _size ({ id, token }, done) { return this.client .get(`https://graph.instagram.com/${id}`) @@ -180,4 +171,7 @@ class Instagram extends Provider { Instagram.version = 2 +Instagram.prototype.list = promisify(Instagram.prototype._list) +Instagram.prototype.size = promisify(Instagram.prototype._size) + module.exports = Instagram diff --git a/packages/@uppy/companion/src/server/provider/onedrive/index.js b/packages/@uppy/companion/src/server/provider/onedrive/index.js index fc19b28c3b..b7ee7a62ae 100644 --- a/packages/@uppy/companion/src/server/provider/onedrive/index.js +++ b/packages/@uppy/companion/src/server/provider/onedrive/index.js @@ -1,9 +1,8 @@ -const Provider = require('../Provider') - const request = require('request') const purest = require('purest')({ request }) const { promisify } = require('util') +const Provider = require('../Provider') const logger = require('../../logger') const adapter = require('./adapter') const { ProviderApiError, ProviderAuthError } = require('../error') @@ -33,10 +32,6 @@ class OneDrive extends Provider { .request(done) } - async list (options) { - return promisify(this._list.bind(this))(options) - } - /** * Makes 2 requests in parallel - 1. to get files, 2. to get user email * it then waits till both requests are done before proceeding with the callback @@ -95,10 +90,6 @@ class OneDrive extends Provider { throw new Error('call to thumbnail is not implemented') } - async size (options) { - return promisify(this._size.bind(this))(options) - } - _size ({ id, query, token }, done) { const rootPath = query.driveId ? `/drives/${query.driveId}` : '/me/drive' return this.client @@ -153,4 +144,7 @@ class OneDrive extends Provider { OneDrive.version = 2 +OneDrive.prototype.list = promisify(OneDrive.prototype._list) +OneDrive.prototype.size = promisify(OneDrive.prototype._size) + module.exports = OneDrive diff --git a/packages/@uppy/companion/src/server/provider/unsplash/index.js b/packages/@uppy/companion/src/server/provider/unsplash/index.js index 0e96d5f818..424f85e0ca 100644 --- a/packages/@uppy/companion/src/server/provider/unsplash/index.js +++ b/packages/@uppy/companion/src/server/provider/unsplash/index.js @@ -41,10 +41,6 @@ function adaptData (body, currentQuery) { * Adapter for API https://api.unsplash.com */ class Unsplash extends SearchProvider { - async list (options) { - return promisify(this._list.bind(this))(options) - } - _list ({ token, query = { cursor: null, q: null } }, done) { const reqOpts = { url: `${BASE_URL}/search/photos`, @@ -115,10 +111,6 @@ class Unsplash extends SearchProvider { } } - async size (options) { - return promisify(this._size.bind(this))(options) - } - _size ({ id, token }, done) { const reqOpts = { url: `${BASE_URL}/photos/${id}`, @@ -161,4 +153,7 @@ class Unsplash extends SearchProvider { Unsplash.version = 2 +Unsplash.prototype.list = promisify(Unsplash.prototype._list) +Unsplash.prototype.size = promisify(Unsplash.prototype._size) + module.exports = Unsplash diff --git a/packages/@uppy/companion/src/server/provider/zoom/index.js b/packages/@uppy/companion/src/server/provider/zoom/index.js index 91bd16c4eb..cd12a1945f 100644 --- a/packages/@uppy/companion/src/server/provider/zoom/index.js +++ b/packages/@uppy/companion/src/server/provider/zoom/index.js @@ -1,9 +1,9 @@ -const Provider = require('../Provider') - const { promisify } = require('util') const request = require('request') const moment = require('moment-timezone') const purest = require('purest')({ request }) + +const Provider = require('../Provider') const logger = require('../../logger') const adapter = require('./adapter') const { ProviderApiError, ProviderAuthError } = require('../error') @@ -33,10 +33,6 @@ class Zoom extends Provider { return 'zoom' } - async list (options) { - return promisify(this._list.bind(this))(options) - } - _list (options, done) { /* - returns list of months by default @@ -151,10 +147,6 @@ class Zoom extends Provider { } } - async size (options) { - return promisify(this._size.bind(this))(options) - } - _size ({ id, token, query }, done) { const meetingId = id const fileId = query.recordingId @@ -259,10 +251,6 @@ class Zoom extends Provider { return data } - async logout (options) { - return promisify(this._logout.bind(this))(options) - } - _logout ({ companion, token }, done) { companion.getProviderCredentials().then(({ key, secret }) => { const encodedAuth = Buffer.from(`${key}:${secret}`, 'binary').toString('base64') @@ -285,10 +273,6 @@ class Zoom extends Provider { }).catch((err) => done(err)) } - async deauthorizationCallback (options) { - return promisify(this._deauthorizationCallback.bind(this))(options) - } - _deauthorizationCallback ({ companion, body, headers }, done) { if (!body || body.event !== DEAUTH_EVENT_NAME) { done(null, { data: {}, status: 400 }) @@ -357,4 +341,9 @@ class Zoom extends Provider { Zoom.version = 2 +Zoom.prototype.list = promisify(Zoom.prototype._list) +Zoom.prototype.size = promisify(Zoom.prototype._size) +Zoom.prototype.logout = promisify(Zoom.prototype._logout) +Zoom.prototype.deauthorizationCallback = promisify(Zoom.prototype._deauthorizationCallback) + module.exports = Zoom From 291d8e351de24fce2d2816b9a0663320cdf4d678 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Fri, 29 Oct 2021 22:36:51 +0700 Subject: [PATCH 46/46] Update packages/@uppy/companion/test/__tests__/uploader.js Co-authored-by: Antoine du Hamel --- packages/@uppy/companion/test/__tests__/uploader.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/@uppy/companion/test/__tests__/uploader.js b/packages/@uppy/companion/test/__tests__/uploader.js index 9a01a05f05..6ba32c07ef 100644 --- a/packages/@uppy/companion/test/__tests__/uploader.js +++ b/packages/@uppy/companion/test/__tests__/uploader.js @@ -180,10 +180,11 @@ describe('uploader with tus protocol', () => { const uploadToken = uploader.token expect(uploader.hasError()).toBe(false) + // validate that the test is resolved on socket connection + uploader.awaitReady().then(uploader.uploadStream(stream)) + socketClient.connect(uploadToken) + return new Promise((resolve, reject) => { - // validate that the test is resolved on socket connection - uploader.awaitReady().then(uploader.uploadStream(stream)) - socketClient.connect(uploadToken) socketClient.onUploadError(uploadToken, (message) => { try { expect(message).toMatchObject({ payload: { error: { message: 'maxFileSize exceeded' } } })