Skip to content

Commit

Permalink
@uppy/xhr-upload: fix stale file references in events (#5499)
Browse files Browse the repository at this point in the history
  • Loading branch information
Murderlon authored Oct 31, 2024
1 parent 3bfa87a commit ba3a3ad
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 5 deletions.
54 changes: 53 additions & 1 deletion packages/@uppy/xhr-upload/src/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { vi, describe, it, expect } from 'vitest'
import nock from 'nock'
import Core from '@uppy/core'
import Core, { type UppyEventMap } from '@uppy/core'
import XHRUpload from './index.ts'

describe('XHRUpload', () => {
Expand Down Expand Up @@ -61,6 +61,58 @@ describe('XHRUpload', () => {
})
})

it('should send response object over upload-error event', async () => {
nock('https://fake-endpoint.uppy.io')
.defaultReplyHeaders({
'access-control-allow-method': 'POST',
'access-control-allow-origin': '*',
})
.options('/')
.reply(204, {})
.post('/')
.reply(400, { status: 400, message: 'Oh no' })

const core = new Core()
const shouldRetry = vi.fn(() => false)

core.use(XHRUpload, {
id: 'XHRUpload',
endpoint: 'https://fake-endpoint.uppy.io',
shouldRetry,
})
const id = core.addFile({
type: 'image/png',
source: 'test',
name: 'test.jpg',
data: new Blob([new Uint8Array(8192)]),
})

const event = new Promise<
Parameters<UppyEventMap<any, any>['upload-error']>
>((resolve) => {
core.once('upload-error', (...args) => resolve(args))
})

await Promise.all([
core.upload(),
event.then(([file, , response]) => {
const newFile = core.getFile(id)
// error and response are set inside upload-error in core.
// When we subscribe to upload-error it is emitted before
// these properties are set in core. Ideally we'd have an internal
// emitter which calls an external one but it is what it is.
delete newFile.error
delete newFile.response
// This is still useful to test because other properties
// might have changed in the meantime
expect(file).toEqual(newFile)
expect(response).toBeInstanceOf(XMLHttpRequest)
}),
])

expect(shouldRetry).toHaveBeenCalledTimes(1)
})

describe('headers', () => {
it('can be a function', async () => {
const scope = nock('https://fake-endpoint.uppy.io').defaultReplyHeaders({
Expand Down
9 changes: 5 additions & 4 deletions packages/@uppy/xhr-upload/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,8 @@ export default class XHRUpload<
},
onUploadProgress: (event) => {
if (event.lengthComputable) {
for (const file of files) {
for (const { id } of files) {
const file = this.uppy.getFile(id)
this.uppy.emit('upload-progress', file, {
uploadStarted: file.progress.uploadStarted ?? 0,
bytesUploaded: (event.loaded / event.total) * file.size!,
Expand All @@ -241,8 +242,8 @@ export default class XHRUpload<

const uploadURL = typeof body?.url === 'string' ? body.url : undefined

for (const file of files) {
this.uppy.emit('upload-success', file, {
for (const { id } of files) {
this.uppy.emit('upload-success', this.uppy.getFile(id), {
status: res.status,
body,
uploadURL,
Expand All @@ -260,7 +261,7 @@ export default class XHRUpload<
for (const file of files) {
this.uppy.emit(
'upload-error',
file,
this.uppy.getFile(file.id),
buildResponseError(request, error),
request,
)
Expand Down

0 comments on commit ba3a3ad

Please sign in to comment.