diff --git a/.changeset/little-clouds-call.md b/.changeset/little-clouds-call.md new file mode 100644 index 000000000..833cf647a --- /dev/null +++ b/.changeset/little-clouds-call.md @@ -0,0 +1,5 @@ +--- +"@uppy/tus": major +--- + +@uppy/tus: don't abort the request on error, so the server response (status + body) is forwarded to the `upload-error` event and `file.response` instead of being reset to status `0`. diff --git a/packages/@uppy/tus/src/index.test.ts b/packages/@uppy/tus/src/index.test.ts index 206dffefd..1a46170e4 100644 --- a/packages/@uppy/tus/src/index.test.ts +++ b/packages/@uppy/tus/src/index.test.ts @@ -1,7 +1,51 @@ -import Core from '@uppy/core' -import { describe, expect, expectTypeOf, it } from 'vitest' +import Core, { type UppyEventMap } from '@uppy/core' +import { describe, expect, expectTypeOf, it, vi } from 'vitest' import Tus, { type TusBody } from './index.js' +// Shared fake XHR object — must be declared via vi.hoisted so it's available +// inside the vi.mock factory (which is hoisted before imports). +const { fakeXhr } = vi.hoisted(() => ({ + fakeXhr: { + status: 403, + responseText: JSON.stringify({ + message: 'File cannot be uploaded as the BIN content type is disallowed!', + status_code: 403, + }), + }, +})) + +// Mock tus-js-client so the upload-error test never touches the network. +// The mock Upload fires onError immediately with a fake DetailedError. +vi.mock('tus-js-client', async (importOriginal) => { + const actual = await importOriginal() + class MockUpload { + private options: Record + + constructor(_file: any, options: Record) { + this.options = options + } + + start() { + const err = Object.assign(new Error('tus: server responded with 403'), { + originalResponse: { + getStatus: () => 403, + getUnderlyingObject: () => fakeXhr, + }, + originalRequest: null, + }) + setTimeout(() => this.options.onError(err), 0) + } + + abort() {} + + // ponytail: tus calls this before start(); return empty so no resume logic runs + findPreviousUploads() { + return Promise.resolve([]) + } + } + return { ...actual, Upload: MockUpload } +}) + describe('Tus', () => { it('Throws errors if autoRetry option is true', () => { const uppy = new Core() @@ -44,4 +88,40 @@ describe('Tus', () => { { xhr: XMLHttpRequest } | undefined >() }) + + describe('upload-error response', () => { + it('sends the server response over the upload-error event', async () => { + const core = new Core() + core.use(Tus, { + endpoint: 'https://fake-endpoint.uppy.io/files/', + retryDelays: [], + }) + const id = core.addFile({ + type: 'application/octet-stream', + source: 'test', + name: 'test.bin', + data: new Blob([new Uint8Array(1024)]), + }) + + const event = new Promise< + Parameters['upload-error']> + >((resolve) => { + core.once('upload-error', (...args) => resolve(args)) + }) + + await Promise.all([ + core.upload().catch(() => { + // Core rejects the upload; we assert on the event/state instead. + }), + event.then(([, , response]) => { + expect(response?.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) + }) + }) }) diff --git a/packages/@uppy/tus/src/index.ts b/packages/@uppy/tus/src/index.ts index 2929cb76e..65b4bb196 100644 --- a/packages/@uppy/tus/src/index.ts +++ b/packages/@uppy/tus/src/index.ts @@ -164,13 +164,28 @@ export default class Tus extends BasePlugin< * 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?: { abort: boolean }): void { + 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) { - uploader.abort() + if (opts?.abortRequest !== false) { + uploader.abort() - if (opts?.abort) { - uploader.abort(true) + if (opts?.abort) { + uploader.abort(true) + } } this.uploaders[fileID] = null @@ -219,6 +234,12 @@ export default class Tus extends BasePlugin< ): Promise { 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. + let errorResponse: + | Omit['response']>, 'uploadURL'> + | undefined + // Create a new tus upload return new Promise((resolve, reject) => { let queuedRequest: ReturnType @@ -291,6 +312,23 @@ export default class Tus extends BasePlugin< uploadOptions.onError = (err) => { this.uppy.log(err) + // tus-js-client only calls `onError` once it has given up retrying, so + // the request has already completed. Capture the server response (status + // + body) and forward it to the `upload-error` event and `file.response`, + // mirroring the shape emitted by `onSuccess`. + const originalResponse = (err as tus.DetailedError).originalResponse + if (originalResponse != null) { + errorResponse = { + status: originalResponse.getStatus(), + body: { + // We have to put `as XMLHttpRequest` because tus-js-client + // returns `any`, as the type differs in Node.js and the browser. + // In the browser it's always `XMLHttpRequest`. + xhr: originalResponse.getUnderlyingObject() as XMLHttpRequest, + } as unknown as B, + } + } + const xhr = (err as tus.DetailedError).originalRequest != null ? (err as tus.DetailedError).originalRequest.getUnderlyingObject() @@ -299,7 +337,11 @@ export default class Tus extends BasePlugin< err = new NetworkError(err, xhr) } - this.resetUploaderReferences(file.id) + // Do not abort the request here: it has already completed, and aborting + // 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 }) queuedRequest?.abort() if (typeof opts.onError === 'function') { @@ -516,7 +558,10 @@ export default class Tus extends BasePlugin< queuedRequest = this.requests.run(qRequest) }) }).catch((err) => { - this.uppy.emit('upload-error', file, err) + // `errorResponse` is captured in the `onError` handler above (the request + // is intentionally not aborted there), so the server response is still + // available here to forward to the `upload-error` event. + this.uppy.emit('upload-error', file, err, errorResponse) throw err }) }