Respond to CR.

Signal/src/util/Backup/OWSBackupAPI.swift

@@ -135,22 +135,15 @@ import PromiseKit
 
     public class func saveRecordsToCloud(records: [CKRecord]) -> Promise<Void> {
 
-        var remainder = records
-        var promise = Promise.value(())
         // CloudKit's internal limit is 400, but I haven't found a constant for this.
         let kMaxBatchSize = 100
-        while remainder.count > 0 {
+        return records.chunked(by: kMaxBatchSize).reduce(Promise.value(())) { (promise, batch) -> Promise<Void> in
-            let batch = Array(remainder[0..<kMaxBatchSize])
+            return promise.then(on: .global()) {
-            remainder = Array(remainder[kMaxBatchSize..<remainder.count])
+                saveRecordsToCloud(records: batch, remainingRetries: maxRetries)
-            promise = promise.then(on: DispatchQueue.global()) { _ in
+                }.done {
-                return saveRecordsToCloud(records: batch,
+                    Logger.verbose("Saved batch: \(batch.count)")
-                                          remainingRetries: maxRetries)
-            }.then(on: DispatchQueue.global()) { _ -> Promise<Void> in
-                Logger.verbose("Saved batch: \(batch.count)")
-                return Promise.value(())
             }
         }
-        return promise
     }
 
     private class func saveRecordsToCloud(records: [CKRecord],

Signal/src/util/Backup/OWSBackupExportJob.m

@@ -794,7 +794,7 @@ NS_ASSUME_NONNULL_BEGIN
                 // OWSAttachmentExport is used to lazily write an encrypted copy of the
                 // attachment to disk.
                 if (![attachmentExport prepareForUpload]) {
-                    // Attachment files are non-critical so any error uploading them is recoverable.
+                    // Attachment files are non-critical so any error preparing them is recoverable.
                     return @(1);
                 }
                 OWSAssertDebug(attachmentExport.relativeFilePath.length > 0);
@@ -814,7 +814,7 @@ NS_ASSUME_NONNULL_BEGIN
             }();
 
             if (!fileUrl) {
-                // Attachment files are non-critical so any error uploading them is recoverable.
+                // Attachment files are non-critical so any error preparing them is recoverable.
                 return @(1);
             }
 
@@ -828,21 +828,20 @@ NS_ASSUME_NONNULL_BEGIN
         });
     }
 
+    void (^cleanup)(void) = ^{
+        for (OWSAttachmentExport *attachmentExport in items) {
+            if (![attachmentExport cleanUp]) {
+                OWSLogError(@"couldn't clean up attachment export.");
+                // Attachment files are non-critical so any error uploading them is recoverable.
+            }
+        }
+    };
+
     // TODO: Expose progress.
-    dispatch_queue_t backgroundQueue = dispatch_get_global_queue(0, 0);
     return promise
         .thenInBackground(^{
             return [OWSBackupAPI saveRecordsToCloudObjcWithRecords:records];
         })
-        .ensureOn(backgroundQueue,
-            ^{
-                for (OWSAttachmentExport *attachmentExport in items) {
-                    if (![attachmentExport cleanUp]) {
-                        OWSLogError(@"couldn't clean up attachment export.");
-                        // Attachment files are non-critical so any error uploading them is recoverable.
-                    }
-                }
-            })
         .thenInBackground(^{
             OWSAssertDebug(items.count == records.count);
             NSUInteger count = MIN(items.count, records.count);
@@ -874,9 +873,13 @@ NS_ASSUME_NONNULL_BEGIN
                     attachmentExport.relativeFilePath);
             }
         })
-        .catchInBackground(^{
+        .thenInBackground(^{
-            // Attachment files are non-critical so any error uploading them is recoverable.
+            cleanup();
-            return [AnyPromise promiseWithValue:@(1)];
+        })
+        .catchInBackground(^(NSError *error) {
+            cleanup();
+
+            return error;
         });
 }