diff --git a/examples/custom-provider/server/index.cjs b/examples/custom-provider/server/index.cjs index fb9dc91ab7..e7f8538b73 100644 --- a/examples/custom-provider/server/index.cjs +++ b/examples/custom-provider/server/index.cjs @@ -71,7 +71,7 @@ app.use((req, res) => { // handle server errors app.use((err, req, res) => { console.error('\x1b[31m', err.stack, '\x1b[0m') - res.status(err.status || 500).json({ message: err.message, error: err }) + res.status(500).json({ message: err.message, error: err }) }) uppy.socket(app.listen(3020), uppyOptions) diff --git a/examples/uppy-with-companion/server/index.js b/examples/uppy-with-companion/server/index.js index 204fc4b27a..ed85fa3f79 100644 --- a/examples/uppy-with-companion/server/index.js +++ b/examples/uppy-with-companion/server/index.js @@ -64,7 +64,7 @@ app.use((req, res) => { // handle server errors app.use((err, req, res) => { console.error('\x1b[31m', err.stack, '\x1b[0m') - res.status(err.status || 500).json({ message: err.message, error: err }) + res.status(500).json({ message: err.message, error: err }) }) companion.socket(app.listen(3020)) diff --git a/packages/@uppy/companion/src/server/controllers/get.js b/packages/@uppy/companion/src/server/controllers/get.js index 6364399967..75406f33c1 100644 --- a/packages/@uppy/companion/src/server/controllers/get.js +++ b/packages/@uppy/companion/src/server/controllers/get.js @@ -1,5 +1,7 @@ const logger = require('../logger') const { startDownUpload } = require('../helpers/upload') +const { respondWithError } = require('../provider/error') + async function get (req, res) { const { id } = req.params @@ -17,6 +19,7 @@ async function get (req, res) { await startDownUpload({ req, res, getSize, download }) } catch (err) { logger.error(err, 'controller.get.error', req.id) + if (respondWithError(err, res)) return res.status(500).json({ message: 'Failed to download file' }) } } diff --git a/packages/@uppy/companion/src/server/controllers/googlePicker.js b/packages/@uppy/companion/src/server/controllers/googlePicker.js index 4b3dd051ee..b08d66a605 100644 --- a/packages/@uppy/companion/src/server/controllers/googlePicker.js +++ b/packages/@uppy/companion/src/server/controllers/googlePicker.js @@ -7,6 +7,7 @@ const { getURLMeta } = require('../helpers/request') const logger = require('../logger') const { downloadURL } = require('../download') const { getGoogleFileSize, streamGoogleFile } = require('../provider/google/drive'); +const { respondWithError } = require('../provider/error') const getAuthHeader = (token) => ({ authorization: `Bearer ${token}` }); @@ -49,7 +50,8 @@ const get = async (req, res) => { await startDownUpload({ req, res, getSize, download }) } catch (err) { logger.error(err, 'controller.googlePicker.error', req.id) - res.status(err.status || 500).json({ message: 'failed to fetch Google Picker URL' }) + if (respondWithError(err, res)) return + res.status(500).json({ message: 'failed to fetch Google Picker URL' }) } } diff --git a/packages/@uppy/companion/src/server/controllers/url.js b/packages/@uppy/companion/src/server/controllers/url.js index 838d659ee7..99b1e96667 100644 --- a/packages/@uppy/companion/src/server/controllers/url.js +++ b/packages/@uppy/companion/src/server/controllers/url.js @@ -5,6 +5,7 @@ const { downloadURL } = require('../download') const { validateURL } = require('../helpers/request') const { getURLMeta } = require('../helpers/request') const logger = require('../logger') +const { respondWithError } = require('../provider/error') /** * @callback downloadCallback @@ -24,14 +25,16 @@ const meta = async (req, res) => { const { allowLocalUrls } = req.companion.options if (!validateURL(req.body.url, allowLocalUrls)) { logger.debug('Invalid request body detected. Exiting url meta handler.', null, req.id) - return res.status(400).json({ error: 'Invalid request body' }) + res.status(400).json({ error: 'Invalid request body' }) + return } const urlMeta = await getURLMeta(req.body.url, allowLocalUrls) - return res.json(urlMeta) + res.json(urlMeta) } catch (err) { logger.error(err, 'controller.url.meta.error', req.id) - return res.status(err.status || 500).json({ message: 'failed to fetch URL metadata' }) + if (respondWithError(err, res)) return + res.status(500).json({ message: 'failed to fetch URL metadata' }) } } @@ -62,7 +65,8 @@ const get = async (req, res) => { await startDownUpload({ req, res, getSize, download }) } catch (err) { logger.error(err, 'controller.url.error', req.id) - res.status(err.status || 500).json({ message: 'failed to fetch URL' }) + if (respondWithError(err, res)) return + res.status(500).json({ message: 'failed to fetch URL' }) } } diff --git a/packages/@uppy/companion/src/server/helpers/upload.js b/packages/@uppy/companion/src/server/helpers/upload.js index b628634eeb..b508c52dff 100644 --- a/packages/@uppy/companion/src/server/helpers/upload.js +++ b/packages/@uppy/companion/src/server/helpers/upload.js @@ -1,51 +1,38 @@ const Uploader = require('../Uploader') const logger = require('../logger') -const { respondWithError } = require('../provider/error') async function startDownUpload({ req, res, getSize, download }) { - try { - logger.debug('Starting download stream.', null, req.id) - const { stream, size: maybeSize } = await download() + logger.debug('Starting download stream.', null, req.id) + const { stream, size: maybeSize } = await download() - let size - // if the provider already knows the size, we can use that - if (typeof maybeSize === 'number' && !Number.isNaN(maybeSize) && maybeSize > 0) { - size = maybeSize - } - // if not we need to get the size - if (size == null) { - size = await getSize() - } - const { clientSocketConnectTimeout } = req.companion.options - - logger.debug('Instantiating uploader.', null, req.id) - const uploader = new Uploader(Uploader.reqToOptions(req, size)) - - // "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(clientSocketConnectTimeout) - logger.debug('Socket connection received. Starting remote download/upload.', null, req.id) + let size + // if the provider already knows the size, we can use that + if (typeof maybeSize === 'number' && !Number.isNaN(maybeSize) && maybeSize > 0) { + size = maybeSize + } + // if not we need to get the size + if (size == null) { + size = await getSize() + } + const { clientSocketConnectTimeout } = req.companion.options - await uploader.tryUploadStream(stream, req) - })().catch((err) => logger.error(err)) + logger.debug('Instantiating uploader.', null, req.id) + const uploader = new Uploader(Uploader.reqToOptions(req, size)) - // Respond the request - // NOTE: the Uploader will continue running after the http request is responded - res.status(200).json({ token: uploader.token }) - } catch (err) { - if (err.name === 'ValidationError') { - logger.debug(err.message, 'uploader.validator.fail') - res.status(400).json({ message: err.message }) - return - } + // "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(clientSocketConnectTimeout) + logger.debug('Socket connection received. Starting remote download/upload.', null, req.id) - if (respondWithError(err, res)) return + await uploader.tryUploadStream(stream, req) + })().catch((err) => logger.error(err)) - throw err - } + // Respond the request + // NOTE: the Uploader will continue running after the http request is responded + res.status(200).json({ token: uploader.token }) } module.exports = { startDownUpload } diff --git a/packages/@uppy/companion/src/server/helpers/utils.js b/packages/@uppy/companion/src/server/helpers/utils.js index 4578f0d224..26fb46c330 100644 --- a/packages/@uppy/companion/src/server/helpers/utils.js +++ b/packages/@uppy/companion/src/server/helpers/utils.js @@ -148,6 +148,9 @@ module.exports.decrypt = (encrypted, secret) => { module.exports.defaultGetKey = ({ filename }) => `${crypto.randomUUID()}-${filename}` +/** + * Our own HttpError in cases where we can't use `got`'s `HTTPError` + */ class HttpError extends Error { statusCode @@ -176,7 +179,7 @@ module.exports.prepareStream = async (stream) => new Promise((resolve, reject) = }) .on('error', (err) => { // In this case the error object is not a normal GOT HTTPError where json is already parsed, - // we create our own HttpError error for this case + // we use our own HttpError error for this scenario. if (typeof err.response?.body === 'string' && typeof err.response?.statusCode === 'number') { let responseJson try { diff --git a/packages/@uppy/companion/src/server/provider/error.js b/packages/@uppy/companion/src/server/provider/error.js index efb4a53381..1f76212bff 100644 --- a/packages/@uppy/companion/src/server/provider/error.js +++ b/packages/@uppy/companion/src/server/provider/error.js @@ -30,6 +30,8 @@ class ProviderUserError extends ProviderApiError { /** * AuthError is error returned when an adapter encounters * an authorization error while communication with its corresponding provider + * this signals to the client that the access token is invalid and needs to be + * refreshed or the user needs to re-authenticate */ class ProviderAuthError extends ProviderApiError { constructor() { @@ -39,10 +41,27 @@ class ProviderAuthError extends ProviderApiError { } } +function parseHttpError(err) { + if (err?.name === 'HTTPError') { + return { + statusCode: err.response?.statusCode, + body: err.response?.body, + } + } + if (err?.name === 'HttpError') { + return { + statusCode: err.statusCode, + body: err.responseJson, + } + } + return undefined +} + /** * Convert an error instance to an http response if possible * * @param {Error | ProviderApiError} err the error instance to convert to an http json response + * @returns {object | undefined} an object with a code and json field if the error can be converted to a response */ function errorToResponse(err) { // @ts-ignore @@ -50,6 +69,10 @@ function errorToResponse(err) { return { code: 401, json: { message: err.message } } } + if (err?.name === 'ValidationError') { + return { code: 400, json: { message: err.message } } + } + if (err?.name === 'ProviderUserError') { // @ts-ignore return { code: 400, json: err.json } @@ -74,6 +97,12 @@ function errorToResponse(err) { } } + const httpError = parseHttpError(err) + if (httpError) { + // We proxy the response purely for ease of debugging + return { code: 500, json: { statusCode: httpError.statusCode, body: httpError.body } } + } + return undefined } @@ -86,4 +115,4 @@ function respondWithError(err, res) { return false } -module.exports = { ProviderAuthError, ProviderApiError, ProviderUserError, respondWithError } +module.exports = { ProviderAuthError, ProviderApiError, ProviderUserError, respondWithError, parseHttpError } diff --git a/packages/@uppy/companion/src/server/provider/providerErrors.js b/packages/@uppy/companion/src/server/provider/providerErrors.js index 2497efff63..621fec9209 100644 --- a/packages/@uppy/companion/src/server/provider/providerErrors.js +++ b/packages/@uppy/companion/src/server/provider/providerErrors.js @@ -1,5 +1,5 @@ const logger = require('../logger') -const { ProviderApiError, ProviderUserError, ProviderAuthError } = require('./error') +const { ProviderApiError, ProviderUserError, ProviderAuthError, parseHttpError } = require('./error') /** * @@ -37,18 +37,11 @@ async function withProviderErrorHandling({ try { return await fn() } catch (err) { - let statusCode - let body + const httpError = parseHttpError(err) - if (err?.name === 'HTTPError') { - statusCode = err.response?.statusCode - body = err.response?.body - } else if (err?.name === 'HttpError') { - statusCode = err.statusCode - body = err.responseJson - } - - if (statusCode != null) { + // Wrap all HTTP errors according to the provider's desired error handling + if (httpError) { + const { statusCode, body } = httpError let knownErr if (isAuthError({ statusCode, body })) { knownErr = new ProviderAuthError() @@ -62,8 +55,8 @@ async function withProviderErrorHandling({ throw knownErr } + // non HTTP errors will be passed through logger.error(err, tag) - throw err } } @@ -81,4 +74,4 @@ async function withGoogleErrorHandling (providerName, tag, fn) { }) } -module.exports = { withProviderErrorHandling, withGoogleErrorHandling } +module.exports = { withProviderErrorHandling, withGoogleErrorHandling, parseHttpError } diff --git a/packages/@uppy/companion/src/standalone/index.js b/packages/@uppy/companion/src/standalone/index.js index fefaf64f8e..5afa2f2fde 100644 --- a/packages/@uppy/companion/src/standalone/index.js +++ b/packages/@uppy/companion/src/standalone/index.js @@ -182,10 +182,10 @@ module.exports = function server(inputCompanionOptions) { } else { logger.error(err, 'root.error', req.id) } - res.status(err.status || 500).json({ message: 'Something went wrong', requestId: req.id }) + res.status(500).json({ message: 'Something went wrong', requestId: req.id }) } else { logger.error(err, 'root.error', req.id) - res.status(err.status || 500).json({ message: err.message, error: err, requestId: req.id }) + res.status(500).json({ message: err.message, error: err, requestId: req.id }) } })