diff --git a/packages/@uppy/tus/src/index.test.ts b/packages/@uppy/tus/src/index.test.ts index 1a46170e4..12c69ae58 100644 --- a/packages/@uppy/tus/src/index.test.ts +++ b/packages/@uppy/tus/src/index.test.ts @@ -36,7 +36,12 @@ vi.mock('tus-js-client', async (importOriginal) => { setTimeout(() => this.options.onError(err), 0) } - abort() {} + abort() { + // Mirrors XMLHttpRequest.abort(): the response object is reset. This + // makes the test fail if Tus aborts the completed errored request. + fakeXhr.status = 0 + fakeXhr.responseText = '' + } // ponytail: tus calls this before start(); return empty so no resume logic runs findPreviousUploads() { @@ -115,13 +120,19 @@ describe('Tus', () => { }), event.then(([, , response]) => { expect(response?.status).toBe(403) + expect(response?.body?.xhr.status).toBe(403) expect(JSON.parse(response!.body!.xhr.responseText).message).toBe( 'File cannot be uploaded as the BIN content type is disallowed!', ) }), ]) - expect(core.getFile(id).response?.status).toBe(403) + const file = core.getFile(id) + expect(file.response?.status).toBe(403) + expect(file.response?.body?.xhr.status).toBe(403) + expect(JSON.parse(file.response!.body!.xhr.responseText).message).toBe( + 'File cannot be uploaded as the BIN content type is disallowed!', + ) }) }) }) diff --git a/packages/@uppy/tus/src/index.ts b/packages/@uppy/tus/src/index.ts index 65b4bb196..114c2d22f 100644 --- a/packages/@uppy/tus/src/index.ts +++ b/packages/@uppy/tus/src/index.ts @@ -160,34 +160,25 @@ export default class Tus extends BasePlugin< this.uploaderEvents = Object.create(null) } + /** + * Stop the upload in Tus. If `terminate` is `true`, it will also terminate the + * upload on the Tus server by sending a `DELETE` request. If not, it will just + * cancel any current upload request and leave the upload in a half-uploaded state. + * + * @param fileID + * @param terminate Whether to terminate the upload on the server. + */ + #abortUploader(fileID: string, terminate?: boolean) { + const uploader = this.uploaders[fileID] + uploader?.abort(terminate) + } + /** * Clean up all references for a file's upload: the tus.Upload instance, * any events related to the file, and the Companion WebSocket connection. */ - resetUploaderReferences( - fileID: string, - opts?: { - /** Terminate the upload on the server (sends a `DELETE` request). */ - abort?: boolean - /** - * Abort the underlying request. Defaults to `true`. Set to `false` when - * the request has already completed (e.g. in the error handler), so the - * underlying `xhr` — and thus the server response — is preserved instead - * of being reset by `abort()`. - */ - abortRequest?: boolean - }, - ): void { - const uploader = this.uploaders[fileID] - if (uploader) { - if (opts?.abortRequest !== false) { - uploader.abort() - - if (opts?.abort) { - uploader.abort(true) - } - } - + #resetUploaderReferences(fileID: string): void { + if (this.uploaders[fileID]) { this.uploaders[fileID] = null } if (this.uploaderEvents[fileID]) { @@ -218,7 +209,7 @@ export default class Tus extends BasePlugin< * * When working on this function, keep in mind: * - When an upload is completed or cancelled for any reason, the tus.Upload and EventManager instances need to be cleaned - * up using this.resetUploaderReferences(). + * up using this.#resetUploaderReferences() and this.#abortUploader(). * - When an upload is cancelled or paused, for any reason, it needs to be removed from the `this.requests` queue using * `queuedRequest.abort()`. * - When an upload is completed for any reason, including errors, it needs to be marked as such using @@ -232,7 +223,8 @@ export default class Tus extends BasePlugin< async #uploadLocalFile( file: LocalUppyFile, ): Promise { - this.resetUploaderReferences(file.id) + this.#abortUploader(file.id) + this.#resetUploaderReferences(file.id) // Captured in `onError` and forwarded to the `upload-error` event in the // `.catch` below, so consumers can read the failing server response. @@ -341,7 +333,7 @@ export default class Tus extends BasePlugin< // it would reset the underlying `xhr` (status `0`, empty body) and // discard the response we just captured. We still drop our references // and remove the event listeners. - this.resetUploaderReferences(file.id, { abortRequest: false }) + this.#resetUploaderReferences(file.id) queuedRequest?.abort() if (typeof opts.onError === 'function') { @@ -380,7 +372,8 @@ export default class Tus extends BasePlugin< this.uppy.emit('upload-success', this.uppy.getFile(file.id), uploadResp) - this.resetUploaderReferences(file.id) + this.#abortUploader(file.id) + this.#resetUploaderReferences(file.id) queuedRequest.done() if (upload.url) { @@ -502,7 +495,7 @@ export default class Tus extends BasePlugin< upload.start() } // Don't do anything here, the caller will take care of cancelling the upload itself - // using resetUploaderReferences(). This is because resetUploaderReferences() has to be + // 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 @@ -523,7 +516,8 @@ export default class Tus extends BasePlugin< eventManager.onFileRemove(file.id, (targetFileID) => { queuedRequest.abort() - this.resetUploaderReferences(file.id, { abort: !!upload.url }) + this.#abortUploader(file.id, !!upload.url) + this.#resetUploaderReferences(file.id) resolve(`upload ${targetFileID} was removed`) }) @@ -546,7 +540,8 @@ export default class Tus extends BasePlugin< eventManager.onCancelAll(file.id, () => { queuedRequest.abort() - this.resetUploaderReferences(file.id, { abort: !!upload.url }) + this.#abortUploader(file.id, !!upload.url) + this.#resetUploaderReferences(file.id) resolve(`upload ${file.id} was canceled`) })