From 654605e8c48d8fd13a56cf6a75e5492e1c36a4c7 Mon Sep 17 00:00:00 2001 From: Xavier Abad Date: Mon, 2 Nov 2026 16:53:45 +0100 Subject: [PATCH 01/12] refactor(upload): let the user retry what failed in an upload, uploading each item once --- src/app/tasks/types.ts | 13 ++ src/app/upload/Upload.test.ts | 186 ++++++++++++++++++++++++++- src/app/upload/Upload.ts | 67 +++++++++- src/app/upload/UploadManager.test.ts | 10 ++ src/app/upload/UploadManager.ts | 12 +- src/app/upload/failedUploadItem.ts | 28 ++++ 6 files changed, 300 insertions(+), 16 deletions(-) create mode 100644 src/app/upload/failedUploadItem.ts diff --git a/src/app/tasks/types.ts b/src/app/tasks/types.ts index b213d66f5c..aa6ae820c0 100644 --- a/src/app/tasks/types.ts +++ b/src/app/tasks/types.ts @@ -40,6 +40,17 @@ export enum TaskEvent { TaskError = 'task-error', } +export interface FailedUploadItem { + id: string; + name: string; + type?: string; + size: number; + modifiedAt: number; + isFolder: boolean; + isRetryable: boolean; + isRetrying: boolean; +} + export interface BaseTask { id: string; relatedTaskId?: string; @@ -51,6 +62,8 @@ export interface BaseTask { downloadedProgress?: number; subtitle?: string; stop?: () => Promise; + failedItems?: FailedUploadItem[]; + retry?: (failedItem?: FailedUploadItem) => Promise; } export interface CreateFolderTask extends BaseTask { diff --git a/src/app/upload/Upload.test.ts b/src/app/upload/Upload.test.ts index b270b668c0..97450daa9c 100644 --- a/src/app/upload/Upload.test.ts +++ b/src/app/upload/Upload.test.ts @@ -6,6 +6,7 @@ import { IRoot } from 'app/store/slices/storage/types'; import tasksService from 'app/tasks/services/tasks.service'; import { TaskStatus, TaskType } from 'app/tasks/types'; import errorService from 'services/error.service'; +import referralService from 'services/referral.service'; import { AppError } from '@internxt/sdk'; import { HTTP_STATUS_CODES } from 'app/core/constants'; import { @@ -52,13 +53,9 @@ const createUpload = ( callbacks: Partial[0]> = {}, ) => new Upload({ destinationFolderUuid: DESTINATION_UUID, roots, files, uploadRepository, ...callbacks }); -const tasksOf = (action: TaskType) => tasksService.getTasks().filter((task) => task.action === action); +const uploadedNames = () => vi.mocked(uploadFile).mock.calls.map(([file]) => file.name); -const failUploadsOf = (failingNames: string[], error: unknown = new Error('Server error')) => - (uploadFile as Mock).mockImplementation(async (file: { name: string }) => { - if (failingNames.includes(file.name)) throw error; - return { uuid: `${file.name}-uuid` }; - }); +const tasksOf = (action: TaskType) => tasksService.getTasks().filter((task) => task.action === action); describe('Uploading what the user picked', () => { beforeEach(() => { @@ -208,6 +205,173 @@ describe('Uploading what the user picked', () => { expect(newStorageService.deleteFolderByUuid).toHaveBeenCalledWith('Photos-uuid'); expect(uploadFile).not.toHaveBeenCalled(); }); +}); + +const failUploadsOf = (failingNames: string[], error: unknown = new Error('Server error')) => + (uploadFile as Mock).mockImplementation(async (file: { name: string }) => { + if (failingNames.includes(file.name)) throw error; + return { uuid: `${file.name}-uuid` }; + }); + +const uploadSucceeds = () => + (uploadFile as Mock).mockImplementation(async (file: { name: string }) => ({ uuid: `${file.name}-uuid` })); + +describe('Retrying what failed', () => { + beforeEach(() => { + vi.clearAllMocks(); + tasksService.clearTasks(); + vi.spyOn(errorService, 'reportError').mockReturnValue(); + fakeFolderCreationTaking(); + }); + + test('When files inside a folder fail, then they are listed on the folder task', async () => { + failUploadsOf(['broken']); + + await createUpload( + [getFolderToUpload('Photos', { childrenFiles: [getFileToUpload('broken.png'), getFileToUpload('beach.png')] })], + [], + ).run(); + + expect(tasksOf(TaskType.UploadFolder)[0].failedItems).toEqual([ + expect.objectContaining({ name: 'broken', type: 'png', isFolder: false, isRetryable: true, isRetrying: false }), + ]); + }); + + test('When a folder and a file both fail, then each task lists only its own failed items', async () => { + failUploadsOf(['broken', 'report']); + + await createUpload( + [getFolderToUpload('Photos', { childrenFiles: [getFileToUpload('broken.png')] })], + [getFileToUpload('report.pdf')], + ).run(); + + expect(tasksOf(TaskType.UploadFolder)[0].failedItems).toEqual([expect.objectContaining({ name: 'broken' })]); + expect(tasksOf(TaskType.UploadFile)[0].failedItems).toEqual([expect.objectContaining({ name: 'report' })]); + }); + + test('When the user retries a folder, then its failed files are uploaded again into it and the folder ends with nothing failed', async () => { + failUploadsOf(['broken']); + const upload = createUpload( + [getFolderToUpload('Photos', { childrenFiles: [getFileToUpload('broken.png'), getFileToUpload('beach.png')] })], + [], + ); + await upload.run(); + uploadSucceeds(); + + await tasksOf(TaskType.UploadFolder)[0].retry?.(); + + const [folderTask] = tasksOf(TaskType.UploadFolder); + expect(uploadFile).toHaveBeenLastCalledWith( + expect.objectContaining({ name: 'broken', parentFolderId: 'Photos-uuid' }), + expect.any(Function), + expect.any(Object), + expect.any(Object), + ); + expect(folderTask).toEqual(expect.objectContaining({ status: TaskStatus.Success, progress: 1, failedItems: [] })); + }); + + test('When the user retries one failed file, then only that file is uploaded again', async () => { + failUploadsOf(['retried', 'still-failing']); + await createUpload( + [ + getFolderToUpload('Photos', { + childrenFiles: [getFileToUpload('retried.png'), getFileToUpload('still-failing.png')], + }), + ], + [], + ).run(); + uploadSucceeds(); + vi.mocked(uploadFile).mockClear(); + const [folderTask] = tasksOf(TaskType.UploadFolder); + const retriedFailure = folderTask.failedItems?.find(({ name }) => name === 'retried'); + + await folderTask.retry?.(retriedFailure); + + expect(uploadedNames()).toEqual(['retried']); + expect(tasksService.findTask(folderTask.id)?.failedItems).toEqual([ + expect.objectContaining({ name: 'still-failing' }), + ]); + }); + + test('When the user retries everything while a file is already being retried, then that file is uploaded only once', async () => { + failUploadsOf(['retrying', 'waiting']); + await createUpload( + [ + getFolderToUpload('Photos', { + childrenFiles: [getFileToUpload('retrying.png'), getFileToUpload('waiting.png')], + }), + ], + [], + ).run(); + fakeUploadsTaking(100); + vi.mocked(uploadFile).mockClear(); + const [folderTask] = tasksOf(TaskType.UploadFolder); + const retryingFailure = folderTask.failedItems?.find(({ name }) => name === 'retrying'); + + const retryingOne = folderTask.retry?.(retryingFailure); + const retryingEverything = tasksService.findTask(folderTask.id)?.retry?.(); + await Promise.all([retryingOne, retryingEverything]); + + expect(uploadedNames()).toEqual(['retrying', 'waiting']); + }); + + test('When the failed files of a folder are retried one by one at the same time, then the folder upload is counted once', async () => { + failUploadsOf(['report', 'invoice']); + await createUpload( + [getFolderToUpload('Docs', { childrenFiles: [getFileToUpload('report.pdf'), getFileToUpload('invoice.pdf')] })], + [], + ).run(); + fakeUploadsTaking(100); + const [folderTask] = tasksOf(TaskType.UploadFolder); + const [reportFailure, invoiceFailure] = folderTask.failedItems ?? []; + + await Promise.all([folderTask.retry?.(reportFailure), folderTask.retry?.(invoiceFailure)]); + + expect(referralService.trackFolderUpload).toHaveBeenCalledOnce(); + }); + + test('When a folder that could not be created is retried, then it is created with all its content', async () => { + vi.mocked(folderService.createFolderByUuid).mockImplementation( + () => [Promise.reject(new Error('Server error')), { cancel: vi.fn() }] as never, + ); + await createUpload([getFolderToUpload('Photos', { childrenFiles: [getFileToUpload('beach.png')] })], []).run(); + fakeFolderCreationTaking(); + + await tasksOf(TaskType.UploadFolder)[0].retry?.(); + + expect(uploadFile).toHaveBeenCalledWith( + expect.objectContaining({ name: 'beach', parentFolderId: 'Photos-uuid' }), + expect.any(Function), + expect.any(Object), + expect.any(Object), + ); + expect(tasksOf(TaskType.UploadFolder)[0]).toEqual(expect.objectContaining({ status: TaskStatus.Success })); + }); + + test('When a failed file is retried, then its own task succeeds', async () => { + failUploadsOf(['report']); + await createUpload([], [getFileToUpload('report.pdf')]).run(); + uploadSucceeds(); + + await tasksOf(TaskType.UploadFile)[0].retry?.(); + + expect(tasksOf(TaskType.UploadFile)[0]).toEqual( + expect.objectContaining({ status: TaskStatus.Success, subtitle: undefined, failedItems: [] }), + ); + }); + + test('When the storage was full and the user retries after freeing space, then the files upload', async () => { + failUploadsOf(['report', 'invoice'], new AppError('No space left', HTTP_STATUS_CODES.MAX_SPACE_USED)); + await createUpload( + [getFolderToUpload('Docs', { childrenFiles: [getFileToUpload('report.pdf'), getFileToUpload('invoice.pdf')] })], + [], + ).run(); + uploadSucceeds(); + + await tasksOf(TaskType.UploadFolder)[0].retry?.(); + + expect(tasksOf(TaskType.UploadFolder)[0].failedItems).toEqual([]); + }); test('When the storage fills up while uploading, then the user is told once, however many items fail', async () => { const onStorageFull = vi.fn(); @@ -217,4 +381,14 @@ describe('Uploading what the user picked', () => { expect(onStorageFull).toHaveBeenCalledOnce(); }); + + test('When a retry also finds the storage full, then the user is told again', async () => { + const onStorageFull = vi.fn(); + failUploadsOf(['report'], new AppError('No space left', HTTP_STATUS_CODES.MAX_SPACE_USED)); + await createUpload([], [getFileToUpload('report.pdf')], { onStorageFull }).run(); + + await tasksOf(TaskType.UploadFile)[0].retry?.(); + + expect(onStorageFull).toHaveBeenCalledTimes(2); + }); }); diff --git a/src/app/upload/Upload.ts b/src/app/upload/Upload.ts index 7b7334e99d..c2696c1835 100644 --- a/src/app/upload/Upload.ts +++ b/src/app/upload/Upload.ts @@ -16,10 +16,12 @@ import { createFileItem, createFolderItem, UploadManager } from './UploadManager import { PersistUploadRepository } from 'app/repositories/DatabaseUploadRepository'; import { IRoot } from 'app/store/slices/storage/types'; import tasksService from 'app/tasks/services/tasks.service'; -import { TaskStatus, TaskType, UploadFileTask, UploadFolderTask } from 'app/tasks/types'; +import { FailedUploadItem, TaskStatus, TaskType, UploadFileTask, UploadFolderTask } from 'app/tasks/types'; import { errorMerge } from 'app/tasks/upload/uploadTaskErrors'; import errorService from 'services/error.service'; import referralService from 'services/referral.service'; +import { uniqueId } from 'utils/lodash'; +import { toFailedUploadItem } from './failedUploadItem'; interface UploadProps { destinationFolderUuid: string; @@ -37,7 +39,7 @@ interface RootFolderUpload { totalItems: number; uploadedItems: number; isCancelled: boolean; - hasFailedItems: boolean; + isUploadTracked: boolean; createdFolder?: DriveFolderData; } @@ -69,6 +71,7 @@ export class Upload { private readonly items: UploadItem[]; private readonly taskIdByItem = new Map(); private readonly rootFolders = new Map(); + private readonly failedItems = new Map(); constructor(props: UploadProps) { this.props = props; @@ -83,6 +86,11 @@ export class Upload { for (const item of this.items) { if (item.kind === UploadItemKind.Folder) this.trackRootFolder(item); else this.trackFile(item); + + tasksService.updateTask({ + taskId: this.getTaskIdOf(item), + merge: { retry: (failedItem) => this.retry(item, failedItem) }, + }); } } @@ -124,7 +132,7 @@ export class Upload { totalItems: countItemsIn(item.folder), uploadedItems: 0, isCancelled: false, - hasFailedItems: false, + isUploadTracked: false, }); } @@ -179,6 +187,7 @@ export class Upload { const isFile = item.kind === UploadItemKind.File; const isFolder = item.kind === UploadItemKind.Folder; + this.clearFailure(item); this.props.onItemUploaded?.(uploadedItem, getParentFolderUuid(item), isFolder); if (isFile) { @@ -219,8 +228,7 @@ export class Upload { } private onItemFailed(item: UploadItem, failure: UploadFailure): void { - const rootFolder = this.rootFolders.get(getTopLevelItem(item)); - if (rootFolder) rootFolder.hasFailedItems = true; + this.recordFailure(item, failure); const taskId = this.taskIdByItem.get(item); if (!taskId) return; @@ -267,10 +275,55 @@ export class Upload { merge: { status: TaskStatus.Success, itemUUID: { rootFolderUUID: rootFolder.createdFolder?.uuid } }, }); - const hasEverythingUploaded = !rootFolder.hasFailedItems; - if (hasEverythingUploaded) { + const hasEverythingUploaded = this.getFailuresOf(item).length === 0; + const shouldTrackUpload = hasEverythingUploaded && !rootFolder.isUploadTracked; + if (shouldTrackUpload) { + rootFolder.isUploadTracked = true; referralService.trackFolderUpload(); logNetworkInfoForUpload({ folderName: item.folder.name }); } } + + private async retry(topLevelItem: UploadItem, pickedFailedItem?: FailedUploadItem): Promise { + const failures = this.getFailuresOf(topLevelItem); + const failuresToRetry = pickedFailedItem + ? failures.filter(([, failedItem]) => failedItem.id === pickedFailedItem.id) + : failures; + + const hasNothingToRetry = failuresToRetry.length === 0; + if (hasNothingToRetry) return; + + for (const [item, failedItem] of failuresToRetry) this.failedItems.set(item, { ...failedItem, isRetrying: true }); + this.syncFailedItemsToTask(topLevelItem); + + const taskId = this.getTaskIdOf(topLevelItem); + const rootFolder = this.rootFolders.get(topLevelItem); + const progress = rootFolder ? tasksService.findTask(taskId)?.progress : 0; + tasksService.updateTask({ taskId, merge: { status: TaskStatus.InProcess, subtitle: undefined, progress } }); + + const itemsToRetry = failuresToRetry.map(([item]) => item); + await this.manager.run(itemsToRetry); + + if (rootFolder) this.finishRootFolder(topLevelItem as UploadFolderItem, rootFolder); + } + + private getFailuresOf(topLevelItem: UploadItem): [UploadItem, FailedUploadItem][] { + return [...this.failedItems].filter(([item]) => getTopLevelItem(item) === topLevelItem); + } + + private recordFailure(item: UploadItem, failure: UploadFailure): void { + const failureId = this.failedItems.get(item)?.id ?? uniqueId(); + this.failedItems.set(item, toFailedUploadItem(failureId, item, failure)); + this.syncFailedItemsToTask(getTopLevelItem(item)); + } + + private clearFailure(item: UploadItem): void { + const hadFailed = this.failedItems.delete(item); + if (hadFailed) this.syncFailedItemsToTask(getTopLevelItem(item)); + } + + private syncFailedItemsToTask(topLevelItem: UploadItem): void { + const failedItems = this.getFailuresOf(topLevelItem).map(([, failedItem]) => failedItem); + tasksService.updateTask({ taskId: this.getTaskIdOf(topLevelItem), merge: { failedItems } }); + } } diff --git a/src/app/upload/UploadManager.test.ts b/src/app/upload/UploadManager.test.ts index eb47cfd586..d92050c0f3 100644 --- a/src/app/upload/UploadManager.test.ts +++ b/src/app/upload/UploadManager.test.ts @@ -272,6 +272,16 @@ describe('Uploading files', () => { expect(events.onItemCancelled).not.toHaveBeenCalled(); }); + test('When a file that is already uploading is uploaded again, then it is uploaded only once', async () => { + fakeSlowUploads(); + const report = getUploadFileItem('report'); + const manager = createManager(); + + await failIfHangs(Promise.all([manager.run([report]), manager.run([report])])); + + expect(startedUploadNames()).toEqual(['report']); + }); + test('When a single file is cancelled, then only that file stops', async () => { fakeSlowUploads(); const [cancelledItem, otherItem] = [getUploadFileItem('cancelled'), getUploadFileItem('other')]; diff --git a/src/app/upload/UploadManager.ts b/src/app/upload/UploadManager.ts index 39fab71308..6c7653e0f3 100644 --- a/src/app/upload/UploadManager.ts +++ b/src/app/upload/UploadManager.ts @@ -129,6 +129,7 @@ export class UploadManager { private readonly largeFilesWaitingForFolders: UploadItem[] = []; private readonly cancelledItems = new WeakSet(); private readonly abortActiveUploadOf = new Map void>(); + private readonly itemsInProgress = new Set(); private result: UploadResult = { succeeded: [], failed: [] }; private readonly uploadQueue: AsyncPriorityQueue = priorityQueue((item, done) => { @@ -136,6 +137,9 @@ export class UploadManager { .catch((error) => errorService.reportError(error)) .finally(() => { if (isFolderItem(item)) this.finishFolder(); + + const isWaitingForFolders = this.largeFilesWaitingForFolders.includes(item); + if (!isWaitingForFolders) this.itemsInProgress.delete(item); done(); }); }, MAX_CONCURRENT_UPLOADS); @@ -154,10 +158,12 @@ export class UploadManager { this.uploadQueue.concurrency = MAX_CONCURRENT_UPLOADS; } - const hasNothingToUpload = items.length === 0; - if (hasNothingToUpload) return this.result; + const itemsToUpload = items.filter((item) => !this.itemsInProgress.has(item)); + const hasNothingToWaitFor = itemsToUpload.length === 0 && this.uploadQueue.idle(); + if (hasNothingToWaitFor) return this.result; - this.queue(items); + itemsToUpload.forEach((item) => this.itemsInProgress.add(item)); + this.queue(itemsToUpload); await this.uploadQueue.drain(); return this.result; diff --git a/src/app/upload/failedUploadItem.ts b/src/app/upload/failedUploadItem.ts new file mode 100644 index 0000000000..001c5cb13a --- /dev/null +++ b/src/app/upload/failedUploadItem.ts @@ -0,0 +1,28 @@ +import { UploadItem, UploadItemKind } from 'app/upload/types'; +import { UploadFailure } from 'app/upload/uploadFailure'; +import { FailedUploadItem } from 'app/tasks/types'; + +export const toFailedUploadItem = (id: string, item: UploadItem, { isRetryable }: UploadFailure): FailedUploadItem => { + if (item.kind === UploadItemKind.Folder) { + return { + id, + name: item.folder.name, + size: 0, + modifiedAt: Date.now(), + isFolder: true, + isRetryable, + isRetrying: false, + }; + } + + return { + id, + name: item.file.name, + type: item.file.type, + size: item.file.size, + modifiedAt: item.file.content.lastModified, + isFolder: false, + isRetryable, + isRetrying: false, + }; +}; From e744b386553b19ac26b57d2d798d9a4fa5b94d6d Mon Sep 17 00:00:00 2001 From: Xavier Abad Date: Wed, 4 Nov 2026 13:53:25 +0100 Subject: [PATCH 02/12] refactor(upload): give each task its retry when it is created --- src/app/upload/Upload.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/app/upload/Upload.ts b/src/app/upload/Upload.ts index 9a5244e1f0..5b2690198d 100644 --- a/src/app/upload/Upload.ts +++ b/src/app/upload/Upload.ts @@ -86,11 +86,6 @@ export class Upload { for (const item of this.items) { if (item.kind === UploadItemKind.Folder) this.trackRootFolder(item); else this.trackFile(item); - - tasksService.updateTask({ - taskId: this.getTaskIdOf(item), - merge: { retry: (failedItem) => this.retry(item, failedItem) }, - }); } } @@ -124,6 +119,7 @@ export class Upload { showNotification: true, cancellable: true, stop: () => Promise.resolve(this.cancelRootFolder(item)), + retry: (failedItem) => this.retry(item, failedItem), }); this.taskIdByItem.set(item, taskId); @@ -153,6 +149,7 @@ export class Upload { currentFolderId: destinationFolderUuid, }, stop: () => Promise.resolve(this.manager.cancelItem(item)), + retry: (failedItem) => this.retry(item, failedItem), }); this.taskIdByItem.set(item, taskId); @@ -296,7 +293,7 @@ export class Upload { for (const [item, failedItem] of failuresToRetry) this.failedItems.set(item, { ...failedItem, isRetrying: true }); this.syncFailedItemsToTask(topLevelItem); - const taskId = this.getTaskIdOf(topLevelItem); + const taskId = this.getTaskIdOf(topLevelItem) as string; const rootFolder = this.rootFolders.get(topLevelItem); const progress = rootFolder ? tasksService.findTask(taskId)?.progress : 0; tasksService.updateTask({ taskId, merge: { status: TaskStatus.InProcess, subtitle: undefined, progress } }); @@ -324,6 +321,6 @@ export class Upload { private syncFailedItemsToTask(topLevelItem: UploadItem): void { const failedItems = this.getFailuresOf(topLevelItem).map(([, failedItem]) => failedItem); - tasksService.updateTask({ taskId: this.getTaskIdOf(topLevelItem), merge: { failedItems } }); + tasksService.updateTask({ taskId: this.getTaskIdOf(topLevelItem) as string, merge: { failedItems } }); } } From 968233555b30634113f5c8781d4334975b2c7da1 Mon Sep 17 00:00:00 2001 From: Xavier Abad Date: Wed, 4 Nov 2026 14:59:02 +0100 Subject: [PATCH 03/12] fix(upload): retry everything only retries the failures that retrying can fix --- src/app/upload/Upload.test.ts | 25 +++++++++++++++++++++++++ src/app/upload/Upload.ts | 6 +++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/app/upload/Upload.test.ts b/src/app/upload/Upload.test.ts index 7680b7b5ab..8d924fc4fe 100644 --- a/src/app/upload/Upload.test.ts +++ b/src/app/upload/Upload.test.ts @@ -297,6 +297,31 @@ describe('Retrying what failed', () => { ]); }); + test('When the user retries everything, then the files that retrying cannot fix are not uploaded again', async () => { + (uploadFile as Mock).mockImplementation(async (file: { name: string }) => { + if (file.name === 'over-plan-limit') throw new AppError('Plan limit reached', HTTP_STATUS_CODES.PAYMENT_REQUIRED); + if (file.name === 'broken') throw new Error('Server error'); + return { uuid: `${file.name}-uuid` }; + }); + await createUpload( + [ + getFolderToUpload('Photos', { + childrenFiles: [getFileToUpload('over-plan-limit.png'), getFileToUpload('broken.png')], + }), + ], + [], + ).run(); + uploadSucceeds(); + vi.mocked(uploadFile).mockClear(); + + await tasksOf(TaskType.UploadFolder)[0].retry?.(); + + expect(uploadedNames()).toEqual(['broken']); + expect(tasksOf(TaskType.UploadFolder)[0].failedItems).toEqual([ + expect.objectContaining({ name: 'over-plan-limit', isRetrying: false }), + ]); + }); + test('When the user retries everything while a file is already being retried, then that file is uploaded only once', async () => { failUploadsOf(['retrying', 'waiting']); await createUpload( diff --git a/src/app/upload/Upload.ts b/src/app/upload/Upload.ts index 5b2690198d..aee7d400d6 100644 --- a/src/app/upload/Upload.ts +++ b/src/app/upload/Upload.ts @@ -282,10 +282,10 @@ export class Upload { } private async retry(topLevelItem: UploadItem, pickedFailedItem?: FailedUploadItem): Promise { - const failures = this.getFailuresOf(topLevelItem); + const retryableFailures = this.getFailuresOf(topLevelItem).filter(([, failedItem]) => failedItem.isRetryable); const failuresToRetry = pickedFailedItem - ? failures.filter(([, failedItem]) => failedItem.id === pickedFailedItem.id) - : failures; + ? retryableFailures.filter(([, failedItem]) => failedItem.id === pickedFailedItem.id) + : retryableFailures; const hasNothingToRetry = failuresToRetry.length === 0; if (hasNothingToRetry) return; From 15eb1a489f6397d8dc2e6f10fba439f0e9885088 Mon Sep 17 00:00:00 2001 From: Xavier Abad Date: Mon, 2 Nov 2026 16:53:45 +0100 Subject: [PATCH 04/12] refactor(upload): let the user retry what failed in an upload, uploading each item once --- src/app/tasks/types.ts | 13 ++ src/app/upload/Upload.test.ts | 186 ++++++++++++++++++++++++++- src/app/upload/Upload.ts | 67 +++++++++- src/app/upload/UploadManager.test.ts | 10 ++ src/app/upload/UploadManager.ts | 12 +- src/app/upload/failedUploadItem.ts | 28 ++++ 6 files changed, 300 insertions(+), 16 deletions(-) create mode 100644 src/app/upload/failedUploadItem.ts diff --git a/src/app/tasks/types.ts b/src/app/tasks/types.ts index b213d66f5c..aa6ae820c0 100644 --- a/src/app/tasks/types.ts +++ b/src/app/tasks/types.ts @@ -40,6 +40,17 @@ export enum TaskEvent { TaskError = 'task-error', } +export interface FailedUploadItem { + id: string; + name: string; + type?: string; + size: number; + modifiedAt: number; + isFolder: boolean; + isRetryable: boolean; + isRetrying: boolean; +} + export interface BaseTask { id: string; relatedTaskId?: string; @@ -51,6 +62,8 @@ export interface BaseTask { downloadedProgress?: number; subtitle?: string; stop?: () => Promise; + failedItems?: FailedUploadItem[]; + retry?: (failedItem?: FailedUploadItem) => Promise; } export interface CreateFolderTask extends BaseTask { diff --git a/src/app/upload/Upload.test.ts b/src/app/upload/Upload.test.ts index 3deb83d56e..7680b7b5ab 100644 --- a/src/app/upload/Upload.test.ts +++ b/src/app/upload/Upload.test.ts @@ -6,6 +6,7 @@ import { IRoot } from 'app/store/slices/storage/types'; import tasksService from 'app/tasks/services/tasks.service'; import { TaskStatus, TaskType } from 'app/tasks/types'; import errorService from 'services/error.service'; +import referralService from 'services/referral.service'; import { AppError } from '@internxt/sdk'; import { HTTP_STATUS_CODES } from 'app/core/constants'; import { @@ -56,13 +57,9 @@ const createUpload = ( callbacks: Partial[0]> = {}, ) => new Upload({ destinationFolderUuid: DESTINATION_UUID, roots, files, uploadRepository, ...callbacks }); -const tasksOf = (action: TaskType) => tasksService.getTasks().filter((task) => task.action === action); +const uploadedNames = () => vi.mocked(uploadFile).mock.calls.map(([file]) => file.name); -const failUploadsOf = (failingNames: string[], error: unknown = new Error('Server error')) => - (uploadFile as Mock).mockImplementation(async (file: { name: string }) => { - if (failingNames.includes(file.name)) throw error; - return { uuid: `${file.name}-uuid` }; - }); +const tasksOf = (action: TaskType) => tasksService.getTasks().filter((task) => task.action === action); describe('Uploading what the user picked', () => { beforeEach(() => { @@ -212,6 +209,173 @@ describe('Uploading what the user picked', () => { expect(newStorageService.deleteFolderByUuid).toHaveBeenCalledWith('Photos-uuid'); expect(uploadFile).not.toHaveBeenCalled(); }); +}); + +const failUploadsOf = (failingNames: string[], error: unknown = new Error('Server error')) => + (uploadFile as Mock).mockImplementation(async (file: { name: string }) => { + if (failingNames.includes(file.name)) throw error; + return { uuid: `${file.name}-uuid` }; + }); + +const uploadSucceeds = () => + (uploadFile as Mock).mockImplementation(async (file: { name: string }) => ({ uuid: `${file.name}-uuid` })); + +describe('Retrying what failed', () => { + beforeEach(() => { + vi.clearAllMocks(); + tasksService.clearTasks(); + vi.spyOn(errorService, 'reportError').mockReturnValue(); + fakeFolderCreationTaking(); + }); + + test('When files inside a folder fail, then they are listed on the folder task', async () => { + failUploadsOf(['broken']); + + await createUpload( + [getFolderToUpload('Photos', { childrenFiles: [getFileToUpload('broken.png'), getFileToUpload('beach.png')] })], + [], + ).run(); + + expect(tasksOf(TaskType.UploadFolder)[0].failedItems).toEqual([ + expect.objectContaining({ name: 'broken', type: 'png', isFolder: false, isRetryable: true, isRetrying: false }), + ]); + }); + + test('When a folder and a file both fail, then each task lists only its own failed items', async () => { + failUploadsOf(['broken', 'report']); + + await createUpload( + [getFolderToUpload('Photos', { childrenFiles: [getFileToUpload('broken.png')] })], + [getFileToUpload('report.pdf')], + ).run(); + + expect(tasksOf(TaskType.UploadFolder)[0].failedItems).toEqual([expect.objectContaining({ name: 'broken' })]); + expect(tasksOf(TaskType.UploadFile)[0].failedItems).toEqual([expect.objectContaining({ name: 'report' })]); + }); + + test('When the user retries a folder, then its failed files are uploaded again into it and the folder ends with nothing failed', async () => { + failUploadsOf(['broken']); + const upload = createUpload( + [getFolderToUpload('Photos', { childrenFiles: [getFileToUpload('broken.png'), getFileToUpload('beach.png')] })], + [], + ); + await upload.run(); + uploadSucceeds(); + + await tasksOf(TaskType.UploadFolder)[0].retry?.(); + + const [folderTask] = tasksOf(TaskType.UploadFolder); + expect(uploadFile).toHaveBeenLastCalledWith( + expect.objectContaining({ name: 'broken', parentFolderId: 'Photos-uuid' }), + expect.any(Function), + expect.any(Object), + expect.any(Object), + ); + expect(folderTask).toEqual(expect.objectContaining({ status: TaskStatus.Success, progress: 1, failedItems: [] })); + }); + + test('When the user retries one failed file, then only that file is uploaded again', async () => { + failUploadsOf(['retried', 'still-failing']); + await createUpload( + [ + getFolderToUpload('Photos', { + childrenFiles: [getFileToUpload('retried.png'), getFileToUpload('still-failing.png')], + }), + ], + [], + ).run(); + uploadSucceeds(); + vi.mocked(uploadFile).mockClear(); + const [folderTask] = tasksOf(TaskType.UploadFolder); + const retriedFailure = folderTask.failedItems?.find(({ name }) => name === 'retried'); + + await folderTask.retry?.(retriedFailure); + + expect(uploadedNames()).toEqual(['retried']); + expect(tasksService.findTask(folderTask.id)?.failedItems).toEqual([ + expect.objectContaining({ name: 'still-failing' }), + ]); + }); + + test('When the user retries everything while a file is already being retried, then that file is uploaded only once', async () => { + failUploadsOf(['retrying', 'waiting']); + await createUpload( + [ + getFolderToUpload('Photos', { + childrenFiles: [getFileToUpload('retrying.png'), getFileToUpload('waiting.png')], + }), + ], + [], + ).run(); + fakeUploadsTaking(100); + vi.mocked(uploadFile).mockClear(); + const [folderTask] = tasksOf(TaskType.UploadFolder); + const retryingFailure = folderTask.failedItems?.find(({ name }) => name === 'retrying'); + + const retryingOne = folderTask.retry?.(retryingFailure); + const retryingEverything = tasksService.findTask(folderTask.id)?.retry?.(); + await Promise.all([retryingOne, retryingEverything]); + + expect(uploadedNames()).toEqual(['retrying', 'waiting']); + }); + + test('When the failed files of a folder are retried one by one at the same time, then the folder upload is counted once', async () => { + failUploadsOf(['report', 'invoice']); + await createUpload( + [getFolderToUpload('Docs', { childrenFiles: [getFileToUpload('report.pdf'), getFileToUpload('invoice.pdf')] })], + [], + ).run(); + fakeUploadsTaking(100); + const [folderTask] = tasksOf(TaskType.UploadFolder); + const [reportFailure, invoiceFailure] = folderTask.failedItems ?? []; + + await Promise.all([folderTask.retry?.(reportFailure), folderTask.retry?.(invoiceFailure)]); + + expect(referralService.trackFolderUpload).toHaveBeenCalledOnce(); + }); + + test('When a folder that could not be created is retried, then it is created with all its content', async () => { + vi.mocked(folderService.createFolderByUuid).mockImplementation( + () => [Promise.reject(new Error('Server error')), { cancel: vi.fn() }] as never, + ); + await createUpload([getFolderToUpload('Photos', { childrenFiles: [getFileToUpload('beach.png')] })], []).run(); + fakeFolderCreationTaking(); + + await tasksOf(TaskType.UploadFolder)[0].retry?.(); + + expect(uploadFile).toHaveBeenCalledWith( + expect.objectContaining({ name: 'beach', parentFolderId: 'Photos-uuid' }), + expect.any(Function), + expect.any(Object), + expect.any(Object), + ); + expect(tasksOf(TaskType.UploadFolder)[0]).toEqual(expect.objectContaining({ status: TaskStatus.Success })); + }); + + test('When a failed file is retried, then its own task succeeds', async () => { + failUploadsOf(['report']); + await createUpload([], [getFileToUpload('report.pdf')]).run(); + uploadSucceeds(); + + await tasksOf(TaskType.UploadFile)[0].retry?.(); + + expect(tasksOf(TaskType.UploadFile)[0]).toEqual( + expect.objectContaining({ status: TaskStatus.Success, subtitle: undefined, failedItems: [] }), + ); + }); + + test('When the storage was full and the user retries after freeing space, then the files upload', async () => { + failUploadsOf(['report', 'invoice'], new AppError('No space left', HTTP_STATUS_CODES.MAX_SPACE_USED)); + await createUpload( + [getFolderToUpload('Docs', { childrenFiles: [getFileToUpload('report.pdf'), getFileToUpload('invoice.pdf')] })], + [], + ).run(); + uploadSucceeds(); + + await tasksOf(TaskType.UploadFolder)[0].retry?.(); + + expect(tasksOf(TaskType.UploadFolder)[0].failedItems).toEqual([]); + }); test('When the storage fills up while uploading, then the user is told once, however many items fail', async () => { const onStorageFull = vi.fn(); @@ -221,4 +385,14 @@ describe('Uploading what the user picked', () => { expect(onStorageFull).toHaveBeenCalledOnce(); }); + + test('When a retry also finds the storage full, then the user is told again', async () => { + const onStorageFull = vi.fn(); + failUploadsOf(['report'], new AppError('No space left', HTTP_STATUS_CODES.MAX_SPACE_USED)); + await createUpload([], [getFileToUpload('report.pdf')], { onStorageFull }).run(); + + await tasksOf(TaskType.UploadFile)[0].retry?.(); + + expect(onStorageFull).toHaveBeenCalledTimes(2); + }); }); diff --git a/src/app/upload/Upload.ts b/src/app/upload/Upload.ts index e6cd31e589..9a5244e1f0 100644 --- a/src/app/upload/Upload.ts +++ b/src/app/upload/Upload.ts @@ -16,10 +16,12 @@ import { createFileItem, createFolderItem, UploadManager } from './UploadManager import { PersistUploadRepository } from 'app/repositories/DatabaseUploadRepository'; import { IRoot } from 'app/store/slices/storage/types'; import tasksService from 'app/tasks/services/tasks.service'; -import { TaskStatus, TaskType, UploadFileTask, UploadFolderTask } from 'app/tasks/types'; +import { FailedUploadItem, TaskStatus, TaskType, UploadFileTask, UploadFolderTask } from 'app/tasks/types'; import { errorMerge } from 'app/tasks/upload/uploadTaskErrors'; import errorService from 'services/error.service'; import referralService from 'services/referral.service'; +import { uniqueId } from 'utils/lodash'; +import { toFailedUploadItem } from './failedUploadItem'; interface UploadProps { destinationFolderUuid: string; @@ -37,7 +39,7 @@ interface RootFolderUpload { totalItems: number; uploadedItems: number; isCancelled: boolean; - hasFailedItems: boolean; + isUploadTracked: boolean; createdFolder?: DriveFolderData; } @@ -69,6 +71,7 @@ export class Upload { private readonly items: UploadItem[]; private readonly taskIdByItem = new Map(); private readonly rootFolders = new Map(); + private readonly failedItems = new Map(); constructor(props: UploadProps) { this.props = props; @@ -83,6 +86,11 @@ export class Upload { for (const item of this.items) { if (item.kind === UploadItemKind.Folder) this.trackRootFolder(item); else this.trackFile(item); + + tasksService.updateTask({ + taskId: this.getTaskIdOf(item), + merge: { retry: (failedItem) => this.retry(item, failedItem) }, + }); } } @@ -124,7 +132,7 @@ export class Upload { totalItems: countItemsIn(item.folder), uploadedItems: 0, isCancelled: false, - hasFailedItems: false, + isUploadTracked: false, }); } @@ -179,6 +187,7 @@ export class Upload { const isFile = item.kind === UploadItemKind.File; const isFolder = item.kind === UploadItemKind.Folder; + this.clearFailure(item); this.props.onItemUploaded?.(uploadedItem, getParentFolderUuid(item), isFolder); if (isFile) { @@ -219,8 +228,7 @@ export class Upload { } private onItemFailed(item: UploadItem, failure: UploadFailure): void { - const rootFolder = this.rootFolders.get(getTopLevelItem(item)); - if (rootFolder) rootFolder.hasFailedItems = true; + this.recordFailure(item, failure); const taskId = this.getTaskIdOf(item); if (!taskId) return; @@ -267,10 +275,55 @@ export class Upload { merge: { status: TaskStatus.Success, itemUUID: { rootFolderUUID: createdFolder.uuid } }, }); - const hasEverythingUploaded = !rootFolder.hasFailedItems; - if (hasEverythingUploaded) { + const hasEverythingUploaded = this.getFailuresOf(item).length === 0; + const shouldTrackUpload = hasEverythingUploaded && !rootFolder.isUploadTracked; + if (shouldTrackUpload) { + rootFolder.isUploadTracked = true; referralService.trackFolderUpload(); logNetworkInfoForUpload({ folderName: item.folder.name }); } } + + private async retry(topLevelItem: UploadItem, pickedFailedItem?: FailedUploadItem): Promise { + const failures = this.getFailuresOf(topLevelItem); + const failuresToRetry = pickedFailedItem + ? failures.filter(([, failedItem]) => failedItem.id === pickedFailedItem.id) + : failures; + + const hasNothingToRetry = failuresToRetry.length === 0; + if (hasNothingToRetry) return; + + for (const [item, failedItem] of failuresToRetry) this.failedItems.set(item, { ...failedItem, isRetrying: true }); + this.syncFailedItemsToTask(topLevelItem); + + const taskId = this.getTaskIdOf(topLevelItem); + const rootFolder = this.rootFolders.get(topLevelItem); + const progress = rootFolder ? tasksService.findTask(taskId)?.progress : 0; + tasksService.updateTask({ taskId, merge: { status: TaskStatus.InProcess, subtitle: undefined, progress } }); + + const itemsToRetry = failuresToRetry.map(([item]) => item); + await this.manager.run(itemsToRetry); + + if (rootFolder) this.finishRootFolder(topLevelItem as UploadFolderItem, rootFolder); + } + + private getFailuresOf(topLevelItem: UploadItem): [UploadItem, FailedUploadItem][] { + return [...this.failedItems].filter(([item]) => getTopLevelItem(item) === topLevelItem); + } + + private recordFailure(item: UploadItem, failure: UploadFailure): void { + const failureId = this.failedItems.get(item)?.id ?? uniqueId(); + this.failedItems.set(item, toFailedUploadItem(failureId, item, failure)); + this.syncFailedItemsToTask(getTopLevelItem(item)); + } + + private clearFailure(item: UploadItem): void { + const hadFailed = this.failedItems.delete(item); + if (hadFailed) this.syncFailedItemsToTask(getTopLevelItem(item)); + } + + private syncFailedItemsToTask(topLevelItem: UploadItem): void { + const failedItems = this.getFailuresOf(topLevelItem).map(([, failedItem]) => failedItem); + tasksService.updateTask({ taskId: this.getTaskIdOf(topLevelItem), merge: { failedItems } }); + } } diff --git a/src/app/upload/UploadManager.test.ts b/src/app/upload/UploadManager.test.ts index eb47cfd586..d92050c0f3 100644 --- a/src/app/upload/UploadManager.test.ts +++ b/src/app/upload/UploadManager.test.ts @@ -272,6 +272,16 @@ describe('Uploading files', () => { expect(events.onItemCancelled).not.toHaveBeenCalled(); }); + test('When a file that is already uploading is uploaded again, then it is uploaded only once', async () => { + fakeSlowUploads(); + const report = getUploadFileItem('report'); + const manager = createManager(); + + await failIfHangs(Promise.all([manager.run([report]), manager.run([report])])); + + expect(startedUploadNames()).toEqual(['report']); + }); + test('When a single file is cancelled, then only that file stops', async () => { fakeSlowUploads(); const [cancelledItem, otherItem] = [getUploadFileItem('cancelled'), getUploadFileItem('other')]; diff --git a/src/app/upload/UploadManager.ts b/src/app/upload/UploadManager.ts index 2f5c3f697b..0253e6bdf3 100644 --- a/src/app/upload/UploadManager.ts +++ b/src/app/upload/UploadManager.ts @@ -129,6 +129,7 @@ export class UploadManager { private readonly largeFilesWaitingForFolders: UploadItem[] = []; private readonly cancelledItems = new WeakSet(); private readonly abortActiveUploadOf = new Map void>(); + private readonly itemsInProgress = new Set(); private result: UploadResult = { succeeded: [], failed: [] }; private readonly uploadQueue: AsyncPriorityQueue = priorityQueue((item, done) => { @@ -136,6 +137,9 @@ export class UploadManager { .catch((error) => errorService.reportError(error)) .finally(() => { if (isFolderItem(item)) this.finishFolder(); + + const isWaitingForFolders = this.largeFilesWaitingForFolders.includes(item); + if (!isWaitingForFolders) this.itemsInProgress.delete(item); done(); }); }, MAX_CONCURRENT_UPLOADS); @@ -154,10 +158,12 @@ export class UploadManager { this.uploadQueue.concurrency = MAX_CONCURRENT_UPLOADS; } - const hasNothingToUpload = items.length === 0; - if (hasNothingToUpload) return this.result; + const itemsToUpload = items.filter((item) => !this.itemsInProgress.has(item)); + const hasNothingToWaitFor = itemsToUpload.length === 0 && this.uploadQueue.idle(); + if (hasNothingToWaitFor) return this.result; - this.queue(items); + itemsToUpload.forEach((item) => this.itemsInProgress.add(item)); + this.queue(itemsToUpload); await this.uploadQueue.drain(); return this.result; diff --git a/src/app/upload/failedUploadItem.ts b/src/app/upload/failedUploadItem.ts new file mode 100644 index 0000000000..001c5cb13a --- /dev/null +++ b/src/app/upload/failedUploadItem.ts @@ -0,0 +1,28 @@ +import { UploadItem, UploadItemKind } from 'app/upload/types'; +import { UploadFailure } from 'app/upload/uploadFailure'; +import { FailedUploadItem } from 'app/tasks/types'; + +export const toFailedUploadItem = (id: string, item: UploadItem, { isRetryable }: UploadFailure): FailedUploadItem => { + if (item.kind === UploadItemKind.Folder) { + return { + id, + name: item.folder.name, + size: 0, + modifiedAt: Date.now(), + isFolder: true, + isRetryable, + isRetrying: false, + }; + } + + return { + id, + name: item.file.name, + type: item.file.type, + size: item.file.size, + modifiedAt: item.file.content.lastModified, + isFolder: false, + isRetryable, + isRetrying: false, + }; +}; From 9e45bdec4b99f7e288107fca935a8c4079cb4f8c Mon Sep 17 00:00:00 2001 From: Xavier Abad Date: Wed, 4 Nov 2026 13:53:25 +0100 Subject: [PATCH 05/12] refactor(upload): give each task its retry when it is created --- src/app/upload/Upload.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/app/upload/Upload.ts b/src/app/upload/Upload.ts index 9a5244e1f0..5b2690198d 100644 --- a/src/app/upload/Upload.ts +++ b/src/app/upload/Upload.ts @@ -86,11 +86,6 @@ export class Upload { for (const item of this.items) { if (item.kind === UploadItemKind.Folder) this.trackRootFolder(item); else this.trackFile(item); - - tasksService.updateTask({ - taskId: this.getTaskIdOf(item), - merge: { retry: (failedItem) => this.retry(item, failedItem) }, - }); } } @@ -124,6 +119,7 @@ export class Upload { showNotification: true, cancellable: true, stop: () => Promise.resolve(this.cancelRootFolder(item)), + retry: (failedItem) => this.retry(item, failedItem), }); this.taskIdByItem.set(item, taskId); @@ -153,6 +149,7 @@ export class Upload { currentFolderId: destinationFolderUuid, }, stop: () => Promise.resolve(this.manager.cancelItem(item)), + retry: (failedItem) => this.retry(item, failedItem), }); this.taskIdByItem.set(item, taskId); @@ -296,7 +293,7 @@ export class Upload { for (const [item, failedItem] of failuresToRetry) this.failedItems.set(item, { ...failedItem, isRetrying: true }); this.syncFailedItemsToTask(topLevelItem); - const taskId = this.getTaskIdOf(topLevelItem); + const taskId = this.getTaskIdOf(topLevelItem) as string; const rootFolder = this.rootFolders.get(topLevelItem); const progress = rootFolder ? tasksService.findTask(taskId)?.progress : 0; tasksService.updateTask({ taskId, merge: { status: TaskStatus.InProcess, subtitle: undefined, progress } }); @@ -324,6 +321,6 @@ export class Upload { private syncFailedItemsToTask(topLevelItem: UploadItem): void { const failedItems = this.getFailuresOf(topLevelItem).map(([, failedItem]) => failedItem); - tasksService.updateTask({ taskId: this.getTaskIdOf(topLevelItem), merge: { failedItems } }); + tasksService.updateTask({ taskId: this.getTaskIdOf(topLevelItem) as string, merge: { failedItems } }); } } From 5bf545f990d85b6995bc51d2ca00b52c29a30d77 Mon Sep 17 00:00:00 2001 From: Xavier Abad Date: Wed, 4 Nov 2026 14:59:02 +0100 Subject: [PATCH 06/12] fix(upload): retry everything only retries the failures that retrying can fix --- src/app/upload/Upload.test.ts | 25 +++++++++++++++++++++++++ src/app/upload/Upload.ts | 6 +++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/app/upload/Upload.test.ts b/src/app/upload/Upload.test.ts index 7680b7b5ab..8d924fc4fe 100644 --- a/src/app/upload/Upload.test.ts +++ b/src/app/upload/Upload.test.ts @@ -297,6 +297,31 @@ describe('Retrying what failed', () => { ]); }); + test('When the user retries everything, then the files that retrying cannot fix are not uploaded again', async () => { + (uploadFile as Mock).mockImplementation(async (file: { name: string }) => { + if (file.name === 'over-plan-limit') throw new AppError('Plan limit reached', HTTP_STATUS_CODES.PAYMENT_REQUIRED); + if (file.name === 'broken') throw new Error('Server error'); + return { uuid: `${file.name}-uuid` }; + }); + await createUpload( + [ + getFolderToUpload('Photos', { + childrenFiles: [getFileToUpload('over-plan-limit.png'), getFileToUpload('broken.png')], + }), + ], + [], + ).run(); + uploadSucceeds(); + vi.mocked(uploadFile).mockClear(); + + await tasksOf(TaskType.UploadFolder)[0].retry?.(); + + expect(uploadedNames()).toEqual(['broken']); + expect(tasksOf(TaskType.UploadFolder)[0].failedItems).toEqual([ + expect.objectContaining({ name: 'over-plan-limit', isRetrying: false }), + ]); + }); + test('When the user retries everything while a file is already being retried, then that file is uploaded only once', async () => { failUploadsOf(['retrying', 'waiting']); await createUpload( diff --git a/src/app/upload/Upload.ts b/src/app/upload/Upload.ts index 5b2690198d..aee7d400d6 100644 --- a/src/app/upload/Upload.ts +++ b/src/app/upload/Upload.ts @@ -282,10 +282,10 @@ export class Upload { } private async retry(topLevelItem: UploadItem, pickedFailedItem?: FailedUploadItem): Promise { - const failures = this.getFailuresOf(topLevelItem); + const retryableFailures = this.getFailuresOf(topLevelItem).filter(([, failedItem]) => failedItem.isRetryable); const failuresToRetry = pickedFailedItem - ? failures.filter(([, failedItem]) => failedItem.id === pickedFailedItem.id) - : failures; + ? retryableFailures.filter(([, failedItem]) => failedItem.id === pickedFailedItem.id) + : retryableFailures; const hasNothingToRetry = failuresToRetry.length === 0; if (hasNothingToRetry) return; From cd0c921dd037b86fae35307fd9e60edfa09faf35 Mon Sep 17 00:00:00 2001 From: Xavier Abad Date: Fri, 6 Nov 2026 13:42:42 +0100 Subject: [PATCH 07/12] refactor(upload): simplify the retry flow --- src/app/tasks/types.ts | 4 +- src/app/upload/Upload.test.ts | 141 ++++++++++++++++++++++----- src/app/upload/Upload.ts | 109 +++++++++++++-------- src/app/upload/UploadManager.test.ts | 10 -- src/app/upload/UploadManager.ts | 12 +-- src/app/upload/failedUploadItem.ts | 10 +- 6 files changed, 192 insertions(+), 94 deletions(-) diff --git a/src/app/tasks/types.ts b/src/app/tasks/types.ts index aa6ae820c0..3565597c51 100644 --- a/src/app/tasks/types.ts +++ b/src/app/tasks/types.ts @@ -41,7 +41,6 @@ export enum TaskEvent { } export interface FailedUploadItem { - id: string; name: string; type?: string; size: number; @@ -49,6 +48,7 @@ export interface FailedUploadItem { isFolder: boolean; isRetryable: boolean; isRetrying: boolean; + retry: () => Promise; } export interface BaseTask { @@ -63,7 +63,7 @@ export interface BaseTask { subtitle?: string; stop?: () => Promise; failedItems?: FailedUploadItem[]; - retry?: (failedItem?: FailedUploadItem) => Promise; + retry?: () => Promise; } export interface CreateFolderTask extends BaseTask { diff --git a/src/app/upload/Upload.test.ts b/src/app/upload/Upload.test.ts index 3bc47876cc..ef5805e760 100644 --- a/src/app/upload/Upload.test.ts +++ b/src/app/upload/Upload.test.ts @@ -61,13 +61,22 @@ const uploadedNames = () => vi.mocked(uploadFile).mock.calls.map(([file]) => fil const tasksOf = (action: TaskType) => tasksService.getTasks().filter((task) => task.action === action); +const failUploadsOf = (failingNames: string[], error: unknown = new Error('Server error')) => + (uploadFile as Mock).mockImplementation(async (file: { name: string }) => { + if (failingNames.includes(file.name)) throw error; + return { uuid: `${file.name}-uuid` }; + }); + +const uploadSucceeds = () => + (uploadFile as Mock).mockImplementation(async (file: { name: string }) => ({ uuid: `${file.name}-uuid` })); + describe('Uploading what the user picked', () => { beforeEach(() => { vi.clearAllMocks(); tasksService.clearTasks(); vi.spyOn(errorService, 'reportError').mockReturnValue(); vi.mocked(newStorageService.deleteFolderByUuid).mockResolvedValue(undefined as never); - (uploadFile as Mock).mockImplementation(async (file: { name: string }) => ({ uuid: `${file.name}-uuid` })); + uploadSucceeds(); fakeFolderCreationTaking(); }); @@ -111,6 +120,12 @@ describe('Uploading what the user picked', () => { expect(onItemUploaded).toHaveBeenCalledWith(expect.objectContaining({ name: 'report' }), DESTINATION_UUID, false); }); + test('When a folder is uploaded with everything in it, then it is counted as a folder upload', async () => { + await createUpload([getFolderToUpload('Photos', { childrenFiles: [getFileToUpload('beach.png')] })], []).run(); + + expect(referralService.trackFolderUpload).toHaveBeenCalledOnce(); + }); + test('When reporting an uploaded item fails, then the upload still finishes and each task succeeds', async () => { const onItemUploaded = vi.fn(() => { throw new Error('Listener error'); @@ -135,10 +150,7 @@ describe('Uploading what the user picked', () => { }); test('When a file inside a folder fails, then the folder still succeeds with what could be uploaded', async () => { - (uploadFile as Mock).mockImplementation(async (file: { name: string }) => { - if (file.name === 'broken') throw new Error('Server error'); - return { uuid: `${file.name}-uuid` }; - }); + failUploadsOf(['broken']); const upload = createUpload( [getFolderToUpload('Photos', { childrenFiles: [getFileToUpload('broken.png'), getFileToUpload('beach.png')] })], [], @@ -272,20 +284,12 @@ describe('Uploading what the user picked', () => { }); }); -const failUploadsOf = (failingNames: string[], error: unknown = new Error('Server error')) => - (uploadFile as Mock).mockImplementation(async (file: { name: string }) => { - if (failingNames.includes(file.name)) throw error; - return { uuid: `${file.name}-uuid` }; - }); - -const uploadSucceeds = () => - (uploadFile as Mock).mockImplementation(async (file: { name: string }) => ({ uuid: `${file.name}-uuid` })); - describe('Retrying what failed', () => { beforeEach(() => { vi.clearAllMocks(); tasksService.clearTasks(); vi.spyOn(errorService, 'reportError').mockReturnValue(); + vi.mocked(newStorageService.deleteFolderByUuid).mockResolvedValue(undefined as never); fakeFolderCreationTaking(); }); @@ -335,6 +339,31 @@ describe('Retrying what failed', () => { expect(folderTask).toEqual(expect.objectContaining({ status: TaskStatus.Success, progress: 1, failedItems: [] })); }); + test('When the user cancels a folder while it is being retried, then the retry finishes only once the folder is removed', async () => { + failUploadsOf(['broken']); + const upload = createUpload([getFolderToUpload('Photos', { childrenFiles: [getFileToUpload('broken.png')] })], []); + await upload.run(); + vi.mocked(uploadFile).mockClear(); + fakeUploadsTaking(500); + let finishRemoval: () => void = () => undefined; + vi.mocked(newStorageService.deleteFolderByUuid).mockReturnValue( + new Promise((resolve) => (finishRemoval = resolve)) as never, + ); + const onRetryFinished = vi.fn(); + const [folderTask] = tasksOf(TaskType.UploadFolder); + + const retrying = folderTask.retry?.().then(onRetryFinished); + await vi.waitFor(() => expect(uploadFile).toHaveBeenCalled()); + await tasksService.cancelTask(folderTask.id); + await vi.waitFor(() => expect(newStorageService.deleteFolderByUuid).toHaveBeenCalledWith('Photos-uuid')); + await new Promise((resolve) => setTimeout(resolve, 100)); + + expect(onRetryFinished).not.toHaveBeenCalled(); + finishRemoval(); + await retrying; + expect(onRetryFinished).toHaveBeenCalled(); + }); + test('When the user retries one failed file, then only that file is uploaded again', async () => { failUploadsOf(['retried', 'still-failing']); await createUpload( @@ -350,7 +379,7 @@ describe('Retrying what failed', () => { const [folderTask] = tasksOf(TaskType.UploadFolder); const retriedFailure = folderTask.failedItems?.find(({ name }) => name === 'retried'); - await folderTask.retry?.(retriedFailure); + await retriedFailure?.retry(); expect(uploadedNames()).toEqual(['retried']); expect(tasksService.findTask(folderTask.id)?.failedItems).toEqual([ @@ -398,26 +427,22 @@ describe('Retrying what failed', () => { const [folderTask] = tasksOf(TaskType.UploadFolder); const retryingFailure = folderTask.failedItems?.find(({ name }) => name === 'retrying'); - const retryingOne = folderTask.retry?.(retryingFailure); + const retryingOne = retryingFailure?.retry(); const retryingEverything = tasksService.findTask(folderTask.id)?.retry?.(); await Promise.all([retryingOne, retryingEverything]); expect(uploadedNames()).toEqual(['retrying', 'waiting']); }); - test('When the failed files of a folder are retried one by one at the same time, then the folder upload is counted once', async () => { - failUploadsOf(['report', 'invoice']); - await createUpload( - [getFolderToUpload('Docs', { childrenFiles: [getFileToUpload('report.pdf'), getFileToUpload('invoice.pdf')] })], - [], - ).run(); - fakeUploadsTaking(100); - const [folderTask] = tasksOf(TaskType.UploadFolder); - const [reportFailure, invoiceFailure] = folderTask.failedItems ?? []; + test('When a folder is only fully uploaded after a retry, then it is not counted as a folder upload', async () => { + failUploadsOf(['report']); + await createUpload([getFolderToUpload('Docs', { childrenFiles: [getFileToUpload('report.pdf')] })], []).run(); + uploadSucceeds(); - await Promise.all([folderTask.retry?.(reportFailure), folderTask.retry?.(invoiceFailure)]); + await tasksOf(TaskType.UploadFolder)[0].retry?.(); - expect(referralService.trackFolderUpload).toHaveBeenCalledOnce(); + expect(tasksOf(TaskType.UploadFolder)[0].failedItems).toEqual([]); + expect(referralService.trackFolderUpload).not.toHaveBeenCalled(); }); test('When a folder that could not be created is retried, then it is created with all its content', async () => { @@ -481,4 +506,66 @@ describe('Retrying what failed', () => { expect(onStorageFull).toHaveBeenCalledTimes(2); }); + + test('When a cancelled folder is retried, then nothing is uploaded again', async () => { + failUploadsOf(['broken']); + await createUpload([getFolderToUpload('Photos', { childrenFiles: [getFileToUpload('broken.png')] })], []).run(); + const [folderTask] = tasksOf(TaskType.UploadFolder); + await tasksService.cancelTask(folderTask.id); + vi.mocked(uploadFile).mockClear(); + + await folderTask.retry?.(); + + expect(uploadFile).not.toHaveBeenCalled(); + expect(tasksService.findTask(folderTask.id)).toEqual( + expect.objectContaining({ + status: TaskStatus.Cancelled, + failedItems: [expect.objectContaining({ name: 'broken', isRetrying: false })], + }), + ); + }); + + test('When the user frees space and retries while another file is still uploading, then the retried file uploads', async () => { + let isStorageFull = true; + (uploadFile as Mock).mockImplementation(async (file: { name: string }) => { + if (file.name === 'video') await new Promise((resolve) => setTimeout(resolve, 500)); + else if (isStorageFull) throw new AppError('No space left', HTTP_STATUS_CODES.MAX_SPACE_USED); + return { uuid: `${file.name}-uuid` }; + }); + const upload = createUpload([], [getFileToUpload('report.pdf'), getFileToUpload('video.mp4')]); + const running = upload.run(); + const [reportTask, videoTask] = tasksOf(TaskType.UploadFile); + await vi.waitFor(() => expect(tasksService.findTask(reportTask.id)?.status).toBe(TaskStatus.Error)); + isStorageFull = false; + + await reportTask.retry?.(); + + expect(tasksService.findTask(reportTask.id)?.status).toBe(TaskStatus.Success); + expect(tasksService.findTask(videoTask.id)?.status).toBe(TaskStatus.InProcess); + await running; + }); + + test('When the failed files of two folders are retried, then each folder finishes when its own files do', async () => { + failUploadsOf(['broken', 'movie']); + await createUpload( + [ + getFolderToUpload('Photos', { childrenFiles: [getFileToUpload('broken.png')] }), + getFolderToUpload('Videos', { childrenFiles: [getFileToUpload('movie.mp4')] }), + ], + [], + ).run(); + (uploadFile as Mock).mockImplementation(async (file: { name: string }) => { + if (file.name === 'movie') await new Promise((resolve) => setTimeout(resolve, 500)); + return { uuid: `${file.name}-uuid` }; + }); + const [photosTask, videosTask] = tasksOf(TaskType.UploadFolder); + + const retryingVideos = videosTask.retry?.(); + await photosTask.retry?.(); + + expect(tasksService.findTask(photosTask.id)?.status).toBe(TaskStatus.Success); + expect(tasksService.findTask(videosTask.id)?.status).toBe(TaskStatus.InProcess); + await retryingVideos; + expect(tasksService.findTask(videosTask.id)?.status).toBe(TaskStatus.Success); + }); }); diff --git a/src/app/upload/Upload.ts b/src/app/upload/Upload.ts index ace1d7bac2..5c40ed7797 100644 --- a/src/app/upload/Upload.ts +++ b/src/app/upload/Upload.ts @@ -28,7 +28,6 @@ import { FailedUploadItem, TaskStatus, TaskType, UploadFileTask, UploadFolderTas import { errorMerge, folderRemovalFailedMerge } from 'app/tasks/upload/uploadTaskErrors'; import errorService from 'services/error.service'; import referralService from 'services/referral.service'; -import { uniqueId } from 'utils/lodash'; import { toFailedUploadItem } from './failedUploadItem'; interface UploadProps { @@ -47,7 +46,6 @@ interface RootFolderUpload { totalItems: number; uploadedItems: number; isCancelled: boolean; - isUploadTracked: boolean; createdFolder?: DriveFolderData; } @@ -63,36 +61,58 @@ const isTaskStopped = (taskId: string): boolean => { export class Upload { private readonly props: UploadProps; - private readonly manager: UploadManager; private readonly items: UploadItem[]; private readonly taskIdByItem = new Map(); private readonly rootFolders = new Map(); private readonly cancelledRootFolderDeletions: Promise[] = []; - private readonly failedItems = new Map(); + private readonly failuresByTopLevelItem = new Map>(); + private readonly activeManagers = new Set(); constructor(props: UploadProps) { this.props = props; this.items = createItemsToUpload(props); - this.manager = new UploadManager({ - ownerUserAuthenticationData: props.ownerUserAuthenticationData, - uploadRepository: props.uploadRepository, - getPauseKey: (item) => this.getTaskIdOf(getTopLevelItem(item)) as string, - events: this.createManagerEvents(), - }); for (const item of this.items) { if (item.kind === UploadItemKind.Folder) this.trackRootFolder(item); else this.trackFile(item); + this.failuresByTopLevelItem.set(item, new Map()); } } async run(): Promise { - const result = await this.manager.run(this.items); + const result = await this.uploadAndFinish(this.items, this.items); + this.rootFolders.forEach((rootFolder, item) => this.trackFolderUpload(item as UploadFolderItem, rootFolder)); + return result; + } + + private async uploadAndFinish(items: UploadItem[], topLevelItems: UploadItem[]): Promise { + const manager = this.createManager(); + this.activeManagers.add(manager); + const result = await manager.run(items); + this.activeManagers.delete(manager); await Promise.all(this.cancelledRootFolderDeletions); - this.rootFolders.forEach((rootFolder, item) => this.finishRootFolder(item as UploadFolderItem, rootFolder)); + + for (const topLevelItem of topLevelItems) { + const rootFolder = this.rootFolders.get(topLevelItem); + if (rootFolder) this.finishRootFolder(topLevelItem as UploadFolderItem, rootFolder); + } + return result; } + private createManager(): UploadManager { + return new UploadManager({ + ownerUserAuthenticationData: this.props.ownerUserAuthenticationData, + uploadRepository: this.props.uploadRepository, + getPauseKey: (item) => this.getTaskIdOf(getTopLevelItem(item)) as string, + events: this.createManagerEvents(), + }); + } + + private cancelItem(item: UploadItem): void { + this.activeManagers.forEach((manager) => manager.cancelItem(item)); + } + private createManagerEvents(): UploadManagerEvents { return { onItemStarted: (item) => this.onItemStarted(item), @@ -117,7 +137,7 @@ export class Upload { showNotification: true, cancellable: true, stop: () => Promise.resolve(this.cancelRootFolder(item)), - retry: (failedItem) => this.retry(item, failedItem), + retry: () => this.retry(item), }); this.taskIdByItem.set(item, taskId); @@ -126,7 +146,6 @@ export class Upload { totalItems: countItemsIn(item.folder), uploadedItems: 0, isCancelled: false, - isUploadTracked: false, }); } @@ -146,8 +165,8 @@ export class Upload { isDeepFolder: false, currentFolderId: destinationFolderUuid, }, - stop: () => Promise.resolve(this.manager.cancelItem(item)), - retry: (failedItem) => this.retry(item, failedItem), + stop: () => Promise.resolve(this.cancelItem(item)), + retry: () => this.retry(item), }); this.taskIdByItem.set(item, taskId); @@ -183,9 +202,9 @@ export class Upload { const isFileWithItsOwnTask = !!taskId && !isFolderItem(item); if (isFileWithItsOwnTask) this.finishFileTask(taskId, { fileUUID: uploadedItem.uuid }); else this.countUploadedInRootFolder(item, uploadedItem); + this.clearFailure(item); const isFile = item.kind === UploadItemKind.File; - this.clearFailure(item); this.props.onItemUploaded?.(uploadedItem, getParentFolderUuid(item), !isFile); if (isFile) { @@ -244,7 +263,7 @@ export class Upload { private cancelRootFolder(item: UploadFolderItem): void { const rootFolder = this.rootFolders.get(item) as RootFolderUpload; rootFolder.isCancelled = true; - this.manager.cancelItem(item); + this.cancelItem(item); if (rootFolder.createdFolder) { this.cancelledRootFolderDeletions.push(this.deleteCancelledRootFolder(item, rootFolder)); } @@ -275,56 +294,60 @@ export class Upload { taskId: rootFolder.taskId, merge: { status: TaskStatus.Success, itemUUID: { rootFolderUUID: createdFolder.uuid } }, }); + } - const hasEverythingUploaded = this.getFailuresOf(item).length === 0; - const shouldTrackUpload = hasEverythingUploaded && !rootFolder.isUploadTracked; - if (shouldTrackUpload) { - rootFolder.isUploadTracked = true; - referralService.trackFolderUpload(); - logNetworkInfoForUpload({ folderName: item.folder.name }); - } + private trackFolderUpload(item: UploadFolderItem, { isCancelled, createdFolder }: RootFolderUpload): void { + const hasEverythingUploaded = !isCancelled && !!createdFolder && this.getFailuresOf(item).size === 0; + if (!hasEverythingUploaded) return; + + referralService.trackFolderUpload(); + logNetworkInfoForUpload({ folderName: item.folder.name }); } - private async retry(topLevelItem: UploadItem, pickedFailedItem?: FailedUploadItem): Promise { - const retryableFailures = this.getFailuresOf(topLevelItem).filter(([, failedItem]) => failedItem.isRetryable); - const failuresToRetry = pickedFailedItem - ? retryableFailures.filter(([, failedItem]) => failedItem.id === pickedFailedItem.id) - : retryableFailures; + private async retry(topLevelItem: UploadItem, pickedItem?: UploadItem): Promise { + const taskId = this.getTaskIdOf(topLevelItem) as string; + const isTaskCancelled = tasksService.findTask(taskId)?.status === TaskStatus.Cancelled; + if (isTaskCancelled) return; + + const failures = this.getFailuresOf(topLevelItem); + const retryableFailures = [...failures].filter( + ([, failedItem]) => failedItem.isRetryable && !failedItem.isRetrying, + ); + const failuresToRetry = pickedItem ? retryableFailures.filter(([item]) => item === pickedItem) : retryableFailures; const hasNothingToRetry = failuresToRetry.length === 0; if (hasNothingToRetry) return; - for (const [item, failedItem] of failuresToRetry) this.failedItems.set(item, { ...failedItem, isRetrying: true }); + for (const [item, failedItem] of failuresToRetry) failures.set(item, { ...failedItem, isRetrying: true }); this.syncFailedItemsToTask(topLevelItem); - const taskId = this.getTaskIdOf(topLevelItem) as string; const rootFolder = this.rootFolders.get(topLevelItem); const progress = rootFolder ? tasksService.findTask(taskId)?.progress : 0; tasksService.updateTask({ taskId, merge: { status: TaskStatus.InProcess, subtitle: undefined, progress } }); const itemsToRetry = failuresToRetry.map(([item]) => item); - await this.manager.run(itemsToRetry); - - if (rootFolder) this.finishRootFolder(topLevelItem as UploadFolderItem, rootFolder); + await this.uploadAndFinish(itemsToRetry, [topLevelItem]); } - private getFailuresOf(topLevelItem: UploadItem): [UploadItem, FailedUploadItem][] { - return [...this.failedItems].filter(([item]) => getTopLevelItem(item) === topLevelItem); + private getFailuresOf(topLevelItem: UploadItem): Map { + return this.failuresByTopLevelItem.get(topLevelItem) as Map; } private recordFailure(item: UploadItem, failure: UploadFailure): void { - const failureId = this.failedItems.get(item)?.id ?? uniqueId(); - this.failedItems.set(item, toFailedUploadItem(failureId, item, failure)); - this.syncFailedItemsToTask(getTopLevelItem(item)); + const topLevelItem = getTopLevelItem(item); + const retry = () => this.retry(topLevelItem, item); + this.getFailuresOf(topLevelItem).set(item, toFailedUploadItem(item, failure, retry)); + this.syncFailedItemsToTask(topLevelItem); } private clearFailure(item: UploadItem): void { - const hadFailed = this.failedItems.delete(item); - if (hadFailed) this.syncFailedItemsToTask(getTopLevelItem(item)); + const topLevelItem = getTopLevelItem(item); + const hadFailed = this.getFailuresOf(topLevelItem).delete(item); + if (hadFailed) this.syncFailedItemsToTask(topLevelItem); } private syncFailedItemsToTask(topLevelItem: UploadItem): void { - const failedItems = this.getFailuresOf(topLevelItem).map(([, failedItem]) => failedItem); + const failedItems = [...this.getFailuresOf(topLevelItem).values()]; tasksService.updateTask({ taskId: this.getTaskIdOf(topLevelItem) as string, merge: { failedItems } }); } } diff --git a/src/app/upload/UploadManager.test.ts b/src/app/upload/UploadManager.test.ts index bcc33ac2cf..2d7e363828 100644 --- a/src/app/upload/UploadManager.test.ts +++ b/src/app/upload/UploadManager.test.ts @@ -273,16 +273,6 @@ describe('Uploading files', () => { expect(events.onItemCancelled).not.toHaveBeenCalled(); }); - test('When a file that is already uploading is uploaded again, then it is uploaded only once', async () => { - fakeSlowUploads(); - const report = getUploadFileItem('report'); - const manager = createManager(); - - await failIfHangs(Promise.all([manager.run([report]), manager.run([report])])); - - expect(startedUploadNames()).toEqual(['report']); - }); - test('When a single file is cancelled, then only that file stops', async () => { fakeSlowUploads(); const [cancelledItem, otherItem] = [getUploadFileItem('cancelled'), getUploadFileItem('other')]; diff --git a/src/app/upload/UploadManager.ts b/src/app/upload/UploadManager.ts index 705cffdc2a..78f745a032 100644 --- a/src/app/upload/UploadManager.ts +++ b/src/app/upload/UploadManager.ts @@ -95,7 +95,6 @@ export class UploadManager { private readonly largeFilesWaitingForFolders: UploadItem[] = []; private readonly cancelledItems = new WeakSet(); private readonly abortActiveUploadOf = new Map void>(); - private readonly itemsInProgress = new Set(); private result: UploadResult = { succeeded: [], failed: [] }; private readonly uploadQueue: AsyncPriorityQueue = priorityQueue((item, done) => { @@ -103,9 +102,6 @@ export class UploadManager { .catch((error) => errorService.reportError(error)) .finally(() => { if (isFolderItem(item)) this.finishFolder(); - - const isWaitingForFolders = this.largeFilesWaitingForFolders.includes(item); - if (!isWaitingForFolders) this.itemsInProgress.delete(item); done(); }); }, MAX_CONCURRENT_UPLOADS); @@ -124,12 +120,10 @@ export class UploadManager { this.uploadQueue.concurrency = MAX_CONCURRENT_UPLOADS; } - const itemsToUpload = items.filter((item) => !this.itemsInProgress.has(item)); - const hasNothingToWaitFor = itemsToUpload.length === 0 && this.uploadQueue.idle(); - if (hasNothingToWaitFor) return this.result; + const hasNothingToUpload = items.length === 0; + if (hasNothingToUpload) return this.result; - itemsToUpload.forEach((item) => this.itemsInProgress.add(item)); - this.queue(itemsToUpload); + this.queue(items); await this.uploadQueue.drain(); return this.result; diff --git a/src/app/upload/failedUploadItem.ts b/src/app/upload/failedUploadItem.ts index 001c5cb13a..3472abac00 100644 --- a/src/app/upload/failedUploadItem.ts +++ b/src/app/upload/failedUploadItem.ts @@ -2,21 +2,24 @@ import { UploadItem, UploadItemKind } from 'app/upload/types'; import { UploadFailure } from 'app/upload/uploadFailure'; import { FailedUploadItem } from 'app/tasks/types'; -export const toFailedUploadItem = (id: string, item: UploadItem, { isRetryable }: UploadFailure): FailedUploadItem => { +export const toFailedUploadItem = ( + item: UploadItem, + { isRetryable }: UploadFailure, + retry: () => Promise, +): FailedUploadItem => { if (item.kind === UploadItemKind.Folder) { return { - id, name: item.folder.name, size: 0, modifiedAt: Date.now(), isFolder: true, isRetryable, isRetrying: false, + retry, }; } return { - id, name: item.file.name, type: item.file.type, size: item.file.size, @@ -24,5 +27,6 @@ export const toFailedUploadItem = (id: string, item: UploadItem, { isRetryable } isFolder: false, isRetryable, isRetrying: false, + retry, }; }; From e6c25868341b7a156562f5ccbd303b528665c291 Mon Sep 17 00:00:00 2001 From: Xavier Abad Date: Fri, 6 Nov 2026 16:03:47 +0100 Subject: [PATCH 08/12] fix(upload): retry only finished tasks --- src/app/upload/Upload.test.ts | 10 ++++++++-- src/app/upload/Upload.ts | 9 ++++----- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/app/upload/Upload.test.ts b/src/app/upload/Upload.test.ts index ef5805e760..eaf9131cde 100644 --- a/src/app/upload/Upload.test.ts +++ b/src/app/upload/Upload.test.ts @@ -412,7 +412,7 @@ describe('Retrying what failed', () => { ]); }); - test('When the user retries everything while a file is already being retried, then that file is uploaded only once', async () => { + test('When the user retries while a retry of the same task is still running, then nothing else is uploaded', async () => { failUploadsOf(['retrying', 'waiting']); await createUpload( [ @@ -431,7 +431,13 @@ describe('Retrying what failed', () => { const retryingEverything = tasksService.findTask(folderTask.id)?.retry?.(); await Promise.all([retryingOne, retryingEverything]); - expect(uploadedNames()).toEqual(['retrying', 'waiting']); + expect(uploadedNames()).toEqual(['retrying']); + expect(tasksService.findTask(folderTask.id)).toEqual( + expect.objectContaining({ + status: TaskStatus.Success, + failedItems: [expect.objectContaining({ name: 'waiting' })], + }), + ); }); test('When a folder is only fully uploaded after a retry, then it is not counted as a folder upload', async () => { diff --git a/src/app/upload/Upload.ts b/src/app/upload/Upload.ts index 5c40ed7797..642d3f6a0c 100644 --- a/src/app/upload/Upload.ts +++ b/src/app/upload/Upload.ts @@ -306,13 +306,12 @@ export class Upload { private async retry(topLevelItem: UploadItem, pickedItem?: UploadItem): Promise { const taskId = this.getTaskIdOf(topLevelItem) as string; - const isTaskCancelled = tasksService.findTask(taskId)?.status === TaskStatus.Cancelled; - if (isTaskCancelled) return; + const status = tasksService.findTask(taskId)?.status; + const isTaskFinished = status === TaskStatus.Success || status === TaskStatus.Error; + if (!isTaskFinished) return; const failures = this.getFailuresOf(topLevelItem); - const retryableFailures = [...failures].filter( - ([, failedItem]) => failedItem.isRetryable && !failedItem.isRetrying, - ); + const retryableFailures = [...failures].filter(([, failedItem]) => failedItem.isRetryable); const failuresToRetry = pickedItem ? retryableFailures.filter(([item]) => item === pickedItem) : retryableFailures; const hasNothingToRetry = failuresToRetry.length === 0; From a563214c9c0eabe677c921df58949d89bb63a4aa Mon Sep 17 00:00:00 2001 From: Xavier Abad Date: Mon, 9 Nov 2026 14:22:07 +0100 Subject: [PATCH 09/12] refactor(upload): drop unused shared auth data --- src/app/upload/Upload.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/app/upload/Upload.ts b/src/app/upload/Upload.ts index 642d3f6a0c..1e9f2d04b2 100644 --- a/src/app/upload/Upload.ts +++ b/src/app/upload/Upload.ts @@ -150,8 +150,6 @@ export class Upload { } private trackFile(item: UploadFileItem): void { - const { ownerUserAuthenticationData, destinationFolderUuid } = this.props; - const taskId = tasksService.create({ action: TaskType.UploadFile, item: { uploadFile: item.file.content, parentFolderId: item.file.parentFolderId }, @@ -160,11 +158,6 @@ export class Upload { isFileNameValidated: true, showNotification: true, cancellable: true, - sharedItemAuthenticationData: ownerUserAuthenticationData && { - ownerUserAuthenticationData, - isDeepFolder: false, - currentFolderId: destinationFolderUuid, - }, stop: () => Promise.resolve(this.cancelItem(item)), retry: () => this.retry(item), }); From efddfe845c608db8a91871aa36991bf6e2120859 Mon Sep 17 00:00:00 2001 From: Xavier Abad Date: Mon, 9 Nov 2026 14:22:46 +0100 Subject: [PATCH 10/12] fix(upload): stop showing cancelled items retrying --- src/app/upload/Upload.test.ts | 34 ++++++++++++++++++++++++++++++++++ src/app/upload/Upload.ts | 2 ++ 2 files changed, 36 insertions(+) diff --git a/src/app/upload/Upload.test.ts b/src/app/upload/Upload.test.ts index eaf9131cde..e2c18d0cda 100644 --- a/src/app/upload/Upload.test.ts +++ b/src/app/upload/Upload.test.ts @@ -574,4 +574,38 @@ describe('Retrying what failed', () => { await retryingVideos; expect(tasksService.findTask(videosTask.id)?.status).toBe(TaskStatus.Success); }); + + test('When the user cancels a folder while its failed files are retried, then they are no longer shown as retrying', async () => { + failUploadsOf(['broken']); + await createUpload([getFolderToUpload('Photos', { childrenFiles: [getFileToUpload('broken.png')] })], []).run(); + fakeUploadsTaking(500); + const [folderTask] = tasksOf(TaskType.UploadFolder); + + const retrying = folderTask.retry?.(); + await vi.waitFor(() => + expect(tasksService.findTask(folderTask.id)?.failedItems).toEqual([ + expect.objectContaining({ name: 'broken', isRetrying: true }), + ]), + ); + await tasksService.cancelTask(folderTask.id); + await retrying; + + expect(tasksService.findTask(folderTask.id)?.failedItems).toEqual([]); + }); + + test('When the user cancels a failed file while it is retried, then it is no longer shown as retrying', async () => { + failUploadsOf(['report']); + await createUpload([], [getFileToUpload('report.pdf')]).run(); + fakeUploadsTaking(500); + const [fileTask] = tasksOf(TaskType.UploadFile); + + const retrying = fileTask.retry?.(); + await vi.waitFor(() => expect(uploadFile).toHaveBeenCalledTimes(2)); + await tasksService.cancelTask(fileTask.id); + await retrying; + + expect(tasksService.findTask(fileTask.id)).toEqual( + expect.objectContaining({ status: TaskStatus.Cancelled, failedItems: [] }), + ); + }); }); diff --git a/src/app/upload/Upload.ts b/src/app/upload/Upload.ts index 1e9f2d04b2..6e086285ca 100644 --- a/src/app/upload/Upload.ts +++ b/src/app/upload/Upload.ts @@ -245,6 +245,8 @@ export class Upload { } private onItemCancelled(item: UploadItem): void { + this.clearFailure(item); + const taskId = this.getTaskIdOf(item); const isFileWithItsOwnTask = !!taskId && item.kind === UploadItemKind.File; if (!isFileWithItsOwnTask) return; From f37e4fbb0f41fd8e17a006a53f2f52741926abdf Mon Sep 17 00:00:00 2001 From: Xavier Abad Date: Mon, 9 Nov 2026 14:23:17 +0100 Subject: [PATCH 11/12] fix(upload): finish a folder after its last run --- src/app/upload/Upload.test.ts | 35 +++++++++++++++++++++++++++++++++++ src/app/upload/Upload.ts | 33 ++++++++++++++++++++++++++------- 2 files changed, 61 insertions(+), 7 deletions(-) diff --git a/src/app/upload/Upload.test.ts b/src/app/upload/Upload.test.ts index e2c18d0cda..d203cafdb8 100644 --- a/src/app/upload/Upload.test.ts +++ b/src/app/upload/Upload.test.ts @@ -9,6 +9,7 @@ import errorService from 'services/error.service'; import referralService from 'services/referral.service'; import { AppError } from '@internxt/sdk'; import { HTTP_STATUS_CODES } from 'app/core/constants'; +import { MAX_UPLOAD_ATTEMPTS } from 'app/network/networkConstants'; import { getCreatedFolder, getFileToUpload, @@ -575,6 +576,40 @@ describe('Retrying what failed', () => { expect(tasksService.findTask(videosTask.id)?.status).toBe(TaskStatus.Success); }); + test('When a folder is retried while the rest of the upload is still running, then it only finishes once its retry does', async () => { + let failedPhotosCreations = 0; + vi.mocked(folderService.createFolderByUuid).mockImplementation((_parentUuid, name) => { + const shouldFail = name === 'Photos' && failedPhotosCreations < MAX_UPLOAD_ATTEMPTS; + if (shouldFail) failedPhotosCreations++; + const creation = shouldFail ? Promise.reject(new Error('Server error')) : Promise.resolve(getCreatedFolder(name)); + return [creation, { cancel: vi.fn() }] as never; + }); + (uploadFile as Mock).mockImplementation(async (file: { name: string }) => { + const uploadTimeMs = file.name === 'movie' ? 300 : 600; + await new Promise((resolve) => setTimeout(resolve, uploadTimeMs)); + return { uuid: `${file.name}-uuid` }; + }); + const upload = createUpload( + [ + getFolderToUpload('Photos', { childrenFiles: [getFileToUpload('beach.png')] }), + getFolderToUpload('Videos', { childrenFiles: [getFileToUpload('movie.mp4')] }), + ], + [], + ); + const running = upload.run(); + const [photosTask, videosTask] = tasksOf(TaskType.UploadFolder); + await vi.waitFor(() => expect(tasksService.findTask(photosTask.id)?.status).toBe(TaskStatus.Error)); + + const retryingPhotos = photosTask.retry?.(); + await running; + + expect(tasksService.findTask(videosTask.id)?.status).toBe(TaskStatus.Success); + expect(tasksService.findTask(photosTask.id)?.status).toBe(TaskStatus.InProcess); + expect(referralService.trackFolderUpload).toHaveBeenCalledOnce(); + await retryingPhotos; + expect(tasksService.findTask(photosTask.id)?.status).toBe(TaskStatus.Success); + }); + test('When the user cancels a folder while its failed files are retried, then they are no longer shown as retrying', async () => { failUploadsOf(['broken']); await createUpload([getFolderToUpload('Photos', { childrenFiles: [getFileToUpload('broken.png')] })], []).run(); diff --git a/src/app/upload/Upload.ts b/src/app/upload/Upload.ts index 6e086285ca..3d0b1f12ba 100644 --- a/src/app/upload/Upload.ts +++ b/src/app/upload/Upload.ts @@ -46,6 +46,7 @@ interface RootFolderUpload { totalItems: number; uploadedItems: number; isCancelled: boolean; + activeRuns: number; createdFolder?: DriveFolderData; } @@ -85,21 +86,33 @@ export class Upload { return result; } + /** + * A root folder can be in more than one run at a time (the upload and a retry of what failed in it), so it + * only finishes when its last run does. + */ private async uploadAndFinish(items: UploadItem[], topLevelItems: UploadItem[]): Promise { + const rootFoldersInRun = topLevelItems.filter((item) => this.rootFolders.has(item)) as UploadFolderItem[]; + rootFoldersInRun.forEach((item) => this.getRootFolder(item).activeRuns++); + const manager = this.createManager(); this.activeManagers.add(manager); const result = await manager.run(items); this.activeManagers.delete(manager); await Promise.all(this.cancelledRootFolderDeletions); - for (const topLevelItem of topLevelItems) { - const rootFolder = this.rootFolders.get(topLevelItem); - if (rootFolder) this.finishRootFolder(topLevelItem as UploadFolderItem, rootFolder); + for (const item of rootFoldersInRun) { + const rootFolder = this.getRootFolder(item); + rootFolder.activeRuns--; + if (rootFolder.activeRuns === 0) this.finishRootFolder(item, rootFolder); } return result; } + private getRootFolder(item: UploadItem): RootFolderUpload { + return this.rootFolders.get(item) as RootFolderUpload; + } + private createManager(): UploadManager { return new UploadManager({ ownerUserAuthenticationData: this.props.ownerUserAuthenticationData, @@ -146,6 +159,7 @@ export class Upload { totalItems: countItemsIn(item.folder), uploadedItems: 0, isCancelled: false, + activeRuns: 0, }); } @@ -208,7 +222,7 @@ export class Upload { private countUploadedInRootFolder(item: UploadItem, uploadedItem: UploadedItem): void { const topLevelItem = getTopLevelItem(item); - const rootFolder = this.rootFolders.get(topLevelItem) as RootFolderUpload; + const rootFolder = this.getRootFolder(topLevelItem); const isRootFolderItself = item === topLevelItem; if (isRootFolderItself) { @@ -256,7 +270,7 @@ export class Upload { } private cancelRootFolder(item: UploadFolderItem): void { - const rootFolder = this.rootFolders.get(item) as RootFolderUpload; + const rootFolder = this.getRootFolder(item); rootFolder.isCancelled = true; this.cancelItem(item); if (rootFolder.createdFolder) { @@ -291,8 +305,13 @@ export class Upload { }); } - private trackFolderUpload(item: UploadFolderItem, { isCancelled, createdFolder }: RootFolderUpload): void { - const hasEverythingUploaded = !isCancelled && !!createdFolder && this.getFailuresOf(item).size === 0; + private trackFolderUpload( + item: UploadFolderItem, + { isCancelled, activeRuns, createdFolder }: RootFolderUpload, + ): void { + const isStillUploading = activeRuns > 0; + const hasEverythingUploaded = + !isCancelled && !isStillUploading && !!createdFolder && this.getFailuresOf(item).size === 0; if (!hasEverythingUploaded) return; referralService.trackFolderUpload(); From d9908864febaf978f8bc128b23b409524abc9b7a Mon Sep 17 00:00:00 2001 From: Xavier Abad Date: Wed, 11 Nov 2026 10:41:27 +0100 Subject: [PATCH 12/12] fix(upload): track a folder completed by a retry --- src/app/upload/Upload.test.ts | 4 ++-- src/app/upload/Upload.ts | 19 ++++++------------- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/src/app/upload/Upload.test.ts b/src/app/upload/Upload.test.ts index d203cafdb8..9c16de45c2 100644 --- a/src/app/upload/Upload.test.ts +++ b/src/app/upload/Upload.test.ts @@ -441,7 +441,7 @@ describe('Retrying what failed', () => { ); }); - test('When a folder is only fully uploaded after a retry, then it is not counted as a folder upload', async () => { + test('When a folder is only fully uploaded after a retry, then it is counted as a folder upload', async () => { failUploadsOf(['report']); await createUpload([getFolderToUpload('Docs', { childrenFiles: [getFileToUpload('report.pdf')] })], []).run(); uploadSucceeds(); @@ -449,7 +449,7 @@ describe('Retrying what failed', () => { await tasksOf(TaskType.UploadFolder)[0].retry?.(); expect(tasksOf(TaskType.UploadFolder)[0].failedItems).toEqual([]); - expect(referralService.trackFolderUpload).not.toHaveBeenCalled(); + expect(referralService.trackFolderUpload).toHaveBeenCalledOnce(); }); test('When a folder that could not be created is retried, then it is created with all its content', async () => { diff --git a/src/app/upload/Upload.ts b/src/app/upload/Upload.ts index 3d0b1f12ba..7370ed6083 100644 --- a/src/app/upload/Upload.ts +++ b/src/app/upload/Upload.ts @@ -80,10 +80,8 @@ export class Upload { } } - async run(): Promise { - const result = await this.uploadAndFinish(this.items, this.items); - this.rootFolders.forEach((rootFolder, item) => this.trackFolderUpload(item as UploadFolderItem, rootFolder)); - return result; + run(): Promise { + return this.uploadAndFinish(this.items, this.items); } /** @@ -303,17 +301,12 @@ export class Upload { taskId: rootFolder.taskId, merge: { status: TaskStatus.Success, itemUUID: { rootFolderUUID: createdFolder.uuid } }, }); - } - private trackFolderUpload( - item: UploadFolderItem, - { isCancelled, activeRuns, createdFolder }: RootFolderUpload, - ): void { - const isStillUploading = activeRuns > 0; - const hasEverythingUploaded = - !isCancelled && !isStillUploading && !!createdFolder && this.getFailuresOf(item).size === 0; - if (!hasEverythingUploaded) return; + const hasEverythingUploaded = this.getFailuresOf(item).size === 0; + if (hasEverythingUploaded) this.trackFolderUpload(item); + } + private trackFolderUpload(item: UploadFolderItem): void { referralService.trackFolderUpload(); logNetworkInfoForUpload({ folderName: item.folder.name }); }