@uppy/core: make upload() idempotent

packages/@uppy/core/src/Uppy.test.ts

@@ -821,27 +821,6 @@ describe('src/Core', () => {
         uploadStarted: null,
       })
     })
-
-    it('should report an error if post-processing a file fails', () => {
-      const core = new Core()
-
-      core.addFile({
-        source: 'vi',
-        name: 'foo.jpg',
-        type: 'image/jpeg',
-        data: testImage,
-      })
-
-      const fileId = Object.keys(core.getState().files)[0]
-      const file = core.getFile(fileId)
-      core.emit('error', new Error('foooooo'), file)
-
-      expect(core.getState().error).toEqual('foooooo')
-
-      expect(core.upload()).resolves.toMatchObject({
-        failed: [{ name: 'foo.jpg' }],
-      })
-    })
   })
 
   describe('uploaders', () => {
@@ -1357,6 +1336,74 @@ describe('src/Core', () => {
       })
       await expect(core.upload()).resolves.toBeDefined()
     })
+
+    it('upload() is idempotent when called multiple times', async () => {
+      const onUpload = vi.fn()
+      const onRetryAll = vi.fn()
+      const onUploadError = vi.fn()
+      const core = new Core()
+      let hasError = false
+
+      core.addUploader((fileIDs) => {
+        fileIDs.forEach((fileID) => {
+          const file = core.getFile(fileID)
+          if (!hasError) {
+            // @ts-ignore
+            core.emit('upload-error', file, new Error('foo'))
+            hasError = true
+          }
+        })
+        return Promise.resolve()
+      })
+      core.on('upload', onUpload)
+      core.on('retry-all', onRetryAll)
+      core.on('upload-error', onUploadError)
+
+      const firstFileID = core.addFile({
+        source: 'vi',
+        name: 'foo.jpg',
+        type: 'image/jpeg',
+        data: testImage,
+      })
+      // First time 'upload' and 'upload-error' should be emitted
+      await core.upload()
+      expect(onRetryAll).not.toHaveBeenCalled()
+      expect(onUpload).toHaveBeenCalled()
+      expect(onUploadError).toHaveBeenCalled()
+
+      // One failed file in memory but we add a new one before uploading
+      const secondFileID = core.addFile({
+        source: 'vi',
+        name: 'bar.jpg',
+        type: 'image/jpeg',
+        data: testImage,
+      })
+      const onComplete = vi.fn()
+      core.on('complete', onComplete)
+      // Second time 'upload' and 'retry-all' should be emitted
+      // but only the error file should have been uploaded, not the new one as well.
+      await core.upload()
+      expect(onRetryAll).toHaveBeenCalled()
+      expect(onUpload).toBeCalledTimes(2) // one more
+      expect(onUploadError).toBeCalledTimes(1) // still the same
+      expect(onComplete).toHaveBeenCalled()
+      const result = onComplete.mock.calls[0][0]
+      expect(result.successful?.length).toBe(1)
+      expect(result.failed?.length).toBe(0)
+      expect(result.successful?.[0].id).toBe(firstFileID)
+
+      // Third time 'upload' should be emitted and succeed with the newly added file
+      await core.upload()
+      expect(onRetryAll).toHaveBeenCalledTimes(1) // still one
+      expect(onUpload).toBeCalledTimes(3) // one more
+      expect(onUploadError).toBeCalledTimes(1) // still one
+      expect(onComplete).toBeCalledTimes(2) // one more
+      const result2 = onComplete.mock.calls[1][0]
+      // Even though it's another upload the file from the previous upload is included again
+      expect(result2.successful?.length).toBe(2)
+      expect(result2.failed?.length).toBe(0)
+      expect(result2.successful?.[1].id).toBe(secondFileID)
+    })
   })
 
   describe('removing a file', () => {

packages/@uppy/core/src/Uppy.ts

@@ -2248,6 +2248,37 @@ export class Uppy<
 
     let { files } = this.getState()
 
+    // Check if we have files with errors (for retry behavior)
+    const filesToRetry = Object.keys(files).filter(
+      (fileID) => files[fileID].error,
+    )
+    const hasFilesToRetry = filesToRetry.length > 0
+
+    // If we have files to retry, behave like retryAll() and ignore any new files
+    if (hasFilesToRetry) {
+      const updatedFiles = { ...files }
+      filesToRetry.forEach((fileID) => {
+        updatedFiles[fileID] = {
+          ...updatedFiles[fileID],
+          isPaused: false,
+          error: null,
+        }
+      })
+
+      this.setState({
+        files: updatedFiles,
+        error: null,
+      })
+
+      this.emit('retry-all', Object.values(updatedFiles))
+
+      const uploadID = this.#createUpload(filesToRetry, {
+        forceAllowNewUpload: true, // create new upload even if allowNewUpload: false
+      })
+      return this.#runUpload(uploadID)
+    }
+
+    // If no files to retry, proceed with original upload() behavior for new files
     const onBeforeUploadResult = this.opts.onBeforeUpload(files)
 
     if (onBeforeUploadResult === false) {