@uppy/tus: don't abort errored request (#6312)

this fixes #6287 , another similar attempted fix which didn't got merged
: #5683

in #5683 we discussed a more longer term solution but IMO that would be
a breaking change as it would change the event contract

AI Disclaimer : AI Used 

This fix is Manually Tested across **Chrome and Firefox**
This commit is contained in:
Prakash 2026-05-29 18:42:58 +05:30 committed by GitHub
parent 5519b84409
commit 51acfe9e6c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 138 additions and 8 deletions

View file

@ -44,6 +44,7 @@
"devDependencies": {
"@uppy/core": "workspace:^",
"jsdom": "^26.1.0",
"nock": "^13.1.0",
"typescript": "^5.8.3",
"vitest": "^3.2.4"
},

View file

@ -1,5 +1,6 @@
import Core from '@uppy/core'
import { describe, expect, expectTypeOf, it } from 'vitest'
import Core, { type UppyEventMap } from '@uppy/core'
import nock from 'nock'
import { afterEach, describe, expect, expectTypeOf, it } from 'vitest'
import Tus, { type TusBody } from './index.js'
describe('Tus', () => {
@ -44,4 +45,61 @@ describe('Tus', () => {
{ xhr: XMLHttpRequest } | undefined
>()
})
describe('upload-error response', () => {
afterEach(() => {
nock.cleanAll()
})
it('sends the server response over the upload-error event', async () => {
nock('https://fake-endpoint.uppy.io')
.post('/files/')
.reply(
403,
JSON.stringify({
message:
'File cannot be uploaded as the BIN content type is disallowed!',
status_code: 403,
}),
{ 'Content-Type': 'application/json' },
)
const core = new Core<any, TusBody>()
core.use(Tus, {
endpoint: 'https://fake-endpoint.uppy.io/files/',
// Avoid retrying so the failure surfaces immediately.
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<UppyEventMap<any, TusBody>['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)
const { xhr } = response!.body!
expect(xhr).toBeInstanceOf(XMLHttpRequest)
expect(JSON.parse(xhr.responseText).message).toBe(
'File cannot be uploaded as the BIN content type is disallowed!',
)
}),
])
// The response is also persisted on the file so it is available on the
// `complete` result and via `getFile`.
expect(core.getFile(id).response?.status).toBe(403)
})
})
})

View file

@ -164,13 +164,28 @@ export default class Tus<M extends Meta, B extends Body> 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<M extends Meta, B extends Body> extends BasePlugin<
): Promise<tus.Upload | string> {
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<NonNullable<UppyFile<M, B>['response']>, 'uploadURL'>
| undefined
// Create a new tus upload
return new Promise<tus.Upload | string>((resolve, reject) => {
let queuedRequest: ReturnType<RateLimitedQueue['run']>
@ -291,6 +312,23 @@ export default class Tus<M extends Meta, B extends Body> 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<M extends Meta, B extends Body> 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<M extends Meta, B extends Body> 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
})
}

View file

@ -0,0 +1,25 @@
import { defineConfig } from 'vitest/config'
// In the browser (where Uppy runs) tus-js-client uses its browser build, which
// accepts `Blob`/`File` and `XMLHttpRequest`. Under the default Node resolution
// Vitest would load the Node build, which only accepts `Buffer`/`Readable`.
// Alias to the browser build so jsdom tests exercise the same code path as
// production.
export default defineConfig({
resolve: {
alias: {
'tus-js-client': 'tus-js-client/lib.esm/browser/index.js',
},
},
test: {
environment: 'jsdom',
// Run with the document origin set to the upload endpoint so requests in
// tests are same-origin. Otherwise jsdom treats them as cross-origin and
// hides the response status (`xhr.status === 0`).
environmentOptions: {
jsdom: {
url: 'https://fake-endpoint.uppy.io',
},
},
},
})

View file

@ -9089,6 +9089,7 @@ __metadata:
"@uppy/core": "workspace:^"
"@uppy/utils": "workspace:^"
jsdom: "npm:^26.1.0"
nock: "npm:^13.1.0"
tus-js-client: "npm:^4.2.3"
typescript: "npm:^5.8.3"
vitest: "npm:^3.2.4"