Cleanup after canceled local backup export

Co-authored-by: trevor-signal <131492920+trevor-signal@users.noreply.github.com>

ts/services/backups/export.preload.ts

@@ -293,12 +293,27 @@ export class BackupExportStream extends Readable {
     super();
   }
 
+  async #cleanupAfterError() {
+    log.warn('Cleaning up after error...');
+    await resumeWriteAccess();
+  }
+
+  override _destroy(
+    error: Error | null,
+    callback: (error?: Error | null) => void
+  ): void {
+    if (error) {
+      drop(this.#cleanupAfterError());
+    }
+    callback(error);
+  }
+
   public run(): void {
     drop(
       (async () => {
-        log.info('BackupExportStream: starting...');
+        log.info('starting...');
         drop(AttachmentBackupManager.stop());
-        log.info('BackupExportStream: message migration starting...');
+        log.info('message migration starting...');
         await migrateAllMessages();
 
         await pauseWriteAccess();
@@ -317,7 +332,7 @@ export class BackupExportStream extends Readable {
                 this.#attachmentBackupJobs.map(job => {
                   if (job.type === 'local') {
                     log.error(
-                      "BackupExportStream: Can't enqueue local backup jobs during remote backup, skipping"
+                      "Can't enqueue local backup jobs during remote backup, skipping"
                     );
                     return Promise.resolve();
                   }
@@ -338,7 +353,7 @@ export class BackupExportStream extends Readable {
           }
           log.info('finished successfully');
         } catch (error) {
-          await resumeWriteAccess();
+          await this.#cleanupAfterError();
           log.error('errored', toLogFormat(error));
           this.emit('error', error);
         } finally {

ts/services/backups/index.preload.ts

@@ -22,10 +22,7 @@ import * as Bytes from '../../Bytes.std.js';
 import { strictAssert } from '../../util/assert.std.js';
 import { drop } from '../../util/drop.std.js';
 import { TEMP_PATH } from '../../util/basePaths.preload.js';
-import {
-  getAbsoluteDownloadsPath,
-  saveAttachmentToDisk,
-} from '../../util/migrations.preload.js';
+import { getAbsoluteDownloadsPath } from '../../util/migrations.preload.js';
 import { waitForAllBatchers } from '../../util/batcher.std.js';
 import { flushAllWaitBatchers } from '../../util/waitBatcher.std.js';
 import { DelimitedStream } from '../../util/DelimitedStream.node.js';
@@ -51,7 +48,7 @@ import {
 } from '../../types/backups.node.js';
 import { HTTPError } from '../../types/HTTPError.std.js';
 import { constantTimeEqual } from '../../Crypto.node.js';
-import { measureSize } from '../../AttachmentCrypto.node.js';
+import { measureSize, safeUnlink } from '../../AttachmentCrypto.node.js';
 import { signalProtocolStore } from '../../SignalProtocolStore.preload.js';
 import { isTestOrMockEnvironment } from '../../environment.std.js';
 import { runStorageServiceSyncJob } from '../storage.preload.js';
@@ -94,6 +91,8 @@ import {
   writeLocalBackupFilesList,
   readLocalBackupFilesList,
   validateLocalBackupStructure,
+  getAllPathsInLocalBackupFilesDirectory,
+  getLocalBackupFilesDirectory,
 } from './util/localBackup.node.js';
 import {
   AttachmentPermanentlyMissingError,
@@ -327,6 +326,7 @@ export class BackupsService {
       const { totalBytes } = await this.exportToDisk(filePath, {
         type: 'remote',
         level: backupLevel,
+        abortSignal: new AbortController().signal,
       });
 
       await this.api.upload(filePath, totalBytes);
@@ -358,37 +358,78 @@ export class BackupsService {
       options.backupsBaseDir,
       `signal-backup-${getTimestampForFolder()}`
     );
-    await mkdir(snapshotDir, { recursive: true });
 
-    const freeSpaceBytes = await getFreeDiskSpace(snapshotDir);
+    const freeSpaceBytes = await getFreeDiskSpace(options.backupsBaseDir);
     const bytesNeeded = MIMINUM_DISK_SPACE_FOR_LOCAL_EXPORT - freeSpaceBytes;
     if (bytesNeeded > 0) {
       fnLog.info(
         `Not enough storage; only ${freeSpaceBytes} available, ${MIMINUM_DISK_SPACE_FOR_LOCAL_EXPORT} is minimum needed`
       );
-      throw new NotEnoughStorageError(bytesNeeded);
     }
 
-    const exportResult = await this.exportToDisk(join(snapshotDir, 'main'), {
-      type: 'local-encrypted',
-      snapshotDir,
+    const filesDir = getLocalBackupFilesDirectory({
+      backupsBaseDir: options.backupsBaseDir,
     });
 
-    const metadataArgs = {
-      snapshotDir,
-      backupId: getBackupId(),
-      metadataKey: getLocalBackupMetadataKey(),
-    };
-    await writeLocalBackupMetadata(metadataArgs);
-    await verifyLocalBackupMetadata(metadataArgs);
-    await this.#runLocalAttachmentBackupJobs({
-      attachmentBackupJobs: exportResult.attachmentBackupJobs,
-      baseDir: options.backupsBaseDir,
-      onProgress: options.onProgress,
-      abortSignal: options.abortSignal,
-    });
+    await mkdir(filesDir, { recursive: true });
 
-    return { ...exportResult, snapshotDir };
+    const attachmentPathsWrittenBeforeThisBackup =
+      await getAllPathsInLocalBackupFilesDirectory({
+        backupsBaseDir: options.backupsBaseDir,
+      });
+
+    try {
+      await mkdir(snapshotDir, { recursive: true });
+
+      const exportResult = await this.exportToDisk(join(snapshotDir, 'main'), {
+        type: 'local-encrypted',
+        snapshotDir,
+        abortSignal: options.abortSignal,
+      });
+
+      const metadataArgs = {
+        snapshotDir,
+        backupId: getBackupId(),
+        metadataKey: getLocalBackupMetadataKey(),
+      };
+      await writeLocalBackupMetadata(metadataArgs);
+      await verifyLocalBackupMetadata(metadataArgs);
+      await this.#runLocalAttachmentBackupJobs({
+        attachmentBackupJobs: exportResult.attachmentBackupJobs,
+        baseDir: options.backupsBaseDir,
+        onProgress: options.onProgress,
+        abortSignal: options.abortSignal,
+      });
+
+      return { ...exportResult, snapshotDir };
+    } catch (e) {
+      if (options.abortSignal.aborted) {
+        log.warn('exportLocalBackup aborted', Errors.toLogFormat(e));
+      } else {
+        log.error('exportLocalBackup encountered error', Errors.toLogFormat(e));
+      }
+
+      log.info('Deleting snapshot directory');
+      await rm(snapshotDir, { recursive: true, force: true });
+      log.info('Deleted Snapshot directory');
+
+      const attachmentPathsAfterBackup =
+        await getAllPathsInLocalBackupFilesDirectory({
+          backupsBaseDir: options.backupsBaseDir,
+        });
+
+      const pathsAdded = new Set(attachmentPathsAfterBackup).difference(
+        new Set(attachmentPathsWrittenBeforeThisBackup)
+      );
+
+      if (pathsAdded.size > 0) {
+        log.info(`Deleting ${pathsAdded.size} newly written files`);
+        await Promise.all([...pathsAdded].map(safeUnlink));
+        log.info(`Deleted ${pathsAdded.size} files`);
+      }
+
+      throw e;
+    }
   }
 
   async #runLocalAttachmentBackupJobs({
@@ -600,6 +641,7 @@ export class BackupsService {
         join(exportDir, 'main.jsonl'),
         {
           type: 'plaintext-export',
+          abortSignal,
         }
       );
 
@@ -667,6 +709,7 @@ export class BackupsService {
       const recordStream = new BackupExportStream({
         type: 'remote',
         level: BackupLevel.Free,
+        abortSignal: new AbortController().signal,
       });
 
       recordStream.run();
@@ -688,19 +731,6 @@ export class BackupsService {
     }
   }
 
-  // Test harness
-  public async exportWithDialog(): Promise<void> {
-    const { data } = await this.exportBackupData({
-      type: 'remote',
-      level: BackupLevel.Free,
-    });
-
-    await saveAttachmentToDisk({
-      name: 'backup.bin',
-      data,
-    });
-  }
-
   public async importFromDisk(
     backupFile: string,
     options: BackupImportOptions
@@ -1146,7 +1176,8 @@ export class BackupsService {
                 totalBytes = size;
               },
             }),
-            sink
+            sink,
+            { signal: options.abortSignal }
           );
           break;
         case 'cross-client-integration-test':
@@ -1161,7 +1192,8 @@ export class BackupsService {
                 totalBytes = size;
               },
             }),
-            sink
+            sink,
+            { signal: options.abortSignal }
           );
           break;
         case 'plaintext-export':
@@ -1172,7 +1204,8 @@ export class BackupsService {
                 totalBytes = size;
               },
             }),
-            sink
+            sink,
+            { signal: options.abortSignal }
           );
           break;
         default:

ts/services/backups/types.std.ts

@@ -34,7 +34,7 @@ export type OnProgressCallback = (
   totalBytes: number
 ) => void;
 
-export type BackupExportOptions =
+export type BackupExportOptions = { abortSignal: AbortSignal } & (
   | {
       type: 'remote' | 'cross-client-integration-test';
       level: BackupLevel;
@@ -45,7 +45,8 @@ export type BackupExportOptions =
   | {
       type: 'local-encrypted';
       snapshotDir: string;
-    };
+    }
+);
 
 export type BackupImportOptions = (
   | { type: 'remote' | 'cross-client-integration-test' }

ts/services/backups/util/localBackup.node.ts

@@ -3,7 +3,7 @@
 
 import { randomBytes } from 'node:crypto';
 import { dirname, join } from 'node:path';
-import { readFile, stat, writeFile } from 'node:fs/promises';
+import { readdir, readFile, stat, writeFile } from 'node:fs/promises';
 import { createReadStream, createWriteStream } from 'node:fs';
 import { Transform } from 'node:stream';
 import { pipeline } from 'node:stream/promises';
@@ -25,6 +25,29 @@ const log = createLogger('localBackup');
 
 const { Reader } = protobuf;
 
+export function getLocalBackupFilesDirectory({
+  backupsBaseDir,
+}: {
+  backupsBaseDir: string;
+}): string {
+  return join(backupsBaseDir, 'files');
+}
+
+export async function getAllPathsInLocalBackupFilesDirectory({
+  backupsBaseDir,
+}: {
+  backupsBaseDir: string;
+}): Promise<Array<string>> {
+  const filesDir = getLocalBackupFilesDirectory({ backupsBaseDir });
+  const allEntries = await readdir(filesDir, {
+    withFileTypes: true,
+    recursive: true,
+  });
+  return allEntries
+    .filter(entry => entry.isFile())
+    .map(entry => join(entry.parentPath, entry.name));
+}
+
 export function getLocalBackupDirectoryForMediaName({
   backupsBaseDir,
   mediaName,
@@ -36,7 +59,10 @@ export function getLocalBackupDirectoryForMediaName({
     throw new Error('Invalid mediaName input');
   }
 
-  return join(backupsBaseDir, 'files', mediaName.substring(0, 2));
+  return join(
+    getLocalBackupFilesDirectory({ backupsBaseDir }),
+    mediaName.substring(0, 2)
+  );
 }
 
 export function getLocalBackupPathForMediaName({

ts/sql/main.main.ts

@@ -204,7 +204,10 @@ export class MainSQL {
 
   public resumeWriteAccess(): void {
     const pauseWaiters = this.#pauseWaiters;
-    strictAssert(pauseWaiters != null, 'Not paused');
+    if (pauseWaiters == null) {
+      return;
+    }
+
     this.#pauseWaiters = undefined;
 
     for (const waiter of pauseWaiters) {

ts/test-electron/backup/filePointer_test.preload.ts

@@ -241,6 +241,7 @@ describe('getFilePointerForAttachment', () => {
       backupOptions: {
         type: 'remote',
         level: BackupLevel.Paid,
+        abortSignal: new AbortController().signal,
       },
       getBackupCdnInfo: notInBackupCdn,
       messageReceivedAt: 100,
@@ -271,6 +272,7 @@ describe('getFilePointerForAttachment', () => {
         backupOptions: {
           type: 'remote',
           level: BackupLevel.Paid,
+          abortSignal: new AbortController().signal,
         },
         getBackupCdnInfo: notInBackupCdn,
         messageReceivedAt: 100,
@@ -298,6 +300,7 @@ describe('getFilePointerForAttachment', () => {
         backupOptions: {
           type: 'remote',
           level: BackupLevel.Free,
+          abortSignal: new AbortController().signal,
         },
         getBackupCdnInfo: notInBackupCdn,
         messageReceivedAt: 100,
@@ -326,6 +329,7 @@ describe('getFilePointerForAttachment', () => {
         backupOptions: {
           type: 'remote',
           level: BackupLevel.Free,
+          abortSignal: new AbortController().signal,
         },
         getBackupCdnInfo: notInBackupCdn,
         messageReceivedAt: 100,
@@ -354,6 +358,7 @@ describe('getFilePointerForAttachment', () => {
         backupOptions: {
           type: 'remote',
           level: BackupLevel.Free,
+          abortSignal: new AbortController().signal,
         },
         getBackupCdnInfo: notInBackupCdn,
         messageReceivedAt: 100,
@@ -378,6 +383,7 @@ describe('getFilePointerForAttachment', () => {
         backupOptions: {
           type: 'remote',
           level: BackupLevel.Paid,
+          abortSignal: new AbortController().signal,
         },
         getBackupCdnInfo: notInBackupCdn,
         messageReceivedAt: 100,
@@ -422,6 +428,7 @@ describe('getFilePointerForAttachment', () => {
         backupOptions: {
           type: 'remote',
           level: BackupLevel.Paid,
+          abortSignal: new AbortController().signal,
         },
         getBackupCdnInfo: notInBackupCdn,
         messageReceivedAt: 100,
@@ -459,6 +466,7 @@ describe('getFilePointerForAttachment', () => {
           backupOptions: {
             type: 'local-encrypted',
             snapshotDir: '/root/backups/signal-backup-12-12-12',
+            abortSignal: new AbortController().signal,
           },
           getBackupCdnInfo: notInBackupCdn,
           messageReceivedAt: 100,
@@ -498,6 +506,7 @@ describe('getFilePointerForAttachment', () => {
           backupOptions: {
             type: 'local-encrypted',
             snapshotDir: '/root/backups/signal-backup-12-12-12',
+            abortSignal: new AbortController().signal,
           },
           getBackupCdnInfo: notInBackupCdn,
           messageReceivedAt: 100,

ts/test-electron/backup/helpers.preload.ts

@@ -245,6 +245,7 @@ export async function asymmetricRoundtripHarness(
     await backupsService.exportToDisk(targetOutputFile, {
       type: 'remote',
       level: options.backupLevel,
+      abortSignal: new AbortController().signal,
     });
 
     await updateConvoIdToTitle();

ts/test-electron/backup/integration_test.preload.ts

@@ -59,6 +59,7 @@ describe('backup/integration', () => {
       const { data: exported } = await backupsService.exportBackupData({
         type: 'cross-client-integration-test',
         level: BackupLevel.Paid,
+        abortSignal: new AbortController().signal,
       });
 
       const actualStream = new MemoryStream(Buffer.from(exported));