From adad14fb6125e2d80a6d5576979806ca597f2785 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Wed, 18 Oct 2023 16:06:30 +0200 Subject: [PATCH 1/8] New priority queue & exponential backoff client --- .../companion-client/src/RequestClient.js | 255 ++++++------ packages/@uppy/core/package.json | 1 + packages/@uppy/core/src/Uppy.js | 8 + packages/@uppy/core/types/index.d.ts | 3 + packages/@uppy/utils/package.json | 1 + packages/@uppy/utils/src/fetcher.js | 141 +++++++ packages/@uppy/xhr-upload/src/index.js | 376 +++++------------- packages/@uppy/xhr-upload/src/index.test.js | 4 +- yarn.lock | 1 + 9 files changed, 387 insertions(+), 403 deletions(-) create mode 100644 packages/@uppy/utils/src/fetcher.js diff --git a/packages/@uppy/companion-client/src/RequestClient.js b/packages/@uppy/companion-client/src/RequestClient.js index 8f96224d8d..b288ea33d2 100644 --- a/packages/@uppy/companion-client/src/RequestClient.js +++ b/packages/@uppy/companion-client/src/RequestClient.js @@ -4,13 +4,24 @@ 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 { getUppyAbortController } from '@uppy/utils/lib/fetcher' import AuthError from './AuthError.js' import Socket from './Socket.js' import packageJson from '../package.json' +function defer () { + const deferred = {} + + deferred.promise = new Promise((resolve, reject) => { + deferred.resolve = resolve + deferred.reject = reject + }) + + return deferred +} + // Remove the trailing slash so we can always safely append /xyz. function stripSlash (url) { return url.replace(/\/$/, '') @@ -47,29 +58,28 @@ export default class RequestClient { #companionHeaders - constructor (uppy, opts, getQueue) { + constructor(uppy, opts) { this.uppy = uppy this.opts = opts - this.getQueue = getQueue this.onReceiveResponse = this.onReceiveResponse.bind(this) this.#companionHeaders = opts?.companionHeaders } - setCompanionHeaders (headers) { + setCompanionHeaders(headers) { this.#companionHeaders = headers } - [Symbol.for('uppy test: getCompanionHeaders')] () { + [Symbol.for('uppy test: getCompanionHeaders')]() { return this.#companionHeaders } - get hostname () { + get hostname() { const { companion } = this.uppy.getState() const host = this.opts.companionUrl return stripSlash(companion && companion[host] ? companion[host] : host) } - async headers () { + async headers() { const defaultHeaders = { Accept: 'application/json', 'Content-Type': 'application/json', @@ -82,7 +92,7 @@ export default class RequestClient { } } - onReceiveResponse ({ headers }) { + onReceiveResponse({ headers }) { const state = this.uppy.getState() const companion = state.companion || {} const host = this.opts.companionUrl @@ -95,7 +105,7 @@ export default class RequestClient { } } - #getUrl (url) { + #getUrl(url) { if (/^(https?:|)\/\//.test(url)) { return url } @@ -116,15 +126,11 @@ export default class RequestClient { Subsequent requests use the cached result of the preflight. However if there is an error retrieving the allowed headers, we will try again next time */ - async preflight (path) { + async preflight(path) { 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 { @@ -161,7 +167,7 @@ export default class RequestClient { return promise } - async preflightAndHeaders (path) { + async preflightAndHeaders(path) { const [allowedHeaders, headers] = await Promise.all([ this.preflight(path), this.headers(), @@ -181,7 +187,7 @@ export default class RequestClient { } /** @protected */ - async request ({ path, method = 'GET', data, skipPostResponse, signal }) { + async request({ path, method = 'GET', data, skipPostResponse, signal }) { try { const headers = await this.preflightAndHeaders(path) const response = await fetchWithNetworkError(this.#getUrl(path), { @@ -201,47 +207,45 @@ export default class RequestClient { } } - async get (path, options = undefined) { + async get(path, options = undefined) { // TODO: remove boolean support for options that was added for backward compatibility. // eslint-disable-next-line no-param-reassign if (typeof options === 'boolean') options = { skipPostResponse: options } return this.request({ ...options, path }) } - async post (path, data, options = undefined) { + async post(path, data, options = undefined) { // TODO: remove boolean support for options that was added for backward compatibility. // eslint-disable-next-line no-param-reassign if (typeof options === 'boolean') options = { skipPostResponse: options } return this.request({ ...options, path, method: 'POST', data }) } - async delete (path, data = undefined, options) { + async delete(path, data = undefined, options) { // TODO: remove boolean support for options that was added for backward compatibility. // eslint-disable-next-line no-param-reassign if (typeof options === 'boolean') options = { skipPostResponse: options } return this.request({ ...options, path, method: 'DELETE', data }) } - async uploadRemoteFile (file, reqBody, options = {}) { + async uploadRemoteFile(file, reqBody) { + // TODO: handle signal internally try { if (file.serverToken) { - return await this.connectToServerSocket(file, this.getQueue()) + return await this.connectToServerSocket(file) } - const queueRequestSocketToken = this.getQueue().wrapPromiseFunction( - this.#requestSocketToken, - { priority: -1 }, - ) - const serverToken = await queueRequestSocketToken(file, reqBody).abortOn( - options.signal, + const controller = getUppyAbortController(this.uppy, file.id) + const serverToken = await this.uppy.queue.add( + ({ signal }) => { + return this.#requestSocketToken(file, reqBody, signal) + }, + { signal: controller.signal, priority: -1 }, ) if (!this.uppy.getState().files[file.id]) return undefined this.uppy.setFileState(file.id, { serverToken }) - return await this.connectToServerSocket( - this.uppy.getFile(file.id), - this.getQueue(), - ) + 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 @@ -254,7 +258,7 @@ export default class RequestClient { } } - #requestSocketToken = async (file, postBody) => { + #requestSocketToken = async (file, postBody, signal) => { if (file.remote.url == null) { throw new Error('Cannot connect to an undefined URL') } @@ -262,6 +266,7 @@ export default class RequestClient { const res = await this.post(file.remote.url, { ...file.remote.body, ...postBody, + signal, }) return res.token @@ -270,69 +275,24 @@ export default class RequestClient { /** * @param {UppyFile} file */ - 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 eventManager = new EventManager(this.uppy) - - let queuedRequest - - eventManager.onFileRemove(file.id, () => { + async connectToServerSocket(file) { + const token = file.serverToken + const host = getSocketHost(file.remote.companionUrl) + const socket = new Socket({ target: `${host}/api/${token}`, autoOpen: false }) + const { capabilities } = this.uppy.getState() + const deferred = defer() + const controller = new AbortController() + + this.uppy.on('cancel-all', ({reason}) => { + if (reason === 'user') { socket.send('cancel', {}) - queuedRequest.abort() - 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 = queue.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() - } - resolve(`upload ${file.id} was canceled`) - }) - - eventManager.onResumeAll(file.id, () => { - queuedRequest.abort() - if (file.error) { - socket.send('pause', {}) - } - queuedRequest = queue.run(() => { - socket.open() - socket.send('resume', {}) - - return () => {} - }) - }) + controller.abort() + } + deferred.resolve(`upload ${file.id} was canceled`) + }) - eventManager.onRetry(file.id, () => { + this.uppy.on('upload-retry', (id) => { + if (file.id === 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 @@ -341,60 +301,93 @@ export default class RequestClient { socket.send('pause', {}) socket.send('resume', {}) } - }) + } + }) - eventManager.onRetryAll(file.id, () => { - // See the comment in the onRetry() call + this.uppy.on('retry-all', () => { + if (this.uppy.getFile(file.id)) { 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 (capabilities.resumableUploads) { + this.uppy.on('pause-all', () => { + if (this.uppy.getFile(file.id)) { + socket.send('pause', {}) + } + }) - // 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) { - // Remove the serverToken so that a new one will be created for the retry. - this.uppy.setFileState(file.id, { - serverToken: null, + this.uppy.on('resume-all', () => { + if (this.uppy.getFile(file.id)) { + controller.abort() + if (file.error) { + socket.send('pause', {}) + } + this.uppy.queue.add(() => { + socket.open() + socket.send('resume', {}) }) - } else { - socket.close() } - - this.uppy.emit('upload-error', file, error) - queuedRequest.done() - reject(error) }) + } - socket.on('success', (data) => { - const uploadResp = { - uploadURL: data.url, + if (capabilities.individualCancellation) { + this.uppy.on('file-removed', () => { + if (this.uppy.getFile(file.id)) { + socket.send('cancel', {}) + controller.abort() + deferred.resolve(`upload ${file.id} was removed`) } + }) + } - this.uppy.emit('upload-success', file, uploadResp) - queuedRequest.done() - socket.close() - resolve() + 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, }) - queuedRequest = queue.run(() => { - if (file.isPaused) { - socket.send('pause', {}) - } else { - socket.open() + // 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) { + if (this.uppy.getFile(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() + } - return () => {} - }) + this.uppy.emit('upload-error', file, error) + deferred.reject(error) + }) + + socket.on('success', (data) => { + const uploadResp = { + uploadURL: data.url, + } + + this.uppy.emit('upload-success', file, uploadResp) + socket.close() + deferred.resolve() }) + + this.uppy.queue.add(() => { + if (file.isPaused) { + socket.send('pause', {}) + } else { + socket.open() + } + return deferred.promise + }, { signal: controller.signal }) } } diff --git a/packages/@uppy/core/package.json b/packages/@uppy/core/package.json index 5c537e0438..fc43efce20 100644 --- a/packages/@uppy/core/package.json +++ b/packages/@uppy/core/package.json @@ -29,6 +29,7 @@ "mime-match": "^1.0.2", "namespace-emitter": "^2.0.1", "nanoid": "^4.0.0", + "p-queue": "^7.4.1", "preact": "^10.5.13" }, "devDependencies": { diff --git a/packages/@uppy/core/src/Uppy.js b/packages/@uppy/core/src/Uppy.js index fcde7efdfc..37cb31ca0a 100644 --- a/packages/@uppy/core/src/Uppy.js +++ b/packages/@uppy/core/src/Uppy.js @@ -6,6 +6,7 @@ import ee from 'namespace-emitter' import { nanoid } from 'nanoid/non-secure' import throttle from 'lodash/throttle.js' import DefaultStore from '@uppy/store-default' +import PQueue from 'p-queue' import getFileType from '@uppy/utils/lib/getFileType' import getFileNameAndExtension from '@uppy/utils/lib/getFileNameAndExtension' import { getSafeFileId } from '@uppy/utils/lib/generateFileID' @@ -44,6 +45,8 @@ class Uppy { #postProcessors = new Set() + queue = new PQueue({ concurrency: 6, autoStart: false }) + /** * Instantiate Uppy * @@ -773,6 +776,7 @@ class Uppy { } pauseAll () { + this.queue.pause() const updatedFiles = { ...this.getState().files } const inProgressUpdatedFiles = Object.keys(updatedFiles).filter((file) => { return !updatedFiles[file].progress.uploadComplete @@ -789,6 +793,7 @@ class Uppy { } resumeAll () { + this.queue.start() const updatedFiles = { ...this.getState().files } const inProgressUpdatedFiles = Object.keys(updatedFiles).filter((file) => { return !updatedFiles[file].progress.uploadComplete @@ -843,6 +848,7 @@ class Uppy { } cancelAll ({ reason = 'user' } = {}) { + this.queue.clear() this.emit('cancel-all', { reason }) // Only remove existing uploads if user is canceling @@ -1624,11 +1630,13 @@ class Uppy { }) const uploadID = this.#createUpload(waitingFileIDs) + this.queue.start() return this.#runUpload(uploadID) }) .catch((err) => { this.emit('error', err) this.log(err, 'error') + this.queue.clear() throw err }) } diff --git a/packages/@uppy/core/types/index.d.ts b/packages/@uppy/core/types/index.d.ts index c6d136f5a4..49f3844068 100644 --- a/packages/@uppy/core/types/index.d.ts +++ b/packages/@uppy/core/types/index.d.ts @@ -1,4 +1,5 @@ import * as UppyUtils from '@uppy/utils' +import PQueue from 'p-queue' // Utility types type OmitKey = Pick> @@ -294,6 +295,8 @@ export interface UppyEventMap< export class Uppy { constructor(opts?: UppyOptions) + queue: PQueue + on(event: K, callback: UppyEventMap[K]): this on>( diff --git a/packages/@uppy/utils/package.json b/packages/@uppy/utils/package.json index b95ba2c059..4a516a4fa2 100644 --- a/packages/@uppy/utils/package.json +++ b/packages/@uppy/utils/package.json @@ -57,6 +57,7 @@ "./lib/truncateString": "./lib/truncateString.js", "./lib/remoteFileObjToLocal": "./lib/remoteFileObjToLocal.js", "./lib/fetchWithNetworkError": "./lib/fetchWithNetworkError.js", + "./lib/fetcher": "./lib/fetcher.js", "./lib/ErrorWithCause": "./lib/ErrorWithCause.js", "./lib/delay": "./lib/delay.js", "./lib/hasProperty": "./lib/hasProperty.js", diff --git a/packages/@uppy/utils/src/fetcher.js b/packages/@uppy/utils/src/fetcher.js new file mode 100644 index 0000000000..3997f164d6 --- /dev/null +++ b/packages/@uppy/utils/src/fetcher.js @@ -0,0 +1,141 @@ +/* eslint-disable max-len */ + +export class XhrError extends Error { + /** + * Create an XHR error. + * + * @param {string} message + * Message. + * @param {XMLHttpRequest} xhr + * Message. + * @returns + * XHR error. + */ + constructor (message, xhr) { + super(message) + this.name = 'XhrError' + this.xhr = xhr + } +} + +/** + * Get an abort signal that will be aborted when the specified Uppy file is removed or all uploads are cancelled. + * + * @param {Uppy} uppy + * The Uppy instance. + * @param {string} [id] + * The ID of the file to watch for removal. + * @param {AbortSignal} [additionalSignal] + * An optional additional abort signal. + * @returns {AbortController} + * The abort signal. + */ +export function getUppyAbortController(uppy, id, additionalSignal) { + const controller = new AbortController() + + uppy.once('cancel-all', () => { + controller.abort() + }) + + if (id) { + uppy.on('file-removed', (file) => { + if (id === file.id) { + controller.abort() + } + }) + } + + if (additionalSignal) { + additionalSignal.addEventListener('abort', () => { + controller.abort() + }) + } + + return controller +} + +/** + * Fetches data from a specified URL using XMLHttpRequest, with optional retry functionality and progress tracking. + * + * @param {string} url + * The URL to send the request to. + * @param {object} [options] + * Optional settings for the fetch operation. + * @param {string} [options.method='GET'] + * The HTTP method to use for the request. + * @param {string} [options.body=null] + * The request payload, if any. + * @param {Record} [options.headers] + * An object representing any headers to send with the request. + * @param {number} [options.retries=3] + * The number of retry attempts to make if the request fails. + * @param {number} [options.delay=1000] + * The initial delay between retry attempts, in milliseconds. The delay doubles with each retry. + * @param {function(Event): void} [options.onUploadProgress] + * A callback function for tracking upload progress. + * @param {function(XMLHttpRequest): boolean} [options.shouldRetry] + * A function to determine whether to retry the request. + * @param {AbortSignal} [options.signal] + * signal to abort the upload. + * @returns {Promise} + * A Promise that resolves to the response text if the request succeeds, and rejects with an error if it fails. + */ +export function fetcher(url, options = {}) { + const retries = options.retries || 3; + const delay = (retryCount) => 0.3 * (2 ** (retryCount - 1)) * 1000 + const onUploadProgress = options.onUploadProgress || (() => {}); + const shouldRetry = options.shouldRetry || (() => true); + + function requestWithRetry(retryCount = 0) { + return new Promise((resolve, reject) => { + const xhr = new XMLHttpRequest(); + xhr.open(options.method || 'GET', url, true); + + if (options.signal) { + options.signal.addEventListener('abort', () => { + xhr.abort() + // Using DOMException for abort errors aligns with + // the convention established by the Fetch API. + reject(new DOMException('Aborted', 'AbortError')) + }) + } + + xhr.onload = () => { + if (xhr.status >= 200 && xhr.status < 300) { + resolve(xhr) + } else if (shouldRetry(xhr) && retryCount < retries) { + setTimeout(() => { + requestWithRetry(retryCount + 1).then(resolve, reject) + }, delay(retryCount)) + } else { + reject(new XhrError(xhr.statusText, xhr)); + } + }; + + xhr.onerror = () => { + if (shouldRetry(xhr) && retryCount < retries) { + setTimeout(() => { + requestWithRetry(retryCount + 1).then(resolve, reject) + }, delay(retryCount)) + } else { + reject(new XhrError(xhr.statusText, xhr)); + } + }; + + xhr.upload.onprogress = (event) => { + onUploadProgress(event); + }; + + if (options.headers) { + Object.keys(options.headers).forEach(key => { + xhr.setRequestHeader(key, options.headers[key]); + }); + } + + xhr.send(options.body); + }); + } + + return requestWithRetry(); +} + diff --git a/packages/@uppy/xhr-upload/src/index.js b/packages/@uppy/xhr-upload/src/index.js index 8de78482f4..237f0b1518 100644 --- a/packages/@uppy/xhr-upload/src/index.js +++ b/packages/@uppy/xhr-upload/src/index.js @@ -1,16 +1,14 @@ 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' -import ProgressTimeout from '@uppy/utils/lib/ProgressTimeout' -import { RateLimitedQueue, internalRateLimitedQueue } from '@uppy/utils/lib/RateLimitedQueue' import NetworkError from '@uppy/utils/lib/NetworkError' import isNetworkError from '@uppy/utils/lib/isNetworkError' import { filterNonFailedFiles, filterFilesToEmitUploadStarted } from '@uppy/utils/lib/fileFilters' +import { fetcher, XhrError, getUppyAbortController } from '@uppy/utils/lib/fetcher' import packageJson from '../package.json' import locale from './locale.js' +// TODO: do we really need this? function buildResponseError (xhr, err) { let error = err // No error message @@ -48,7 +46,9 @@ export default class XHRUpload extends BasePlugin { // eslint-disable-next-line global-require static VERSION = packageJson.version - constructor (uppy, opts) { + #uppyFetch + + constructor(uppy, opts) { super(uppy, opts) this.type = 'uploader' this.id = this.opts.id || 'XHRUpload' @@ -70,24 +70,15 @@ export default class XHRUpload extends BasePlugin { withCredentials: false, responseType: '', /** - * @param {string} responseText the response body string + * @param {XMLHttpRequest} response */ - getResponseData (responseText) { - let parsedResponse = {} - try { - parsedResponse = JSON.parse(responseText) - } catch (err) { - uppy.log(err) - } - - return parsedResponse + async getResponseData(response) { + return JSON.parse(response.responseText) }, /** - * - * @param {string} _ the response body string - * @param {XMLHttpRequest | respObj} response the response object (XHR or similar) + * @param {XMLHttpRequest} response */ - getResponseError (_, response) { + getResponseError(response) { let error = new Error('Upload error') if (isNetworkError(response)) { @@ -97,23 +88,76 @@ export default class XHRUpload extends BasePlugin { return error }, /** - * Check if the response from the upload endpoint indicates that the upload was successful. - * - * @param {number} status the response status code + * @param {XMLHttpRequest} response */ - validateStatus (status) { - return status >= 200 && status < 300 + validateStatus(response) { + return response.status >= 200 && response.status < 300 }, } this.opts = { ...defaultOptions, ...opts } this.i18nInit() - // Simultaneous upload limiting is shared across all uploads with this plugin. - if (internalRateLimitedQueue in this.opts) { - this.requests = this.opts[internalRateLimitedQueue] - } else { - this.requests = new RateLimitedQueue(this.opts.limit) + /** + * xhr-upload wrapper for `fetcher` to handle user options + * `validateStatus`, `getResponseError`, `getResponseData` + * and to emit `upload-progress`, `upload-error`, and `upload-success` events. + * + * @param {import('@uppy/core').UppyFile[]} files + */ + this.#uppyFetch = (files) => { + /** @type {typeof fetcher} */ + return async (url, options) => { + try { + const response = await fetcher(url, { + ...options, + onUploadProgress: (event) => { + if (event.lengthComputable) { + for (const file of files) { + this.uppy.emit('upload-progress', file, { + uploader: this, + bytesUploaded: (event.loaded / event.total) * file.size, + bytesTotal: file.size, + }) + } + } + }, + }) + + if (!this.opts.validateStatus(response)) { + throw new XhrError(response.statusText, response) + } + + const body = await this.opts.getResponseData(response) + const uploadUrl = body[this.opts.responseUrlFieldName] + + for (const file of files) { + this.uppy.emit('upload-success', file, { + status: response.status, + body, + uploadUrl, + }) + } + + return response + } catch (error) { + if (error.name === 'AbortError') { + return undefined + } + if (error instanceof XhrError) { + const { xhr } = error + const customError = buildResponseError( + xhr, + this.opts.getResponseError(xhr.responseText, xhr), + ) + for (const file of files) { + this.uppy.emit('upload-error', file, customError) + } + } + + throw error + } + } } if (this.opts.bundle && !this.opts.formData) { @@ -124,10 +168,12 @@ export default class XHRUpload extends BasePlugin { throw new Error('The `metaFields` option has been renamed to `allowedMetaFields`.') } - this.uploaderEvents = Object.create(null) + if (opts.bundle && typeof this.opts.headers === 'function') { + throw new TypeError('`headers` may not be a function when the `bundle: true` option is set') + } } - getOptions (file) { + getOptions(file) { const overrides = this.uppy.getState().xhrUpload const { headers } = this.opts @@ -213,231 +259,36 @@ export default class XHRUpload extends BasePlugin { return formPost } - async #uploadLocalFile (file, current, total) { + async #uploadLocalFile(file) { const opts = this.getOptions(file) - - this.uppy.log(`uploading ${current} of ${total}`) - return new Promise((resolve, reject) => { - const data = opts.formData - ? this.createFormDataUpload(file, opts) - : file.data - - const xhr = new XMLHttpRequest() - const eventManager = new EventManager(this.uppy) - this.uploaderEvents[file.id] = eventManager - let queuedRequest - - const timer = new ProgressTimeout(opts.timeout, () => { - const error = new Error(this.i18n('uploadStalled', { seconds: Math.ceil(opts.timeout / 1000) })) - this.uppy.emit('upload-stalled', error, [file]) - }) - - const id = nanoid() - - xhr.upload.addEventListener('loadstart', () => { - this.uppy.log(`[XHRUpload] ${id} started`) - }) - - xhr.upload.addEventListener('progress', (ev) => { - this.uppy.log(`[XHRUpload] ${id} progress: ${ev.loaded} / ${ev.total}`) - // Begin checking for timeouts when progress starts, instead of loading, - // to avoid timing out requests on browser concurrency queue - timer.progress() - - if (ev.lengthComputable) { - this.uppy.emit('upload-progress', file, { - uploader: this, - bytesUploaded: ev.loaded, - bytesTotal: ev.total, - }) - } - }) - - xhr.addEventListener('load', () => { - this.uppy.log(`[XHRUpload] ${id} finished`) - timer.done() - queuedRequest.done() - if (this.uploaderEvents[file.id]) { - this.uploaderEvents[file.id].remove() - this.uploaderEvents[file.id] = null - } - - if (opts.validateStatus(xhr.status, xhr.responseText, xhr)) { - const body = opts.getResponseData(xhr.responseText, xhr) - const uploadURL = body[opts.responseUrlFieldName] - - const uploadResp = { - status: xhr.status, - body, - uploadURL, - } - - this.uppy.emit('upload-success', file, uploadResp) - - if (uploadURL) { - this.uppy.log(`Download ${file.name} from ${uploadURL}`) - } - - return resolve(file) - } - const body = opts.getResponseData(xhr.responseText, xhr) - const error = buildResponseError(xhr, opts.getResponseError(xhr.responseText, xhr)) - - const response = { - status: xhr.status, - body, - } - - this.uppy.emit('upload-error', file, error, response) - return reject(error) - }) - - xhr.addEventListener('error', () => { - this.uppy.log(`[XHRUpload] ${id} errored`) - timer.done() - queuedRequest.done() - if (this.uploaderEvents[file.id]) { - this.uploaderEvents[file.id].remove() - this.uploaderEvents[file.id] = null - } - - const error = buildResponseError(xhr, opts.getResponseError(xhr.responseText, xhr)) - this.uppy.emit('upload-error', file, error) - return reject(error) - }) - - xhr.open(opts.method.toUpperCase(), opts.endpoint, true) - // IE10 does not allow setting `withCredentials` and `responseType` - // before `open()` is called. - xhr.withCredentials = opts.withCredentials - if (opts.responseType !== '') { - xhr.responseType = opts.responseType - } - - queuedRequest = this.requests.run(() => { - // When using an authentication system like JWT, the bearer token goes as a header. This - // header needs to be fresh each time the token is refreshed so computing and setting the - // headers just before the upload starts enables this kind of authentication to work properly. - // Otherwise, half-way through the list of uploads the token could be stale and the upload would fail. - const currentOpts = this.getOptions(file) - - Object.keys(currentOpts.headers).forEach((header) => { - xhr.setRequestHeader(header, currentOpts.headers[header]) - }) - - xhr.send(data) - - return () => { - timer.done() - xhr.abort() - } - }) - - eventManager.onFileRemove(file.id, () => { - queuedRequest.abort() - reject(new Error('File removed')) - }) - - eventManager.onCancelAll(file.id, ({ reason }) => { - if (reason === 'user') { - queuedRequest.abort() - } - reject(new Error('Upload cancelled')) + const uppyFetch = this.#uppyFetch([file]) + const body = opts.formData + ? this.createFormDataUpload(file, opts) + : file.data + + await this.uppy.queue.add(async () => { + return uppyFetch(opts.endpoint, { + method: opts.method, + headers: opts.headers, + body, }) }) - } - - #uploadBundle (files) { - return new Promise((resolve, reject) => { - const { endpoint } = this.opts - const { method } = this.opts - - const optsFromState = this.uppy.getState().xhrUpload - const formData = this.createBundledUpload(files, { - ...this.opts, - ...(optsFromState || {}), - }) - - const xhr = new XMLHttpRequest() - - const emitError = (error) => { - files.forEach((file) => { - this.uppy.emit('upload-error', file, error) - }) - } - - const timer = new ProgressTimeout(this.opts.timeout, () => { - const error = new Error(this.i18n('uploadStalled', { seconds: Math.ceil(this.opts.timeout / 1000) })) - this.uppy.emit('upload-stalled', error, files) - }) - - xhr.upload.addEventListener('loadstart', () => { - this.uppy.log('[XHRUpload] started uploading bundle') - timer.progress() - }) - - xhr.upload.addEventListener('progress', (ev) => { - timer.progress() - - if (!ev.lengthComputable) return - files.forEach((file) => { - this.uppy.emit('upload-progress', file, { - uploader: this, - bytesUploaded: (ev.loaded / ev.total) * file.size, - bytesTotal: file.size, - }) - }) - }) - - xhr.addEventListener('load', (ev) => { - timer.done() - - if (this.opts.validateStatus(ev.target.status, xhr.responseText, xhr)) { - const body = this.opts.getResponseData(xhr.responseText, xhr) - const uploadResp = { - status: ev.target.status, - body, - } - files.forEach((file) => { - this.uppy.emit('upload-success', file, uploadResp) - }) - return resolve() - } - - const error = this.opts.getResponseError(xhr.responseText, xhr) || new Error('Upload error') - error.request = xhr - emitError(error) - return reject(error) - }) - - xhr.addEventListener('error', () => { - timer.done() - - const error = this.opts.getResponseError(xhr.responseText, xhr) || new Error('Upload error') - emitError(error) - return reject(error) - }) - - this.uppy.on('cancel-all', ({ reason } = {}) => { - if (reason !== 'user') return - timer.done() - xhr.abort() - }) - - xhr.open(method.toUpperCase(), endpoint, true) - // IE10 does not allow setting `withCredentials` and `responseType` - // before `open()` is called. - xhr.withCredentials = this.opts.withCredentials - if (this.opts.responseType !== '') { - xhr.responseType = this.opts.responseType - } + return file + } - Object.keys(this.opts.headers).forEach((header) => { - xhr.setRequestHeader(header, this.opts.headers[header]) - }) + async #uploadBundle(files) { + const { endpoint, method, headers } = this.opts + const optsFromState = this.uppy.getState().xhrUpload ?? {} + const uppyFetch = this.#uppyFetch(files) + const { signal } = getUppyAbortController(this.uppy) + const body = this.createBundledUpload(files, { + ...this.opts, + ...optsFromState, + }) - xhr.send(formData) + await this.uppy.queue.add(async () => { + return uppyFetch(endpoint, { method, body, headers, signal }) }) } @@ -468,27 +319,12 @@ export default class XHRUpload extends BasePlugin { if (file.isRemote) { // INFO: the url plugin needs to use RequestClient, // while others use Provider + // TODO: would be nice if we can always use Provider with an option rather than RequestClient const Client = file.remote.providerOptions.provider ? Provider : RequestClient - const getQueue = () => this.requests - const client = new Client(this.uppy, file.remote.providerOptions, getQueue) - const controller = new AbortController() - - const removedHandler = (removedFile) => { - if (removedFile.id === file.id) controller.abort() - } - this.uppy.on('file-removed', removedHandler) - - const uploadPromise = client.uploadRemoteFile( - file, - this.#getCompanionClientArgs(file), - { signal: controller.signal }, - ) - - this.requests.wrapSyncFunction(() => { - this.uppy.off('file-removed', removedHandler) - }, { priority: -1 })() + const client = new Client(this.uppy, file.remote.providerOptions) + const reqBody = this.#getCompanionClientArgs(file) - return uploadPromise + return client.uploadRemoteFile(file, reqBody) } return this.#uploadLocalFile(file, current, total) @@ -503,7 +339,7 @@ export default class XHRUpload extends BasePlugin { // No limit configured by the user, and no RateLimitedQueue passed in by a "parent" plugin // (basically just AwsS3) using the internal symbol - if (this.opts.limit === 0 && !this.opts[internalRateLimitedQueue]) { + if (this.opts.limit === 0) { this.uppy.log( '[XHRUpload] When uploading multiple files at once, consider setting the `limit` option (to `10` for example), to limit the number of concurrent uploads, which helps prevent memory and network issues: https://uppy.io/docs/xhr-upload/#limit-0', 'warning', diff --git a/packages/@uppy/xhr-upload/src/index.test.js b/packages/@uppy/xhr-upload/src/index.test.js index 520ae19274..9e2135b3cc 100644 --- a/packages/@uppy/xhr-upload/src/index.test.js +++ b/packages/@uppy/xhr-upload/src/index.test.js @@ -50,8 +50,8 @@ describe('XHRUpload', () => { }) const core = new Core() - const validateStatus = vi.fn((status, responseText) => { - return JSON.parse(responseText).code !== 40000 + const validateStatus = vi.fn((response) => { + return JSON.parse(response.responseText).code !== 40000 }) core.use(XHRUpload, { diff --git a/yarn.lock b/yarn.lock index bda7de9512..037411c851 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9852,6 +9852,7 @@ __metadata: mime-match: ^1.0.2 namespace-emitter: ^2.0.1 nanoid: ^4.0.0 + p-queue: ^7.4.1 preact: ^10.5.13 vitest: ^0.34.5 languageName: unknown From e2c626d530e4b1d53ecc20d2854553e05d8974f0 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Thu, 19 Oct 2023 10:15:07 +0200 Subject: [PATCH 2/8] Reuse NetworkError --- packages/@uppy/utils/src/fetcher.js | 22 +++------------------- packages/@uppy/xhr-upload/src/index.js | 6 +++--- 2 files changed, 6 insertions(+), 22 deletions(-) diff --git a/packages/@uppy/utils/src/fetcher.js b/packages/@uppy/utils/src/fetcher.js index 3997f164d6..28aa97c121 100644 --- a/packages/@uppy/utils/src/fetcher.js +++ b/packages/@uppy/utils/src/fetcher.js @@ -1,22 +1,6 @@ /* eslint-disable max-len */ -export class XhrError extends Error { - /** - * Create an XHR error. - * - * @param {string} message - * Message. - * @param {XMLHttpRequest} xhr - * Message. - * @returns - * XHR error. - */ - constructor (message, xhr) { - super(message) - this.name = 'XhrError' - this.xhr = xhr - } -} +import NetworkError from './NetworkError.js' /** * Get an abort signal that will be aborted when the specified Uppy file is removed or all uploads are cancelled. @@ -108,7 +92,7 @@ export function fetcher(url, options = {}) { requestWithRetry(retryCount + 1).then(resolve, reject) }, delay(retryCount)) } else { - reject(new XhrError(xhr.statusText, xhr)); + reject(new NetworkError(xhr.statusText, xhr)); } }; @@ -118,7 +102,7 @@ export function fetcher(url, options = {}) { requestWithRetry(retryCount + 1).then(resolve, reject) }, delay(retryCount)) } else { - reject(new XhrError(xhr.statusText, xhr)); + reject(new NetworkError(xhr.statusText, xhr)); } }; diff --git a/packages/@uppy/xhr-upload/src/index.js b/packages/@uppy/xhr-upload/src/index.js index 237f0b1518..ac25702d31 100644 --- a/packages/@uppy/xhr-upload/src/index.js +++ b/packages/@uppy/xhr-upload/src/index.js @@ -3,7 +3,7 @@ import { Provider, RequestClient } from '@uppy/companion-client' import NetworkError from '@uppy/utils/lib/NetworkError' import isNetworkError from '@uppy/utils/lib/isNetworkError' import { filterNonFailedFiles, filterFilesToEmitUploadStarted } from '@uppy/utils/lib/fileFilters' -import { fetcher, XhrError, getUppyAbortController } from '@uppy/utils/lib/fetcher' +import { fetcher, getUppyAbortController } from '@uppy/utils/lib/fetcher' import packageJson from '../package.json' import locale from './locale.js' @@ -125,7 +125,7 @@ export default class XHRUpload extends BasePlugin { }) if (!this.opts.validateStatus(response)) { - throw new XhrError(response.statusText, response) + throw new NetworkError(response.statusText, response) } const body = await this.opts.getResponseData(response) @@ -144,7 +144,7 @@ export default class XHRUpload extends BasePlugin { if (error.name === 'AbortError') { return undefined } - if (error instanceof XhrError) { + if (error instanceof NetworkError) { const { xhr } = error const customError = buildResponseError( xhr, From f527b613991cf90bc684770a47b7cb40808c374b Mon Sep 17 00:00:00 2001 From: Murderlon Date: Thu, 19 Oct 2023 10:18:25 +0200 Subject: [PATCH 3/8] Format unrelated change --- private/js2ts/index.mjs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/private/js2ts/index.mjs b/private/js2ts/index.mjs index c1071d2ecc..5468367b29 100755 --- a/private/js2ts/index.mjs +++ b/private/js2ts/index.mjs @@ -32,8 +32,7 @@ const references = Object.keys(packageJSON.dependencies || {}) .map((pkg) => ({ path: `../${pkg.slice('@uppy/'.length)}` })) const depsNotYetConvertedToTS = references.filter( - (ref) => - !existsSync(new URL(`${ref.path}/tsconfig.json`, packageRoot)), + (ref) => !existsSync(new URL(`${ref.path}/tsconfig.json`, packageRoot)), ) if (depsNotYetConvertedToTS.length) { From 0655ef01c72ac54ee9cd487562ab5d4f4c81c016 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Mon, 23 Oct 2023 12:20:11 +0200 Subject: [PATCH 4/8] Make it backwards compatible --- packages/@uppy/xhr-upload/src/index.js | 33 ++++++++++++++++++-------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/packages/@uppy/xhr-upload/src/index.js b/packages/@uppy/xhr-upload/src/index.js index ac25702d31..d434d7cd08 100644 --- a/packages/@uppy/xhr-upload/src/index.js +++ b/packages/@uppy/xhr-upload/src/index.js @@ -70,15 +70,24 @@ export default class XHRUpload extends BasePlugin { withCredentials: false, responseType: '', /** - * @param {XMLHttpRequest} response + * @param {string} responseText the response body string */ - async getResponseData(response) { - return JSON.parse(response.responseText) + getResponseData (responseText) { + let parsedResponse = {} + try { + parsedResponse = JSON.parse(responseText) + } catch (err) { + uppy.log(err) + } + + return parsedResponse }, /** - * @param {XMLHttpRequest} response + * + * @param {string} _ the response body string + * @param {XMLHttpRequest | respObj} response the response object (XHR or similar) */ - getResponseError(response) { + getResponseError (_, response) { let error = new Error('Upload error') if (isNetworkError(response)) { @@ -88,16 +97,20 @@ export default class XHRUpload extends BasePlugin { return error }, /** - * @param {XMLHttpRequest} response + * Check if the response from the upload endpoint indicates that the upload was successful. + * + * @param {number} status the response status code */ - validateStatus(response) { - return response.status >= 200 && response.status < 300 + validateStatus (status) { + return status >= 200 && status < 300 }, } this.opts = { ...defaultOptions, ...opts } this.i18nInit() + this.uppy.queue.concurrency = this.opts.limit + /** * xhr-upload wrapper for `fetcher` to handle user options * `validateStatus`, `getResponseError`, `getResponseData` @@ -124,11 +137,11 @@ export default class XHRUpload extends BasePlugin { }, }) - if (!this.opts.validateStatus(response)) { + if (!this.opts.validateStatus(response.status)) { throw new NetworkError(response.statusText, response) } - const body = await this.opts.getResponseData(response) + const body = await this.opts.getResponseData(response.responseText) const uploadUrl = body[this.opts.responseUrlFieldName] for (const file of files) { From a8b298c94fdaffdaae98cf131bb190e8fab76077 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Mon, 23 Oct 2023 16:23:43 +0200 Subject: [PATCH 5/8] Full backwards compat --- packages/@uppy/utils/src/fetcher.js | 71 ++++++++++++++------- packages/@uppy/xhr-upload/src/index.js | 30 +++++---- packages/@uppy/xhr-upload/src/index.test.js | 4 +- 3 files changed, 68 insertions(+), 37 deletions(-) diff --git a/packages/@uppy/utils/src/fetcher.js b/packages/@uppy/utils/src/fetcher.js index 28aa97c121..775014bd5e 100644 --- a/packages/@uppy/utils/src/fetcher.js +++ b/packages/@uppy/utils/src/fetcher.js @@ -1,6 +1,10 @@ +/* eslint-disable no-param-reassign */ /* eslint-disable max-len */ import NetworkError from './NetworkError.js' +import ProgressTimeout from './ProgressTimeout.js' + +const noop = () => {} /** * Get an abort signal that will be aborted when the specified Uppy file is removed or all uploads are cancelled. @@ -49,6 +53,8 @@ export function getUppyAbortController(uppy, id, additionalSignal) { * The HTTP method to use for the request. * @param {string} [options.body=null] * The request payload, if any. + * @param {string} [options.timeout=30000] + * Miliseconds between XMLHttpRequest upload progress events before the request is aborted. * @param {Record} [options.headers] * An object representing any headers to send with the request. * @param {number} [options.retries=3] @@ -59,24 +65,42 @@ export function getUppyAbortController(uppy, id, additionalSignal) { * A callback function for tracking upload progress. * @param {function(XMLHttpRequest): boolean} [options.shouldRetry] * A function to determine whether to retry the request. + * @param {function(): void} [options.onTimeout] + * Called when when no XMLHttpRequest upload progress events have been received for `timeout` ms. * @param {AbortSignal} [options.signal] * signal to abort the upload. * @returns {Promise} * A Promise that resolves to the response text if the request succeeds, and rejects with an error if it fails. */ export function fetcher(url, options = {}) { - const retries = options.retries || 3; - const delay = (retryCount) => 0.3 * (2 ** (retryCount - 1)) * 1000 - const onUploadProgress = options.onUploadProgress || (() => {}); - const shouldRetry = options.shouldRetry || (() => true); + const { + body = null, + headers = {}, + method = 'GET', + onTimeout = noop, + onUploadProgress = noop, + responseType, + retries = 3, + shouldRetry = () => true, + signal = null, + timeout = 30 * 1000, + withCredentials = false, + } = options + const delay = (attempt) => 0.3 * 2 ** (attempt - 1) * 1000 + const timer = new ProgressTimeout(timeout, onTimeout) function requestWithRetry(retryCount = 0) { return new Promise((resolve, reject) => { - const xhr = new XMLHttpRequest(); - xhr.open(options.method || 'GET', url, true); + const xhr = new XMLHttpRequest() + + xhr.open(method || 'GET', url, true) + xhr.withCredentials = withCredentials + if (responseType !== '') { + xhr.responseType = responseType + } - if (options.signal) { - options.signal.addEventListener('abort', () => { + if (signal) { + signal.addEventListener('abort', () => { xhr.abort() // Using DOMException for abort errors aligns with // the convention established by the Fetch API. @@ -86,15 +110,17 @@ export function fetcher(url, options = {}) { xhr.onload = () => { if (xhr.status >= 200 && xhr.status < 300) { + timer.done() resolve(xhr) } else if (shouldRetry(xhr) && retryCount < retries) { setTimeout(() => { requestWithRetry(retryCount + 1).then(resolve, reject) }, delay(retryCount)) } else { - reject(new NetworkError(xhr.statusText, xhr)); + timer.done() + reject(new NetworkError(xhr.statusText, xhr)) } - }; + } xhr.onerror = () => { if (shouldRetry(xhr) && retryCount < retries) { @@ -102,24 +128,25 @@ export function fetcher(url, options = {}) { requestWithRetry(retryCount + 1).then(resolve, reject) }, delay(retryCount)) } else { - reject(new NetworkError(xhr.statusText, xhr)); + timer.done() + reject(new NetworkError(xhr.statusText, xhr)) } - }; + } xhr.upload.onprogress = (event) => { - onUploadProgress(event); - }; + timer.progress() + onUploadProgress(event) + } - if (options.headers) { - Object.keys(options.headers).forEach(key => { - xhr.setRequestHeader(key, options.headers[key]); - }); + if (headers) { + Object.keys(headers).forEach((key) => { + xhr.setRequestHeader(key, headers[key]) + }) } - xhr.send(options.body); - }); + xhr.send(body) + }) } - return requestWithRetry(); + return requestWithRetry() } - diff --git a/packages/@uppy/xhr-upload/src/index.js b/packages/@uppy/xhr-upload/src/index.js index d434d7cd08..2a8ead0997 100644 --- a/packages/@uppy/xhr-upload/src/index.js +++ b/packages/@uppy/xhr-upload/src/index.js @@ -67,6 +67,7 @@ export default class XHRUpload extends BasePlugin { headers: {}, timeout: 30 * 1000, limit: 5, + // TODO: remove in next major withCredentials: false, responseType: '', /** @@ -99,9 +100,11 @@ export default class XHRUpload extends BasePlugin { /** * Check if the response from the upload endpoint indicates that the upload was successful. * - * @param {number} status the response status code + * @param {number} status + * @param {string} responseText */ - validateStatus (status) { + // eslint-disable-next-line no-unused-vars + validateStatus (status, responseText) { return status >= 200 && status < 300 }, } @@ -124,6 +127,11 @@ export default class XHRUpload extends BasePlugin { try { const response = await fetcher(url, { ...options, + onTimeout: () => { + const seconds = Math.ceil(options.timeout / 1000) + const error = new Error(this.i18n('uploadStalled', { seconds })) + this.uppy.emit('upload-stalled', error, files) + }, onUploadProgress: (event) => { if (event.lengthComputable) { for (const file of files) { @@ -137,7 +145,7 @@ export default class XHRUpload extends BasePlugin { }, }) - if (!this.opts.validateStatus(response.status)) { + if (!this.opts.validateStatus(response.status, response.responseText)) { throw new NetworkError(response.statusText, response) } @@ -158,10 +166,10 @@ export default class XHRUpload extends BasePlugin { return undefined } if (error instanceof NetworkError) { - const { xhr } = error + const { request } = error const customError = buildResponseError( - xhr, - this.opts.getResponseError(xhr.responseText, xhr), + request, + this.opts.getResponseError(request.responseText, request), ) for (const file of files) { this.uppy.emit('upload-error', file, customError) @@ -280,18 +288,14 @@ export default class XHRUpload extends BasePlugin { : file.data await this.uppy.queue.add(async () => { - return uppyFetch(opts.endpoint, { - method: opts.method, - headers: opts.headers, - body, - }) + return uppyFetch(opts.endpoint, { ...opts, body }) }) return file } async #uploadBundle(files) { - const { endpoint, method, headers } = this.opts + const { endpoint } = this.opts const optsFromState = this.uppy.getState().xhrUpload ?? {} const uppyFetch = this.#uppyFetch(files) const { signal } = getUppyAbortController(this.uppy) @@ -301,7 +305,7 @@ export default class XHRUpload extends BasePlugin { }) await this.uppy.queue.add(async () => { - return uppyFetch(endpoint, { method, body, headers, signal }) + return uppyFetch(endpoint, { ...this.opts, body, signal }) }) } diff --git a/packages/@uppy/xhr-upload/src/index.test.js b/packages/@uppy/xhr-upload/src/index.test.js index 9e2135b3cc..7cf60b7539 100644 --- a/packages/@uppy/xhr-upload/src/index.test.js +++ b/packages/@uppy/xhr-upload/src/index.test.js @@ -50,8 +50,8 @@ describe('XHRUpload', () => { }) const core = new Core() - const validateStatus = vi.fn((response) => { - return JSON.parse(response.responseText).code !== 40000 + const validateStatus = vi.fn(() => { + return false }) core.use(XHRUpload, { From 48363401fc478a80e27b81fd442c2d0f07d28812 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Mon, 23 Oct 2023 16:25:30 +0200 Subject: [PATCH 6/8] fixup! Full backwards compat --- packages/@uppy/utils/src/fetcher.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/@uppy/utils/src/fetcher.js b/packages/@uppy/utils/src/fetcher.js index 775014bd5e..7446b9a62c 100644 --- a/packages/@uppy/utils/src/fetcher.js +++ b/packages/@uppy/utils/src/fetcher.js @@ -55,6 +55,10 @@ export function getUppyAbortController(uppy, id, additionalSignal) { * The request payload, if any. * @param {string} [options.timeout=30000] * Miliseconds between XMLHttpRequest upload progress events before the request is aborted. + * @param {string} [options.withCredentials=false] + * Sets the withCredentials property of the XMLHttpRequest object. + * @param {string} [options.responseType=''] + * Sets the responseType property of the XMLHttpRequest object. * @param {Record} [options.headers] * An object representing any headers to send with the request. * @param {number} [options.retries=3] From 9aeae6c321f31876f317ca11ebad1f596ba597cd Mon Sep 17 00:00:00 2001 From: Murderlon Date: Mon, 23 Oct 2023 16:31:34 +0200 Subject: [PATCH 7/8] fixup! fixup! Full backwards compat --- packages/@uppy/utils/src/fetcher.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@uppy/utils/src/fetcher.js b/packages/@uppy/utils/src/fetcher.js index 7446b9a62c..b6d1b62ffc 100644 --- a/packages/@uppy/utils/src/fetcher.js +++ b/packages/@uppy/utils/src/fetcher.js @@ -99,7 +99,7 @@ export function fetcher(url, options = {}) { xhr.open(method || 'GET', url, true) xhr.withCredentials = withCredentials - if (responseType !== '') { + if (responseType) { xhr.responseType = responseType } From 92995e847ebbed790c63032f56ebed660e2099c5 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Mon, 23 Oct 2023 16:35:49 +0200 Subject: [PATCH 8/8] Remove todo comment --- packages/@uppy/xhr-upload/src/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/@uppy/xhr-upload/src/index.js b/packages/@uppy/xhr-upload/src/index.js index 2a8ead0997..3c1991daf4 100644 --- a/packages/@uppy/xhr-upload/src/index.js +++ b/packages/@uppy/xhr-upload/src/index.js @@ -67,7 +67,6 @@ export default class XHRUpload extends BasePlugin { headers: {}, timeout: 30 * 1000, limit: 5, - // TODO: remove in next major withCredentials: false, responseType: '', /**