From ba3a3ad8b582c621bab395ccb444a5444036f1aa Mon Sep 17 00:00:00 2001
From: Merlijn Vos <merlijn@soverin.net>
Date: Thu, 31 Oct 2024 10:16:17 +0100
Subject: [PATCH] @uppy/xhr-upload: fix stale file references in events (#5499)

---
 packages/@uppy/xhr-upload/src/index.test.ts | 54 ++++++++++++++++++++-
 packages/@uppy/xhr-upload/src/index.ts      |  9 ++--
 2 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/packages/@uppy/xhr-upload/src/index.test.ts b/packages/@uppy/xhr-upload/src/index.test.ts
index 19cb8b6059..127444b339 100644
--- a/packages/@uppy/xhr-upload/src/index.test.ts
+++ b/packages/@uppy/xhr-upload/src/index.test.ts
@@ -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', () => {
@@ -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({
diff --git a/packages/@uppy/xhr-upload/src/index.ts b/packages/@uppy/xhr-upload/src/index.ts
index 1ad2ed7755..dcab608307 100644
--- a/packages/@uppy/xhr-upload/src/index.ts
+++ b/packages/@uppy/xhr-upload/src/index.ts
@@ -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!,
@@ -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,
@@ -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,
               )