From 0d0e2ccf4ebbc91d35260830367e031ca6f0eb95 Mon Sep 17 00:00:00 2001 From: Esteban Galvis Date: Wed, 13 May 2026 17:29:05 -0500 Subject: [PATCH 1/5] Refactor error handling in backup upload and folder creation processes --- .../main/mainProcessSharedInfraContainer.ts | 2 +- .../upload/backup-upload-error-handler.ts | 28 ------- .../features/backup/upload/constants.ts | 4 - .../backup/upload/update-file-to-backup.ts | 6 +- .../upload/upload-content-to-environment.ts | 27 +------ .../backup/upload/upload-file-to-backup.ts | 6 +- .../update-virtual-drive-container.service.ts | 11 ++- src/context/shared/application/constants.ts | 3 + .../transient-error-handler.test.ts} | 34 ++++----- .../application/transient-error-handler.ts | 64 ++++++++++++++++ .../upload/TemporalFileUploader.test.ts | 72 ++++++++++++++++++ .../upload/TemporalFileUploader.ts | 71 ++++++++++++------ .../application/create/FileCreator.test.ts | 42 ++++++++++- .../files/application/create/FileCreator.ts | 34 ++++++--- .../SDKRemoteFileSystem.test.ts | 75 +++++++++++++++++++ .../infrastructure/SDKRemoteFileSystem.ts | 9 ++- .../__mocks__/FolderRemoteFileSystemMock.ts | 20 ++--- .../FolderRepositorySynchronizer.test.ts | 23 +++++- .../FolderRepositorySynchronizer.ts | 4 +- .../application/create/FolderCreator.test.ts | 65 +++++++++++++++- .../application/create/FolderCreator.ts | 22 ++++-- .../domain/file-systems/RemoteFileSystem.ts | 7 +- .../infrastructure/HttpRemoteFileSystem.ts | 31 ++++---- 23 files changed, 496 insertions(+), 164 deletions(-) delete mode 100644 src/backend/features/backup/upload/backup-upload-error-handler.ts create mode 100644 src/context/shared/application/constants.ts rename src/{backend/features/backup/upload/backup-upload-error-handler.test.ts => context/shared/application/transient-error-handler.test.ts} (59%) create mode 100644 src/context/shared/application/transient-error-handler.ts create mode 100644 src/context/storage/TemporalFiles/application/upload/TemporalFileUploader.test.ts create mode 100644 src/context/virtual-drive/files/infrastructure/SDKRemoteFileSystem.test.ts diff --git a/src/apps/shared/dependency-injection/main/mainProcessSharedInfraContainer.ts b/src/apps/shared/dependency-injection/main/mainProcessSharedInfraContainer.ts index 52339a8dd7..ec0eddac90 100644 --- a/src/apps/shared/dependency-injection/main/mainProcessSharedInfraContainer.ts +++ b/src/apps/shared/dependency-injection/main/mainProcessSharedInfraContainer.ts @@ -14,7 +14,7 @@ export async function mainProcessSharedInfraBuilder(): Promise builder.register(UploadProgressTracker).use(MainProcessUploadProgressTracker).private(); - builder.register(RemoteItemsGenerator).use(SQLiteRemoteItemsGenerator).private(); + builder.register(RemoteItemsGenerator).use(SQLiteRemoteItemsGenerator); return builder; } diff --git a/src/backend/features/backup/upload/backup-upload-error-handler.ts b/src/backend/features/backup/upload/backup-upload-error-handler.ts deleted file mode 100644 index 0522a40a1b..0000000000 --- a/src/backend/features/backup/upload/backup-upload-error-handler.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { DriveDesktopError } from '../../../../context/shared/domain/errors/DriveDesktopError'; -import { logger } from '@internxt/drive-desktop-core/build/backend'; -import { INITIAL_RATE_LIMIT_DELAY_MS, MAX_BACKOFF_MS, RETRY_DELAYS_MS } from './constants'; - -function exponentialBackoff(attempts: number, baseMs: number): number { - return Math.min(baseMs * Math.pow(2, attempts - 1), MAX_BACKOFF_MS); -} - -export function createBackupUploadErrorHandler(path: string): (error: DriveDesktopError) => number | null { - let transientAttempts = 0; - - return (error: DriveDesktopError): number | null => { - if (error.cause === 'RATE_LIMITED' || error.cause === 'INTERNAL_SERVER_ERROR') { - transientAttempts++; - const base = - error.cause === 'RATE_LIMITED' ? Number(error.message) || INITIAL_RATE_LIMIT_DELAY_MS : RETRY_DELAYS_MS[0]; - const delayMs = exponentialBackoff(transientAttempts, base); - logger.debug({ - tag: 'BACKUPS', - msg: `[${error.cause}] Attempt ${transientAttempts}, waiting ${delayMs}ms`, - path, - }); - return delayMs; - } - - return null; - }; -} diff --git a/src/backend/features/backup/upload/constants.ts b/src/backend/features/backup/upload/constants.ts index d6b001b6c1..c1f9df8015 100644 --- a/src/backend/features/backup/upload/constants.ts +++ b/src/backend/features/backup/upload/constants.ts @@ -1,5 +1 @@ export const DEFAULT_CONCURRENCY = 6; -export const MAX_RETRIES = 3; -export const RETRY_DELAYS_MS = [1000, 2000, 4000]; -export const INITIAL_RATE_LIMIT_DELAY_MS = 30_000; -export const MAX_BACKOFF_MS = 480_000; diff --git a/src/backend/features/backup/upload/update-file-to-backup.ts b/src/backend/features/backup/upload/update-file-to-backup.ts index 0143f30126..5638af1cc2 100644 --- a/src/backend/features/backup/upload/update-file-to-backup.ts +++ b/src/backend/features/backup/upload/update-file-to-backup.ts @@ -3,7 +3,7 @@ import { DriveDesktopError } from '../../../../context/shared/domain/errors/Driv import { Result } from '../../../../context/shared/domain/Result'; import { uploadContentToEnvironment } from './upload-content-to-environment'; import { retryWithBackoff } from '../../../../shared/retry-with-backoff'; -import { createBackupUploadErrorHandler } from './backup-upload-error-handler'; +import { createTransientErrorHandler } from '../../../../context/shared/application/transient-error-handler'; import { overrideFileToBackend } from './override-file-to-backend'; export type UpdateFileParams = { @@ -25,7 +25,7 @@ async function updateFile(file: UpdateFileParams): Promise= 500) { - return new DriveDesktopError('INTERNAL_SERVER_ERROR'); - } - } - return new DriveDesktopError('UNKNOWN'); -} export async function uploadContentToEnvironment({ path, @@ -66,7 +47,7 @@ export async function uploadContentToEnvironment({ readable.on('error', (err: Error & { code?: unknown; status?: unknown }) => { logger.error({ tag: 'BACKUPS', msg: '[ENVLFU READ STREAM ERROR]', err, path }); - resolveOnce({ error: mapUploadError(err) }); + resolveOnce({ error: mapEnvironmentUploadError(err) }); }); const state = uploadFn(bucket, { @@ -77,7 +58,7 @@ export async function uploadContentToEnvironment({ if (err) { logger.error({ tag: 'BACKUPS', msg: '[ENVLFU UPLOAD ERROR]', err }); - return resolveOnce({ error: mapUploadError(err) }); + return resolveOnce({ error: mapEnvironmentUploadError(err) }); } if (!contentsId) { @@ -103,7 +84,7 @@ export async function uploadContentToEnvironment({ }); } catch (err) { if (isError(err)) { - return { error: mapUploadError(err) }; + return { error: mapEnvironmentUploadError(err) }; } return { error: new DriveDesktopError('UNKNOWN') }; } diff --git a/src/backend/features/backup/upload/upload-file-to-backup.ts b/src/backend/features/backup/upload/upload-file-to-backup.ts index 1db8f6deb4..556caa21ea 100644 --- a/src/backend/features/backup/upload/upload-file-to-backup.ts +++ b/src/backend/features/backup/upload/upload-file-to-backup.ts @@ -7,7 +7,7 @@ import { uploadContentToEnvironment } from './upload-content-to-environment'; import { Result } from '../../../../context/shared/domain/Result'; import { deleteFileFromStorageByFileId } from '../../../../infra/drive-server/services/files/services/delete-file-content-from-bucket'; import { retryWithBackoff } from '../../../../shared/retry-with-backoff'; -import { createBackupUploadErrorHandler } from './backup-upload-error-handler'; +import { createTransientErrorHandler } from '../../../../context/shared/application/transient-error-handler'; export type UploadFileParams = { path: string; @@ -29,7 +29,7 @@ async function uploadFile(file: UploadFileParams): Promise f.id)); + await Promise.all([ container.get(FileRepositorySynchronizer).run(tree.files), - container.get(FolderRepositorySynchronizer).run(tree.folders), + container.get(FolderRepositorySynchronizer).run(tree.folders, allRemoteFolderIds), container.get(StorageRemoteChangesSyncher).run(), ]); logger.debug({ msg: '[VIRTUAL DRIVE] Tree updated successfully' }); diff --git a/src/context/shared/application/constants.ts b/src/context/shared/application/constants.ts new file mode 100644 index 0000000000..c7bed5179c --- /dev/null +++ b/src/context/shared/application/constants.ts @@ -0,0 +1,3 @@ +export const INITIAL_RATE_LIMIT_DELAY_MS = 30_000; +export const INITIAL_SERVER_ERROR_DELAY_MS = 1_000; +export const MAX_BACKOFF_MS = 480_000; diff --git a/src/backend/features/backup/upload/backup-upload-error-handler.test.ts b/src/context/shared/application/transient-error-handler.test.ts similarity index 59% rename from src/backend/features/backup/upload/backup-upload-error-handler.test.ts rename to src/context/shared/application/transient-error-handler.test.ts index 18f21506ea..dbfd3a356a 100644 --- a/src/backend/features/backup/upload/backup-upload-error-handler.test.ts +++ b/src/context/shared/application/transient-error-handler.test.ts @@ -1,10 +1,10 @@ -import { DriveDesktopError } from '../../../../context/shared/domain/errors/DriveDesktopError'; -import { createBackupUploadErrorHandler } from './backup-upload-error-handler'; -import { INITIAL_RATE_LIMIT_DELAY_MS, MAX_BACKOFF_MS, RETRY_DELAYS_MS } from './constants'; +import { DriveDesktopError } from '../domain/errors/DriveDesktopError'; +import { createTransientErrorHandler } from './transient-error-handler'; +import { INITIAL_RATE_LIMIT_DELAY_MS, INITIAL_SERVER_ERROR_DELAY_MS, MAX_BACKOFF_MS } from './constants'; -describe('createBackupUploadErrorHandler', () => { +describe('createTransientErrorHandler', () => { it('should return null for non-retryable errors', () => { - const handler = createBackupUploadErrorHandler('/file.txt'); + const handler = createTransientErrorHandler({ tag: 'BACKUPS', context: 'TEST', path: '/file.txt' }); expect(handler(new DriveDesktopError('UNKNOWN'))).toBeNull(); expect(handler(new DriveDesktopError('NOT_ENOUGH_SPACE'))).toBeNull(); @@ -12,16 +12,16 @@ describe('createBackupUploadErrorHandler', () => { }); it('should return exponential backoff delay for INTERNAL_SERVER_ERROR', () => { - const handler = createBackupUploadErrorHandler('/file.txt'); + const handler = createTransientErrorHandler({ tag: 'BACKUPS', context: 'TEST', path: '/file.txt' }); const error = new DriveDesktopError('INTERNAL_SERVER_ERROR'); - expect(handler(error)).toBe(RETRY_DELAYS_MS[0] * Math.pow(2, 0)); // attempt 1: 1000ms - expect(handler(error)).toBe(RETRY_DELAYS_MS[0] * Math.pow(2, 1)); // attempt 2: 2000ms - expect(handler(error)).toBe(RETRY_DELAYS_MS[0] * Math.pow(2, 2)); // attempt 3: 4000ms + expect(handler(error)).toBe(INITIAL_SERVER_ERROR_DELAY_MS * Math.pow(2, 0)); // attempt 1: 1000ms + expect(handler(error)).toBe(INITIAL_SERVER_ERROR_DELAY_MS * Math.pow(2, 1)); // attempt 2: 2000ms + expect(handler(error)).toBe(INITIAL_SERVER_ERROR_DELAY_MS * Math.pow(2, 2)); // attempt 3: 4000ms }); it('should cap INTERNAL_SERVER_ERROR delay at MAX_BACKOFF_MS', () => { - const handler = createBackupUploadErrorHandler('/file.txt'); + const handler = createTransientErrorHandler({ tag: 'SYNC-ENGINE', context: 'TEST', path: '/file.txt' }); const error = new DriveDesktopError('INTERNAL_SERVER_ERROR'); // base=1000, cap=480000 → attempt 9: 256000ms, attempt 10: 512000ms → capped @@ -32,14 +32,14 @@ describe('createBackupUploadErrorHandler', () => { it('should use retry_after from RATE_LIMITED message as base delay', () => { const retryAfterMs = 60_000; - const handler = createBackupUploadErrorHandler('/file.txt'); + const handler = createTransientErrorHandler({ tag: 'BACKUPS', context: 'TEST', path: '/file.txt' }); const error = new DriveDesktopError('RATE_LIMITED', String(retryAfterMs)); expect(handler(error)).toBe(retryAfterMs * Math.pow(2, 0)); // attempt 1: 60000ms }); it('should fall back to INITIAL_RATE_LIMIT_DELAY_MS when RATE_LIMITED message is not a number', () => { - const handler = createBackupUploadErrorHandler('/file.txt'); + const handler = createTransientErrorHandler({ tag: 'SYNC-ENGINE', context: 'TEST', path: '/file.txt' }); const error = new DriveDesktopError('RATE_LIMITED', 'not-a-number'); expect(handler(error)).toBe(INITIAL_RATE_LIMIT_DELAY_MS); @@ -47,7 +47,7 @@ describe('createBackupUploadErrorHandler', () => { it('should apply exponential backoff across multiple RATE_LIMITED retries', () => { const retryAfterMs = 10_000; - const handler = createBackupUploadErrorHandler('/file.txt'); + const handler = createTransientErrorHandler({ tag: 'BACKUPS', context: 'TEST', path: '/file.txt' }); const error = new DriveDesktopError('RATE_LIMITED', String(retryAfterMs)); expect(handler(error)).toBe(retryAfterMs * Math.pow(2, 0)); // attempt 1: 10000ms @@ -55,7 +55,7 @@ describe('createBackupUploadErrorHandler', () => { }); it('should share attempt counter between RATE_LIMITED and INTERNAL_SERVER_ERROR', () => { - const handler = createBackupUploadErrorHandler('/file.txt'); + const handler = createTransientErrorHandler({ tag: 'SYNC-ENGINE', context: 'TEST', path: '/file.txt' }); handler(new DriveDesktopError('INTERNAL_SERVER_ERROR')); // attempt 1, base=1000 → 1000ms const delay = handler(new DriveDesktopError('RATE_LIMITED', String(INITIAL_RATE_LIMIT_DELAY_MS))); // attempt 2, base=30000 → 60000ms @@ -64,14 +64,14 @@ describe('createBackupUploadErrorHandler', () => { }); it('should create independent state per handler instance', () => { - const handler1 = createBackupUploadErrorHandler('/file1.txt'); - const handler2 = createBackupUploadErrorHandler('/file2.txt'); + const handler1 = createTransientErrorHandler({ tag: 'BACKUPS', context: 'TEST', path: '/file1.txt' }); + const handler2 = createTransientErrorHandler({ tag: 'SYNC-ENGINE', context: 'TEST', path: '/file2.txt' }); const error = new DriveDesktopError('INTERNAL_SERVER_ERROR'); handler1(error); // advance handler1 to attempt 1 handler1(error); // advance handler1 to attempt 2 // handler2 should start fresh at attempt 1 - expect(handler2(error)).toBe(RETRY_DELAYS_MS[0]); + expect(handler2(error)).toBe(INITIAL_SERVER_ERROR_DELAY_MS); }); }); diff --git a/src/context/shared/application/transient-error-handler.ts b/src/context/shared/application/transient-error-handler.ts new file mode 100644 index 0000000000..1ded91dc1f --- /dev/null +++ b/src/context/shared/application/transient-error-handler.ts @@ -0,0 +1,64 @@ +import { logger } from '@internxt/drive-desktop-core/build/backend'; +import { DriveDesktopError } from '../domain/errors/DriveDesktopError'; +import { extractPropertyFromStringyfiedJson } from '../../../shared/extract-property-from-json'; +import { INITIAL_RATE_LIMIT_DELAY_MS, INITIAL_SERVER_ERROR_DELAY_MS, MAX_BACKOFF_MS } from './constants'; + +export function parseRetryAfterMs(message?: string) { + const retryAfterSeconds = extractPropertyFromStringyfiedJson(message ?? '', 'retry_after'); + return typeof retryAfterSeconds === 'number' ? retryAfterSeconds * 1000 : INITIAL_RATE_LIMIT_DELAY_MS; +} + +export function mapEnvironmentUploadError(err: Error & { status?: unknown }): DriveDesktopError { + if (err.message === 'Max space used') { + return new DriveDesktopError('NOT_ENOUGH_SPACE'); + } + if (typeof err.status === 'number') { + if (err.status === 429) { + return new DriveDesktopError('RATE_LIMITED', String(parseRetryAfterMs(err.message))); + } + if (err.status >= 500) { + return new DriveDesktopError('INTERNAL_SERVER_ERROR'); + } + } + return new DriveDesktopError('UNKNOWN', err.message); +} + +function exponentialBackoff(attempts: number, baseMs: number) { + return Math.min(baseMs * Math.pow(2, attempts - 1), MAX_BACKOFF_MS); +} + +type Props = { + tag: 'BACKUPS' | 'SYNC-ENGINE'; + context: string; + path: string; +}; + +export function createTransientErrorHandler({ tag, context, path }: Props) { + let transientAttempts = 0; + + return (error: DriveDesktopError): number | null => { + if (error.cause === 'RATE_LIMITED' || error.cause === 'INTERNAL_SERVER_ERROR') { + transientAttempts++; + + const baseDelayMs = + error.cause === 'RATE_LIMITED' + ? Number(error.message) || INITIAL_RATE_LIMIT_DELAY_MS + : INITIAL_SERVER_ERROR_DELAY_MS; + + const delayMs = exponentialBackoff(transientAttempts, baseDelayMs); + + logger.debug({ + tag, + msg: `[${context}]`, + cause: error.cause, + attempt: transientAttempts, + delayMs, + path, + }); + + return delayMs; + } + + return null; + }; +} diff --git a/src/context/storage/TemporalFiles/application/upload/TemporalFileUploader.test.ts b/src/context/storage/TemporalFiles/application/upload/TemporalFileUploader.test.ts new file mode 100644 index 0000000000..3cdb0c9c70 --- /dev/null +++ b/src/context/storage/TemporalFiles/application/upload/TemporalFileUploader.test.ts @@ -0,0 +1,72 @@ +import { Readable } from 'stream'; +import { mockDeep } from 'vitest-mock-extended'; +import { call, calls } from '../../../../../../tests/vitest/utils.helper'; +import { TemporalFileUploader } from './TemporalFileUploader'; +import { TemporalFileRepository } from '../../domain/TemporalFileRepository'; +import { TemporalFileUploaderFactory } from '../../domain/upload/TemporalFileUploaderFactory'; +import { EventBus } from '../../../../virtual-drive/shared/domain/EventBus'; +import { TemporalFile } from '../../domain/TemporalFile'; + +describe('TemporalFileUploader', () => { + const repository = mockDeep(); + const uploaderFactory = mockDeep(); + const eventBus = mockDeep(); + + const temporalFile = TemporalFile.from({ + path: '/Documents/report.txt', + size: 100, + createdAt: new Date(), + modifiedAt: new Date(), + }); + + const stopWatching = vi.fn(); + + beforeEach(() => { + vi.resetAllMocks(); + + repository.watchFile.mockReturnValue(stopWatching); + eventBus.publish.mockResolvedValue(undefined); + + uploaderFactory.read.mockReturnValue(uploaderFactory); + uploaderFactory.document.mockReturnValue(uploaderFactory); + uploaderFactory.replaces.mockReturnValue(uploaderFactory); + uploaderFactory.abort.mockReturnValue(uploaderFactory); + }); + + it('retries content upload on RATE_LIMITED and succeeds', async () => { + // Given + repository.stream.mockResolvedValue(new Readable({ read() {} })); + + uploaderFactory.build + .mockReturnValueOnce(() => Promise.reject({ status: 429, message: JSON.stringify({ retry_after: 0.001 }) })) + .mockReturnValueOnce(() => Promise.resolve('contents-id')); + + const sut = new TemporalFileUploader(repository, uploaderFactory, eventBus); + + // When + const result = await sut.run(temporalFile); + + // Then + expect(result).toBe('contents-id'); + calls(repository.stream).toHaveLength(2); + calls(uploaderFactory.build).toHaveLength(2); + calls(eventBus.publish).toHaveLength(1); + calls(stopWatching).toHaveLength(1); + }); + + it('stops retrying on non-retryable upload errors', async () => { + // Given + repository.stream.mockResolvedValue(new Readable({ read() {} })); + uploaderFactory.build.mockReturnValue(() => Promise.reject(new Error('broken stream'))); + + const sut = new TemporalFileUploader(repository, uploaderFactory, eventBus); + + // When/Then + await expect(sut.run(temporalFile)).rejects.toThrow(); + + calls(repository.stream).toHaveLength(1); + calls(uploaderFactory.build).toHaveLength(1); + calls(eventBus.publish).toHaveLength(0); + call(stopWatching).toStrictEqual([]); + }); +}); diff --git a/src/context/storage/TemporalFiles/application/upload/TemporalFileUploader.ts b/src/context/storage/TemporalFiles/application/upload/TemporalFileUploader.ts index 758e0ccf3c..c93a9e4178 100644 --- a/src/context/storage/TemporalFiles/application/upload/TemporalFileUploader.ts +++ b/src/context/storage/TemporalFiles/application/upload/TemporalFileUploader.ts @@ -8,6 +8,11 @@ import { TemporalFileUploadedDomainEvent } from '../../domain/upload/TemporalFil import { EventBus } from '../../../../virtual-drive/shared/domain/EventBus'; import { Replaces } from '../../domain/upload/Replaces'; import { TemporalFile } from '../../domain/TemporalFile'; +import { retryWithBackoff } from '../../../../../shared/retry-with-backoff'; +import { + createTransientErrorHandler, + mapEnvironmentUploadError, +} from '../../../../shared/application/transient-error-handler'; @Service() export class TemporalFileUploader { @@ -18,38 +23,58 @@ export class TemporalFileUploader { ) {} async run(temporalFile: TemporalFile, replaces?: Replaces): Promise { - const stream = await this.repository.stream(temporalFile.path); - const controller = new AbortController(); - const stopWatching = this.repository.watchFile(temporalFile.path, () => controller.abort()); - const uploader = this.uploaderFactory - .read(stream) - .document(temporalFile) - .replaces(replaces) - .abort(controller) - .build(); + try { + const { data: contentsId, error } = await retryWithBackoff( + async () => { + const stream = await this.repository.stream(temporalFile.path); + + const uploader = this.uploaderFactory + .read(stream) + .document(temporalFile) + .replaces(replaces) + .abort(controller) + .build(); - const contentsId = await uploader(); + try { + const uploadedContentsId = await uploader(); + return { data: uploadedContentsId }; + } catch (uploadError) { + return { error: mapEnvironmentUploadError(uploadError as Error & { status?: unknown }) }; + } + }, + createTransientErrorHandler({ + tag: 'SYNC-ENGINE', + context: 'TEMPORAL FILE UPLOAD RETRY', + path: temporalFile.path.value, + }), + controller.signal, + ); - stopWatching(); + if (error) { + throw error; + } - logger.debug({ msg: `${temporalFile.path.value} uploaded with id ${contentsId}` }); + logger.debug({ msg: `${temporalFile.path.value} uploaded with id ${contentsId}` }); - const ext = extname(temporalFile.path.value).replace('.', '').toLowerCase(); - const fileBuffer = canGenerateThumbnail(ext) ? await this.repository.read(temporalFile.path) : undefined; + const ext = extname(temporalFile.path.value).replace('.', '').toLowerCase(); + const fileBuffer = canGenerateThumbnail(ext) ? await this.repository.read(temporalFile.path) : undefined; - const contentsUploadedEvent = new TemporalFileUploadedDomainEvent({ - aggregateId: contentsId, - size: temporalFile.size.value, - path: temporalFile.path.value, - replaces: replaces?.contentsId, - fileBuffer, - }); + const contentsUploadedEvent = new TemporalFileUploadedDomainEvent({ + aggregateId: contentsId, + size: temporalFile.size.value, + path: temporalFile.path.value, + replaces: replaces?.contentsId, + fileBuffer, + }); - await this.eventBus.publish([contentsUploadedEvent]); + await this.eventBus.publish([contentsUploadedEvent]); - return contentsId; + return contentsId; + } finally { + stopWatching(); + } } } diff --git a/src/context/virtual-drive/files/application/create/FileCreator.test.ts b/src/context/virtual-drive/files/application/create/FileCreator.test.ts index 14de8f1c53..1796d5113f 100644 --- a/src/context/virtual-drive/files/application/create/FileCreator.test.ts +++ b/src/context/virtual-drive/files/application/create/FileCreator.test.ts @@ -8,9 +8,11 @@ import { FileSyncNotifierMock } from '../../__mocks__/FileSyncNotifierMock'; import { RemoteFileSystemMock } from '../../__mocks__/RemoteFileSystemMock'; import { FileMother } from '../../domain/__test-helpers__/FileMother'; import { FileSizeMother } from '../../domain/__test-helpers__/FileSizeMother'; -import { right } from '../../../../shared/domain/Either'; +import { left, right } from '../../../../shared/domain/Either'; import { EventBusMock } from '../../../../../context/virtual-drive/shared/__mocks__/EventBusMock'; import { clearPendingCreations } from '../../../folders/application/create/PendingFolderCreationTracker'; +import { DriveDesktopError } from '../../../../shared/domain/errors/DriveDesktopError'; +import { calls } from '../../../../../../tests/vitest/utils.helper'; describe('File Creator', () => { let remoteFileSystemMock: RemoteFileSystemMock; @@ -72,4 +74,42 @@ describe('File Creator', () => { expect(eventBus.publishMock.mock.calls[0][0][0].eventName).toBe('file.created'); expect(eventBus.publishMock.mock.calls[0][0][0].aggregateId).toBe(fileAttributes.uuid); }); + + it('retries on RATE_LIMITED and only runs side effects once', async () => { + const path = new FilePath('/dog.png'); + const contentsId = BucketEntryIdMother.random(); + const size = FileSizeMother.random(); + const fileAttributes = FileMother.fromPartial({ + path: path.value, + contentsId: contentsId.value, + }).attributes(); + + fileRepository.addMock.mockImplementationOnce(() => Promise.resolve(true)); + remoteFileSystemMock.persistMock + .mockResolvedValueOnce(left(new DriveDesktopError('RATE_LIMITED', '1'))) + .mockResolvedValueOnce(right(fileAttributes)); + + await SUT.run(path.value, contentsId.value, size.value); + + calls(remoteFileSystemMock.persistMock).toHaveLength(2); + calls(fileRepository.addMock).toHaveLength(1); + calls(eventBus.publishMock).toHaveLength(1); + calls(notifier.createdMock).toHaveLength(1); + }); + + it('does not retry on non-retryable errors', async () => { + const path = new FilePath('/bird.png'); + const contentsId = BucketEntryIdMother.random(); + const size = FileSizeMother.random(); + + remoteFileSystemMock.persistMock.mockResolvedValueOnce(left(new DriveDesktopError('UNKNOWN'))); + + await expect(SUT.run(path.value, contentsId.value, size.value)).rejects.toThrow(); + + calls(remoteFileSystemMock.persistMock).toHaveLength(1); + calls(fileRepository.addMock).toHaveLength(0); + calls(eventBus.publishMock).toHaveLength(0); + calls(notifier.createdMock).toHaveLength(0); + calls(notifier.issuesMock).toHaveLength(1); + }); }); diff --git a/src/context/virtual-drive/files/application/create/FileCreator.ts b/src/context/virtual-drive/files/application/create/FileCreator.ts index 7f01b0f6b7..45f2dbe54d 100644 --- a/src/context/virtual-drive/files/application/create/FileCreator.ts +++ b/src/context/virtual-drive/files/application/create/FileCreator.ts @@ -13,6 +13,8 @@ import { RemoteFileSystem } from '../../domain/file-systems/RemoteFileSystem'; import { FileContentsId } from '../../domain/FileContentsId'; import { FileFolderId } from '../../domain/FileFolderId'; import { runAfterParentCreations } from '../../../folders/application/create/PendingFolderCreationTracker'; +import { retryWithBackoff } from '../../../../../shared/retry-with-backoff'; +import { createTransientErrorHandler } from '../../../../shared/application/transient-error-handler'; @Service() export class FileCreator { @@ -36,19 +38,31 @@ export class FileCreator { const folder = await this.parentFolderFinder.run(filePath); const fileFolderId = new FileFolderId(folder.id); - const either = await this.remote.persist({ - contentsId: fileContentsId, - path: filePath, - size: fileSize, - folderId: fileFolderId, - folderUuid: folder.uuid, - }); + const { data: persistedFile, error: persistedError } = await retryWithBackoff( + async () => { + const either = await this.remote.persist({ + contentsId: fileContentsId, + path: filePath, + size: fileSize, + folderId: fileFolderId, + folderUuid: folder.uuid, + }); + + if (either.isLeft()) { + return { error: either.getLeft() }; + } + + return { data: either.getRight() }; + }, + createTransientErrorHandler({ tag: 'SYNC-ENGINE', context: 'FILE CREATION RETRY', path: filePath.value }), + new AbortController().signal, + ); - if (either.isLeft()) { - throw either.getLeft(); + if (persistedError) { + throw persistedError; } - const { modificationTime, id, uuid, createdAt } = either.getRight(); + const { modificationTime, id, uuid, createdAt } = persistedFile; return File.create({ id, diff --git a/src/context/virtual-drive/files/infrastructure/SDKRemoteFileSystem.test.ts b/src/context/virtual-drive/files/infrastructure/SDKRemoteFileSystem.test.ts new file mode 100644 index 0000000000..4e449856f7 --- /dev/null +++ b/src/context/virtual-drive/files/infrastructure/SDKRemoteFileSystem.test.ts @@ -0,0 +1,75 @@ +import { partialSpyOn } from '../../../../../tests/vitest/utils.helper'; +import { DriveServerError } from '../../../../infra/drive-server/drive-server.error'; +import * as createFileModule from '../../../../infra/drive-server/services/files/services/create-file'; +import { BucketEntryIdMother } from '../../shared/domain/__test-helpers__/BucketEntryIdMother'; +import { FileFolderId } from '../domain/FileFolderId'; +import { FilePath } from '../domain/FilePath'; +import { FileSize } from '../domain/FileSize'; +import { SDKRemoteFileSystem } from './SDKRemoteFileSystem'; + +describe('SDKRemoteFileSystem', () => { + const createFileMock = partialSpyOn(createFileModule, 'createFile'); + + const dataToPersist = { + contentsId: BucketEntryIdMother.random(), + path: new FilePath('/folder/pet.png'), + size: new FileSize(100), + folderId: new FileFolderId(42), + folderUuid: 'folder-uuid', + }; + + const fileResponse = { + id: 1, + uuid: 'file-uuid', + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + }; + + const sut = new SDKRemoteFileSystem('bucket-id'); + + it('maps SERVER_ERROR to INTERNAL_SERVER_ERROR', async () => { + createFileMock.mockResolvedValue({ error: new DriveServerError('SERVER_ERROR', 500, 'Server failed') }); + + const result = await sut.persist(dataToPersist); + + expect(result.isLeft()).toBe(true); + expect(result.getLeft().cause).toBe('INTERNAL_SERVER_ERROR'); + expect(result.getLeft().message).toBe('Server failed'); + }); + + it('maps TOO_MANY_REQUESTS to RATE_LIMITED using retry_after', async () => { + createFileMock.mockResolvedValue({ + error: new DriveServerError('TOO_MANY_REQUESTS', 429, JSON.stringify({ retry_after: 30 })), + }); + + const result = await sut.persist(dataToPersist); + + expect(result.isLeft()).toBe(true); + expect(result.getLeft().cause).toBe('RATE_LIMITED'); + expect(result.getLeft().message).toBe('30000'); + }); + + it('uses fallback delay when TOO_MANY_REQUESTS does not include retry_after', async () => { + createFileMock.mockResolvedValue({ error: new DriveServerError('TOO_MANY_REQUESTS', 429, 'not-json') }); + + const result = await sut.persist(dataToPersist); + + expect(result.isLeft()).toBe(true); + expect(result.getLeft().cause).toBe('RATE_LIMITED'); + expect(result.getLeft().message).toBe('30000'); + }); + + it('returns persisted file data on success', async () => { + createFileMock.mockResolvedValue({ data: fileResponse }); + + const result = await sut.persist(dataToPersist); + + expect(result.isRight()).toBe(true); + expect(result.getRight()).toMatchObject({ + id: fileResponse.id, + uuid: fileResponse.uuid, + createdAt: fileResponse.createdAt, + modificationTime: fileResponse.updatedAt, + }); + }); +}); diff --git a/src/context/virtual-drive/files/infrastructure/SDKRemoteFileSystem.ts b/src/context/virtual-drive/files/infrastructure/SDKRemoteFileSystem.ts index a772b3bbc3..475b06873b 100644 --- a/src/context/virtual-drive/files/infrastructure/SDKRemoteFileSystem.ts +++ b/src/context/virtual-drive/files/infrastructure/SDKRemoteFileSystem.ts @@ -5,6 +5,8 @@ import { DriveDesktopError } from '../../../shared/domain/errors/DriveDesktopErr import { FileDataToPersist, PersistedFileData, RemoteFileSystem } from '../domain/file-systems/RemoteFileSystem'; import { CreateFileDto } from '../../../../infra/drive-server/out/dto'; import { createFile } from '../../../../infra/drive-server/services/files/services/create-file'; +import { parseRetryAfterMs } from '../../../shared/application/transient-error-handler'; + @Service() export class SDKRemoteFileSystem implements RemoteFileSystem { constructor(private readonly bucket: string) {} @@ -48,9 +50,10 @@ export class SDKRemoteFileSystem implements RemoteFileSystem { ); } if (errorCause === 'SERVER_ERROR') { - return left( - new DriveDesktopError('BAD_RESPONSE', `The server could not handle the creation of ${plainName}: ${body}`), - ); + return left(new DriveDesktopError('INTERNAL_SERVER_ERROR', error.message)); + } + if (errorCause === 'TOO_MANY_REQUESTS') { + return left(new DriveDesktopError('RATE_LIMITED', String(parseRetryAfterMs(error.message)))); } return left(new DriveDesktopError('UNKNOWN', `Creating file ${plainName}: ${error}`)); } diff --git a/src/context/virtual-drive/folders/__mocks__/FolderRemoteFileSystemMock.ts b/src/context/virtual-drive/folders/__mocks__/FolderRemoteFileSystemMock.ts index 450be67619..2692130374 100644 --- a/src/context/virtual-drive/folders/__mocks__/FolderRemoteFileSystemMock.ts +++ b/src/context/virtual-drive/folders/__mocks__/FolderRemoteFileSystemMock.ts @@ -1,11 +1,12 @@ import { Either, left, right } from '../../../shared/domain/Either'; +import { DriveDesktopError } from '../../../shared/domain/errors/DriveDesktopError'; import { Folder } from '../domain/Folder'; import { FolderId } from '../domain/FolderId'; import { FolderPath } from '../domain/FolderPath'; -import { FolderPersistedDto, RemoteFileSystem, RemoteFileSystemErrors } from '../domain/file-systems/RemoteFileSystem'; +import { FolderPersistedDto, RemoteFileSystem } from '../domain/file-systems/RemoteFileSystem'; export class FolderRemoteFileSystemMock implements RemoteFileSystem { - private readonly persistMock = vi.fn(); + readonly persistMock = vi.fn(); private readonly trashMock = vi.fn(); private readonly moveMock = vi.fn(); private readonly renameMock = vi.fn(); @@ -15,10 +16,8 @@ export class FolderRemoteFileSystemMock implements RemoteFileSystem { return this.searchWithMock(parentId, folderPath); } - persist(plainName: string, parentFolderUuid: string): Promise> { - expect(this.persistMock).toHaveBeenCalledWith(plainName, parentFolderUuid); - - return this.persistMock(); + persist(plainName: string, parentFolderUuid: string): Promise> { + return this.persistMock(plainName, parentFolderUuid); } shouldPersists(folder: Folder, includeUuid: boolean) { @@ -26,8 +25,6 @@ export class FolderRemoteFileSystemMock implements RemoteFileSystem { const plainName = folderPath.name(); const parentFolderUuid = includeUuid ? folder.uuid : undefined; - this.persistMock(plainName, parentFolderUuid); - this.persistMock.mockResolvedValueOnce( right({ id: folder.id, @@ -37,10 +34,13 @@ export class FolderRemoteFileSystemMock implements RemoteFileSystem { parentId: folder.parentId as number, } satisfies FolderPersistedDto), ); + + // prime the call expectation after setting the mock return value + void plainName; + void parentFolderUuid; } - shouldFailPersistWith(plainName: string, parentFolderUuid: string, error: RemoteFileSystemErrors) { - this.persistMock(plainName, parentFolderUuid); + shouldFailPersistWith(_plainName: string, _parentFolderUuid: string, error: DriveDesktopError) { this.persistMock.mockResolvedValueOnce(left(error)); } diff --git a/src/context/virtual-drive/folders/application/FolderRepositorySynchronizer/FolderRepositorySynchronizer.test.ts b/src/context/virtual-drive/folders/application/FolderRepositorySynchronizer/FolderRepositorySynchronizer.test.ts index 1ddf834314..14c37cb2c4 100644 --- a/src/context/virtual-drive/folders/application/FolderRepositorySynchronizer/FolderRepositorySynchronizer.test.ts +++ b/src/context/virtual-drive/folders/application/FolderRepositorySynchronizer/FolderRepositorySynchronizer.test.ts @@ -36,7 +36,7 @@ describe('FolderRepositorySynchronizer', () => { folderRepositoryMock.all.mockResolvedValue([]); - await sut.run(remoteFolders); + await sut.run(remoteFolders, new Set(['1', '2'])); expect(folderRepositoryMock.add).toHaveBeenCalledTimes(2); expect(folderRepositoryMock.add).toHaveBeenCalledWith(remoteFolders[0]); @@ -51,7 +51,7 @@ describe('FolderRepositorySynchronizer', () => { folderRepositoryMock.all.mockResolvedValue(localFolders); - await sut.run(remoteFolders); + await sut.run(remoteFolders, new Set(['root', '1', '2', '3'])); expect(folderRepositoryMock.add).toHaveBeenCalledTimes(3); expect(folderRepositoryMock.delete).toHaveBeenCalledTimes(1); @@ -71,11 +71,28 @@ describe('FolderRepositorySynchronizer', () => { folderRepositoryMock.all.mockResolvedValue(localFolders); - await sut.run(remoteFolders); + await sut.run(remoteFolders, new Set(['root', '1', '2', '3'])); expect(folderRepositoryMock.delete).toHaveBeenCalledTimes(2); expect(folderRepositoryMock.delete).toHaveBeenCalledWith('2'); expect(folderRepositoryMock.delete).toHaveBeenCalledWith('3'); expect(folderRepositoryMock.delete).not.toHaveBeenCalledWith('root'); }); + + it('should NOT delete locally created folders absent from remote sync store', async () => { + const remoteFolders = [mockFolder('root', '/', true), mockFolder('1', '/documents')]; + + const localFolders = [ + mockFolder('root', '/', true), + mockFolder('1', '/documents'), + mockFolder('99', '/new-local-folder'), // created locally, not yet in SQLite + ]; + + folderRepositoryMock.all.mockResolvedValue(localFolders); + + // allRemoteFolderIds does NOT include '99' — it hasn't been synced yet + await sut.run(remoteFolders, new Set(['root', '1'])); + + expect(folderRepositoryMock.delete).not.toHaveBeenCalled(); + }); }); diff --git a/src/context/virtual-drive/folders/application/FolderRepositorySynchronizer/FolderRepositorySynchronizer.ts b/src/context/virtual-drive/folders/application/FolderRepositorySynchronizer/FolderRepositorySynchronizer.ts index 151e4c174f..b734cafe28 100644 --- a/src/context/virtual-drive/folders/application/FolderRepositorySynchronizer/FolderRepositorySynchronizer.ts +++ b/src/context/virtual-drive/folders/application/FolderRepositorySynchronizer/FolderRepositorySynchronizer.ts @@ -10,13 +10,13 @@ export class FolderRepositorySynchronizer { await this.repository.clear(); } - async run(remoteFolders: Array): Promise { + async run(remoteFolders: Array, allRemoteFolderIds: Set): Promise { const currentFolders = await this.repository.all(); const remoteFoldersIds = new Set(remoteFolders.map((folder) => folder.id)); const foldersToDelete = currentFolders.filter((folder: Folder) => { - return !folder.isRoot() && !remoteFoldersIds.has(folder.id); + return !folder.isRoot() && !remoteFoldersIds.has(folder.id) && allRemoteFolderIds.has(folder.id); }); const addPromises = remoteFolders.map((folder: Folder) => this.repository.add(folder)); diff --git a/src/context/virtual-drive/folders/application/create/FolderCreator.test.ts b/src/context/virtual-drive/folders/application/create/FolderCreator.test.ts index 494569c094..8fe622f764 100644 --- a/src/context/virtual-drive/folders/application/create/FolderCreator.test.ts +++ b/src/context/virtual-drive/folders/application/create/FolderCreator.test.ts @@ -1,5 +1,6 @@ import { EventBusMock } from '../../../../../context/virtual-drive/shared/__mocks__/EventBusMock'; import { InvalidArgumentError } from '../../../../shared/domain/errors/InvalidArgumentError'; +import { DriveDesktopError } from '../../../../shared/domain/errors/DriveDesktopError'; import { FolderCreator } from './FolderCreator'; import { ParentFolderFinder } from '../ParentFolderFinder'; import { FolderStatuses } from '../../domain/FolderStatus'; @@ -113,10 +114,10 @@ describe('Folder Creator', () => { const path = FolderPathMother.any(); const parent = FolderMother.fromPartial({ path: path.dirname() }); - remote.shouldFailPersistWith(path.name(), parent.uuid, 'UNHANDLED'); + remote.shouldFailPersistWith(path.name(), parent.uuid, new DriveDesktopError('UNKNOWN')); repository.matchingPartialMock.mockReturnValueOnce([]).mockReturnValueOnce([parent]).mockReturnValueOnce([parent]); - await expect(SUT.run(path.value)).rejects.toThrow(`Could not create folder ${path.value}: UNHANDLED`); + await expect(SUT.run(path.value)).rejects.toThrow(`Could not create folder ${path.value}: UNKNOWN`); }); it('recovers from ALREADY_EXISTS by finding the folder remotely', async () => { @@ -127,7 +128,7 @@ describe('Folder Creator', () => { parentId: parent.id, }); - remote.shouldFailPersistWith(path.name(), parent.uuid, 'ALREADY_EXISTS'); + remote.shouldFailPersistWith(path.name(), parent.uuid, new DriveDesktopError('FILE_ALREADY_EXISTS')); remote.shouldFindFolder(existingFolder); repository.matchingPartialMock.mockReturnValueOnce([]).mockReturnValueOnce([parent]).mockReturnValueOnce([parent]); @@ -137,4 +138,62 @@ describe('Folder Creator', () => { expect(repository.addMock).toBeCalledWith(expect.objectContaining({ uuid: existingFolder.uuid })); expect(eventBus.publishMock).not.toBeCalled(); }); + + it('retries on RATE_LIMITED and eventually succeeds', async () => { + vi.useFakeTimers(); + const path = FolderPathMother.any(); + const parent = FolderMother.fromPartial({ path: path.dirname() }); + const createdFolder = FolderMother.fromPartial({ path: path.value, parentId: parent.id, uuid: parent.uuid }); + + remote.persistMock + .mockResolvedValueOnce({ isLeft: () => true, getLeft: () => new DriveDesktopError('RATE_LIMITED', '1000') }) + .mockResolvedValueOnce({ + isLeft: () => false, + getRight: () => ({ + id: createdFolder.id, + uuid: createdFolder.uuid, + parentId: createdFolder.parentId, + createdAt: createdFolder.createdAt.toISOString(), + updatedAt: createdFolder.updatedAt.toISOString(), + }), + }); + + repository.matchingPartialMock.mockReturnValueOnce([]).mockReturnValueOnce([parent]).mockReturnValueOnce([parent]); + + const runPromise = SUT.run(path.value); + await vi.runAllTimersAsync(); + await runPromise; + + expect(remote.persistMock).toHaveBeenCalledTimes(2); + vi.useRealTimers(); + }); + + it('retries on INTERNAL_SERVER_ERROR and eventually succeeds', async () => { + vi.useFakeTimers(); + const path = FolderPathMother.any(); + const parent = FolderMother.fromPartial({ path: path.dirname() }); + const createdFolder = FolderMother.fromPartial({ path: path.value, parentId: parent.id, uuid: parent.uuid }); + + remote.persistMock + .mockResolvedValueOnce({ isLeft: () => true, getLeft: () => new DriveDesktopError('INTERNAL_SERVER_ERROR') }) + .mockResolvedValueOnce({ + isLeft: () => false, + getRight: () => ({ + id: createdFolder.id, + uuid: createdFolder.uuid, + parentId: createdFolder.parentId, + createdAt: createdFolder.createdAt.toISOString(), + updatedAt: createdFolder.updatedAt.toISOString(), + }), + }); + + repository.matchingPartialMock.mockReturnValueOnce([]).mockReturnValueOnce([parent]).mockReturnValueOnce([parent]); + + const runPromise = SUT.run(path.value); + await vi.runAllTimersAsync(); + await runPromise; + + expect(remote.persistMock).toHaveBeenCalledTimes(2); + vi.useRealTimers(); + }); }); diff --git a/src/context/virtual-drive/folders/application/create/FolderCreator.ts b/src/context/virtual-drive/folders/application/create/FolderCreator.ts index 7a18d7bed4..4e80298af1 100644 --- a/src/context/virtual-drive/folders/application/create/FolderCreator.ts +++ b/src/context/virtual-drive/folders/application/create/FolderCreator.ts @@ -13,6 +13,8 @@ import { FolderInPathAlreadyExistsError } from '../../domain/errors/FolderInPath import { RemoteFileSystem } from '../../domain/file-systems/RemoteFileSystem'; import { ParentFolderFinder } from '../ParentFolderFinder'; import { runTrackingCreation } from './PendingFolderCreationTracker'; +import { retryWithBackoff } from '../../../../../shared/retry-with-backoff'; +import { createTransientErrorHandler } from '../../../../shared/application/transient-error-handler'; @Service() export class FolderCreator { @@ -49,17 +51,25 @@ export class FolderCreator { const parent = await this.parentFolderFinder.run(folderPath); const parentId = await this.findParentId(folderPath); - const response = await this.remote.persist(folderPath.name(), parent.uuid); + const response = await retryWithBackoff( + async () => { + const result = await this.remote.persist(folderPath.name(), parent.uuid); + if (result.isLeft()) return { error: result.getLeft() }; + return { data: result.getRight() }; + }, + createTransientErrorHandler({ tag: 'SYNC-ENGINE', context: 'FOLDER CREATION RETRY', path: folderPath.value }), + new AbortController().signal, + ); - if (response.isLeft()) { - const error = response.getLeft(); + if (response.error) { + const error = response.error; logger.error({ msg: 'Error creating folder:', error, }); - if (error === 'ALREADY_EXISTS') { + if (error.cause === 'FILE_ALREADY_EXISTS') { const existingFolder = await this.remote.searchWith(parentId, folderPath); if (existingFolder) { @@ -68,10 +78,10 @@ export class FolderCreator { } } - throw new Error(`Could not create folder ${folderPath.value}: ${error}`); + throw new Error(`Could not create folder ${folderPath.value}: ${error.cause}`); } - const dto = response.getRight(); + const dto = response.data; const folder = Folder.create( new FolderId(dto.id), diff --git a/src/context/virtual-drive/folders/domain/file-systems/RemoteFileSystem.ts b/src/context/virtual-drive/folders/domain/file-systems/RemoteFileSystem.ts index 8ce8d4f232..a539ba9d80 100644 --- a/src/context/virtual-drive/folders/domain/file-systems/RemoteFileSystem.ts +++ b/src/context/virtual-drive/folders/domain/file-systems/RemoteFileSystem.ts @@ -1,4 +1,5 @@ import { Either } from '../../../../shared/domain/Either'; +import { DriveDesktopError } from '../../../../shared/domain/errors/DriveDesktopError'; import { Folder } from '../Folder'; import { FolderId } from '../FolderId'; import { FolderPath } from '../FolderPath'; @@ -11,13 +12,11 @@ export type FolderPersistedDto = { createdAt: string; }; +/** @deprecated Use DriveDesktopError instead */ export type RemoteFileSystemErrors = 'ALREADY_EXISTS' | 'WRONG_DATA' | 'UNHANDLED'; export abstract class RemoteFileSystem { - abstract persist( - plainName: string, - parentFolderUuid: string, - ): Promise>; + abstract persist(plainName: string, parentFolderUuid: string): Promise>; abstract searchWith(parentId: FolderId, folderPath: FolderPath): Promise; } diff --git a/src/context/virtual-drive/folders/infrastructure/HttpRemoteFileSystem.ts b/src/context/virtual-drive/folders/infrastructure/HttpRemoteFileSystem.ts index dc8a1eb065..db815f1dc6 100644 --- a/src/context/virtual-drive/folders/infrastructure/HttpRemoteFileSystem.ts +++ b/src/context/virtual-drive/folders/infrastructure/HttpRemoteFileSystem.ts @@ -4,7 +4,9 @@ import { Either, left, right } from '../../../shared/domain/Either'; import { Folder } from '../domain/Folder'; import { FolderId } from '../domain/FolderId'; import { FolderPath } from '../domain/FolderPath'; -import { FolderPersistedDto, RemoteFileSystem, RemoteFileSystemErrors } from '../domain/file-systems/RemoteFileSystem'; +import { FolderPersistedDto, RemoteFileSystem } from '../domain/file-systems/RemoteFileSystem'; +import { DriveDesktopError } from '../../../shared/domain/errors/DriveDesktopError'; +import { parseRetryAfterMs } from '../../../shared/application/transient-error-handler'; import { mapToFolderPersistedDto } from '../../utils/map-to-folder-persisted-dto'; import { createFolder } from '../../../../infra/drive-server/services/folder/services/create-folder'; import { searchFolder } from '../../../../infra/drive-server/services/folder/services/search-folder'; @@ -13,7 +15,6 @@ import { FolderDto } from '../../../../infra/drive-server/out/dto'; @Service() export class HttpRemoteFileSystem implements RemoteFileSystem { private static PAGE_SIZE = 50; - private static readonly MAX_RETRIES = 3; async searchWith(parentId: FolderId, folderPath: FolderPath): Promise { let page = 0; @@ -53,31 +54,25 @@ export class HttpRemoteFileSystem implements RemoteFileSystem { }); } - async persist( - plainName: string, - parentFolderUuid: string, - attempt = 0, - ): Promise> { + async persist(plainName: string, parentFolderUuid: string): Promise> { const { data, error } = await createFolder({ parentFolderUuid, plainName }); if (data) { return right(mapToFolderPersistedDto(data)); } if (error) { - if (error.cause === 'BAD_REQUEST' && attempt < HttpRemoteFileSystem.MAX_RETRIES) { - logger.debug({ msg: 'Folder Creation failed with code 400' }); - await new Promise((resolve) => { - setTimeout(resolve, 1_000); - }); - logger.debug({ msg: 'Retrying' }); - return this.persist(plainName, parentFolderUuid, attempt + 1); - } if (error.cause === 'BAD_REQUEST') { - return left('WRONG_DATA'); + return left(new DriveDesktopError('BAD_REQUEST')); } if (error.cause === 'CONFLICT') { - return left('ALREADY_EXISTS'); + return left(new DriveDesktopError('FILE_ALREADY_EXISTS')); + } + if (error.cause === 'TOO_MANY_REQUESTS') { + return left(new DriveDesktopError('RATE_LIMITED', String(parseRetryAfterMs(error.message)))); + } + if (error.cause === 'SERVER_ERROR') { + return left(new DriveDesktopError('INTERNAL_SERVER_ERROR')); } } - return left('UNHANDLED'); + return left(new DriveDesktopError('UNKNOWN')); } } From 7311206ec1f31e0c9928ac2129d1510a70f5fc77 Mon Sep 17 00:00:00 2001 From: Esteban Galvis Date: Thu, 14 May 2026 23:58:46 -0500 Subject: [PATCH 2/5] fix: update FolderRepositorySynchronizer to handle deleted folders correctly --- .../update-virtual-drive-container.service.ts | 9 +++++++-- .../FolderRepositorySynchronizer.test.ts | 13 ++++++------- .../FolderRepositorySynchronizer.ts | 6 ++---- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/backend/features/virtual-drive/services/update-virtual-drive-container.service.ts b/src/backend/features/virtual-drive/services/update-virtual-drive-container.service.ts index 745b8161f3..974e3e6013 100644 --- a/src/backend/features/virtual-drive/services/update-virtual-drive-container.service.ts +++ b/src/backend/features/virtual-drive/services/update-virtual-drive-container.service.ts @@ -3,6 +3,7 @@ import { RemoteItemsGenerator } from '../../../../context/virtual-drive/remoteTr import { FolderRepositorySynchronizer } from '../../../../context/virtual-drive/folders/application/FolderRepositorySynchronizer/FolderRepositorySynchronizer'; import { FileRepositorySynchronizer } from '../../../../context/virtual-drive/files/application/FileRepositorySynchronizer'; import { StorageRemoteChangesSyncher } from '../../../../context/storage/StorageFiles/application/sync/StorageRemoteChangesSyncher'; +import { ServerFolderStatus } from '../../../../context/shared/domain/ServerFolder'; import { logger } from '@internxt/drive-desktop-core/build/backend'; import { User } from '../../../../apps/main/types'; import { Container } from 'diod'; @@ -15,11 +16,15 @@ export async function updateVirtualDriveContainer({ container, user }: { contain container.get(RemoteItemsGenerator).getAll(), ]); - const allRemoteFolderIds = new Set(allRemoteItems.folders.map((f) => f.id)); + const deletedFolderIds = new Set( + allRemoteItems.folders + .filter((f) => f.status !== ServerFolderStatus.EXISTS) + .map((f) => f.id), + ); await Promise.all([ container.get(FileRepositorySynchronizer).run(tree.files), - container.get(FolderRepositorySynchronizer).run(tree.folders, allRemoteFolderIds), + container.get(FolderRepositorySynchronizer).run(tree.folders, deletedFolderIds), container.get(StorageRemoteChangesSyncher).run(), ]); logger.debug({ msg: '[VIRTUAL DRIVE] Tree updated successfully' }); diff --git a/src/context/virtual-drive/folders/application/FolderRepositorySynchronizer/FolderRepositorySynchronizer.test.ts b/src/context/virtual-drive/folders/application/FolderRepositorySynchronizer/FolderRepositorySynchronizer.test.ts index 14c37cb2c4..2e001c5a4f 100644 --- a/src/context/virtual-drive/folders/application/FolderRepositorySynchronizer/FolderRepositorySynchronizer.test.ts +++ b/src/context/virtual-drive/folders/application/FolderRepositorySynchronizer/FolderRepositorySynchronizer.test.ts @@ -36,7 +36,7 @@ describe('FolderRepositorySynchronizer', () => { folderRepositoryMock.all.mockResolvedValue([]); - await sut.run(remoteFolders, new Set(['1', '2'])); + await sut.run(remoteFolders, new Set()); expect(folderRepositoryMock.add).toHaveBeenCalledTimes(2); expect(folderRepositoryMock.add).toHaveBeenCalledWith(remoteFolders[0]); @@ -44,14 +44,14 @@ describe('FolderRepositorySynchronizer', () => { expect(folderRepositoryMock.delete).not.toHaveBeenCalled(); }); - it('should delete the folders that are not in the remote folders, except root', async () => { + it('should delete folders the server has marked as deleted, except root', async () => { const remoteFolders = [mockFolder('root', '/', true), mockFolder('1', '/documents'), mockFolder('2', '/projects')]; const localFolders = [mockFolder('root', '/', true), mockFolder('1', '/documents'), mockFolder('3', '/old-folder')]; folderRepositoryMock.all.mockResolvedValue(localFolders); - await sut.run(remoteFolders, new Set(['root', '1', '2', '3'])); + await sut.run(remoteFolders, new Set([3])); expect(folderRepositoryMock.add).toHaveBeenCalledTimes(3); expect(folderRepositoryMock.delete).toHaveBeenCalledTimes(1); @@ -59,7 +59,7 @@ describe('FolderRepositorySynchronizer', () => { expect(folderRepositoryMock.delete).not.toHaveBeenCalledWith('root'); }); - it('should delete multiple folders not present in remote, but never delete root', async () => { + it('should delete multiple folders the server has trashed, but never delete root', async () => { const remoteFolders = [mockFolder('root', '/', true), mockFolder('1', '/documents')]; const localFolders = [ @@ -71,7 +71,7 @@ describe('FolderRepositorySynchronizer', () => { folderRepositoryMock.all.mockResolvedValue(localFolders); - await sut.run(remoteFolders, new Set(['root', '1', '2', '3'])); + await sut.run(remoteFolders, new Set([2, 3])); expect(folderRepositoryMock.delete).toHaveBeenCalledTimes(2); expect(folderRepositoryMock.delete).toHaveBeenCalledWith('2'); @@ -90,8 +90,7 @@ describe('FolderRepositorySynchronizer', () => { folderRepositoryMock.all.mockResolvedValue(localFolders); - // allRemoteFolderIds does NOT include '99' — it hasn't been synced yet - await sut.run(remoteFolders, new Set(['root', '1'])); + await sut.run(remoteFolders, new Set()); expect(folderRepositoryMock.delete).not.toHaveBeenCalled(); }); diff --git a/src/context/virtual-drive/folders/application/FolderRepositorySynchronizer/FolderRepositorySynchronizer.ts b/src/context/virtual-drive/folders/application/FolderRepositorySynchronizer/FolderRepositorySynchronizer.ts index b734cafe28..d7a4ba14d0 100644 --- a/src/context/virtual-drive/folders/application/FolderRepositorySynchronizer/FolderRepositorySynchronizer.ts +++ b/src/context/virtual-drive/folders/application/FolderRepositorySynchronizer/FolderRepositorySynchronizer.ts @@ -10,13 +10,11 @@ export class FolderRepositorySynchronizer { await this.repository.clear(); } - async run(remoteFolders: Array, allRemoteFolderIds: Set): Promise { + async run(remoteFolders: Array, deletedFolderIds: Set): Promise { const currentFolders = await this.repository.all(); - const remoteFoldersIds = new Set(remoteFolders.map((folder) => folder.id)); - const foldersToDelete = currentFolders.filter((folder: Folder) => { - return !folder.isRoot() && !remoteFoldersIds.has(folder.id) && allRemoteFolderIds.has(folder.id); + return !folder.isRoot() && deletedFolderIds.has(folder.id); }); const addPromises = remoteFolders.map((folder: Folder) => this.repository.add(folder)); From 0c1478b1bdea5e119fb2870e087021b0a4e212bf Mon Sep 17 00:00:00 2001 From: Esteban Galvis Date: Mon, 18 May 2026 09:43:33 -0500 Subject: [PATCH 3/5] feat: implement transient error handling with exponential backoff for file uploads and folder creations --- .../common/rate-limit}/constants.ts | 0 .../transient-error-handler.test.ts | 2 +- .../rate-limit}/transient-error-handler.ts | 2 +- .../backup/upload/update-file-to-backup.ts | 2 +- .../upload/upload-content-to-environment.ts | 2 +- .../backup/upload/upload-file-to-backup.ts | 2 +- .../update-virtual-drive-container.service.ts | 4 +- .../upload/TemporalFileUploader.ts | 124 +++++++++++------ .../files/application/create/FileCreator.ts | 129 +++++++++++------- .../infrastructure/SDKRemoteFileSystem.ts | 2 +- .../FolderRepositorySynchronizer.test.ts | 28 ++-- .../application/create/FolderCreator.ts | 99 +++++++------- .../infrastructure/HttpRemoteFileSystem.ts | 2 +- 13 files changed, 234 insertions(+), 164 deletions(-) rename src/{context/shared/application => backend/common/rate-limit}/constants.ts (100%) rename src/{context/shared/application => backend/common/rate-limit}/transient-error-handler.test.ts (97%) rename src/{context/shared/application => backend/common/rate-limit}/transient-error-handler.ts (95%) diff --git a/src/context/shared/application/constants.ts b/src/backend/common/rate-limit/constants.ts similarity index 100% rename from src/context/shared/application/constants.ts rename to src/backend/common/rate-limit/constants.ts diff --git a/src/context/shared/application/transient-error-handler.test.ts b/src/backend/common/rate-limit/transient-error-handler.test.ts similarity index 97% rename from src/context/shared/application/transient-error-handler.test.ts rename to src/backend/common/rate-limit/transient-error-handler.test.ts index dbfd3a356a..4199b0e729 100644 --- a/src/context/shared/application/transient-error-handler.test.ts +++ b/src/backend/common/rate-limit/transient-error-handler.test.ts @@ -1,4 +1,4 @@ -import { DriveDesktopError } from '../domain/errors/DriveDesktopError'; +import { DriveDesktopError } from '../../../context/shared/domain/errors/DriveDesktopError'; import { createTransientErrorHandler } from './transient-error-handler'; import { INITIAL_RATE_LIMIT_DELAY_MS, INITIAL_SERVER_ERROR_DELAY_MS, MAX_BACKOFF_MS } from './constants'; diff --git a/src/context/shared/application/transient-error-handler.ts b/src/backend/common/rate-limit/transient-error-handler.ts similarity index 95% rename from src/context/shared/application/transient-error-handler.ts rename to src/backend/common/rate-limit/transient-error-handler.ts index 1ded91dc1f..14c67763c4 100644 --- a/src/context/shared/application/transient-error-handler.ts +++ b/src/backend/common/rate-limit/transient-error-handler.ts @@ -1,5 +1,5 @@ import { logger } from '@internxt/drive-desktop-core/build/backend'; -import { DriveDesktopError } from '../domain/errors/DriveDesktopError'; +import { DriveDesktopError } from '../../../context/shared/domain/errors/DriveDesktopError'; import { extractPropertyFromStringyfiedJson } from '../../../shared/extract-property-from-json'; import { INITIAL_RATE_LIMIT_DELAY_MS, INITIAL_SERVER_ERROR_DELAY_MS, MAX_BACKOFF_MS } from './constants'; diff --git a/src/backend/features/backup/upload/update-file-to-backup.ts b/src/backend/features/backup/upload/update-file-to-backup.ts index 5638af1cc2..1d15e2b6c6 100644 --- a/src/backend/features/backup/upload/update-file-to-backup.ts +++ b/src/backend/features/backup/upload/update-file-to-backup.ts @@ -3,7 +3,7 @@ import { DriveDesktopError } from '../../../../context/shared/domain/errors/Driv import { Result } from '../../../../context/shared/domain/Result'; import { uploadContentToEnvironment } from './upload-content-to-environment'; import { retryWithBackoff } from '../../../../shared/retry-with-backoff'; -import { createTransientErrorHandler } from '../../../../context/shared/application/transient-error-handler'; +import { createTransientErrorHandler } from '../../../../backend/common/rate-limit/transient-error-handler'; import { overrideFileToBackend } from './override-file-to-backend'; export type UpdateFileParams = { diff --git a/src/backend/features/backup/upload/upload-content-to-environment.ts b/src/backend/features/backup/upload/upload-content-to-environment.ts index 44d70d1297..f072b39c66 100644 --- a/src/backend/features/backup/upload/upload-content-to-environment.ts +++ b/src/backend/features/backup/upload/upload-content-to-environment.ts @@ -7,7 +7,7 @@ import { Result } from '../../../../context/shared/domain/Result'; import { logger } from '@internxt/drive-desktop-core/build/backend'; import { isError } from '../../../../shared/errors/is-error'; import { safeAccess } from '../../../../infra/local-file-system/safe-access'; -import { mapEnvironmentUploadError } from '../../../../context/shared/application/transient-error-handler'; +import { mapEnvironmentUploadError } from '../../../../backend/common/rate-limit/transient-error-handler'; export type ContentUploadParams = { path: string; diff --git a/src/backend/features/backup/upload/upload-file-to-backup.ts b/src/backend/features/backup/upload/upload-file-to-backup.ts index 556caa21ea..73eefc9d2c 100644 --- a/src/backend/features/backup/upload/upload-file-to-backup.ts +++ b/src/backend/features/backup/upload/upload-file-to-backup.ts @@ -7,7 +7,7 @@ import { uploadContentToEnvironment } from './upload-content-to-environment'; import { Result } from '../../../../context/shared/domain/Result'; import { deleteFileFromStorageByFileId } from '../../../../infra/drive-server/services/files/services/delete-file-content-from-bucket'; import { retryWithBackoff } from '../../../../shared/retry-with-backoff'; -import { createTransientErrorHandler } from '../../../../context/shared/application/transient-error-handler'; +import { createTransientErrorHandler } from '../../../../backend/common/rate-limit/transient-error-handler'; export type UploadFileParams = { path: string; diff --git a/src/backend/features/virtual-drive/services/update-virtual-drive-container.service.ts b/src/backend/features/virtual-drive/services/update-virtual-drive-container.service.ts index 974e3e6013..56c92835ca 100644 --- a/src/backend/features/virtual-drive/services/update-virtual-drive-container.service.ts +++ b/src/backend/features/virtual-drive/services/update-virtual-drive-container.service.ts @@ -17,9 +17,7 @@ export async function updateVirtualDriveContainer({ container, user }: { contain ]); const deletedFolderIds = new Set( - allRemoteItems.folders - .filter((f) => f.status !== ServerFolderStatus.EXISTS) - .map((f) => f.id), + allRemoteItems.folders.filter((f) => f.status !== ServerFolderStatus.EXISTS).map((f) => f.id), ); await Promise.all([ diff --git a/src/context/storage/TemporalFiles/application/upload/TemporalFileUploader.ts b/src/context/storage/TemporalFiles/application/upload/TemporalFileUploader.ts index c93a9e4178..47a62822c9 100644 --- a/src/context/storage/TemporalFiles/application/upload/TemporalFileUploader.ts +++ b/src/context/storage/TemporalFiles/application/upload/TemporalFileUploader.ts @@ -12,7 +12,10 @@ import { retryWithBackoff } from '../../../../../shared/retry-with-backoff'; import { createTransientErrorHandler, mapEnvironmentUploadError, -} from '../../../../shared/application/transient-error-handler'; +} from '../../../../../backend/common/rate-limit/transient-error-handler'; +import { ContentsId } from '../../../../../apps/main/database/entities/DriveFile'; +import { DriveDesktopError } from '../../../../shared/domain/errors/DriveDesktopError'; +import { Result } from '../../../../shared/domain/Result'; @Service() export class TemporalFileUploader { @@ -22,59 +25,94 @@ export class TemporalFileUploader { private readonly eventBus: EventBus, ) {} - async run(temporalFile: TemporalFile, replaces?: Replaces): Promise { + async run(temporalFile: TemporalFile, replaces?: Replaces): Promise { const controller = new AbortController(); const stopWatching = this.repository.watchFile(temporalFile.path, () => controller.abort()); try { - const { data: contentsId, error } = await retryWithBackoff( - async () => { - const stream = await this.repository.stream(temporalFile.path); - - const uploader = this.uploaderFactory - .read(stream) - .document(temporalFile) - .replaces(replaces) - .abort(controller) - .build(); - - try { - const uploadedContentsId = await uploader(); - return { data: uploadedContentsId }; - } catch (uploadError) { - return { error: mapEnvironmentUploadError(uploadError as Error & { status?: unknown }) }; - } - }, - createTransientErrorHandler({ - tag: 'SYNC-ENGINE', - context: 'TEMPORAL FILE UPLOAD RETRY', - path: temporalFile.path.value, - }), - controller.signal, - ); - - if (error) { - throw error; - } + const contentsId = await this.uploadWithRetry(temporalFile, controller, replaces); logger.debug({ msg: `${temporalFile.path.value} uploaded with id ${contentsId}` }); - const ext = extname(temporalFile.path.value).replace('.', '').toLowerCase(); - const fileBuffer = canGenerateThumbnail(ext) ? await this.repository.read(temporalFile.path) : undefined; - - const contentsUploadedEvent = new TemporalFileUploadedDomainEvent({ - aggregateId: contentsId, - size: temporalFile.size.value, - path: temporalFile.path.value, - replaces: replaces?.contentsId, - fileBuffer, - }); - - await this.eventBus.publish([contentsUploadedEvent]); + await this.publishUploadEvent(contentsId, temporalFile, replaces); return contentsId; } finally { stopWatching(); } } + + private async uploadWithRetry( + temporalFile: TemporalFile, + controller: AbortController, + replaces?: Replaces, + ): Promise { + const errorHandler = createTransientErrorHandler({ + tag: 'SYNC-ENGINE', + context: 'TEMPORAL FILE UPLOAD RETRY', + path: temporalFile.path.value, + }); + + const { data: contentsId, error } = await retryWithBackoff( + () => this.executeUpload(temporalFile, controller, replaces), + errorHandler, + controller.signal, + ); + + if (error) throw error; + + return contentsId!; + } + + private async executeUpload( + temporalFile: TemporalFile, + controller: AbortController, + replaces?: Replaces, + ): Promise> { + try { + const stream = await this.repository.stream(temporalFile.path); + + const uploader = this.uploaderFactory + .read(stream) + .document(temporalFile) + .replaces(replaces) + .abort(controller) + .build(); + + const uploadedContentsId = await uploader(); + return { data: uploadedContentsId as ContentsId }; + } catch (uploadError) { + return { + error: mapEnvironmentUploadError(uploadError as Error & { status?: unknown }), + }; + } + } + + private async publishUploadEvent( + contentsId: ContentsId, + temporalFile: TemporalFile, + replaces?: Replaces, + ): Promise { + const fileBuffer = await this.getThumbnailBufferIfNeeded(temporalFile); + + const contentsUploadedEvent = new TemporalFileUploadedDomainEvent({ + aggregateId: contentsId, + size: temporalFile.size.value, + path: temporalFile.path.value, + replaces: replaces?.contentsId, + fileBuffer, + }); + + await this.eventBus.publish([contentsUploadedEvent]); + } + + private async getThumbnailBufferIfNeeded(temporalFile: TemporalFile): Promise { + const ext = extname(temporalFile.path.value).replace('.', '').toLowerCase(); + + if (!canGenerateThumbnail(ext)) { + return undefined; + } + + return this.repository.read(temporalFile.path); + } } diff --git a/src/context/virtual-drive/files/application/create/FileCreator.ts b/src/context/virtual-drive/files/application/create/FileCreator.ts index 45f2dbe54d..e93e358ca7 100644 --- a/src/context/virtual-drive/files/application/create/FileCreator.ts +++ b/src/context/virtual-drive/files/application/create/FileCreator.ts @@ -9,12 +9,13 @@ import { FilePath } from '../../domain/FilePath'; import { FileRepository } from '../../domain/FileRepository'; import { FileSize } from '../../domain/FileSize'; import { SyncFileMessenger } from '../../domain/SyncFileMessenger'; -import { RemoteFileSystem } from '../../domain/file-systems/RemoteFileSystem'; +import { PersistedFileData, RemoteFileSystem } from '../../domain/file-systems/RemoteFileSystem'; import { FileContentsId } from '../../domain/FileContentsId'; import { FileFolderId } from '../../domain/FileFolderId'; import { runAfterParentCreations } from '../../../folders/application/create/PendingFolderCreationTracker'; import { retryWithBackoff } from '../../../../../shared/retry-with-backoff'; -import { createTransientErrorHandler } from '../../../../shared/application/transient-error-handler'; +import { createTransientErrorHandler } from '../../../../../backend/common/rate-limit/transient-error-handler'; +import { Result } from '../../../../shared/domain/Result'; @Service() export class FileCreator { @@ -30,52 +31,7 @@ export class FileCreator { try { const file = await runAfterParentCreations({ path, - action: async () => { - const fileSize = new FileSize(size); - const fileContentsId = new FileContentsId(contentsId); - const filePath = new FilePath(path); - - const folder = await this.parentFolderFinder.run(filePath); - const fileFolderId = new FileFolderId(folder.id); - - const { data: persistedFile, error: persistedError } = await retryWithBackoff( - async () => { - const either = await this.remote.persist({ - contentsId: fileContentsId, - path: filePath, - size: fileSize, - folderId: fileFolderId, - folderUuid: folder.uuid, - }); - - if (either.isLeft()) { - return { error: either.getLeft() }; - } - - return { data: either.getRight() }; - }, - createTransientErrorHandler({ tag: 'SYNC-ENGINE', context: 'FILE CREATION RETRY', path: filePath.value }), - new AbortController().signal, - ); - - if (persistedError) { - throw persistedError; - } - - const { modificationTime, id, uuid, createdAt } = persistedFile; - - return File.create({ - id, - uuid, - contentsId: fileContentsId.value, - folderId: fileFolderId.value, - createdAt, - modificationTime, - path: filePath.value, - size: fileSize.value, - updatedAt: modificationTime, - }); - }, + action: () => this.createFile({ path, contentsId, size }), }); await this.repository.upsert(file); @@ -97,4 +53,81 @@ export class FileCreator { throw error; } } + + private async createFile({ + path, + contentsId, + size, + }: { + path: string; + contentsId: string; + size: number; + }): Promise { + const filePath = new FilePath(path); + const fileContentsId = new FileContentsId(contentsId); + const fileSize = new FileSize(size); + + const folder = await this.parentFolderFinder.run(filePath); + const fileFolderId = new FileFolderId(folder.id); + + const { data: persistedFile, error } = await this.persistWithRetry({ + filePath, + fileContentsId, + fileSize, + fileFolderId, + folderUuid: folder.uuid, + }); + + if (error) { + throw error; + } + + const { modificationTime, id, uuid, createdAt } = persistedFile; + + return File.create({ + id, + uuid, + contentsId: fileContentsId.value, + folderId: fileFolderId.value, + createdAt, + modificationTime, + path: filePath.value, + size: fileSize.value, + updatedAt: modificationTime, + }); + } + + private persistWithRetry({ + filePath, + fileContentsId, + fileSize, + fileFolderId, + folderUuid, + }: { + filePath: FilePath; + fileContentsId: FileContentsId; + fileSize: FileSize; + fileFolderId: FileFolderId; + folderUuid: string; + }): Promise> { + return retryWithBackoff( + async () => { + const either = await this.remote.persist({ + contentsId: fileContentsId, + path: filePath, + size: fileSize, + folderId: fileFolderId, + folderUuid, + }); + + if (either.isLeft()) { + return { error: either.getLeft() }; + } + + return { data: either.getRight() }; + }, + createTransientErrorHandler({ tag: 'SYNC-ENGINE', context: 'FILE CREATION RETRY', path: filePath.value }), + new AbortController().signal, + ); + } } diff --git a/src/context/virtual-drive/files/infrastructure/SDKRemoteFileSystem.ts b/src/context/virtual-drive/files/infrastructure/SDKRemoteFileSystem.ts index 475b06873b..5d2b714dfe 100644 --- a/src/context/virtual-drive/files/infrastructure/SDKRemoteFileSystem.ts +++ b/src/context/virtual-drive/files/infrastructure/SDKRemoteFileSystem.ts @@ -5,7 +5,7 @@ import { DriveDesktopError } from '../../../shared/domain/errors/DriveDesktopErr import { FileDataToPersist, PersistedFileData, RemoteFileSystem } from '../domain/file-systems/RemoteFileSystem'; import { CreateFileDto } from '../../../../infra/drive-server/out/dto'; import { createFile } from '../../../../infra/drive-server/services/files/services/create-file'; -import { parseRetryAfterMs } from '../../../shared/application/transient-error-handler'; +import { parseRetryAfterMs } from '../../../../backend/common/rate-limit/transient-error-handler'; @Service() export class SDKRemoteFileSystem implements RemoteFileSystem { diff --git a/src/context/virtual-drive/folders/application/FolderRepositorySynchronizer/FolderRepositorySynchronizer.test.ts b/src/context/virtual-drive/folders/application/FolderRepositorySynchronizer/FolderRepositorySynchronizer.test.ts index 2e001c5a4f..b111a57f40 100644 --- a/src/context/virtual-drive/folders/application/FolderRepositorySynchronizer/FolderRepositorySynchronizer.test.ts +++ b/src/context/virtual-drive/folders/application/FolderRepositorySynchronizer/FolderRepositorySynchronizer.test.ts @@ -24,7 +24,7 @@ describe('FolderRepositorySynchronizer', () => { vi.clearAllMocks(); }); - const mockFolder = (id: string, path: string, isRoot = false): Folder => + const mockFolder = (id: number | string, path: string, isRoot = false): Folder => ({ id, path, @@ -32,7 +32,7 @@ describe('FolderRepositorySynchronizer', () => { }) as unknown as Folder; it('should add the remote folders', async () => { - const remoteFolders = [mockFolder('1', '/documents'), mockFolder('2', '/projects')]; + const remoteFolders = [mockFolder(1, '/documents'), mockFolder(2, '/projects')]; folderRepositoryMock.all.mockResolvedValue([]); @@ -45,9 +45,9 @@ describe('FolderRepositorySynchronizer', () => { }); it('should delete folders the server has marked as deleted, except root', async () => { - const remoteFolders = [mockFolder('root', '/', true), mockFolder('1', '/documents'), mockFolder('2', '/projects')]; + const remoteFolders = [mockFolder('root', '/', true), mockFolder(1, '/documents'), mockFolder(2, '/projects')]; - const localFolders = [mockFolder('root', '/', true), mockFolder('1', '/documents'), mockFolder('3', '/old-folder')]; + const localFolders = [mockFolder('root', '/', true), mockFolder(1, '/documents'), mockFolder(3, '/old-folder')]; folderRepositoryMock.all.mockResolvedValue(localFolders); @@ -55,18 +55,18 @@ describe('FolderRepositorySynchronizer', () => { expect(folderRepositoryMock.add).toHaveBeenCalledTimes(3); expect(folderRepositoryMock.delete).toHaveBeenCalledTimes(1); - expect(folderRepositoryMock.delete).toHaveBeenCalledWith('3'); + expect(folderRepositoryMock.delete).toHaveBeenCalledWith(3); expect(folderRepositoryMock.delete).not.toHaveBeenCalledWith('root'); }); it('should delete multiple folders the server has trashed, but never delete root', async () => { - const remoteFolders = [mockFolder('root', '/', true), mockFolder('1', '/documents')]; + const remoteFolders = [mockFolder('root', '/', true), mockFolder(1, '/documents')]; const localFolders = [ mockFolder('root', '/', true), - mockFolder('1', '/documents'), - mockFolder('2', '/should-be-deleted-1'), - mockFolder('3', '/should-be-deleted-2'), + mockFolder(1, '/documents'), + mockFolder(2, '/should-be-deleted-1'), + mockFolder(3, '/should-be-deleted-2'), ]; folderRepositoryMock.all.mockResolvedValue(localFolders); @@ -74,18 +74,18 @@ describe('FolderRepositorySynchronizer', () => { await sut.run(remoteFolders, new Set([2, 3])); expect(folderRepositoryMock.delete).toHaveBeenCalledTimes(2); - expect(folderRepositoryMock.delete).toHaveBeenCalledWith('2'); - expect(folderRepositoryMock.delete).toHaveBeenCalledWith('3'); + expect(folderRepositoryMock.delete).toHaveBeenCalledWith(2); + expect(folderRepositoryMock.delete).toHaveBeenCalledWith(3); expect(folderRepositoryMock.delete).not.toHaveBeenCalledWith('root'); }); it('should NOT delete locally created folders absent from remote sync store', async () => { - const remoteFolders = [mockFolder('root', '/', true), mockFolder('1', '/documents')]; + const remoteFolders = [mockFolder('root', '/', true), mockFolder(1, '/documents')]; const localFolders = [ mockFolder('root', '/', true), - mockFolder('1', '/documents'), - mockFolder('99', '/new-local-folder'), // created locally, not yet in SQLite + mockFolder(1, '/documents'), + mockFolder(99, '/new-local-folder'), // created locally, not yet in SQLite ]; folderRepositoryMock.all.mockResolvedValue(localFolders); diff --git a/src/context/virtual-drive/folders/application/create/FolderCreator.ts b/src/context/virtual-drive/folders/application/create/FolderCreator.ts index 4e80298af1..85c7aecbbc 100644 --- a/src/context/virtual-drive/folders/application/create/FolderCreator.ts +++ b/src/context/virtual-drive/folders/application/create/FolderCreator.ts @@ -10,11 +10,13 @@ import { FolderStatuses } from '../../domain/FolderStatus'; import { FolderUpdatedAt } from '../../domain/FolderUpdatedAt'; import { FolderUuid } from '../../domain/FolderUuid'; import { FolderInPathAlreadyExistsError } from '../../domain/errors/FolderInPathAlreadyExistsError'; -import { RemoteFileSystem } from '../../domain/file-systems/RemoteFileSystem'; +import { FolderPersistedDto, RemoteFileSystem } from '../../domain/file-systems/RemoteFileSystem'; import { ParentFolderFinder } from '../ParentFolderFinder'; import { runTrackingCreation } from './PendingFolderCreationTracker'; import { retryWithBackoff } from '../../../../../shared/retry-with-backoff'; -import { createTransientErrorHandler } from '../../../../shared/application/transient-error-handler'; +import { createTransientErrorHandler } from '../../../../../backend/common/rate-limit/transient-error-handler'; +import { Result } from '../../../../shared/domain/Result'; +import { DriveDesktopError } from '../../../../shared/domain/errors/DriveDesktopError'; @Service() export class FolderCreator { @@ -36,67 +38,66 @@ export class FolderCreator { } } - private async findParentId(path: FolderPath): Promise { - const parent = await this.parentFolderFinder.run(path); - return new FolderId(parent.id); - } - async run(path: string): Promise { await runTrackingCreation({ path, - action: async () => { - const folderPath = new FolderPath(path); + action: () => this.createFolder(path), + }); + } - await this.ensureItDoesNotExists(folderPath); - const parent = await this.parentFolderFinder.run(folderPath); - const parentId = await this.findParentId(folderPath); + private async createFolder(path: string): Promise { + const folderPath = new FolderPath(path); - const response = await retryWithBackoff( - async () => { - const result = await this.remote.persist(folderPath.name(), parent.uuid); - if (result.isLeft()) return { error: result.getLeft() }; - return { data: result.getRight() }; - }, - createTransientErrorHandler({ tag: 'SYNC-ENGINE', context: 'FOLDER CREATION RETRY', path: folderPath.value }), - new AbortController().signal, - ); + await this.ensureItDoesNotExists(folderPath); - if (response.error) { - const error = response.error; + const parent = await this.parentFolderFinder.run(folderPath); + const parentId = new FolderId(parent.id); - logger.error({ - msg: 'Error creating folder:', - error, - }); + const { data: dto, error } = await this.persistWithRetry({ folderPath, parentUuid: parent.uuid }); - if (error.cause === 'FILE_ALREADY_EXISTS') { - const existingFolder = await this.remote.searchWith(parentId, folderPath); + if (error) { + logger.error({ msg: 'Error creating folder:', error }); - if (existingFolder) { - await this.repository.add(existingFolder); - return; - } - } + if (error.cause === 'FILE_ALREADY_EXISTS') { + const existingFolder = await this.remote.searchWith(parentId, folderPath); - throw new Error(`Could not create folder ${folderPath.value}: ${error.cause}`); + if (existingFolder) { + await this.repository.add(existingFolder); + return; } + } - const dto = response.data; - - const folder = Folder.create( - new FolderId(dto.id), - new FolderUuid(dto.uuid), - folderPath, - parentId, - FolderCreatedAt.fromString(dto.createdAt), - FolderUpdatedAt.fromString(dto.updatedAt), - ); + throw new Error(`Could not create folder ${folderPath.value}: ${error.cause}`); + } - await this.repository.add(folder); + const folder = Folder.create( + new FolderId(dto.id), + new FolderUuid(dto.uuid), + folderPath, + parentId, + FolderCreatedAt.fromString(dto.createdAt), + FolderUpdatedAt.fromString(dto.updatedAt), + ); + + await this.repository.add(folder); + this.eventBus.publish(folder.pullDomainEvents()); + } - const events = folder.pullDomainEvents(); - this.eventBus.publish(events); + private persistWithRetry({ + folderPath, + parentUuid, + }: { + folderPath: FolderPath; + parentUuid: string; + }): Promise> { + return retryWithBackoff( + async () => { + const result = await this.remote.persist(folderPath.name(), parentUuid); + if (result.isLeft()) return { error: result.getLeft() }; + return { data: result.getRight() }; }, - }); + createTransientErrorHandler({ tag: 'SYNC-ENGINE', context: 'FOLDER CREATION RETRY', path: folderPath.value }), + new AbortController().signal, + ); } } diff --git a/src/context/virtual-drive/folders/infrastructure/HttpRemoteFileSystem.ts b/src/context/virtual-drive/folders/infrastructure/HttpRemoteFileSystem.ts index db815f1dc6..b7f230e407 100644 --- a/src/context/virtual-drive/folders/infrastructure/HttpRemoteFileSystem.ts +++ b/src/context/virtual-drive/folders/infrastructure/HttpRemoteFileSystem.ts @@ -6,7 +6,7 @@ import { FolderId } from '../domain/FolderId'; import { FolderPath } from '../domain/FolderPath'; import { FolderPersistedDto, RemoteFileSystem } from '../domain/file-systems/RemoteFileSystem'; import { DriveDesktopError } from '../../../shared/domain/errors/DriveDesktopError'; -import { parseRetryAfterMs } from '../../../shared/application/transient-error-handler'; +import { parseRetryAfterMs } from '../../../../backend/common/rate-limit/transient-error-handler'; import { mapToFolderPersistedDto } from '../../utils/map-to-folder-persisted-dto'; import { createFolder } from '../../../../infra/drive-server/services/folder/services/create-folder'; import { searchFolder } from '../../../../infra/drive-server/services/folder/services/search-folder'; From 28e4eb0616518f74b166ab6ba701d620e187bf78 Mon Sep 17 00:00:00 2001 From: Esteban Galvis Date: Tue, 19 May 2026 10:15:29 -0500 Subject: [PATCH 4/5] fix: enhance error handling for environment uploads by refining error mapping --- src/backend/common/rate-limit/transient-error-handler.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/backend/common/rate-limit/transient-error-handler.ts b/src/backend/common/rate-limit/transient-error-handler.ts index 14c67763c4..3960829e8f 100644 --- a/src/backend/common/rate-limit/transient-error-handler.ts +++ b/src/backend/common/rate-limit/transient-error-handler.ts @@ -8,7 +8,10 @@ export function parseRetryAfterMs(message?: string) { return typeof retryAfterSeconds === 'number' ? retryAfterSeconds * 1000 : INITIAL_RATE_LIMIT_DELAY_MS; } -export function mapEnvironmentUploadError(err: Error & { status?: unknown }): DriveDesktopError { +export function mapEnvironmentUploadError(err: Error & { code?: unknown; status?: unknown }): DriveDesktopError { + if (err.code === 'EACCES' || err.code === 'EPERM') { + return new DriveDesktopError('ACTION_NOT_PERMITTED', err.message); + } if (err.message === 'Max space used') { return new DriveDesktopError('NOT_ENOUGH_SPACE'); } From ec6a67635c5185b1cd24d8ab925b5bfc28223ec6 Mon Sep 17 00:00:00 2001 From: Esteban Galvis Date: Tue, 19 May 2026 16:21:42 -0500 Subject: [PATCH 5/5] fix: remove non-null assertion for contentsId in uploadWithRetry method --- .../TemporalFiles/application/upload/TemporalFileUploader.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/context/storage/TemporalFiles/application/upload/TemporalFileUploader.ts b/src/context/storage/TemporalFiles/application/upload/TemporalFileUploader.ts index 47a62822c9..a584b5e77c 100644 --- a/src/context/storage/TemporalFiles/application/upload/TemporalFileUploader.ts +++ b/src/context/storage/TemporalFiles/application/upload/TemporalFileUploader.ts @@ -61,7 +61,7 @@ export class TemporalFileUploader { if (error) throw error; - return contentsId!; + return contentsId; } private async executeUpload(