Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
654605e
refactor(upload): let the user retry what failed in an upload, upload…
xabg2 Nov 2, 2026
4e097a9
Merge branch 'refactor/upload-action' into refactor/upload-action-retry
xabg2 Nov 3, 2026
a7a45d5
Merge branch 'refactor/upload-action' into refactor/upload-action-retry
xabg2 Nov 4, 2026
e744b38
refactor(upload): give each task its retry when it is created
xabg2 Nov 4, 2026
9682335
fix(upload): retry everything only retries the failures that retrying…
xabg2 Nov 4, 2026
15eb1a4
refactor(upload): let the user retry what failed in an upload, upload…
xabg2 Nov 2, 2026
9e45bde
refactor(upload): give each task its retry when it is created
xabg2 Nov 4, 2026
5bf545f
fix(upload): retry everything only retries the failures that retrying…
xabg2 Nov 4, 2026
baf222d
Merge branch 'refactor/upload-action-retry' of github.com:internxt/dr…
xabg2 Nov 6, 2026
0ee4887
Merge branch 'refactor/upload-action' into refactor/upload-action-retry
xabg2 Nov 6, 2026
cd0c921
refactor(upload): simplify the retry flow
xabg2 Nov 6, 2026
e6c2586
fix(upload): retry only finished tasks
xabg2 Nov 6, 2026
a563214
refactor(upload): drop unused shared auth data
xabg2 Nov 9, 2026
efddfe8
fix(upload): stop showing cancelled items retrying
xabg2 Nov 9, 2026
f37e4fb
fix(upload): finish a folder after its last run
xabg2 Nov 9, 2026
2d07964
Merge branch 'refactor/upload-action' into refactor/upload-action-retry
xabg2 Nov 9, 2026
d990886
fix(upload): track a folder completed by a retry
xabg2 Nov 11, 2026
b14759d
Merge branch 'refactor/upload-action' into refactor/upload-action-retry
xabg2 Nov 11, 2026
5ccb6e9
Merge branch 'refactor/upload-action' into refactor/upload-action-retry
xabg2 Nov 12, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/app/tasks/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -51,6 +62,8 @@ export interface BaseTask {
downloadedProgress?: number;
subtitle?: string;
stop?: () => Promise<void>;
failedItems?: FailedUploadItem[];
retry?: (failedItem?: FailedUploadItem) => Promise<void>;
}

export interface CreateFolderTask extends BaseTask {
Expand Down
210 changes: 204 additions & 6 deletions src/app/upload/Upload.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,9 @@ const createUpload = (
callbacks: Partial<ConstructorParameters<typeof Upload>[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(() => {
Expand Down Expand Up @@ -274,6 +270,198 @@ 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, 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(
[
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();
Expand All @@ -283,4 +471,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);
});
});
64 changes: 57 additions & 7 deletions src/app/upload/Upload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@ import {
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, 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 {
destinationFolderUuid: string;
Expand All @@ -45,7 +47,7 @@ interface RootFolderUpload {
totalItems: number;
uploadedItems: number;
isCancelled: boolean;
hasFailedItems: boolean;
isUploadTracked: boolean;
createdFolder?: DriveFolderData;
}

Expand All @@ -66,6 +68,7 @@ export class Upload {
private readonly taskIdByItem = new Map<UploadItem, string>();
private readonly rootFolders = new Map<UploadItem, RootFolderUpload>();
private readonly cancelledRootFolderDeletions: Promise<void>[] = [];
private readonly failedItems = new Map<UploadItem, FailedUploadItem>();

constructor(props: UploadProps) {
this.props = props;
Expand Down Expand Up @@ -114,6 +117,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);
Expand All @@ -122,7 +126,7 @@ export class Upload {
totalItems: countItemsIn(item.folder),
uploadedItems: 0,
isCancelled: false,
hasFailedItems: false,
isUploadTracked: false,
});
}

Expand All @@ -143,6 +147,7 @@ export class Upload {
currentFolderId: destinationFolderUuid,
},
stop: () => Promise.resolve(this.manager.cancelItem(item)),
retry: (failedItem) => this.retry(item, failedItem),
});

this.taskIdByItem.set(item, taskId);
Expand Down Expand Up @@ -180,6 +185,7 @@ export class Upload {
else this.countUploadedInRootFolder(item, uploadedItem);

const isFile = item.kind === UploadItemKind.File;
this.clearFailure(item);
this.props.onItemUploaded?.(uploadedItem, getParentFolderUuid(item), !isFile);

if (isFile) {
Expand Down Expand Up @@ -217,8 +223,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;
Expand Down Expand Up @@ -271,10 +276,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<void> {
const retryableFailures = this.getFailuresOf(topLevelItem).filter(([, failedItem]) => failedItem.isRetryable);
const failuresToRetry = pickedFailedItem
? retryableFailures.filter(([, failedItem]) => failedItem.id === pickedFailedItem.id)
: retryableFailures;

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) 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 } });
Comment thread
xabg2 marked this conversation as resolved.
Resolved

const itemsToRetry = failuresToRetry.map(([item]) => item);
await this.manager.run(itemsToRetry);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

manager.run waits for the whole queue to drain, so retrying one file of folder A keeps A in process until folder B finishes too, even if the file was uploaded long before. I'd wait only for the retried items here, so each task finishes when its own items do.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since each retry has its own manager, it only waits for its own items.


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 {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every failure and every success calls syncFailedItemsToTask, which goes through the whole map and sends a new array to the task. With storage full in a big folder that's quadratic: 3000 failed files end up in 3000 task updates and 4.5M items copied. What about keeping the failures grouped by top-level item, or syncing them once when the run finishes?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failures are now grouped by top-level item: with storage full in one folder, 10,000 files went from 2.5 s to 0.57 s. We kept the live sync so the list updates during the upload.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Live updates make sense. Throttling the sync (e.g. every 200 ms) keeps them live and avoids 3003 updates for 3000 failures in one folder.

const failedItems = this.getFailuresOf(topLevelItem).map(([, failedItem]) => failedItem);
tasksService.updateTask({ taskId: this.getTaskIdOf(topLevelItem) as string, merge: { failedItems } });
}
}
Loading
Loading