Skip to content

Commit cef447b

Browse files
authored
Merge branch 'main' into angular-v21-support-attempt-2
2 parents 0fff6a5 + 08bd2ef commit cef447b

3 files changed

Lines changed: 138 additions & 8 deletions

File tree

.changeset/little-clouds-call.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@uppy/tus": major
3+
---
4+
5+
@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`.

packages/@uppy/tus/src/index.test.ts

Lines changed: 82 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,51 @@
1-
import Core from '@uppy/core'
2-
import { describe, expect, expectTypeOf, it } from 'vitest'
1+
import Core, { type UppyEventMap } from '@uppy/core'
2+
import { describe, expect, expectTypeOf, it, vi } from 'vitest'
33
import Tus, { type TusBody } from './index.js'
44

5+
// Shared fake XHR object — must be declared via vi.hoisted so it's available
6+
// inside the vi.mock factory (which is hoisted before imports).
7+
const { fakeXhr } = vi.hoisted(() => ({
8+
fakeXhr: {
9+
status: 403,
10+
responseText: JSON.stringify({
11+
message: 'File cannot be uploaded as the BIN content type is disallowed!',
12+
status_code: 403,
13+
}),
14+
},
15+
}))
16+
17+
// Mock tus-js-client so the upload-error test never touches the network.
18+
// The mock Upload fires onError immediately with a fake DetailedError.
19+
vi.mock('tus-js-client', async (importOriginal) => {
20+
const actual = await importOriginal<typeof import('tus-js-client')>()
21+
class MockUpload {
22+
private options: Record<string, any>
23+
24+
constructor(_file: any, options: Record<string, any>) {
25+
this.options = options
26+
}
27+
28+
start() {
29+
const err = Object.assign(new Error('tus: server responded with 403'), {
30+
originalResponse: {
31+
getStatus: () => 403,
32+
getUnderlyingObject: () => fakeXhr,
33+
},
34+
originalRequest: null,
35+
})
36+
setTimeout(() => this.options.onError(err), 0)
37+
}
38+
39+
abort() {}
40+
41+
// ponytail: tus calls this before start(); return empty so no resume logic runs
42+
findPreviousUploads() {
43+
return Promise.resolve([])
44+
}
45+
}
46+
return { ...actual, Upload: MockUpload }
47+
})
48+
549
describe('Tus', () => {
650
it('Throws errors if autoRetry option is true', () => {
751
const uppy = new Core()
@@ -44,4 +88,40 @@ describe('Tus', () => {
4488
{ xhr: XMLHttpRequest } | undefined
4589
>()
4690
})
91+
92+
describe('upload-error response', () => {
93+
it('sends the server response over the upload-error event', async () => {
94+
const core = new Core<any, TusBody>()
95+
core.use(Tus, {
96+
endpoint: 'https://fake-endpoint.uppy.io/files/',
97+
retryDelays: [],
98+
})
99+
const id = core.addFile({
100+
type: 'application/octet-stream',
101+
source: 'test',
102+
name: 'test.bin',
103+
data: new Blob([new Uint8Array(1024)]),
104+
})
105+
106+
const event = new Promise<
107+
Parameters<UppyEventMap<any, TusBody>['upload-error']>
108+
>((resolve) => {
109+
core.once('upload-error', (...args) => resolve(args))
110+
})
111+
112+
await Promise.all([
113+
core.upload().catch(() => {
114+
// Core rejects the upload; we assert on the event/state instead.
115+
}),
116+
event.then(([, , response]) => {
117+
expect(response?.status).toBe(403)
118+
expect(JSON.parse(response!.body!.xhr.responseText).message).toBe(
119+
'File cannot be uploaded as the BIN content type is disallowed!',
120+
)
121+
}),
122+
])
123+
124+
expect(core.getFile(id).response?.status).toBe(403)
125+
})
126+
})
47127
})

packages/@uppy/tus/src/index.ts

Lines changed: 51 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -164,13 +164,28 @@ export default class Tus<M extends Meta, B extends Body> extends BasePlugin<
164164
* Clean up all references for a file's upload: the tus.Upload instance,
165165
* any events related to the file, and the Companion WebSocket connection.
166166
*/
167-
resetUploaderReferences(fileID: string, opts?: { abort: boolean }): void {
167+
resetUploaderReferences(
168+
fileID: string,
169+
opts?: {
170+
/** Terminate the upload on the server (sends a `DELETE` request). */
171+
abort?: boolean
172+
/**
173+
* Abort the underlying request. Defaults to `true`. Set to `false` when
174+
* the request has already completed (e.g. in the error handler), so the
175+
* underlying `xhr` — and thus the server response — is preserved instead
176+
* of being reset by `abort()`.
177+
*/
178+
abortRequest?: boolean
179+
},
180+
): void {
168181
const uploader = this.uploaders[fileID]
169182
if (uploader) {
170-
uploader.abort()
183+
if (opts?.abortRequest !== false) {
184+
uploader.abort()
171185

172-
if (opts?.abort) {
173-
uploader.abort(true)
186+
if (opts?.abort) {
187+
uploader.abort(true)
188+
}
174189
}
175190

176191
this.uploaders[fileID] = null
@@ -219,6 +234,12 @@ export default class Tus<M extends Meta, B extends Body> extends BasePlugin<
219234
): Promise<tus.Upload | string> {
220235
this.resetUploaderReferences(file.id)
221236

237+
// Captured in `onError` and forwarded to the `upload-error` event in the
238+
// `.catch` below, so consumers can read the failing server response.
239+
let errorResponse:
240+
| Omit<NonNullable<UppyFile<M, B>['response']>, 'uploadURL'>
241+
| undefined
242+
222243
// Create a new tus upload
223244
return new Promise<tus.Upload | string>((resolve, reject) => {
224245
let queuedRequest: ReturnType<RateLimitedQueue['run']>
@@ -291,6 +312,23 @@ export default class Tus<M extends Meta, B extends Body> extends BasePlugin<
291312
uploadOptions.onError = (err) => {
292313
this.uppy.log(err)
293314

315+
// tus-js-client only calls `onError` once it has given up retrying, so
316+
// the request has already completed. Capture the server response (status
317+
// + body) and forward it to the `upload-error` event and `file.response`,
318+
// mirroring the shape emitted by `onSuccess`.
319+
const originalResponse = (err as tus.DetailedError).originalResponse
320+
if (originalResponse != null) {
321+
errorResponse = {
322+
status: originalResponse.getStatus(),
323+
body: {
324+
// We have to put `as XMLHttpRequest` because tus-js-client
325+
// returns `any`, as the type differs in Node.js and the browser.
326+
// In the browser it's always `XMLHttpRequest`.
327+
xhr: originalResponse.getUnderlyingObject() as XMLHttpRequest,
328+
} as unknown as B,
329+
}
330+
}
331+
294332
const xhr =
295333
(err as tus.DetailedError).originalRequest != null
296334
? (err as tus.DetailedError).originalRequest.getUnderlyingObject()
@@ -299,7 +337,11 @@ export default class Tus<M extends Meta, B extends Body> extends BasePlugin<
299337
err = new NetworkError(err, xhr)
300338
}
301339

302-
this.resetUploaderReferences(file.id)
340+
// Do not abort the request here: it has already completed, and aborting
341+
// it would reset the underlying `xhr` (status `0`, empty body) and
342+
// discard the response we just captured. We still drop our references
343+
// and remove the event listeners.
344+
this.resetUploaderReferences(file.id, { abortRequest: false })
303345
queuedRequest?.abort()
304346

305347
if (typeof opts.onError === 'function') {
@@ -516,7 +558,10 @@ export default class Tus<M extends Meta, B extends Body> extends BasePlugin<
516558
queuedRequest = this.requests.run(qRequest)
517559
})
518560
}).catch((err) => {
519-
this.uppy.emit('upload-error', file, err)
561+
// `errorResponse` is captured in the `onError` handler above (the request
562+
// is intentionally not aborted there), so the server response is still
563+
// available here to forward to the `upload-error` event.
564+
this.uppy.emit('upload-error', file, err, errorResponse)
520565
throw err
521566
})
522567
}

0 commit comments

Comments
 (0)