From 5e35987079366c82322e5cc88bee4aee22d234aa Mon Sep 17 00:00:00 2001 From: Artur Paikin Date: Fri, 28 Jun 2019 20:25:17 +0300 Subject: [PATCH] Log error in uppy.addFile try/catch (#1680) * Log error in uppy.addFile try/catch * Add `.isRestriction` to a custom RestrictionError, check for that before logging * keep throwing for robodog.upload //cc @goto-bus-stop --- packages/@uppy/core/src/index.js | 49 +++++++++++-------- packages/@uppy/dashboard/src/index.js | 4 +- packages/@uppy/drag-drop/src/index.js | 4 +- packages/@uppy/file-input/src/index.js | 4 +- packages/@uppy/provider-views/src/index.js | 14 ++++-- .../@uppy/robodog/src/AttachFileInputs.js | 4 +- packages/@uppy/url/src/index.js | 11 ++--- packages/@uppy/webcam/src/index.js | 10 +++- 8 files changed, 61 insertions(+), 39 deletions(-) diff --git a/packages/@uppy/core/src/index.js b/packages/@uppy/core/src/index.js index cf0565e06..9d826ff05 100644 --- a/packages/@uppy/core/src/index.js +++ b/packages/@uppy/core/src/index.js @@ -12,6 +12,13 @@ const getTimeStamp = require('@uppy/utils/lib/getTimeStamp') const supportsUploadProgress = require('./supportsUploadProgress') const Plugin = require('./Plugin') // Exported from here. +class RestrictionError extends Error { + constructor (...args) { + super(...args) + this.isRestriction = true + } +} + /** * Uppy Core module. * Manages plugins, state updates, acts as an event bus, @@ -22,7 +29,8 @@ class Uppy { /** * Instantiate Uppy - * @param {object} opts — Uppy options + * + * @param {Object} opts — Uppy options */ constructor (opts) { this.defaultLocale = { @@ -191,7 +199,7 @@ class Uppy { /** * Updates state with a patch * - * @param {object} patch {foo: 'bar'} + * @param {Object} patch {foo: 'bar'} */ setState (patch) { this.store.setState(patch) @@ -199,7 +207,8 @@ class Uppy { /** * Returns current state. - * @return {object} + * + * @returns {Object} */ getState () { return this.store.getState() @@ -341,7 +350,7 @@ class Uppy { _checkMinNumberOfFiles (files) { const { minNumberOfFiles } = this.opts.restrictions if (Object.keys(files).length < minNumberOfFiles) { - throw new Error(`${this.i18n('youHaveToAtLeastSelectX', { smart_count: minNumberOfFiles })}`) + throw new RestrictionError(`${this.i18n('youHaveToAtLeastSelectX', { smart_count: minNumberOfFiles })}`) } } @@ -349,7 +358,7 @@ class Uppy { * Check if file passes a set of restrictions set in options: maxFileSize, * maxNumberOfFiles and allowedFileTypes. * - * @param {object} file object to check + * @param {Object} file object to check * @private */ _checkRestrictions (file) { @@ -357,7 +366,7 @@ class Uppy { if (maxNumberOfFiles) { if (Object.keys(this.getState().files).length + 1 > maxNumberOfFiles) { - throw new Error(`${this.i18n('youCanOnlyUploadX', { smart_count: maxNumberOfFiles })}`) + throw new RestrictionError(`${this.i18n('youCanOnlyUploadX', { smart_count: maxNumberOfFiles })}`) } } @@ -380,14 +389,14 @@ class Uppy { if (!isCorrectFileType) { const allowedFileTypesString = allowedFileTypes.join(', ') - throw new Error(this.i18n('youCanOnlyUploadFileTypes', { types: allowedFileTypesString })) + throw new RestrictionError(this.i18n('youCanOnlyUploadFileTypes', { types: allowedFileTypesString })) } } // We can't check maxFileSize if the size is unknown. if (maxFileSize && file.data.size != null) { if (file.data.size > maxFileSize) { - throw new Error(`${this.i18n('exceedsSize')} ${prettyBytes(maxFileSize)}`) + throw new RestrictionError(`${this.i18n('exceedsSize')} ${prettyBytes(maxFileSize)}`) } } } @@ -397,7 +406,7 @@ class Uppy { * try to guess file type in a clever way, check file against restrictions, * and start an upload if `autoProceed === true`. * - * @param {object} file object to add + * @param {Object} file object to add */ addFile (file) { const { files, allowNewUpload } = this.getState() @@ -880,9 +889,9 @@ class Uppy { /** * Registers a plugin with Core. * - * @param {object} Plugin object - * @param {object} [opts] object with options to be passed to Plugin - * @return {Object} self for chaining + * @param {Object} Plugin object + * @param {Object} [opts] object with options to be passed to Plugin + * @returns {Object} self for chaining */ use (Plugin, opts) { if (typeof Plugin !== 'function') { @@ -926,7 +935,7 @@ class Uppy { * Find one Plugin by name. * * @param {string} id plugin id - * @return {object | boolean} + * @returns {Object|boolean} */ getPlugin (id) { let foundPlugin = null @@ -942,7 +951,7 @@ class Uppy { /** * Iterate through all `use`d plugins. * - * @param {function} method that will be run on each plugin + * @param {Function} method that will be run on each plugin */ iteratePlugins (method) { Object.keys(this.plugins).forEach(pluginType => { @@ -953,7 +962,7 @@ class Uppy { /** * Uninstall and remove a plugin. * - * @param {object} instance The plugin instance to remove. + * @param {Object} instance The plugin instance to remove. */ removePlugin (instance) { this.log(`Removing plugin ${instance.id}`) @@ -1036,8 +1045,8 @@ class Uppy { /** * Logs stuff to console, only if `debug` is set to true. Silent in production. * - * @param {String|Object} message to log - * @param {String} [type] optional `error` or `warning` + * @param {string|Object} message to log + * @param {string} [type] optional `error` or `warning` */ log (message, type) { if (!this.opts.debug) { @@ -1085,7 +1094,7 @@ class Uppy { * Create an upload for a bunch of files. * * @param {Array} fileIDs File IDs to include in this upload. - * @return {string} ID of this upload. + * @returns {string} ID of this upload. */ _createUpload (fileIDs) { const { allowNewUpload, currentUploads } = this.getState() @@ -1126,7 +1135,7 @@ class Uppy { * Add data to an upload's result object. * * @param {string} uploadID The ID of the upload. - * @param {object} data Data properties to add to the result object. + * @param {Object} data Data properties to add to the result object. */ addResultData (uploadID, data) { if (!this._getUpload(uploadID)) { @@ -1250,7 +1259,7 @@ class Uppy { /** * Start an upload for all the files that are not currently being uploaded. * - * @return {Promise} + * @returns {Promise} */ upload () { if (!this.plugins.uploader) { diff --git a/packages/@uppy/dashboard/src/index.js b/packages/@uppy/dashboard/src/index.js index d2233bae2..c896438fe 100644 --- a/packages/@uppy/dashboard/src/index.js +++ b/packages/@uppy/dashboard/src/index.js @@ -376,7 +376,9 @@ module.exports = class Dashboard extends Plugin { } }) } catch (err) { - // Nothing, restriction errors handled in Core + if (!err.isRestriction) { + this.uppy.log(err) + } } } diff --git a/packages/@uppy/drag-drop/src/index.js b/packages/@uppy/drag-drop/src/index.js index 922b3625d..5644ebd19 100644 --- a/packages/@uppy/drag-drop/src/index.js +++ b/packages/@uppy/drag-drop/src/index.js @@ -67,7 +67,9 @@ module.exports = class DragDrop extends Plugin { } }) } catch (err) { - // Nothing, restriction errors handled in Core + if (!err.isRestriction) { + this.uppy.log(err) + } } } diff --git a/packages/@uppy/file-input/src/index.js b/packages/@uppy/file-input/src/index.js index 676eb4ad1..3204eb227 100644 --- a/packages/@uppy/file-input/src/index.js +++ b/packages/@uppy/file-input/src/index.js @@ -55,7 +55,9 @@ module.exports = class FileInput extends Plugin { data: file }) } catch (err) { - // Nothing, restriction errors handled in Core + if (!err.isRestriction) { + this.uppy.log(err) + } } }) diff --git a/packages/@uppy/provider-views/src/index.js b/packages/@uppy/provider-views/src/index.js index 3808e9b07..8564450d8 100644 --- a/packages/@uppy/provider-views/src/index.js +++ b/packages/@uppy/provider-views/src/index.js @@ -41,7 +41,7 @@ module.exports = class ProviderView { static VERSION = require('../package.json').version /** - * @param {object} instance of the plugin + * @param {Object} instance of the plugin */ constructor (plugin, opts) { this.plugin = plugin @@ -113,8 +113,9 @@ module.exports = class ProviderView { /** * Based on folder ID, fetch a new folder and update it to state - * @param {String} id Folder id - * @return {Promise} Folders/files in folder + * + * @param {string} id Folder id + * @returns {Promise} Folders/files in folder */ getFolder (id, name) { return this._loaderWrapper( @@ -142,8 +143,9 @@ module.exports = class ProviderView { /** * Fetches new folder + * * @param {Object} Folder - * @param {String} title Folder title + * @param {string} title Folder title */ getNextFolder (folder) { this.getFolder(folder.requestPath, folder.name) @@ -180,7 +182,9 @@ module.exports = class ProviderView { try { this.plugin.uppy.addFile(tagFile) } catch (err) { - // Nothing, restriction errors handled in Core + if (!err.isRestriction) { + this.plugin.uppy.log(err) + } } } diff --git a/packages/@uppy/robodog/src/AttachFileInputs.js b/packages/@uppy/robodog/src/AttachFileInputs.js index d63e9429c..dd0983e84 100644 --- a/packages/@uppy/robodog/src/AttachFileInputs.js +++ b/packages/@uppy/robodog/src/AttachFileInputs.js @@ -31,7 +31,9 @@ class AttachFileInputs extends Plugin { data: file }) } catch (err) { - // Nothing, restriction errors handled in Core + if (!err.isRestriction) { + this.uppy.log(err) + } } }) } diff --git a/packages/@uppy/url/src/index.js b/packages/@uppy/url/src/index.js index 5b29ffe7e..7ef84f9f3 100644 --- a/packages/@uppy/url/src/index.js +++ b/packages/@uppy/url/src/index.js @@ -133,16 +133,11 @@ module.exports = class Url extends Plugin { try { this.uppy.addFile(tagFile) } catch (err) { - // Nothing, restriction errors handled in Core + if (!err.isRestriction) { + this.uppy.log(err) + } } }) - .then(() => { - // Close the Dashboard panel if plugin is installed - // into Dashboard (could be other parent UI plugin) - // if (this.parent && this.parent.hideAllPanels) { - // this.parent.hideAllPanels() - // } - }) .catch((err) => { this.uppy.log(err) this.uppy.info({ diff --git a/packages/@uppy/webcam/src/index.js b/packages/@uppy/webcam/src/index.js index 700ab7581..474496bde 100644 --- a/packages/@uppy/webcam/src/index.js +++ b/packages/@uppy/webcam/src/index.js @@ -177,7 +177,10 @@ module.exports = class Webcam extends Plugin { try { this.uppy.addFile(file) } catch (err) { - // Nothing, restriction errors handled in Core + // Logging the error, exept restrictions, which is handled in Core + if (!err.isRestriction) { + this.uppy.log(err) + } } }).then(() => { this.recordingChunks = null @@ -253,7 +256,10 @@ module.exports = class Webcam extends Plugin { try { this.uppy.addFile(tagFile) } catch (err) { - // Nothing, restriction errors handled in Core + // Logging the error, exept restrictions, which is handled in Core + if (!err.isRestriction) { + this.uppy.log(err) + } } }, (error) => { this.captureInProgress = false