diff --git a/packages/@uppy/companion-client/src/Provider.js b/packages/@uppy/companion-client/src/Provider.js index 66231b0ef..081f3cdc6 100644 --- a/packages/@uppy/companion-client/src/Provider.js +++ b/packages/@uppy/companion-client/src/Provider.js @@ -135,8 +135,8 @@ export default class Provider extends RequestClient { return this.get(`${this.id}/list/${directory || ''}`, options) } - async logout () { - const response = await this.get(`${this.id}/logout`) + async logout (options) { + const response = await this.get(`${this.id}/logout`, options) await this.#removeAuthToken() return response } diff --git a/packages/@uppy/provider-views/src/ProviderView/ProviderView.jsx b/packages/@uppy/provider-views/src/ProviderView/ProviderView.jsx index 653cbe3db..7bc38b0cc 100644 --- a/packages/@uppy/provider-views/src/ProviderView/ProviderView.jsx +++ b/packages/@uppy/provider-views/src/ProviderView/ProviderView.jsx @@ -95,6 +95,29 @@ export default class ProviderView extends View { // Nothing. } + #abortController + + async #withAbort (op) { + // prevent multiple requests in parallel from causing race conditions + this.#abortController?.abort() + const abortController = new AbortController() + this.#abortController = abortController + const cancelRequest = () => { + abortController.abort() + this.clearSelection() + } + try { + this.plugin.uppy.on('dashboard:close-panel', cancelRequest) + this.plugin.uppy.on('cancel-all', cancelRequest) + + await op(abortController.signal) + } finally { + this.plugin.uppy.off('dashboard:close-panel', cancelRequest) + this.plugin.uppy.off('cancel-all', cancelRequest) + this.#abortController = undefined + } + } + async #list ({ requestPath, absDirPath, signal }) { const { username, nextPagePath, items } = await this.provider.list(requestPath, { signal }) this.username = username || this.username @@ -139,58 +162,45 @@ export default class ProviderView extends View { * @returns {Promise} Folders/files in folder */ async getFolder (requestPath, name) { - const controller = new AbortController() - const cancelRequest = () => { - controller.abort() - this.clearSelection() - } - - this.plugin.uppy.on('dashboard:close-panel', cancelRequest) - this.plugin.uppy.on('cancel-all', cancelRequest) - this.setLoading(true) try { - this.lastCheckbox = undefined + await this.#withAbort(async (signal) => { + this.lastCheckbox = undefined - let { breadcrumbs } = this.plugin.getPluginState() + let { breadcrumbs } = this.plugin.getPluginState() - const index = breadcrumbs.findIndex((dir) => requestPath === dir.requestPath) + const index = breadcrumbs.findIndex((dir) => requestPath === dir.requestPath) - if (index !== -1) { - // means we navigated back to a known directory (already in the stack), so cut the stack off there - breadcrumbs = breadcrumbs.slice(0, index + 1) - } else { - // we have navigated into a new (unknown) folder, add it to the stack - breadcrumbs = [...breadcrumbs, { requestPath, name }] - } + if (index !== -1) { + // means we navigated back to a known directory (already in the stack), so cut the stack off there + breadcrumbs = breadcrumbs.slice(0, index + 1) + } else { + // we have navigated into a new (unknown) folder, add it to the stack + breadcrumbs = [...breadcrumbs, { requestPath, name }] + } - this.nextPagePath = requestPath - let files = [] - let folders = [] - do { - const { files: newFiles, folders: newFolders } = await this.#listFilesAndFolders({ - breadcrumbs, signal: controller.signal, - }) + this.nextPagePath = requestPath + let files = [] + let folders = [] + do { + const { files: newFiles, folders: newFolders } = await this.#listFilesAndFolders({ + breadcrumbs, signal, + }) - files = files.concat(newFiles) - folders = folders.concat(newFolders) + files = files.concat(newFiles) + folders = folders.concat(newFolders) - this.setLoading(this.plugin.uppy.i18n('loadedXFiles', { numFiles: files.length + folders.length })) - } while ( - this.opts.loadAllFiles && this.nextPagePath - ) + this.setLoading(this.plugin.uppy.i18n('loadedXFiles', { numFiles: files.length + folders.length })) + } while ( + this.opts.loadAllFiles && this.nextPagePath + ) - this.plugin.setPluginState({ folders, files, breadcrumbs, filterInput: '' }) + this.plugin.setPluginState({ folders, files, breadcrumbs, filterInput: '' }) + }) } catch (err) { - if (err.cause?.name === 'AbortError') { - // Expected, user clicked “cancel” - return - } this.handleError(err) } finally { this.setLoading(false) - this.plugin.uppy.off('dashboard:close-panel', cancelRequest) - this.plugin.uppy.off('cancel-all', cancelRequest) } } @@ -207,9 +217,10 @@ export default class ProviderView extends View { /** * Removes session token on client side. */ - logout () { - this.provider.logout() - .then((res) => { + async logout () { + try { + await this.#withAbort(async (signal) => { + const res = await this.provider.logout({ signal }) if (res.ok) { if (!res.revoked) { const message = this.plugin.uppy.i18n('companionUnauthorizeHint', { @@ -228,7 +239,10 @@ export default class ProviderView extends View { } this.plugin.setPluginState(newState) } - }).catch(this.handleError) + }) + } catch (err) { + this.handleError(err) + } } filterQuery (input) { @@ -286,14 +300,18 @@ export default class ProviderView extends View { this.isHandlingScroll = true try { - const { files, folders, breadcrumbs } = this.plugin.getPluginState() + await this.#withAbort(async (signal) => { + const { files, folders, breadcrumbs } = this.plugin.getPluginState() - const { files: newFiles, folders: newFolders } = await this.#listFilesAndFolders({ breadcrumbs }) + const { files: newFiles, folders: newFolders } = await this.#listFilesAndFolders({ + breadcrumbs, signal, + }) - const combinedFiles = files.concat(newFiles) - const combinedFolders = folders.concat(newFolders) + const combinedFiles = files.concat(newFiles) + const combinedFolders = folders.concat(newFolders) - this.plugin.setPluginState({ folders: combinedFolders, files: combinedFiles }) + this.plugin.setPluginState({ folders: combinedFolders, files: combinedFiles }) + }) } catch (error) { this.handleError(error) } finally { @@ -302,11 +320,11 @@ export default class ProviderView extends View { } } - async #recursivelyListAllFiles ({ requestPath, absDirPath, relDirPath, queue, onFiles }) { + async #recursivelyListAllFiles ({ requestPath, absDirPath, relDirPath, queue, onFiles, signal }) { let curPath = requestPath while (curPath) { - const res = await this.#list({ requestPath: curPath, absDirPath }) + const res = await this.#list({ requestPath: curPath, absDirPath, signal }) curPath = res.nextPagePath const files = res.items.filter((item) => !item.isFolder) @@ -322,6 +340,7 @@ export default class ProviderView extends View { relDirPath: prependPath(relDirPath, folder.name), queue, onFiles, + signal, }) ))) await Promise.all(promises) // in case we get an error @@ -331,87 +350,90 @@ export default class ProviderView extends View { async donePicking () { this.setLoading(true) try { - const { currentSelection } = this.plugin.getPluginState() + await this.#withAbort(async (signal) => { + const { currentSelection } = this.plugin.getPluginState() - const messages = [] - const newFiles = [] + const messages = [] + const newFiles = [] - for (const selectedItem of currentSelection) { - const { requestPath } = selectedItem + for (const selectedItem of currentSelection) { + const { requestPath } = selectedItem - const withRelDirPath = (newItem) => ({ - ...newItem, - // calculate the file's path relative to the user's selected item's path - // see https://github.com/transloadit/uppy/pull/4537#issuecomment-1614236655 - relDirPath: newItem.absDirPath.replace(selectedItem.absDirPath, '').replace(/^\//, ''), - }) - - if (selectedItem.isFolder) { - let isEmpty = true - let numNewFiles = 0 - - const queue = new PQueue({ concurrency: 6 }) - - const onFiles = (files) => { - for (const newFile of files) { - const tagFile = this.getTagFile(newFile) - const id = getSafeFileId(tagFile) - // If the same folder is added again, we don't want to send - // X amount of duplicate file notifications, we want to say - // the folder was already added. This checks if all files are duplicate, - // if that's the case, we don't add the files. - if (!this.plugin.uppy.checkIfFileAlreadyExists(id)) { - newFiles.push(withRelDirPath(newFile)) - numNewFiles++ - this.setLoading(this.plugin.uppy.i18n('addedNumFiles', { numFiles: numNewFiles })) - } - isEmpty = false - } - } - - await this.#recursivelyListAllFiles({ - requestPath, - absDirPath: prependPath(selectedItem.absDirPath, selectedItem.name), - relDirPath: selectedItem.name, - queue, - onFiles, + const withRelDirPath = (newItem) => ({ + ...newItem, + // calculate the file's path relative to the user's selected item's path + // see https://github.com/transloadit/uppy/pull/4537#issuecomment-1614236655 + relDirPath: newItem.absDirPath.replace(selectedItem.absDirPath, '').replace(/^\//, ''), }) - await queue.onIdle() - let message - if (isEmpty) { - message = this.plugin.uppy.i18n('emptyFolderAdded') - } else if (numNewFiles === 0) { - message = this.plugin.uppy.i18n('folderAlreadyAdded', { - folder: selectedItem.name, + if (selectedItem.isFolder) { + let isEmpty = true + let numNewFiles = 0 + + const queue = new PQueue({ concurrency: 6 }) + + const onFiles = (files) => { + for (const newFile of files) { + const tagFile = this.getTagFile(newFile) + const id = getSafeFileId(tagFile) + // If the same folder is added again, we don't want to send + // X amount of duplicate file notifications, we want to say + // the folder was already added. This checks if all files are duplicate, + // if that's the case, we don't add the files. + if (!this.plugin.uppy.checkIfFileAlreadyExists(id)) { + newFiles.push(withRelDirPath(newFile)) + numNewFiles++ + this.setLoading(this.plugin.uppy.i18n('addedNumFiles', { numFiles: numNewFiles })) + } + isEmpty = false + } + } + + await this.#recursivelyListAllFiles({ + requestPath, + absDirPath: prependPath(selectedItem.absDirPath, selectedItem.name), + relDirPath: selectedItem.name, + queue, + onFiles, + signal, }) + await queue.onIdle() + + let message + if (isEmpty) { + message = this.plugin.uppy.i18n('emptyFolderAdded') + } else if (numNewFiles === 0) { + message = this.plugin.uppy.i18n('folderAlreadyAdded', { + folder: selectedItem.name, + }) + } else { + // TODO we don't really know at this point whether any files were actually added + // (only later after addFiles has been called) so we should probably rewrite this. + // Example: If all files fail to add due to restriction error, it will still say "Added 100 files from folder" + message = this.plugin.uppy.i18n('folderAdded', { + smart_count: numNewFiles, folder: selectedItem.name, + }) + } + + messages.push(message) } else { - // TODO we don't really know at this point whether any files were actually added - // (only later after addFiles has been called) so we should probably rewrite this. - // Example: If all files fail to add due to restriction error, it will still say "Added 100 files from folder" - message = this.plugin.uppy.i18n('folderAdded', { - smart_count: numNewFiles, folder: selectedItem.name, - }) + newFiles.push(withRelDirPath(selectedItem)) } - - messages.push(message) - } else { - newFiles.push(withRelDirPath(selectedItem)) } - } - // Note: this.plugin.uppy.addFiles must be only run once we are done fetching all files, - // because it will cause the loading screen to disappear, - // and that will allow the user to start the upload, so we need to make sure we have - // finished all async operations before we add any file - // see https://github.com/transloadit/uppy/pull/4384 - this.plugin.uppy.log('Adding remote provider files') - this.plugin.uppy.addFiles(newFiles.map((file) => this.getTagFile(file))) + // Note: this.plugin.uppy.addFiles must be only run once we are done fetching all files, + // because it will cause the loading screen to disappear, + // and that will allow the user to start the upload, so we need to make sure we have + // finished all async operations before we add any file + // see https://github.com/transloadit/uppy/pull/4384 + this.plugin.uppy.log('Adding remote provider files') + this.plugin.uppy.addFiles(newFiles.map((file) => this.getTagFile(file))) - this.plugin.setPluginState({ filterInput: '' }) - messages.forEach(message => this.plugin.uppy.info(message)) + this.plugin.setPluginState({ filterInput: '' }) + messages.forEach(message => this.plugin.uppy.info(message)) - this.clearSelection() + this.clearSelection() + }) } catch (err) { this.handleError(err) } finally { diff --git a/packages/@uppy/provider-views/src/View.js b/packages/@uppy/provider-views/src/View.js index 81fffef3d..7126cbd9a 100644 --- a/packages/@uppy/provider-views/src/View.js +++ b/packages/@uppy/provider-views/src/View.js @@ -48,7 +48,9 @@ export default class View { uppy.log(error.toString()) - if (error.isAuthError) { + if (error.isAuthError || error.cause?.name === 'AbortError') { + // authError just means we're not authenticated, don't show to user + // AbortError means the user has clicked "cancel" on an operation return }