From e3452c8c9cb38e43f4bd49ea29b54f45c92023b5 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Thu, 13 Jul 2023 15:35:38 +0200 Subject: [PATCH 01/12] First working version --- .../companion-client/src/RequestClient.js | 156 ++++++++++ packages/@uppy/tus/src/index.js | 272 +++--------------- packages/@uppy/utils/src/EventManager.js | 89 +++++- 3 files changed, 281 insertions(+), 236 deletions(-) diff --git a/packages/@uppy/companion-client/src/RequestClient.js b/packages/@uppy/companion-client/src/RequestClient.js index 69fcc882a6..d77781d7c3 100644 --- a/packages/@uppy/companion-client/src/RequestClient.js +++ b/packages/@uppy/companion-client/src/RequestClient.js @@ -2,7 +2,12 @@ import fetchWithNetworkError from '@uppy/utils/lib/fetchWithNetworkError' import ErrorWithCause from '@uppy/utils/lib/ErrorWithCause' +import emitSocketProgress from '@uppy/utils/lib/emitSocketProgress' +import getSocketHost from '@uppy/utils/lib/getSocketHost' +import EventManager from '@uppy/utils/lib/EventManager' + import AuthError from './AuthError.js' +import Socket from './Socket.js' import packageJson from '../package.json' @@ -43,6 +48,8 @@ export default class RequestClient { this.opts = opts this.onReceiveResponse = this.onReceiveResponse.bind(this) this.#companionHeaders = opts?.companionHeaders + this.uploaderEvents = Object.create(null) + this.uploaderSockets = Object.create(null) } setCompanionHeaders (headers) { @@ -189,4 +196,153 @@ export default class RequestClient { if (typeof options === 'boolean') options = { skipPostResponse: options } return this.request({ ...options, path, method: 'DELETE', data }) } + + /** + * @param {UppyFile} file + */ + async connectToServerSocket (file, requests) { + return new Promise((resolve, reject) => { + const token = file.serverToken + const host = getSocketHost(file.remote.companionUrl) + const socket = new Socket({ target: `${host}/api/${token}`, autoOpen: false }) + const eventManager = new EventManager(this.uppy) + this.uploaderSockets[file.id] = socket + this.uploaderEvents[file.id] = eventManager + + let queuedRequest + + eventManager.onFileRemove(file.id, () => { + socket.send('cancel', {}) + queuedRequest.abort() + this.resetUploaderReferences(file.id) + resolve(`upload ${file.id} was removed`) + }) + + eventManager.onPause(file.id, (isPaused) => { + if (isPaused) { + // Remove this file from the queue so another file can start in its place. + socket.send('pause', {}) + queuedRequest.abort() + } else { + // Resuming an upload should be queued, else you could pause and then + // resume a queued upload to make it skip the queue. + queuedRequest.abort() + queuedRequest = requests.run(() => { + socket.open() + socket.send('resume', {}) + + return () => {} + }) + } + }) + + eventManager.onPauseAll(file.id, () => { + socket.send('pause', {}) + queuedRequest.abort() + }) + + eventManager.onCancelAll(file.id, ({ reason } = {}) => { + if (reason === 'user') { + socket.send('cancel', {}) + queuedRequest.abort() + this.resetUploaderReferences(file.id) + } + resolve(`upload ${file.id} was canceled`) + }) + + eventManager.onResumeAll(file.id, () => { + queuedRequest.abort() + if (file.error) { + socket.send('pause', {}) + } + queuedRequest = requests.run(() => { + socket.open() + socket.send('resume', {}) + + return () => {} + }) + }) + + eventManager.onRetry(file.id, () => { + // Only do the retry if the upload is actually in progress; + // else we could try to send these messages when the upload is still queued. + // We may need a better check for this since the socket may also be closed + // for other reasons, like network failures. + if (socket.isOpen) { + socket.send('pause', {}) + socket.send('resume', {}) + } + }) + + eventManager.onRetryAll(file.id, () => { + // See the comment in the onRetry() call + if (socket.isOpen) { + socket.send('pause', {}) + socket.send('resume', {}) + } + }) + + socket.on('progress', (progressData) => emitSocketProgress(this, progressData, file)) + + socket.on('error', (errData) => { + const { message } = errData.error + const error = Object.assign(new Error(message), { cause: errData.error }) + + // If the remote retry optimisation should not be used, + // close the socket—this will tell companion to clear state and delete the file. + if (!this.opts.useFastRemoteRetry) { + this.resetUploaderReferences(file.id) + // Remove the serverToken so that a new one will be created for the retry. + this.uppy.setFileState(file.id, { + serverToken: null, + }) + } else { + socket.close() + } + + this.uppy.emit('upload-error', file, error) + queuedRequest.done() + reject(error) + }) + + socket.on('success', (data) => { + const uploadResp = { + uploadURL: data.url, + } + + this.uppy.emit('upload-success', file, uploadResp) + this.resetUploaderReferences(file.id) + queuedRequest.done() + socket.close() + resolve() + }) + + queuedRequest = requests.run(() => { + if (file.isPaused) { + socket.send('pause', {}) + } else { + socket.open() + } + + // Just close the socket here, the caller will take care of cancelling the upload itself + // using resetUploaderReferences(). This is because resetUploaderReferences() has to be + // called when this request is still in the queue, and has not been started yet, too. At + // that point this cancellation function is not going to be called. + // Also, we need to remove the request from the queue _without_ destroying everything + // related to this upload to handle pauses. + return () => {} + }) + }) + } + + resetUploaderReferences (fileID) { + if (this.uploaderEvents[fileID]) { + this.uploaderEvents[fileID].remove() + this.uploaderEvents[fileID] = null + } + if (this.uploaderSockets[fileID]) { + this.uploaderSockets[fileID].close() + this.uploaderSockets[fileID] = null + } + } } diff --git a/packages/@uppy/tus/src/index.js b/packages/@uppy/tus/src/index.js index 39022f3d82..5ecc96c29d 100644 --- a/packages/@uppy/tus/src/index.js +++ b/packages/@uppy/tus/src/index.js @@ -1,8 +1,6 @@ -import UploaderPlugin from '@uppy/core/lib/UploaderPlugin.js' +import BasePlugin from '@uppy/core/lib/BasePlugin.js' import * as tus from 'tus-js-client' -import { Provider, RequestClient, Socket } from '@uppy/companion-client' -import emitSocketProgress from '@uppy/utils/lib/emitSocketProgress' -import getSocketHost from '@uppy/utils/lib/getSocketHost' +import { Provider, RequestClient } from '@uppy/companion-client' import EventManager from '@uppy/utils/lib/EventManager' import NetworkError from '@uppy/utils/lib/NetworkError' import isNetworkError from '@uppy/utils/lib/isNetworkError' @@ -52,7 +50,7 @@ const tusDefaultOptions = { /** * Tus resumable file uploader */ -export default class Tus extends UploaderPlugin { +export default class Tus extends BasePlugin { static VERSION = packageJson.version #retryDelayIterator @@ -97,7 +95,6 @@ export default class Tus extends UploaderPlugin { this.uploaders = Object.create(null) this.uploaderEvents = Object.create(null) - this.uploaderSockets = Object.create(null) this.handleResetProgress = this.handleResetProgress.bind(this) this.setQueueRequestSocketToken(this.requests.wrapPromiseFunction(this.#requestSocketToken, { priority: -1 })) @@ -139,10 +136,6 @@ export default class Tus extends UploaderPlugin { this.uploaderEvents[fileID].remove() this.uploaderEvents[fileID] = null } - if (this.uploaderSockets[fileID]) { - this.uploaderSockets[fileID].close() - this.uploaderSockets[fileID] = null - } } /** @@ -180,7 +173,7 @@ export default class Tus extends UploaderPlugin { * @param {UppyFile} file for use with upload * @returns {Promise} */ - #upload (file) { + #uploadLocalFile (file) { this.resetUploaderReferences(file.id) // Create a new tus upload @@ -361,7 +354,8 @@ export default class Tus extends UploaderPlugin { upload = new tus.Upload(file.data, uploadOptions) this.uploaders[file.id] = upload - this.uploaderEvents[file.id] = new EventManager(this.uppy) + const eventManager = new EventManager(this.uppy) + this.uploaderEvents[file.id] = eventManager // eslint-disable-next-line prefer-const qRequest = () => { @@ -387,13 +381,13 @@ export default class Tus extends UploaderPlugin { queuedRequest = this.requests.run(qRequest) - this.onFileRemove(file.id, (targetFileID) => { + eventManager.onFileRemove(file.id, (targetFileID) => { queuedRequest.abort() this.resetUploaderReferences(file.id, { abort: !!upload.url }) resolve(`upload ${targetFileID} was removed`) }) - this.onPause(file.id, (isPaused) => { + eventManager.onPause(file.id, (isPaused) => { queuedRequest.abort() if (isPaused) { // Remove this file from the queue so another file can start in its place. @@ -405,12 +399,12 @@ export default class Tus extends UploaderPlugin { } }) - this.onPauseAll(file.id, () => { + eventManager.onPauseAll(file.id, () => { queuedRequest.abort() upload.abort() }) - this.onCancelAll(file.id, ({ reason } = {}) => { + eventManager.onCancelAll(file.id, ({ reason } = {}) => { if (reason === 'user') { queuedRequest.abort() this.resetUploaderReferences(file.id, { abort: !!upload.url }) @@ -418,7 +412,7 @@ export default class Tus extends UploaderPlugin { resolve(`upload ${file.id} was canceled`) }) - this.onResumeAll(file.id, () => { + eventManager.onResumeAll(file.id, () => { queuedRequest.abort() if (file.error) { upload.abort() @@ -431,9 +425,14 @@ export default class Tus extends UploaderPlugin { }) } - #requestSocketToken = async (file, options) => { + #getCompanionClient = (file) => { const Client = file.remote.providerOptions.provider ? Provider : RequestClient const client = new Client(this.uppy, file.remote.providerOptions) + return client + } + + #requestSocketToken = async (file, options) => { + const client = this.#getCompanionClient(file) const opts = { ...this.opts } if (file.tus) { @@ -453,148 +452,6 @@ export default class Tus extends UploaderPlugin { return res.token } - /** - * See the comment on the upload() method. - * - * Additionally, when an upload is removed, completed, or cancelled, we need to close the WebSocket connection. This is - * handled by the resetUploaderReferences() function, so the same guidelines apply as in upload(). - * - * @param {UppyFile} file - */ - async connectToServerSocket (file) { - return new Promise((resolve, reject) => { - const token = file.serverToken - const host = getSocketHost(file.remote.companionUrl) - const socket = new Socket({ target: `${host}/api/${token}`, autoOpen: false }) - this.uploaderSockets[file.id] = socket - this.uploaderEvents[file.id] = new EventManager(this.uppy) - - let queuedRequest - - this.onFileRemove(file.id, () => { - socket.send('cancel', {}) - queuedRequest.abort() - this.resetUploaderReferences(file.id) - resolve(`upload ${file.id} was removed`) - }) - - this.onPause(file.id, (isPaused) => { - if (isPaused) { - // Remove this file from the queue so another file can start in its place. - socket.send('pause', {}) - queuedRequest.abort() - } else { - // Resuming an upload should be queued, else you could pause and then - // resume a queued upload to make it skip the queue. - queuedRequest.abort() - queuedRequest = this.requests.run(() => { - socket.open() - socket.send('resume', {}) - - return () => {} - }) - } - }) - - this.onPauseAll(file.id, () => { - socket.send('pause', {}) - queuedRequest.abort() - }) - - this.onCancelAll(file.id, ({ reason } = {}) => { - if (reason === 'user') { - socket.send('cancel', {}) - queuedRequest.abort() - this.resetUploaderReferences(file.id) - } - resolve(`upload ${file.id} was canceled`) - }) - - this.onResumeAll(file.id, () => { - queuedRequest.abort() - if (file.error) { - socket.send('pause', {}) - } - queuedRequest = this.requests.run(() => { - socket.open() - socket.send('resume', {}) - - return () => {} - }) - }) - - this.onRetry(file.id, () => { - // Only do the retry if the upload is actually in progress; - // else we could try to send these messages when the upload is still queued. - // We may need a better check for this since the socket may also be closed - // for other reasons, like network failures. - if (socket.isOpen) { - socket.send('pause', {}) - socket.send('resume', {}) - } - }) - - this.onRetryAll(file.id, () => { - // See the comment in the onRetry() call - if (socket.isOpen) { - socket.send('pause', {}) - socket.send('resume', {}) - } - }) - - socket.on('progress', (progressData) => emitSocketProgress(this, progressData, file)) - - socket.on('error', (errData) => { - const { message } = errData.error - const error = Object.assign(new Error(message), { cause: errData.error }) - - // If the remote retry optimisation should not be used, - // close the socket—this will tell companion to clear state and delete the file. - if (!this.opts.useFastRemoteRetry) { - this.resetUploaderReferences(file.id) - // Remove the serverToken so that a new one will be created for the retry. - this.uppy.setFileState(file.id, { - serverToken: null, - }) - } else { - socket.close() - } - - this.uppy.emit('upload-error', file, error) - queuedRequest.done() - reject(error) - }) - - socket.on('success', (data) => { - const uploadResp = { - uploadURL: data.url, - } - - this.uppy.emit('upload-success', file, uploadResp) - this.resetUploaderReferences(file.id) - queuedRequest.done() - socket.close() - resolve() - }) - - queuedRequest = this.requests.run(() => { - if (file.isPaused) { - socket.send('pause', {}) - } else { - socket.open() - } - - // Just close the socket here, the caller will take care of cancelling the upload itself - // using resetUploaderReferences(). This is because resetUploaderReferences() has to be - // called when this request is still in the queue, and has not been started yet, too. At - // that point this cancellation function is not going to be called. - // Also, we need to remove the request from the queue _without_ destroying everything - // related to this upload to handle pauses. - return () => {} - }) - }) - } - /** * Store the uploadUrl on the file options, so that when Golden Retriever * restores state, we will continue uploading to the correct URL. @@ -614,83 +471,36 @@ export default class Tus extends UploaderPlugin { } } - /** - * @param {string} fileID - * @param {function(string): void} cb - */ - onFileRemove (fileID, cb) { - this.uploaderEvents[fileID].on('file-removed', (file) => { - if (fileID === file.id) cb(file.id) - }) - } + #queueRequestSocketToken - /** - * @param {string} fileID - * @param {function(boolean): void} cb - */ - onPause (fileID, cb) { - this.uploaderEvents[fileID].on('upload-pause', (targetFileID, isPaused) => { - if (fileID === targetFileID) { - // const isPaused = this.uppy.pauseResume(fileID) - cb(isPaused) - } - }) + /** @protected */ + setQueueRequestSocketToken (fn) { + this.#queueRequestSocketToken = fn } - /** - * @param {string} fileID - * @param {function(): void} cb - */ - onRetry (fileID, cb) { - this.uploaderEvents[fileID].on('upload-retry', (targetFileID) => { - if (fileID === targetFileID) { - cb() + async uploadRemoteFile (file, options = {}) { + // TODO: we could rewrite this to use server-sent events instead of creating WebSockets. + const client = this.#getCompanionClient(file) + try { + if (file.serverToken) { + return await client.connectToServerSocket(file, this.requests) } - }) - } + const serverToken = await this.#queueRequestSocketToken(file).abortOn(options.signal) - /** - * @param {string} fileID - * @param {function(): void} cb - */ - onRetryAll (fileID, cb) { - this.uploaderEvents[fileID].on('retry-all', () => { - if (!this.uppy.getFile(fileID)) return - cb() - }) - } - - /** - * @param {string} fileID - * @param {function(): void} cb - */ - onPauseAll (fileID, cb) { - this.uploaderEvents[fileID].on('pause-all', () => { - if (!this.uppy.getFile(fileID)) return - cb() - }) - } + if (!this.uppy.getState().files[file.id]) return undefined - /** - * @param {string} fileID - * @param {function(): void} eventHandler - */ - onCancelAll (fileID, eventHandler) { - this.uploaderEvents[fileID].on('cancel-all', (...args) => { - if (!this.uppy.getFile(fileID)) return - eventHandler(...args) - }) - } + this.uppy.setFileState(file.id, { serverToken }) + return await client.connectToServerSocket(this.uppy.getFile(file.id), this.requests) + } catch (err) { + if (err?.cause?.name === 'AbortError') { + // The file upload was aborted, it’s not an error + return undefined + } - /** - * @param {string} fileID - * @param {function(): void} cb - */ - onResumeAll (fileID, cb) { - this.uploaderEvents[fileID].on('resume-all', () => { - if (!this.uppy.getFile(fileID)) return - cb() - }) + this.uppy.setFileState(file.id, { serverToken: undefined }) + this.uppy.emit('upload-error', file, err) + throw err + } } /** @@ -722,7 +532,7 @@ export default class Tus extends UploaderPlugin { return uploadPromise } - return this.#upload(file, current, total) + return this.#uploadLocalFile(file, current, total) })) } diff --git a/packages/@uppy/utils/src/EventManager.js b/packages/@uppy/utils/src/EventManager.js index 012dbe3955..c4bc628757 100644 --- a/packages/@uppy/utils/src/EventManager.js +++ b/packages/@uppy/utils/src/EventManager.js @@ -3,22 +3,101 @@ * all events that were added using the wrapped emitter. */ export default class EventManager { - #emitter + #uppy #events = [] - constructor (emitter) { - this.#emitter = emitter + constructor (uppy) { + this.#uppy = uppy } on (event, fn) { this.#events.push([event, fn]) - return this.#emitter.on(event, fn) + return this.#uppy.on(event, fn) } remove () { for (const [event, fn] of this.#events.splice(0)) { - this.#emitter.off(event, fn) + this.#uppy.off(event, fn) } } + + /** + * @param {string} fileID + * @param {function(string): void} cb + */ + onFileRemove (fileID, cb) { + this.on('file-removed', (file) => { + if (fileID === file.id) cb(file.id) + }) + } + + /** + * @param {string} fileID + * @param {function(boolean): void} cb + */ + onPause (fileID, cb) { + this.on('upload-pause', (targetFileID, isPaused) => { + if (fileID === targetFileID) { + // const isPaused = this.#uppy.pauseResume(fileID) + cb(isPaused) + } + }) + } + + /** + * @param {string} fileID + * @param {function(): void} cb + */ + onRetry (fileID, cb) { + this.on('upload-retry', (targetFileID) => { + if (fileID === targetFileID) { + cb() + } + }) + } + + /** + * @param {string} fileID + * @param {function(): void} cb + */ + onRetryAll (fileID, cb) { + this.on('retry-all', () => { + if (!this.#uppy.getFile(fileID)) return + cb() + }) + } + + /** + * @param {string} fileID + * @param {function(): void} cb + */ + onPauseAll (fileID, cb) { + this.on('pause-all', () => { + if (!this.#uppy.getFile(fileID)) return + cb() + }) + } + + /** + * @param {string} fileID + * @param {function(): void} eventHandler + */ + onCancelAll (fileID, eventHandler) { + this.on('cancel-all', (...args) => { + if (!this.#uppy.getFile(fileID)) return + eventHandler(...args) + }) + } + + /** + * @param {string} fileID + * @param {function(): void} cb + */ + onResumeAll (fileID, cb) { + this.on('resume-all', () => { + if (!this.#uppy.getFile(fileID)) return + cb() + }) + } } From 4f114d9969d263a04414191e92e95295f6794331 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Thu, 13 Jul 2023 15:55:11 +0200 Subject: [PATCH 02/12] Clean up --- .../companion-client/src/RequestClient.js | 25 ------------------- packages/@uppy/tus/src/index.js | 14 +++-------- 2 files changed, 4 insertions(+), 35 deletions(-) diff --git a/packages/@uppy/companion-client/src/RequestClient.js b/packages/@uppy/companion-client/src/RequestClient.js index d77781d7c3..fc4cc0490c 100644 --- a/packages/@uppy/companion-client/src/RequestClient.js +++ b/packages/@uppy/companion-client/src/RequestClient.js @@ -48,8 +48,6 @@ export default class RequestClient { this.opts = opts this.onReceiveResponse = this.onReceiveResponse.bind(this) this.#companionHeaders = opts?.companionHeaders - this.uploaderEvents = Object.create(null) - this.uploaderSockets = Object.create(null) } setCompanionHeaders (headers) { @@ -206,15 +204,12 @@ export default class RequestClient { const host = getSocketHost(file.remote.companionUrl) const socket = new Socket({ target: `${host}/api/${token}`, autoOpen: false }) const eventManager = new EventManager(this.uppy) - this.uploaderSockets[file.id] = socket - this.uploaderEvents[file.id] = eventManager let queuedRequest eventManager.onFileRemove(file.id, () => { socket.send('cancel', {}) queuedRequest.abort() - this.resetUploaderReferences(file.id) resolve(`upload ${file.id} was removed`) }) @@ -245,7 +240,6 @@ export default class RequestClient { if (reason === 'user') { socket.send('cancel', {}) queuedRequest.abort() - this.resetUploaderReferences(file.id) } resolve(`upload ${file.id} was canceled`) }) @@ -291,7 +285,6 @@ export default class RequestClient { // If the remote retry optimisation should not be used, // close the socket—this will tell companion to clear state and delete the file. if (!this.opts.useFastRemoteRetry) { - this.resetUploaderReferences(file.id) // Remove the serverToken so that a new one will be created for the retry. this.uppy.setFileState(file.id, { serverToken: null, @@ -311,7 +304,6 @@ export default class RequestClient { } this.uppy.emit('upload-success', file, uploadResp) - this.resetUploaderReferences(file.id) queuedRequest.done() socket.close() resolve() @@ -324,25 +316,8 @@ export default class RequestClient { socket.open() } - // Just close the socket here, the caller will take care of cancelling the upload itself - // using resetUploaderReferences(). This is because resetUploaderReferences() has to be - // called when this request is still in the queue, and has not been started yet, too. At - // that point this cancellation function is not going to be called. - // Also, we need to remove the request from the queue _without_ destroying everything - // related to this upload to handle pauses. return () => {} }) }) } - - resetUploaderReferences (fileID) { - if (this.uploaderEvents[fileID]) { - this.uploaderEvents[fileID].remove() - this.uploaderEvents[fileID] = null - } - if (this.uploaderSockets[fileID]) { - this.uploaderSockets[fileID].close() - this.uploaderSockets[fileID] = null - } - } } diff --git a/packages/@uppy/tus/src/index.js b/packages/@uppy/tus/src/index.js index 5ecc96c29d..7b55e41f6d 100644 --- a/packages/@uppy/tus/src/index.js +++ b/packages/@uppy/tus/src/index.js @@ -55,6 +55,8 @@ export default class Tus extends BasePlugin { #retryDelayIterator + #queueRequestSocketToken + /** * @param {Uppy} uppy * @param {TusOptions} opts @@ -97,7 +99,7 @@ export default class Tus extends BasePlugin { this.uploaderEvents = Object.create(null) this.handleResetProgress = this.handleResetProgress.bind(this) - this.setQueueRequestSocketToken(this.requests.wrapPromiseFunction(this.#requestSocketToken, { priority: -1 })) + this.#queueRequestSocketToken = this.requests.wrapPromiseFunction(this.#requestSocketToken, { priority: -1 }) } handleResetProgress () { @@ -471,15 +473,7 @@ export default class Tus extends BasePlugin { } } - #queueRequestSocketToken - - /** @protected */ - setQueueRequestSocketToken (fn) { - this.#queueRequestSocketToken = fn - } - async uploadRemoteFile (file, options = {}) { - // TODO: we could rewrite this to use server-sent events instead of creating WebSockets. const client = this.#getCompanionClient(file) try { if (file.serverToken) { @@ -523,7 +517,6 @@ export default class Tus extends BasePlugin { } this.uppy.on('file-removed', removedHandler) - this.resetUploaderReferences(file.id) const uploadPromise = this.uploadRemoteFile(file, { signal: controller.signal }) this.requests.wrapSyncFunction(() => { @@ -532,6 +525,7 @@ export default class Tus extends BasePlugin { return uploadPromise } + return this.#uploadLocalFile(file, current, total) })) } From c37b5e1f0d4f5518718eb01667498147552f2364 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Thu, 13 Jul 2023 16:37:49 +0200 Subject: [PATCH 03/12] Also refactor aws-s3-multipart --- packages/@uppy/aws-s3-multipart/src/index.js | 225 ++---------------- .../companion-client/src/RequestClient.js | 45 ++++ packages/@uppy/tus/src/index.js | 58 +---- 3 files changed, 67 insertions(+), 261 deletions(-) diff --git a/packages/@uppy/aws-s3-multipart/src/index.js b/packages/@uppy/aws-s3-multipart/src/index.js index 05bfba7499..ff7371468f 100644 --- a/packages/@uppy/aws-s3-multipart/src/index.js +++ b/packages/@uppy/aws-s3-multipart/src/index.js @@ -1,8 +1,6 @@ -import UploaderPlugin from '@uppy/core/lib/UploaderPlugin.js' -import { Socket, Provider, RequestClient } from '@uppy/companion-client' +import BasePlugin from '@uppy/core/lib/BasePlugin.js' +import { Provider, RequestClient } from '@uppy/companion-client' import EventManager from '@uppy/utils/lib/EventManager' -import emitSocketProgress from '@uppy/utils/lib/emitSocketProgress' -import getSocketHost from '@uppy/utils/lib/getSocketHost' import { RateLimitedQueue } from '@uppy/utils/lib/RateLimitedQueue' import { filterNonFailedFiles, filterFilesToEmitUploadStarted } from '@uppy/utils/lib/fileFilters' import { createAbortError } from '@uppy/utils/lib/AbortController' @@ -361,13 +359,15 @@ class HTTPCommunicationQueue { } } -export default class AwsS3Multipart extends UploaderPlugin { +export default class AwsS3Multipart extends BasePlugin { static VERSION = packageJson.version #companionCommunicationQueue #client + #queueRequestSocketToken + constructor (uppy, opts) { super(uppy, opts) this.type = 'uploader' @@ -417,8 +417,6 @@ export default class AwsS3Multipart extends UploaderPlugin { this.uploaders = Object.create(null) this.uploaderEvents = Object.create(null) this.uploaderSockets = Object.create(null) - - this.setQueueRequestSocketToken(this.requests.wrapPromiseFunction(this.#requestSocketToken, { priority: -1 })) } [Symbol.for('uppy test: getClient')] () { return this.#client } @@ -683,7 +681,7 @@ export default class AwsS3Multipart extends UploaderPlugin { }) } - #uploadFile (file) { + #uploadLocalFile (file) { return new Promise((resolve, reject) => { const getFile = () => this.uppy.getFile(file.id) || file @@ -745,15 +743,16 @@ export default class AwsS3Multipart extends UploaderPlugin { }) this.uploaders[file.id] = upload - this.uploaderEvents[file.id] = new EventManager(this.uppy) + const eventManager = new EventManager(this.uppy) + this.uploaderEvents[file.id] = eventManager - this.onFileRemove(file.id, (removed) => { + eventManager.onFileRemove(file.id, (removed) => { upload.abort() this.resetUploaderReferences(file.id, { abort: true }) resolve(`upload ${removed.id} was removed`) }) - this.onCancelAll(file.id, ({ reason } = {}) => { + eventManager.onCancelAll(file.id, ({ reason } = {}) => { if (reason === 'user') { upload.abort() this.resetUploaderReferences(file.id, { abort: true }) @@ -761,7 +760,7 @@ export default class AwsS3Multipart extends UploaderPlugin { resolve(`upload ${file.id} was canceled`) }) - this.onFilePause(file.id, (isPaused) => { + eventManager.onFilePause(file.id, (isPaused) => { if (isPaused) { upload.pause() } else { @@ -769,11 +768,11 @@ export default class AwsS3Multipart extends UploaderPlugin { } }) - this.onPauseAll(file.id, () => { + eventManager.onPauseAll(file.id, () => { upload.pause() }) - this.onResumeAll(file.id, () => { + eventManager.onResumeAll(file.id, () => { upload.start() }) @@ -781,155 +780,19 @@ export default class AwsS3Multipart extends UploaderPlugin { }) } - #requestSocketToken = async (file, options) => { - const Client = file.remote.providerOptions.provider ? Provider : RequestClient - const client = new Client(this.uppy, file.remote.providerOptions) - const opts = { ...this.opts } - - if (file.tus) { - // Install file-specific upload overrides. - Object.assign(opts, file.tus) - } - - if (file.remote.url == null) { - throw new Error('Cannot connect to an undefined URL') - } - - const res = await client.post(file.remote.url, { - ...file.remote.body, - protocol: 's3-multipart', - size: file.data.size, - metadata: file.meta, - }, options) - return res.token - } - - async connectToServerSocket (file) { - return new Promise((resolve, reject) => { - let queuedRequest - - const token = file.serverToken - const host = getSocketHost(file.remote.companionUrl) - const socket = new Socket({ target: `${host}/api/${token}`, autoOpen: false }) - this.uploaderSockets[file.id] = socket - this.uploaderEvents[file.id] = new EventManager(this.uppy) - - this.onFileRemove(file.id, () => { - socket.send('cancel', {}) - queuedRequest.abort() - this.resetUploaderReferences(file.id, { abort: true }) - resolve(`upload ${file.id} was removed`) - }) - - this.onFilePause(file.id, (isPaused) => { - if (isPaused) { - // Remove this file from the queue so another file can start in its place. - socket.send('pause', {}) - queuedRequest.abort() - } else { - // Resuming an upload should be queued, else you could pause and then - // resume a queued upload to make it skip the queue. - queuedRequest.abort() - queuedRequest = this.requests.run(() => { - socket.open() - socket.send('resume', {}) - return () => {} - }) - } - }) - - this.onPauseAll(file.id, () => { - // First send the message, then call .abort, - // just to make sure socket is not closed, which .abort used to do - socket.send('pause', {}) - queuedRequest.abort() - }) - - this.onCancelAll(file.id, ({ reason } = {}) => { - if (reason === 'user') { - socket.send('cancel', {}) - queuedRequest.abort() - this.resetUploaderReferences(file.id) - } - resolve(`upload ${file.id} was canceled`) - }) - - this.onResumeAll(file.id, () => { - queuedRequest.abort() - if (file.error) { - socket.send('pause', {}) - } - queuedRequest = this.requests.run(() => { - socket.open() - socket.send('resume', {}) - - return () => {} - }) - }) - - this.onRetry(file.id, () => { - // Only do the retry if the upload is actually in progress; - // else we could try to send these messages when the upload is still queued. - // We may need a better check for this since the socket may also be closed - // for other reasons, like network failures. - if (socket.isOpen) { - socket.send('pause', {}) - socket.send('resume', {}) - } - }) - - this.onRetryAll(file.id, () => { - if (socket.isOpen) { - socket.send('pause', {}) - socket.send('resume', {}) - } - }) - - socket.on('progress', (progressData) => emitSocketProgress(this, progressData, file)) - - socket.on('error', (errData) => { - this.uppy.emit('upload-error', file, new Error(errData.error)) - this.resetUploaderReferences(file.id) - socket.close() - queuedRequest.done() - reject(new Error(errData.error)) - }) - - socket.on('success', (data) => { - const uploadResp = { - uploadURL: data.url, - } - - this.uppy.emit('upload-success', file, uploadResp) - this.resetUploaderReferences(file.id) - socket.close() - queuedRequest.done() - resolve() - }) - - queuedRequest = this.requests.run(() => { - if (file.isPaused) { - socket.send('pause', {}) - } else { - socket.open() - } - - return () => {} - }) - }) - } - #upload = async (fileIDs) => { if (fileIDs.length === 0) return undefined const files = this.uppy.getFilesByIds(fileIDs) - const filesFiltered = filterNonFailedFiles(files) const filesToEmit = filterFilesToEmitUploadStarted(filesFiltered) + this.uppy.emit('upload-start', filesToEmit) const promises = filesFiltered.map((file) => { if (file.isRemote) { + const Client = file.remote.providerOptions.provider ? Provider : RequestClient + const client = new Client(this.uppy, file.remote.providerOptions) this.#setResumableUploadsCapability(false) const controller = new AbortController() @@ -938,8 +801,7 @@ export default class AwsS3Multipart extends UploaderPlugin { } this.uppy.on('file-removed', removedHandler) - this.resetUploaderReferences(file.id) - const uploadPromise = this.uploadRemoteFile(file, { signal: controller.signal }) + const uploadPromise = client.uploadRemoteFile(file, { signal: controller.signal }, this.requests) this.requests.wrapSyncFunction(() => { this.uppy.off('file-removed', removedHandler) @@ -947,7 +809,8 @@ export default class AwsS3Multipart extends UploaderPlugin { return uploadPromise } - return this.#uploadFile(file) + + return this.#uploadLocalFile(file) }) const upload = await Promise.all(promises) @@ -961,56 +824,6 @@ export default class AwsS3Multipart extends UploaderPlugin { this.#client.setCompanionHeaders(this.opts.companionHeaders) } - onFileRemove (fileID, cb) { - this.uploaderEvents[fileID].on('file-removed', (file) => { - if (fileID === file.id) cb(file.id) - }) - } - - onFilePause (fileID, cb) { - this.uploaderEvents[fileID].on('upload-pause', (targetFileID, isPaused) => { - if (fileID === targetFileID) { - cb(isPaused) - } - }) - } - - onRetry (fileID, cb) { - this.uploaderEvents[fileID].on('upload-retry', (targetFileID) => { - if (fileID === targetFileID) { - cb() - } - }) - } - - onRetryAll (fileID, cb) { - this.uploaderEvents[fileID].on('retry-all', () => { - if (!this.uppy.getFile(fileID)) return - cb() - }) - } - - onPauseAll (fileID, cb) { - this.uploaderEvents[fileID].on('pause-all', () => { - if (!this.uppy.getFile(fileID)) return - cb() - }) - } - - onCancelAll (fileID, eventHandler) { - this.uploaderEvents[fileID].on('cancel-all', (...args) => { - if (!this.uppy.getFile(fileID)) return - eventHandler(...args) - }) - } - - onResumeAll (fileID, cb) { - this.uploaderEvents[fileID].on('resume-all', () => { - if (!this.uppy.getFile(fileID)) return - cb() - }) - } - #setResumableUploadsCapability = (boolean) => { const { capabilities } = this.uppy.getState() this.uppy.setState({ diff --git a/packages/@uppy/companion-client/src/RequestClient.js b/packages/@uppy/companion-client/src/RequestClient.js index fc4cc0490c..f812d92f64 100644 --- a/packages/@uppy/companion-client/src/RequestClient.js +++ b/packages/@uppy/companion-client/src/RequestClient.js @@ -195,6 +195,51 @@ export default class RequestClient { return this.request({ ...options, path, method: 'DELETE', data }) } + async uploadRemoteFile (file, options = {}, requests) { + try { + if (file.serverToken) { + return await this.connectToServerSocket(file, this.requests) + } + const queueRequestSocketToken = requests.wrapPromiseFunction(this.#requestSocketToken, { priority: -1 }) + const serverToken = await queueRequestSocketToken(file).abortOn(options.signal) + + if (!this.uppy.getState().files[file.id]) return undefined + + this.uppy.setFileState(file.id, { serverToken }) + return await this.connectToServerSocket(this.uppy.getFile(file.id), requests) + } catch (err) { + if (err?.cause?.name === 'AbortError') { + // The file upload was aborted, it’s not an error + return undefined + } + + this.uppy.setFileState(file.id, { serverToken: undefined }) + this.uppy.emit('upload-error', file, err) + throw err + } + } + + #requestSocketToken = async (file, options) => { + const opts = { ...this.opts } + + if (file.tus) { + // Install file-specific upload overrides. + Object.assign(opts, file.tus) + } + + if (file.remote.url == null) { + throw new Error('Cannot connect to an undefined URL') + } + + const res = await this.post(file.remote.url, { + ...file.remote.body, + protocol: 's3-multipart', + size: file.data.size, + metadata: file.meta, + }, options) + return res.token + } + /** * @param {UppyFile} file */ diff --git a/packages/@uppy/tus/src/index.js b/packages/@uppy/tus/src/index.js index 7b55e41f6d..7ca58b0275 100644 --- a/packages/@uppy/tus/src/index.js +++ b/packages/@uppy/tus/src/index.js @@ -55,8 +55,6 @@ export default class Tus extends BasePlugin { #retryDelayIterator - #queueRequestSocketToken - /** * @param {Uppy} uppy * @param {TusOptions} opts @@ -99,7 +97,6 @@ export default class Tus extends BasePlugin { this.uploaderEvents = Object.create(null) this.handleResetProgress = this.handleResetProgress.bind(this) - this.#queueRequestSocketToken = this.requests.wrapPromiseFunction(this.#requestSocketToken, { priority: -1 }) } handleResetProgress () { @@ -427,33 +424,6 @@ export default class Tus extends BasePlugin { }) } - #getCompanionClient = (file) => { - const Client = file.remote.providerOptions.provider ? Provider : RequestClient - const client = new Client(this.uppy, file.remote.providerOptions) - return client - } - - #requestSocketToken = async (file, options) => { - const client = this.#getCompanionClient(file) - const opts = { ...this.opts } - - if (file.tus) { - // Install file-specific upload overrides. - Object.assign(opts, file.tus) - } - - const res = await client.post(file.remote.url, { - ...file.remote.body, - endpoint: opts.endpoint, - uploadUrl: opts.uploadUrl, - protocol: 'tus', - size: file.data.size, - headers: opts.headers, - metadata: file.meta, - }, options) - return res.token - } - /** * Store the uploadUrl on the file options, so that when Golden Retriever * restores state, we will continue uploading to the correct URL. @@ -473,30 +443,6 @@ export default class Tus extends BasePlugin { } } - async uploadRemoteFile (file, options = {}) { - const client = this.#getCompanionClient(file) - try { - if (file.serverToken) { - return await client.connectToServerSocket(file, this.requests) - } - const serverToken = await this.#queueRequestSocketToken(file).abortOn(options.signal) - - if (!this.uppy.getState().files[file.id]) return undefined - - this.uppy.setFileState(file.id, { serverToken }) - return await client.connectToServerSocket(this.uppy.getFile(file.id), this.requests) - } catch (err) { - if (err?.cause?.name === 'AbortError') { - // The file upload was aborted, it’s not an error - return undefined - } - - this.uppy.setFileState(file.id, { serverToken: undefined }) - this.uppy.emit('upload-error', file, err) - throw err - } - } - /** * @param {(UppyFile | FailedUppyFile)[]} files */ @@ -510,6 +456,8 @@ export default class Tus extends BasePlugin { const total = files.length if (file.isRemote) { + const Client = file.remote.providerOptions.provider ? Provider : RequestClient + const client = new Client(this.uppy, file.remote.providerOptions) const controller = new AbortController() const removedHandler = (removedFile) => { @@ -517,7 +465,7 @@ export default class Tus extends BasePlugin { } this.uppy.on('file-removed', removedHandler) - const uploadPromise = this.uploadRemoteFile(file, { signal: controller.signal }) + const uploadPromise = client.uploadRemoteFile(file, { signal: controller.signal }, this.requests) this.requests.wrapSyncFunction(() => { this.uppy.off('file-removed', removedHandler) From c560206159fc399897e25c1eea6f5a889b3f5ea2 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Thu, 13 Jul 2023 16:39:45 +0200 Subject: [PATCH 04/12] fixup! Also refactor aws-s3-multipart --- packages/@uppy/aws-s3-multipart/src/index.js | 3 +-- packages/@uppy/tus/src/index.js | 1 + 2 files 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 ff7371468f..f35241abdd 100644 --- a/packages/@uppy/aws-s3-multipart/src/index.js +++ b/packages/@uppy/aws-s3-multipart/src/index.js @@ -366,8 +366,6 @@ export default class AwsS3Multipart extends BasePlugin { #client - #queueRequestSocketToken - constructor (uppy, opts) { super(uppy, opts) this.type = 'uploader' @@ -791,6 +789,7 @@ export default class AwsS3Multipart extends BasePlugin { const promises = filesFiltered.map((file) => { if (file.isRemote) { + // TODO: why do we need to do this? why not always one or the other? const Client = file.remote.providerOptions.provider ? Provider : RequestClient const client = new Client(this.uppy, file.remote.providerOptions) this.#setResumableUploadsCapability(false) diff --git a/packages/@uppy/tus/src/index.js b/packages/@uppy/tus/src/index.js index 7ca58b0275..fe535367df 100644 --- a/packages/@uppy/tus/src/index.js +++ b/packages/@uppy/tus/src/index.js @@ -456,6 +456,7 @@ export default class Tus extends BasePlugin { const total = files.length if (file.isRemote) { + // TODO: why do we need to do this? why not always one or the other? const Client = file.remote.providerOptions.provider ? Provider : RequestClient const client = new Client(this.uppy, file.remote.providerOptions) const controller = new AbortController() From ba8acc60ae33438174080f3bc4376a622714f4a8 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Thu, 13 Jul 2023 16:49:15 +0200 Subject: [PATCH 05/12] Also refactor xhr-upload --- packages/@uppy/xhr-upload/src/index.js | 170 ++----------------------- private/dev/Dashboard.js | 2 +- 2 files changed, 13 insertions(+), 159 deletions(-) diff --git a/packages/@uppy/xhr-upload/src/index.js b/packages/@uppy/xhr-upload/src/index.js index a8a331ff78..9aac92b895 100644 --- a/packages/@uppy/xhr-upload/src/index.js +++ b/packages/@uppy/xhr-upload/src/index.js @@ -1,8 +1,6 @@ import UploaderPlugin from '@uppy/core/lib/UploaderPlugin.js' import { nanoid } from 'nanoid/non-secure' -import { Provider, RequestClient, Socket } from '@uppy/companion-client' -import emitSocketProgress from '@uppy/utils/lib/emitSocketProgress' -import getSocketHost from '@uppy/utils/lib/getSocketHost' +import { Provider, RequestClient } from '@uppy/companion-client' import EventManager from '@uppy/utils/lib/EventManager' import ProgressTimeout from '@uppy/utils/lib/ProgressTimeout' import { RateLimitedQueue, internalRateLimitedQueue } from '@uppy/utils/lib/RateLimitedQueue' @@ -127,7 +125,6 @@ export default class XHRUpload extends UploaderPlugin { } this.uploaderEvents = Object.create(null) - this.setQueueRequestSocketToken(this.requests.wrapPromiseFunction(this.#requestSocketToken, { priority: -1 })) } getOptions (file) { @@ -216,7 +213,7 @@ export default class XHRUpload extends UploaderPlugin { return formPost } - async #upload (file, current, total) { + async #uploadLocalFile (file, current, total) { const opts = this.getOptions(file) this.uppy.log(`uploading ${current} of ${total}`) @@ -226,7 +223,8 @@ export default class XHRUpload extends UploaderPlugin { : file.data const xhr = new XMLHttpRequest() - this.uploaderEvents[file.id] = new EventManager(this.uppy) + const eventManager = new EventManager(this.uppy) + this.uploaderEvents[file.id] = eventManager let queuedRequest const timer = new ProgressTimeout(opts.timeout, () => { @@ -335,12 +333,12 @@ export default class XHRUpload extends UploaderPlugin { } }) - this.onFileRemove(file.id, () => { + eventManager.onFileRemove(file.id, () => { queuedRequest.abort() reject(new Error('File removed')) }) - this.onCancelAll(file.id, ({ reason }) => { + eventManager.onCancelAll(file.id, ({ reason }) => { if (reason === 'user') { queuedRequest.abort() } @@ -349,126 +347,6 @@ export default class XHRUpload extends UploaderPlugin { }) } - #requestSocketToken = async (file, options) => { - const opts = this.getOptions(file) - const Client = file.remote.providerOptions.provider ? Provider : RequestClient - const client = new Client(this.uppy, file.remote.providerOptions) - const allowedMetaFields = Array.isArray(opts.allowedMetaFields) - ? opts.allowedMetaFields - // Send along all fields by default. - : Object.keys(file.meta) - const res = await client.post(file.remote.url, { - ...file.remote.body, - protocol: 'multipart', - endpoint: opts.endpoint, - size: file.data.size, - fieldname: opts.fieldName, - metadata: Object.fromEntries(allowedMetaFields.map(name => [name, file.meta[name]])), - httpMethod: opts.method, - useFormData: opts.formData, - headers: opts.headers, - }, options) - return res.token - } - - async connectToServerSocket (file) { - return new Promise((resolve, reject) => { - const opts = this.getOptions(file) - const token = file.serverToken - const host = getSocketHost(file.remote.companionUrl) - let socket - - const createSocket = () => { - if (socket != null) return - - socket = new Socket({ target: `${host}/api/${token}` }) - - socket.on('progress', (progressData) => emitSocketProgress(this, progressData, file)) - - socket.on('success', (data) => { - const body = opts.getResponseData(data.response.responseText, data.response) - const uploadURL = body[opts.responseUrlFieldName] - - const uploadResp = { - status: data.response.status, - body, - uploadURL, - } - - this.uppy.emit('upload-success', file, uploadResp) - queuedRequest.done() // eslint-disable-line no-use-before-define - socket.close() - if (this.uploaderEvents[file.id]) { - this.uploaderEvents[file.id].remove() - this.uploaderEvents[file.id] = null - } - return resolve() - }) - - socket.on('error', (errData) => { - const resp = errData.response - const error = resp - ? opts.getResponseError(resp.responseText, resp) - : Object.assign(new Error(errData.error.message), { cause: errData.error }) - this.uppy.emit('upload-error', file, error) - queuedRequest.done() // eslint-disable-line no-use-before-define - socket.close() - if (this.uploaderEvents[file.id]) { - this.uploaderEvents[file.id].remove() - this.uploaderEvents[file.id] = null - } - reject(error) - }) - } - this.uploaderEvents[file.id] = new EventManager(this.uppy) - - let queuedRequest = this.requests.run(() => { - if (file.isPaused) { - socket?.send('pause', {}) - } else { - createSocket() - } - - return () => {} - }) - - this.onFileRemove(file.id, () => { - socket?.send('cancel', {}) - socket.close() - queuedRequest.abort() - resolve(`upload ${file.id} was removed`) - }) - - this.onCancelAll(file.id, ({ reason } = {}) => { - if (reason === 'user') { - socket?.send('cancel', {}) - queuedRequest.abort() - // socket.close() - } - resolve(`upload ${file.id} was canceled`) - }) - - const onRetryRequest = () => { - if (socket == null) { - queuedRequest.abort() - } else { - queuedRequest.done() - } - queuedRequest = this.requests.run(() => { - if (socket == null) { - createSocket() - } - return () => {} - }) - } - this.onRetry(file.id, onRetryRequest) - this.onRetryAll(file.id, onRetryRequest) - }).catch((err) => { - this.uppy.emit('upload-error', file, err) - return Promise.reject(err) - }) - } - #uploadBundle (files) { return new Promise((resolve, reject) => { const { endpoint } = this.opts @@ -569,6 +447,9 @@ export default class XHRUpload extends UploaderPlugin { const total = files.length if (file.isRemote) { + // TODO: why do we need to do this? why not always one or the other? + const Client = file.remote.providerOptions.provider ? Provider : RequestClient + const client = new Client(this.uppy, file.remote.providerOptions) const controller = new AbortController() const removedHandler = (removedFile) => { @@ -576,7 +457,7 @@ export default class XHRUpload extends UploaderPlugin { } this.uppy.on('file-removed', removedHandler) - const uploadPromise = this.uploadRemoteFile(file, { signal: controller.signal }) + const uploadPromise = client.uploadRemoteFile(file, { signal: controller.signal }, this.requests) this.requests.wrapSyncFunction(() => { this.uppy.off('file-removed', removedHandler) @@ -584,36 +465,9 @@ export default class XHRUpload extends UploaderPlugin { return uploadPromise } - return this.#upload(file, current, total) - })) - } - - onFileRemove (fileID, cb) { - this.uploaderEvents[fileID].on('file-removed', (file) => { - if (fileID === file.id) cb(file.id) - }) - } - onRetry (fileID, cb) { - this.uploaderEvents[fileID].on('upload-retry', (targetFileID) => { - if (fileID === targetFileID) { - cb() - } - }) - } - - onRetryAll (fileID, cb) { - this.uploaderEvents[fileID].on('retry-all', () => { - if (!this.uppy.getFile(fileID)) return - cb() - }) - } - - onCancelAll (fileID, eventHandler) { - this.uploaderEvents[fileID].on('cancel-all', (...args) => { - if (!this.uppy.getFile(fileID)) return - eventHandler(...args) - }) + return this.#uploadLocalFile(file, current, total) + })) } #handleUpload = async (fileIDs) => { diff --git a/private/dev/Dashboard.js b/private/dev/Dashboard.js index ed6933b9c5..ed28fc214d 100644 --- a/private/dev/Dashboard.js +++ b/private/dev/Dashboard.js @@ -125,7 +125,7 @@ export default () => { uppyDashboard.use(AwsS3Multipart, { companionUrl: COMPANION_URL, limit: 6 }) break case 'xhr': - uppyDashboard.use(XHRUpload, { endpoint: XHR_ENDPOINT, limit: 6, bundle: true }) + uppyDashboard.use(XHRUpload, { endpoint: XHR_ENDPOINT, limit: 6, bundle: false }) break case 'transloadit': uppyDashboard.use(Transloadit, { From 5a3ac5122e123ad105e8ffbf6d362f5c00037bad Mon Sep 17 00:00:00 2001 From: Murderlon Date: Thu, 13 Jul 2023 16:52:45 +0200 Subject: [PATCH 06/12] Also refactor aws-s3 --- packages/@uppy/aws-s3/src/MiniXHRUpload.js | 105 --------------------- packages/@uppy/aws-s3/src/index.js | 39 +------- 2 files changed, 4 insertions(+), 140 deletions(-) diff --git a/packages/@uppy/aws-s3/src/MiniXHRUpload.js b/packages/@uppy/aws-s3/src/MiniXHRUpload.js index d6ff265560..4b3dc0a688 100644 --- a/packages/@uppy/aws-s3/src/MiniXHRUpload.js +++ b/packages/@uppy/aws-s3/src/MiniXHRUpload.js @@ -1,7 +1,4 @@ import { nanoid } from 'nanoid/non-secure' -import { Socket } from '@uppy/companion-client' -import emitSocketProgress from '@uppy/utils/lib/emitSocketProgress' -import getSocketHost from '@uppy/utils/lib/getSocketHost' import EventManager from '@uppy/utils/lib/EventManager' import ProgressTimeout from '@uppy/utils/lib/ProgressTimeout' import ErrorWithCause from '@uppy/utils/lib/ErrorWithCause' @@ -235,106 +232,4 @@ export default class MiniXHRUpload { }) }) } - - async connectToServerSocket (file) { - return new Promise((resolve, reject) => { - const opts = this.getOptions(file) - const token = file.serverToken - const host = getSocketHost(file.remote.companionUrl) - let socket - - const createSocket = () => { - if (socket != null) return - - socket = new Socket({ target: `${host}/api/${token}` }) - - socket.on('progress', (progressData) => emitSocketProgress(this, progressData, file)) - - socket.on('success', (data) => { - const body = opts.getResponseData(data.response.responseText, data.response) - const uploadURL = body[opts.responseUrlFieldName] - - const uploadResp = { - status: data.response.status, - body, - uploadURL, - bytesUploaded: data.bytesUploaded, - } - - this.uppy.emit('upload-success', file, uploadResp) - queuedRequest.done() // eslint-disable-line no-use-before-define - socket.close() - if (this.uploaderEvents[file.id]) { - this.uploaderEvents[file.id].remove() - this.uploaderEvents[file.id] = null - } - return resolve() - }) - - socket.on('error', (errData) => { - const resp = errData.response - const error = resp - ? opts.getResponseError(resp.responseText, resp) - : new ErrorWithCause(errData.error.message, { cause: errData.error }) - this.uppy.emit('upload-error', file, error) - queuedRequest.done() // eslint-disable-line no-use-before-define - if (this.uploaderEvents[file.id]) { - this.uploaderEvents[file.id].remove() - this.uploaderEvents[file.id] = null - } - reject(error) - }) - } - this.uploaderEvents[file.id] = new EventManager(this.uppy) - - let queuedRequest = this.requests.run(() => { - if (file.isPaused) { - socket?.send('pause', {}) - } else { - createSocket() - } - - return () => socket.close() - }) - - this.#addEventHandlerForFile('file-removed', file.id, () => { - socket?.send('cancel', {}) - queuedRequest.abort() - resolve(`upload ${file.id} was removed`) - }) - - this.#addEventHandlerIfFileStillExists('cancel-all', file.id, ({ reason } = {}) => { - if (reason === 'user') { - socket?.send('cancel', {}) - queuedRequest.abort() - } - resolve(`upload ${file.id} was canceled`) - }) - - const onRetryRequest = () => { - if (socket == null) { - queuedRequest.abort() - } else { - socket.send('pause', {}) - queuedRequest.done() - } - queuedRequest = this.requests.run(() => { - if (!file.isPaused) { - if (socket == null) { - createSocket() - } else { - socket.send('resume', {}) - } - } - - return () => socket.close() - }) - } - this.#addEventHandlerForFile('upload-retry', file.id, onRetryRequest) - this.#addEventHandlerIfFileStillExists('retry-all', file.id, onRetryRequest) - }).catch((err) => { - this.uppy.emit('upload-error', file, err) - return Promise.reject(err) - }) - } } diff --git a/packages/@uppy/aws-s3/src/index.js b/packages/@uppy/aws-s3/src/index.js index 8d48a35c40..a45a901fa0 100644 --- a/packages/@uppy/aws-s3/src/index.js +++ b/packages/@uppy/aws-s3/src/index.js @@ -144,8 +144,6 @@ export default class AwsS3 extends UploaderPlugin { this.#client = new RequestClient(uppy, opts) this.#requests = new RateLimitedQueue(this.opts.limit) - - this.setQueueRequestSocketToken(this.#requests.wrapPromiseFunction(this.#requestSocketToken, { priority: -1 })) } [Symbol.for('uppy test: getClient')] () { return this.#client } @@ -249,38 +247,6 @@ export default class AwsS3 extends UploaderPlugin { return Promise.resolve() } - connectToServerSocket (file) { - return this.#uploader.connectToServerSocket(file) - } - - #requestSocketToken = async (file) => { - const opts = this.#uploader.getOptions(file) - const Client = file.remote.providerOptions.provider ? Provider : RequestClient - const client = new Client(this.uppy, file.remote.providerOptions) - const allowedMetaFields = Array.isArray(opts.allowedMetaFields) - ? opts.allowedMetaFields - // Send along all fields by default. - : Object.keys(file.meta) - - if (file.tus) { - // Install file-specific upload overrides. - Object.assign(opts, file.tus) - } - - const res = await client.post(file.remote.url, { - ...file.remote.body, - protocol: 'multipart', - endpoint: opts.endpoint, - size: file.data.size, - fieldname: opts.fieldName, - metadata: Object.fromEntries(allowedMetaFields.map(name => [name, file.meta[name]])), - httpMethod: opts.method, - useFormData: opts.formData, - headers: opts.headers, - }) - return res.token - } - uploadFile (id, current, total) { const file = this.uppy.getFile(id) this.uppy.log(`uploading ${current} of ${total}`) @@ -288,6 +254,9 @@ export default class AwsS3 extends UploaderPlugin { if (file.error) throw new Error(file.error) if (file.isRemote) { + // TODO: why do we need to do this? why not always one or the other? + const Client = file.remote.providerOptions.provider ? Provider : RequestClient + const client = new Client(this.uppy, file.remote.providerOptions) const controller = new AbortController() const removedHandler = (removedFile) => { @@ -295,7 +264,7 @@ export default class AwsS3 extends UploaderPlugin { } this.uppy.on('file-removed', removedHandler) - const uploadPromise = this.uploadRemoteFile(file, { signal: controller.signal }) + const uploadPromise = client.uploadRemoteFile(file, { signal: controller.signal }, this.#requests) this.#requests.wrapSyncFunction(() => { this.uppy.off('file-removed', removedHandler) From ac5409b989475aa8419e05637bb49d336f8f728e Mon Sep 17 00:00:00 2001 From: Murderlon Date: Thu, 13 Jul 2023 16:55:21 +0200 Subject: [PATCH 07/12] Remove UploaderPlugin --- packages/@uppy/aws-s3/src/index.js | 4 +-- packages/@uppy/core/src/UploaderPlugin.js | 34 ----------------------- packages/@uppy/xhr-upload/src/index.js | 4 +-- 3 files changed, 4 insertions(+), 38 deletions(-) delete mode 100644 packages/@uppy/core/src/UploaderPlugin.js diff --git a/packages/@uppy/aws-s3/src/index.js b/packages/@uppy/aws-s3/src/index.js index a45a901fa0..fa66676c72 100644 --- a/packages/@uppy/aws-s3/src/index.js +++ b/packages/@uppy/aws-s3/src/index.js @@ -25,7 +25,7 @@ * the XHRUpload code, but at least it's not horrifically broken :) */ -import UploaderPlugin from '@uppy/core/lib/UploaderPlugin.js' +import BasePlugin from '@uppy/core/lib/BasePlugin.js' import AwsS3Multipart from '@uppy/aws-s3-multipart' import { RateLimitedQueue, internalRateLimitedQueue } from '@uppy/utils/lib/RateLimitedQueue' import { RequestClient, Provider } from '@uppy/companion-client' @@ -103,7 +103,7 @@ function defaultGetResponseError (content, xhr) { let warnedSuccessActionStatus = false // TODO deprecate this, will use s3-multipart instead -export default class AwsS3 extends UploaderPlugin { +export default class AwsS3 extends BasePlugin { static VERSION = packageJson.version #client diff --git a/packages/@uppy/core/src/UploaderPlugin.js b/packages/@uppy/core/src/UploaderPlugin.js deleted file mode 100644 index a877bfb418..0000000000 --- a/packages/@uppy/core/src/UploaderPlugin.js +++ /dev/null @@ -1,34 +0,0 @@ -import BasePlugin from './BasePlugin.js' - -export default class UploaderPlugin extends BasePlugin { - #queueRequestSocketToken - - /** @protected */ - setQueueRequestSocketToken (fn) { - this.#queueRequestSocketToken = fn - } - - async uploadRemoteFile (file, options = {}) { - // TODO: we could rewrite this to use server-sent events instead of creating WebSockets. - try { - if (file.serverToken) { - return await this.connectToServerSocket(file) - } - const serverToken = await this.#queueRequestSocketToken(file).abortOn(options.signal) - - if (!this.uppy.getState().files[file.id]) return undefined - - this.uppy.setFileState(file.id, { serverToken }) - return await this.connectToServerSocket(this.uppy.getFile(file.id)) - } catch (err) { - if (err?.cause?.name === 'AbortError') { - // The file upload was aborted, it’s not an error - return undefined - } - - this.uppy.setFileState(file.id, { serverToken: undefined }) - this.uppy.emit('upload-error', file, err) - throw err - } - } -} diff --git a/packages/@uppy/xhr-upload/src/index.js b/packages/@uppy/xhr-upload/src/index.js index 9aac92b895..4c15103759 100644 --- a/packages/@uppy/xhr-upload/src/index.js +++ b/packages/@uppy/xhr-upload/src/index.js @@ -1,4 +1,4 @@ -import UploaderPlugin from '@uppy/core/lib/UploaderPlugin.js' +import BasePlugin from '@uppy/core/lib/BasePlugin.js' import { nanoid } from 'nanoid/non-secure' import { Provider, RequestClient } from '@uppy/companion-client' import EventManager from '@uppy/utils/lib/EventManager' @@ -44,7 +44,7 @@ function setTypeInBlob (file) { return dataWithUpdatedType } -export default class XHRUpload extends UploaderPlugin { +export default class XHRUpload extends BasePlugin { // eslint-disable-next-line global-require static VERSION = packageJson.version From 14b64c8931075d194e29746a28f7df7d5be0e128 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Tue, 18 Jul 2023 13:33:04 +0200 Subject: [PATCH 08/12] Add onFilePause to EventManager --- packages/@uppy/utils/src/EventManager.js | 36 ++++++------------------ 1 file changed, 8 insertions(+), 28 deletions(-) diff --git a/packages/@uppy/utils/src/EventManager.js b/packages/@uppy/utils/src/EventManager.js index c4bc628757..b675443d13 100644 --- a/packages/@uppy/utils/src/EventManager.js +++ b/packages/@uppy/utils/src/EventManager.js @@ -22,20 +22,20 @@ export default class EventManager { } } - /** - * @param {string} fileID - * @param {function(string): void} cb - */ + onFilePause (fileID, cb) { + this.on('upload-pause', (targetFileID, isPaused) => { + if (fileID === targetFileID) { + cb(isPaused) + } + }) + } + onFileRemove (fileID, cb) { this.on('file-removed', (file) => { if (fileID === file.id) cb(file.id) }) } - /** - * @param {string} fileID - * @param {function(boolean): void} cb - */ onPause (fileID, cb) { this.on('upload-pause', (targetFileID, isPaused) => { if (fileID === targetFileID) { @@ -45,10 +45,6 @@ export default class EventManager { }) } - /** - * @param {string} fileID - * @param {function(): void} cb - */ onRetry (fileID, cb) { this.on('upload-retry', (targetFileID) => { if (fileID === targetFileID) { @@ -57,10 +53,6 @@ export default class EventManager { }) } - /** - * @param {string} fileID - * @param {function(): void} cb - */ onRetryAll (fileID, cb) { this.on('retry-all', () => { if (!this.#uppy.getFile(fileID)) return @@ -68,10 +60,6 @@ export default class EventManager { }) } - /** - * @param {string} fileID - * @param {function(): void} cb - */ onPauseAll (fileID, cb) { this.on('pause-all', () => { if (!this.#uppy.getFile(fileID)) return @@ -79,10 +67,6 @@ export default class EventManager { }) } - /** - * @param {string} fileID - * @param {function(): void} eventHandler - */ onCancelAll (fileID, eventHandler) { this.on('cancel-all', (...args) => { if (!this.#uppy.getFile(fileID)) return @@ -90,10 +74,6 @@ export default class EventManager { }) } - /** - * @param {string} fileID - * @param {function(): void} cb - */ onResumeAll (fileID, cb) { this.on('resume-all', () => { if (!this.#uppy.getFile(fileID)) return From baed2dbc3a0025ef50b9a0cf7fb5abd15eaa2515 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Tue, 25 Jul 2023 12:16:12 +0200 Subject: [PATCH 09/12] Inject correct companion POST args into companion-client --- packages/@uppy/aws-s3-multipart/src/index.js | 19 ++- packages/@uppy/aws-s3/src/index.js | 33 +++++- .../@uppy/companion-client/src/Provider.js | 4 +- .../companion-client/src/RequestClient.js | 112 +++++++++++------- packages/@uppy/tus/src/index.js | 28 ++++- packages/@uppy/xhr-upload/src/index.js | 28 ++++- 6 files changed, 174 insertions(+), 50 deletions(-) diff --git a/packages/@uppy/aws-s3-multipart/src/index.js b/packages/@uppy/aws-s3-multipart/src/index.js index 779cba4b7f..894e260a73 100644 --- a/packages/@uppy/aws-s3-multipart/src/index.js +++ b/packages/@uppy/aws-s3-multipart/src/index.js @@ -803,6 +803,16 @@ export default class AwsS3Multipart extends BasePlugin { }) } + // eslint-disable-next-line class-methods-use-this + #getCompanionClientArgs (file) { + return { + ...file.remote.body, + protocol: 's3-multipart', + size: file.data.size, + metadata: file.meta, + } + } + #upload = async (fileIDs) => { if (fileIDs.length === 0) return undefined @@ -816,7 +826,8 @@ export default class AwsS3Multipart extends BasePlugin { if (file.isRemote) { // TODO: why do we need to do this? why not always one or the other? const Client = file.remote.providerOptions.provider ? Provider : RequestClient - const client = new Client(this.uppy, file.remote.providerOptions) + const getQueue = () => this.requests + const client = new Client(this.uppy, file.remote.providerOptions, getQueue) this.#setResumableUploadsCapability(false) const controller = new AbortController() @@ -825,7 +836,11 @@ export default class AwsS3Multipart extends BasePlugin { } this.uppy.on('file-removed', removedHandler) - const uploadPromise = client.uploadRemoteFile(file, { signal: controller.signal }, this.requests) + const uploadPromise = client.uploadRemoteFile( + file, + this.#getCompanionClientArgs(file), + { signal: controller.signal }, + ) this.requests.wrapSyncFunction(() => { this.uppy.off('file-removed', removedHandler) diff --git a/packages/@uppy/aws-s3/src/index.js b/packages/@uppy/aws-s3/src/index.js index fa66676c72..65727b117e 100644 --- a/packages/@uppy/aws-s3/src/index.js +++ b/packages/@uppy/aws-s3/src/index.js @@ -247,6 +247,30 @@ export default class AwsS3 extends BasePlugin { return Promise.resolve() } + #getCompanionClientArgs = (file) => { + const opts = this.#uploader.getOptions(file) + const allowedMetaFields = Array.isArray(opts.allowedMetaFields) + ? opts.allowedMetaFields + // Send along all fields by default. + : Object.keys(file.meta) + // TODO: do we need tus in aws-s3? + if (file.tus) { + // Install file-specific upload overrides. + Object.assign(opts, file.tus) + } + return { + ...file.remote.body, + protocol: 'multipart', + endpoint: opts.endpoint, + size: file.data.size, + fieldname: opts.fieldName, + metadata: Object.fromEntries(allowedMetaFields.map(name => [name, file.meta[name]])), + httpMethod: opts.method, + useFormData: opts.formData, + headers: opts.headers, + } + } + uploadFile (id, current, total) { const file = this.uppy.getFile(id) this.uppy.log(`uploading ${current} of ${total}`) @@ -256,7 +280,8 @@ export default class AwsS3 extends BasePlugin { if (file.isRemote) { // TODO: why do we need to do this? why not always one or the other? const Client = file.remote.providerOptions.provider ? Provider : RequestClient - const client = new Client(this.uppy, file.remote.providerOptions) + const getQueue = () => this.#requests + const client = new Client(this.uppy, file.remote.providerOptions, getQueue) const controller = new AbortController() const removedHandler = (removedFile) => { @@ -264,7 +289,11 @@ export default class AwsS3 extends BasePlugin { } this.uppy.on('file-removed', removedHandler) - const uploadPromise = client.uploadRemoteFile(file, { signal: controller.signal }, this.#requests) + const uploadPromise = client.uploadRemoteFile( + file, + this.#getCompanionClientArgs(file), + { signal: controller.signal }, + ) this.#requests.wrapSyncFunction(() => { this.uppy.off('file-removed', removedHandler) diff --git a/packages/@uppy/companion-client/src/Provider.js b/packages/@uppy/companion-client/src/Provider.js index 081f3cdc65..3a4dda9d69 100644 --- a/packages/@uppy/companion-client/src/Provider.js +++ b/packages/@uppy/companion-client/src/Provider.js @@ -10,8 +10,8 @@ const getName = (id) => { export default class Provider extends RequestClient { #refreshingTokenPromise - constructor (uppy, opts) { - super(uppy, opts) + constructor (uppy, opts, getQueue) { + super(uppy, opts, getQueue) this.provider = opts.provider this.id = this.provider this.name = this.opts.name || getName(this.id) diff --git a/packages/@uppy/companion-client/src/RequestClient.js b/packages/@uppy/companion-client/src/RequestClient.js index f812d92f64..8f96224d8d 100644 --- a/packages/@uppy/companion-client/src/RequestClient.js +++ b/packages/@uppy/companion-client/src/RequestClient.js @@ -30,8 +30,12 @@ async function handleJSONResponse (res) { try { const errData = await jsonPromise errMsg = errData.message ? `${errMsg} message: ${errData.message}` : errMsg - errMsg = errData.requestId ? `${errMsg} request-Id: ${errData.requestId}` : errMsg - } catch { /* if the response contains invalid JSON, let's ignore the error */ } + errMsg = errData.requestId + ? `${errMsg} request-Id: ${errData.requestId}` + : errMsg + } catch { + /* if the response contains invalid JSON, let's ignore the error */ + } throw new Error(errMsg) } @@ -43,9 +47,10 @@ export default class RequestClient { #companionHeaders - constructor (uppy, opts) { + constructor (uppy, opts, getQueue) { this.uppy = uppy this.opts = opts + this.getQueue = getQueue this.onReceiveResponse = this.onReceiveResponse.bind(this) this.#companionHeaders = opts?.companionHeaders } @@ -54,7 +59,9 @@ export default class RequestClient { this.#companionHeaders = headers } - [Symbol.for('uppy test: getCompanionHeaders')] () { return this.#companionHeaders } + [Symbol.for('uppy test: getCompanionHeaders')] () { + return this.#companionHeaders + } get hostname () { const { companion } = this.uppy.getState() @@ -113,7 +120,11 @@ export default class RequestClient { const allowedHeadersCached = allowedHeadersCache.get(this.hostname) if (allowedHeadersCached != null) return allowedHeadersCached - const fallbackAllowedHeaders = ['accept', 'content-type', 'uppy-auth-token'] + const fallbackAllowedHeaders = [ + 'accept', + 'content-type', + 'uppy-auth-token', + ] const promise = (async () => { try { @@ -125,13 +136,20 @@ export default class RequestClient { return fallbackAllowedHeaders } - this.uppy.log(`[CompanionClient] adding allowed preflight headers to companion cache: ${this.hostname} ${header}`) + this.uppy.log( + `[CompanionClient] adding allowed preflight headers to companion cache: ${this.hostname} ${header}`, + ) - const allowedHeaders = header.split(',').map((headerName) => headerName.trim().toLowerCase()) + const allowedHeaders = header + .split(',') + .map((headerName) => headerName.trim().toLowerCase()) allowedHeadersCache.set(this.hostname, allowedHeaders) return allowedHeaders } catch (err) { - this.uppy.log(`[CompanionClient] unable to make preflight request ${err}`, 'warning') + this.uppy.log( + `[CompanionClient] unable to make preflight request ${err}`, + 'warning', + ) // If the user gets a network error or similar, we should try preflight // again next time, or else we might get incorrect behaviour. allowedHeadersCache.delete(this.hostname) // re-fetch next time @@ -144,15 +162,22 @@ export default class RequestClient { } async preflightAndHeaders (path) { - const [allowedHeaders, headers] = await Promise.all([this.preflight(path), this.headers()]) + const [allowedHeaders, headers] = await Promise.all([ + this.preflight(path), + this.headers(), + ]) // filter to keep only allowed Headers - return Object.fromEntries(Object.entries(headers).filter(([header]) => { - if (!allowedHeaders.includes(header.toLowerCase())) { - this.uppy.log(`[CompanionClient] excluding disallowed header ${header}`) - return false - } - return true - })) + return Object.fromEntries( + Object.entries(headers).filter(([header]) => { + if (!allowedHeaders.includes(header.toLowerCase())) { + this.uppy.log( + `[CompanionClient] excluding disallowed header ${header}`, + ) + return false + } + return true + }), + ) } /** @protected */ @@ -170,7 +195,9 @@ export default class RequestClient { return handleJSONResponse(response) } catch (err) { if (err?.isAuthError) throw err - throw new ErrorWithCause(`Could not ${method} ${this.#getUrl(path)}`, { cause: err }) + throw new ErrorWithCause(`Could not ${method} ${this.#getUrl(path)}`, { + cause: err, + }) } } @@ -195,18 +222,26 @@ export default class RequestClient { return this.request({ ...options, path, method: 'DELETE', data }) } - async uploadRemoteFile (file, options = {}, requests) { + async uploadRemoteFile (file, reqBody, options = {}) { try { if (file.serverToken) { - return await this.connectToServerSocket(file, this.requests) + return await this.connectToServerSocket(file, this.getQueue()) } - const queueRequestSocketToken = requests.wrapPromiseFunction(this.#requestSocketToken, { priority: -1 }) - const serverToken = await queueRequestSocketToken(file).abortOn(options.signal) + const queueRequestSocketToken = this.getQueue().wrapPromiseFunction( + this.#requestSocketToken, + { priority: -1 }, + ) + const serverToken = await queueRequestSocketToken(file, reqBody).abortOn( + options.signal, + ) if (!this.uppy.getState().files[file.id]) return undefined this.uppy.setFileState(file.id, { serverToken }) - return await this.connectToServerSocket(this.uppy.getFile(file.id), requests) + return await this.connectToServerSocket( + this.uppy.getFile(file.id), + this.getQueue(), + ) } catch (err) { if (err?.cause?.name === 'AbortError') { // The file upload was aborted, it’s not an error @@ -219,35 +254,30 @@ export default class RequestClient { } } - #requestSocketToken = async (file, options) => { - const opts = { ...this.opts } - - if (file.tus) { - // Install file-specific upload overrides. - Object.assign(opts, file.tus) - } - + #requestSocketToken = async (file, postBody) => { if (file.remote.url == null) { throw new Error('Cannot connect to an undefined URL') } const res = await this.post(file.remote.url, { ...file.remote.body, - protocol: 's3-multipart', - size: file.data.size, - metadata: file.meta, - }, options) + ...postBody, + }) + return res.token } /** * @param {UppyFile} file */ - async connectToServerSocket (file, requests) { + async connectToServerSocket (file, queue) { return new Promise((resolve, reject) => { const token = file.serverToken const host = getSocketHost(file.remote.companionUrl) - const socket = new Socket({ target: `${host}/api/${token}`, autoOpen: false }) + const socket = new Socket({ + target: `${host}/api/${token}`, + autoOpen: false, + }) const eventManager = new EventManager(this.uppy) let queuedRequest @@ -267,7 +297,7 @@ export default class RequestClient { // Resuming an upload should be queued, else you could pause and then // resume a queued upload to make it skip the queue. queuedRequest.abort() - queuedRequest = requests.run(() => { + queuedRequest = queue.run(() => { socket.open() socket.send('resume', {}) @@ -294,7 +324,7 @@ export default class RequestClient { if (file.error) { socket.send('pause', {}) } - queuedRequest = requests.run(() => { + queuedRequest = queue.run(() => { socket.open() socket.send('resume', {}) @@ -325,7 +355,9 @@ export default class RequestClient { socket.on('error', (errData) => { const { message } = errData.error - const error = Object.assign(new Error(message), { cause: errData.error }) + const error = Object.assign(new Error(message), { + cause: errData.error, + }) // If the remote retry optimisation should not be used, // close the socket—this will tell companion to clear state and delete the file. @@ -354,7 +386,7 @@ export default class RequestClient { resolve() }) - queuedRequest = requests.run(() => { + queuedRequest = queue.run(() => { if (file.isPaused) { socket.send('pause', {}) } else { diff --git a/packages/@uppy/tus/src/index.js b/packages/@uppy/tus/src/index.js index fe535367df..22a6efc669 100644 --- a/packages/@uppy/tus/src/index.js +++ b/packages/@uppy/tus/src/index.js @@ -443,6 +443,25 @@ export default class Tus extends BasePlugin { } } + #getCompanionClientArgs (file) { + const opts = { ...this.opts } + + if (file.tus) { + // Install file-specific upload overrides. + Object.assign(opts, file.tus) + } + + return { + ...file.remote.body, + endpoint: opts.endpoint, + uploadUrl: opts.uploadUrl, + protocol: 'tus', + size: file.data.size, + headers: opts.headers, + metadata: file.meta, + } + } + /** * @param {(UppyFile | FailedUppyFile)[]} files */ @@ -458,7 +477,8 @@ export default class Tus extends BasePlugin { if (file.isRemote) { // TODO: why do we need to do this? why not always one or the other? const Client = file.remote.providerOptions.provider ? Provider : RequestClient - const client = new Client(this.uppy, file.remote.providerOptions) + const getQueue = () => this.requests + const client = new Client(this.uppy, file.remote.providerOptions, getQueue) const controller = new AbortController() const removedHandler = (removedFile) => { @@ -466,7 +486,11 @@ export default class Tus extends BasePlugin { } this.uppy.on('file-removed', removedHandler) - const uploadPromise = client.uploadRemoteFile(file, { signal: controller.signal }, this.requests) + const uploadPromise = client.uploadRemoteFile( + file, + this.#getCompanionClientArgs(file), + { signal: controller.signal }, + ) this.requests.wrapSyncFunction(() => { this.uppy.off('file-removed', removedHandler) diff --git a/packages/@uppy/xhr-upload/src/index.js b/packages/@uppy/xhr-upload/src/index.js index 4c15103759..7a09d4f402 100644 --- a/packages/@uppy/xhr-upload/src/index.js +++ b/packages/@uppy/xhr-upload/src/index.js @@ -441,6 +441,25 @@ export default class XHRUpload extends BasePlugin { }) } + #getCompanionClientArgs (file) { + const opts = this.getOptions(file) + const allowedMetaFields = Array.isArray(opts.allowedMetaFields) + ? opts.allowedMetaFields + // Send along all fields by default. + : Object.keys(file.meta) + return { + ...file.remote.body, + protocol: 'multipart', + endpoint: opts.endpoint, + size: file.data.size, + fieldname: opts.fieldName, + metadata: Object.fromEntries(allowedMetaFields.map(name => [name, file.meta[name]])), + httpMethod: opts.method, + useFormData: opts.formData, + headers: opts.headers, + } + } + async #uploadFiles (files) { await Promise.allSettled(files.map((file, i) => { const current = parseInt(i, 10) + 1 @@ -449,7 +468,8 @@ export default class XHRUpload extends BasePlugin { if (file.isRemote) { // TODO: why do we need to do this? why not always one or the other? const Client = file.remote.providerOptions.provider ? Provider : RequestClient - const client = new Client(this.uppy, file.remote.providerOptions) + const getQueue = () => this.requests + const client = new Client(this.uppy, file.remote.providerOptions, getQueue) const controller = new AbortController() const removedHandler = (removedFile) => { @@ -457,7 +477,11 @@ export default class XHRUpload extends BasePlugin { } this.uppy.on('file-removed', removedHandler) - const uploadPromise = client.uploadRemoteFile(file, { signal: controller.signal }, this.requests) + const uploadPromise = client.uploadRemoteFile( + file, + this.#getCompanionClientArgs(file), + { signal: controller.signal }, + ) this.requests.wrapSyncFunction(() => { this.uppy.off('file-removed', removedHandler) From 66c714df3689d00c1f1f7f108e07e8ac33a6dd3e Mon Sep 17 00:00:00 2001 From: Murderlon Date: Thu, 24 Aug 2023 09:55:23 +0100 Subject: [PATCH 10/12] Simplify --- packages/@uppy/aws-s3-multipart/src/index.js | 4 +--- packages/@uppy/aws-s3/src/index.js | 9 +-------- packages/@uppy/tus/src/index.js | 6 ++---- packages/@uppy/xhr-upload/src/index.js | 6 ++---- 4 files changed, 6 insertions(+), 19 deletions(-) diff --git a/packages/@uppy/aws-s3-multipart/src/index.js b/packages/@uppy/aws-s3-multipart/src/index.js index 894e260a73..f94fe99114 100644 --- a/packages/@uppy/aws-s3-multipart/src/index.js +++ b/packages/@uppy/aws-s3-multipart/src/index.js @@ -824,10 +824,8 @@ export default class AwsS3Multipart extends BasePlugin { const promises = filesFiltered.map((file) => { if (file.isRemote) { - // TODO: why do we need to do this? why not always one or the other? - const Client = file.remote.providerOptions.provider ? Provider : RequestClient const getQueue = () => this.requests - const client = new Client(this.uppy, file.remote.providerOptions, getQueue) + const client = new Provider(this.uppy, file.remote.providerOptions, getQueue) this.#setResumableUploadsCapability(false) const controller = new AbortController() diff --git a/packages/@uppy/aws-s3/src/index.js b/packages/@uppy/aws-s3/src/index.js index 65727b117e..c4510c0039 100644 --- a/packages/@uppy/aws-s3/src/index.js +++ b/packages/@uppy/aws-s3/src/index.js @@ -253,11 +253,6 @@ export default class AwsS3 extends BasePlugin { ? opts.allowedMetaFields // Send along all fields by default. : Object.keys(file.meta) - // TODO: do we need tus in aws-s3? - if (file.tus) { - // Install file-specific upload overrides. - Object.assign(opts, file.tus) - } return { ...file.remote.body, protocol: 'multipart', @@ -278,10 +273,8 @@ export default class AwsS3 extends BasePlugin { if (file.error) throw new Error(file.error) if (file.isRemote) { - // TODO: why do we need to do this? why not always one or the other? - const Client = file.remote.providerOptions.provider ? Provider : RequestClient const getQueue = () => this.#requests - const client = new Client(this.uppy, file.remote.providerOptions, getQueue) + const client = new Provider(this.uppy, file.remote.providerOptions, getQueue) const controller = new AbortController() const removedHandler = (removedFile) => { diff --git a/packages/@uppy/tus/src/index.js b/packages/@uppy/tus/src/index.js index 22a6efc669..a35c378f68 100644 --- a/packages/@uppy/tus/src/index.js +++ b/packages/@uppy/tus/src/index.js @@ -1,6 +1,6 @@ import BasePlugin from '@uppy/core/lib/BasePlugin.js' import * as tus from 'tus-js-client' -import { Provider, RequestClient } from '@uppy/companion-client' +import { Provider } from '@uppy/companion-client' import EventManager from '@uppy/utils/lib/EventManager' import NetworkError from '@uppy/utils/lib/NetworkError' import isNetworkError from '@uppy/utils/lib/isNetworkError' @@ -475,10 +475,8 @@ export default class Tus extends BasePlugin { const total = files.length if (file.isRemote) { - // TODO: why do we need to do this? why not always one or the other? - const Client = file.remote.providerOptions.provider ? Provider : RequestClient const getQueue = () => this.requests - const client = new Client(this.uppy, file.remote.providerOptions, getQueue) + const client = new Provider(this.uppy, file.remote.providerOptions, getQueue) const controller = new AbortController() const removedHandler = (removedFile) => { diff --git a/packages/@uppy/xhr-upload/src/index.js b/packages/@uppy/xhr-upload/src/index.js index 7a09d4f402..50b1c390e3 100644 --- a/packages/@uppy/xhr-upload/src/index.js +++ b/packages/@uppy/xhr-upload/src/index.js @@ -1,6 +1,6 @@ import BasePlugin from '@uppy/core/lib/BasePlugin.js' import { nanoid } from 'nanoid/non-secure' -import { Provider, RequestClient } from '@uppy/companion-client' +import { Provider } from '@uppy/companion-client' import EventManager from '@uppy/utils/lib/EventManager' import ProgressTimeout from '@uppy/utils/lib/ProgressTimeout' import { RateLimitedQueue, internalRateLimitedQueue } from '@uppy/utils/lib/RateLimitedQueue' @@ -466,10 +466,8 @@ export default class XHRUpload extends BasePlugin { const total = files.length if (file.isRemote) { - // TODO: why do we need to do this? why not always one or the other? - const Client = file.remote.providerOptions.provider ? Provider : RequestClient const getQueue = () => this.requests - const client = new Client(this.uppy, file.remote.providerOptions, getQueue) + const client = new Provider(this.uppy, file.remote.providerOptions, getQueue) const controller = new AbortController() const removedHandler = (removedFile) => { From e4260d48d609d40d043cda238949270b0df67ad9 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Thu, 24 Aug 2023 12:10:32 +0100 Subject: [PATCH 11/12] Revert "Simplify" This reverts commit 66c714df3689d00c1f1f7f108e07e8ac33a6dd3e. --- packages/@uppy/aws-s3-multipart/src/index.js | 4 +++- packages/@uppy/aws-s3/src/index.js | 9 ++++++++- packages/@uppy/tus/src/index.js | 6 ++++-- packages/@uppy/xhr-upload/src/index.js | 6 ++++-- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/packages/@uppy/aws-s3-multipart/src/index.js b/packages/@uppy/aws-s3-multipart/src/index.js index 44fab43373..8299396eb4 100644 --- a/packages/@uppy/aws-s3-multipart/src/index.js +++ b/packages/@uppy/aws-s3-multipart/src/index.js @@ -831,8 +831,10 @@ export default class AwsS3Multipart extends BasePlugin { const promises = filesFiltered.map((file) => { if (file.isRemote) { + // TODO: why do we need to do this? why not always one or the other? + const Client = file.remote.providerOptions.provider ? Provider : RequestClient const getQueue = () => this.requests - const client = new Provider(this.uppy, file.remote.providerOptions, getQueue) + const client = new Client(this.uppy, file.remote.providerOptions, getQueue) this.#setResumableUploadsCapability(false) const controller = new AbortController() diff --git a/packages/@uppy/aws-s3/src/index.js b/packages/@uppy/aws-s3/src/index.js index 479c59ad59..671512c05d 100644 --- a/packages/@uppy/aws-s3/src/index.js +++ b/packages/@uppy/aws-s3/src/index.js @@ -253,6 +253,11 @@ export default class AwsS3 extends BasePlugin { ? opts.allowedMetaFields // Send along all fields by default. : Object.keys(file.meta) + // TODO: do we need tus in aws-s3? + if (file.tus) { + // Install file-specific upload overrides. + Object.assign(opts, file.tus) + } return { ...file.remote.body, protocol: 'multipart', @@ -273,8 +278,10 @@ export default class AwsS3 extends BasePlugin { if (file.error) throw new Error(file.error) if (file.isRemote) { + // TODO: why do we need to do this? why not always one or the other? + const Client = file.remote.providerOptions.provider ? Provider : RequestClient const getQueue = () => this.#requests - const client = new Provider(this.uppy, file.remote.providerOptions, getQueue) + const client = new Client(this.uppy, file.remote.providerOptions, getQueue) const controller = new AbortController() const removedHandler = (removedFile) => { diff --git a/packages/@uppy/tus/src/index.js b/packages/@uppy/tus/src/index.js index a35c378f68..22a6efc669 100644 --- a/packages/@uppy/tus/src/index.js +++ b/packages/@uppy/tus/src/index.js @@ -1,6 +1,6 @@ import BasePlugin from '@uppy/core/lib/BasePlugin.js' import * as tus from 'tus-js-client' -import { Provider } from '@uppy/companion-client' +import { Provider, RequestClient } from '@uppy/companion-client' import EventManager from '@uppy/utils/lib/EventManager' import NetworkError from '@uppy/utils/lib/NetworkError' import isNetworkError from '@uppy/utils/lib/isNetworkError' @@ -475,8 +475,10 @@ export default class Tus extends BasePlugin { const total = files.length if (file.isRemote) { + // TODO: why do we need to do this? why not always one or the other? + const Client = file.remote.providerOptions.provider ? Provider : RequestClient const getQueue = () => this.requests - const client = new Provider(this.uppy, file.remote.providerOptions, getQueue) + const client = new Client(this.uppy, file.remote.providerOptions, getQueue) const controller = new AbortController() const removedHandler = (removedFile) => { diff --git a/packages/@uppy/xhr-upload/src/index.js b/packages/@uppy/xhr-upload/src/index.js index 50b1c390e3..7a09d4f402 100644 --- a/packages/@uppy/xhr-upload/src/index.js +++ b/packages/@uppy/xhr-upload/src/index.js @@ -1,6 +1,6 @@ import BasePlugin from '@uppy/core/lib/BasePlugin.js' import { nanoid } from 'nanoid/non-secure' -import { Provider } from '@uppy/companion-client' +import { Provider, RequestClient } from '@uppy/companion-client' import EventManager from '@uppy/utils/lib/EventManager' import ProgressTimeout from '@uppy/utils/lib/ProgressTimeout' import { RateLimitedQueue, internalRateLimitedQueue } from '@uppy/utils/lib/RateLimitedQueue' @@ -466,8 +466,10 @@ export default class XHRUpload extends BasePlugin { const total = files.length if (file.isRemote) { + // TODO: why do we need to do this? why not always one or the other? + const Client = file.remote.providerOptions.provider ? Provider : RequestClient const getQueue = () => this.requests - const client = new Provider(this.uppy, file.remote.providerOptions, getQueue) + const client = new Client(this.uppy, file.remote.providerOptions, getQueue) const controller = new AbortController() const removedHandler = (removedFile) => { From 8aa57cf4e49e0237e1e3c357079ecafc7a2d7407 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Thu, 24 Aug 2023 12:14:17 +0100 Subject: [PATCH 12/12] Conditional requestclient vs provider was needed after all --- packages/@uppy/aws-s3-multipart/src/index.js | 3 ++- packages/@uppy/aws-s3/src/index.js | 8 ++------ packages/@uppy/tus/src/index.js | 3 ++- packages/@uppy/xhr-upload/src/index.js | 3 ++- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/@uppy/aws-s3-multipart/src/index.js b/packages/@uppy/aws-s3-multipart/src/index.js index 8299396eb4..45e1b2887f 100644 --- a/packages/@uppy/aws-s3-multipart/src/index.js +++ b/packages/@uppy/aws-s3-multipart/src/index.js @@ -831,7 +831,8 @@ export default class AwsS3Multipart extends BasePlugin { const promises = filesFiltered.map((file) => { if (file.isRemote) { - // TODO: why do we need to do this? why not always one or the other? + // INFO: the url plugin needs to use RequestClient, + // while others use Provider const Client = file.remote.providerOptions.provider ? Provider : RequestClient const getQueue = () => this.requests const client = new Client(this.uppy, file.remote.providerOptions, getQueue) diff --git a/packages/@uppy/aws-s3/src/index.js b/packages/@uppy/aws-s3/src/index.js index 671512c05d..ee2e69b73b 100644 --- a/packages/@uppy/aws-s3/src/index.js +++ b/packages/@uppy/aws-s3/src/index.js @@ -253,11 +253,6 @@ export default class AwsS3 extends BasePlugin { ? opts.allowedMetaFields // Send along all fields by default. : Object.keys(file.meta) - // TODO: do we need tus in aws-s3? - if (file.tus) { - // Install file-specific upload overrides. - Object.assign(opts, file.tus) - } return { ...file.remote.body, protocol: 'multipart', @@ -278,7 +273,8 @@ export default class AwsS3 extends BasePlugin { if (file.error) throw new Error(file.error) if (file.isRemote) { - // TODO: why do we need to do this? why not always one or the other? + // INFO: the url plugin needs to use RequestClient, + // while others use Provider const Client = file.remote.providerOptions.provider ? Provider : RequestClient const getQueue = () => this.#requests const client = new Client(this.uppy, file.remote.providerOptions, getQueue) diff --git a/packages/@uppy/tus/src/index.js b/packages/@uppy/tus/src/index.js index 22a6efc669..377c74c7e6 100644 --- a/packages/@uppy/tus/src/index.js +++ b/packages/@uppy/tus/src/index.js @@ -475,7 +475,8 @@ export default class Tus extends BasePlugin { const total = files.length if (file.isRemote) { - // TODO: why do we need to do this? why not always one or the other? + // INFO: the url plugin needs to use RequestClient, + // while others use Provider const Client = file.remote.providerOptions.provider ? Provider : RequestClient const getQueue = () => this.requests const client = new Client(this.uppy, file.remote.providerOptions, getQueue) diff --git a/packages/@uppy/xhr-upload/src/index.js b/packages/@uppy/xhr-upload/src/index.js index 7a09d4f402..8de78482f4 100644 --- a/packages/@uppy/xhr-upload/src/index.js +++ b/packages/@uppy/xhr-upload/src/index.js @@ -466,7 +466,8 @@ export default class XHRUpload extends BasePlugin { const total = files.length if (file.isRemote) { - // TODO: why do we need to do this? why not always one or the other? + // INFO: the url plugin needs to use RequestClient, + // while others use Provider const Client = file.remote.providerOptions.provider ? Provider : RequestClient const getQueue = () => this.requests const client = new Client(this.uppy, file.remote.providerOptions, getQueue)