Handle 'batchRequestFailed' errors #328

mbrandonw · 2025-12-12T17:30:00Z

Fixes #315

There appears to be a bug in iCloud/CloudKit that causes .batchRequestFailed errors to be emitted even when operations are not atomic. We aren't sure of the exact conditions that causes the error to be emitted, but regardless let's start handling that error. We will simply re-enqueue those records to be saved/deleted, and then they should succeed just fine.

lukaskubanek

Thanks for addressing the issue from #315!

I’ve run manual tests both in the CloudKitDemo sample app and in my own project, where I originally discovered the issue, and everything worked fine!

Logs from CloudKitDemo

Device B: Update c39453bb & insert 6ee059ba

SQLiteData (private.db) nextRecordZoneChangeBatch: scheduled
  ┏━━━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
  ┃ event     ┃ recordType ┃ recordName                                    ┃
  ┡━━━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┩
  │ ➡️ Sending │ counters   │ 6ee059ba-3681-4625-b859-dff3ed59cfbd:counters │
  │ ➡️ Sending │ counters   │ c39453bb-076d-4498-a30b-1a58f995db2f:counters │
  └───────────┴────────────┴───────────────────────────────────────────────┘
Timestamp: 2025-12-13 08:53:32.321688+07:00

Device A: Update c39453bb & insert fd6a5d63

SQLiteData (private.db) nextRecordZoneChangeBatch: scheduled
  ┏━━━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
  ┃ event     ┃ recordType ┃ recordName                                    ┃
  ┡━━━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┩
  │ ➡️ Sending │ counters   │ fd6a5d63-f031-48fc-b661-b7aa1d7ceb87:counters │
  │ ➡️ Sending │ counters   │ c39453bb-076d-4498-a30b-1a58f995db2f:counters │
  └───────────┴────────────┴───────────────────────────────────────────────┘
Timestamp: 2025-12-13 08:53:32.608295+07:00

Device B: Save changes to c39453bb & 6ee059ba

SQLiteData (private.db) sentRecordZoneChanges
  ┏━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
  ┃ action  ┃ recordType ┃ recordName                                    ┃
  ┡━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┩
  │ ✅ Saved │ counters   │ 6ee059ba-3681-4625-b859-dff3ed59cfbd:counters │
  │ ✅ Saved │ counters   │ c39453bb-076d-4498-a30b-1a58f995db2f:counters │
  └─────────┴────────────┴───────────────────────────────────────────────┘
Timestamp: 2025-12-13 08:53:33.372963+07:00

Device A: Fail saving changes to c39453bb & fd6a5d63

SQLiteData (private.db) sentRecordZoneChanges
  ┏━━━━━━━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━┓
  ┃ action        ┃ recordType ┃ recordName                                    ┃ error                    ┃
  ┡━━━━━━━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━┩
  │ 🛑 Save failed │ counters   │ c39453bb-076d-4498-a30b-1a58f995db2f:counters │ serverRecordChanged (14) │
  │ 🛑 Save failed │ counters   │ fd6a5d63-f031-48fc-b661-b7aa1d7ceb87:counters │ batchRequestFailed (22)  │
  └───────────────┴────────────┴───────────────────────────────────────────────┴──────────────────────────┘
Timestamp: 2025-12-13 08:53:34.554332+07:00

Device A: Fetch changes to c39453bb & 6ee059ba from server

SQLiteData (private.db) fetchedDatabaseChanges
  ┏━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━┓
  ┃ action     ┃ zoneName                            ┃ ownerName        ┃
  ┡━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━┩
  │ ✅ Modified │ co.pointfree.SQLiteData.defaultZone │ __defaultOwner__ │
  └────────────┴─────────────────────────────────────┴──────────────────┘
Timestamp: 2025-12-13 08:53:37.376031+07:00

SQLiteData (private.db) fetchedRecordZoneChanges
  ┏━━━━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
  ┃ action     ┃ recordType ┃ recordName                                    ┃
  ┡━━━━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┩
  │ ✅ Modified │ counters   │ c39453bb-076d-4498-a30b-1a58f995db2f:counters │
  │ ✅ Modified │ counters   │ 6ee059ba-3681-4625-b859-dff3ed59cfbd:counters │
  └────────────┴────────────┴───────────────────────────────────────────────┘
Timestamp: 2025-12-13 08:53:37.756558+07:00

Device A: Send changes to c39453bb & fd6a5d63

SQLiteData (private.db) nextRecordZoneChangeBatch: scheduled
  ┏━━━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
  ┃ event     ┃ recordType ┃ recordName                                    ┃
  ┡━━━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┩
  │ ➡️ Sending │ counters   │ fd6a5d63-f031-48fc-b661-b7aa1d7ceb87:counters │
  │ ➡️ Sending │ counters   │ c39453bb-076d-4498-a30b-1a58f995db2f:counters │
  └───────────┴────────────┴───────────────────────────────────────────────┘
Timestamp: 2025-12-13 08:53:37.776724+07:00

SQLiteData (private.db) sentRecordZoneChanges
  ┏━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
  ┃ action  ┃ recordType ┃ recordName                                    ┃
  ┡━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┩
  │ ✅ Saved │ counters   │ fd6a5d63-f031-48fc-b661-b7aa1d7ceb87:counters │
  │ ✅ Saved │ counters   │ c39453bb-076d-4498-a30b-1a58f995db2f:counters │
  └─────────┴────────────┴───────────────────────────────────────────────┘
Timestamp: 2025-12-13 08:53:38.434067+07:00

Device B: Fetch changes to all touched records

SQLiteData (private.db) fetchedDatabaseChanges
  ┏━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━┓
  ┃ action     ┃ zoneName                            ┃ ownerName        ┃
  ┡━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━┩
  │ ✅ Modified │ co.pointfree.SQLiteData.defaultZone │ __defaultOwner__ │
  └────────────┴─────────────────────────────────────┴──────────────────┘
Timestamp: 2025-12-13 08:53:39.692850+07:00

SQLiteData (private.db) fetchedRecordZoneChanges
  ┏━━━━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
  ┃ action     ┃ recordType ┃ recordName                                    ┃
  ┡━━━━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┩
  │ ✅ Modified │ counters   │ 6ee059ba-3681-4625-b859-dff3ed59cfbd:counters │
  │ ✅ Modified │ counters   │ fd6a5d63-f031-48fc-b661-b7aa1d7ceb87:counters │
  │ ✅ Modified │ counters   │ c39453bb-076d-4498-a30b-1a58f995db2f:counters │
  └────────────┴────────────┴───────────────────────────────────────────────┘
Timestamp: 2025-12-13 08:53:40.139455+07:00

I also skimmed through the implementation and the only thing that stood out is that SyncEngine is now aware of the atomicity flag. Since we never want to explicitly configure it to behave atomically, as CKSyncEngine does not guarantee how changes are batched together anyway, I wonder whether it would be more fitting to move the flag into SyncEngine.processPendingRecordZoneChanges(…) and use it to simulate CloudKit’s incorrect behavior in a more contained way, rather than having full support for the flag. That said, it’s more of a detail.

lukaskubanek · 2025-12-13T07:39:45Z

Tests/SQLiteDataTests/CloudKitTests/AtomicTests.swift

+      }
+
+      @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *)
+      @Test func deleteConflictAndNewRecord() async throws {


The deleteConflictAndNewRecord() test is failing for me. It’s missing inline snapshots for the assertions, and even after generating them, the second call to processPendingRecordZoneChanges(scope:) has no pending record zone changes to process. In other words, the batch for the composite modification succeeds, with record 1 deleted and record 2 inserted.

I initially planned to propose adding change tag comparison for deletes to the mock implementation of modifyRecords(…), similar to what’s done for saves. However, after testing the live behavior, it appears there’s no special handling for this case.

From my observation, a conflicting update and deletion behaves as follows:

Update first: if the update reaches the server first, a subsequent deletion is accepted no matter the change tag, resulting in deletion of the record.

Deletion first: if the deletion reaches the server first, a subsequent update results in an .unknownItem partial error. SQLiteData handles this by effectively re-saving the record from scratch and resurrecting the record.

This test falls into the first scenario, which is not expected to produce a partial error, so the test fails. If we want to test deletions as part of an atomicity test, I see two options:

Keep the structure of this test, but swap the update and deletion of record 1. In the composite modification, this would hit the second scenario and produce the .unknownItem error.

Move the conflict to a concurrent update of a record, include a deletion in the composite modification, and have it fail with a .batchRequestFailed partial error.

2. Move the conflict to a concurrent update of a record, include a deletion in the composite modification, and have it fail with a .batchRequestFailed partial error.

Yeah, this is the test I meant to write but did not finish it. I have just pushed an update.

mbrandonw · 2025-12-14T22:54:39Z

Hi @lukaskubanek, taking the time to look through the PR.

I also skimmed through the implementation and the only thing that stood out is that SyncEngine is now aware of the atomicity flag. Since we never want to explicitly configure it to behave atomically, as CKSyncEngine does not guarantee how changes are batched together anyway, I wonder whether it would be more fitting to move the flag into SyncEngine.processPendingRecordZoneChanges(…) and use it to simulate CloudKit’s incorrect behavior in a more contained way, rather than having full support for the flag.

The atomic flag was just forced the sync engine into this state that we feel is actually buggy on Apple's side. It's not publicly exposed and only tweaked in tests. But, I do think pushing it to processPendingRecordZoneChanges works well too, so just pushed a change for that.

lukaskubanek

Looks good to me!

I ran another manual test and everything seems to work as before.

lukaskubanek · 2025-12-15T03:17:13Z

Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift

        return (saveResults: saveResults, deleteResults: deleteResults)
      }
-
      return (saveResults: saveResults, deleteResults: deleteResults)


Can’t we directly return return storage.withValue { … } rather than storing the result of this expression in additional variables and returning those?

Yep, we can do that now. Think previously we had work done after the withValue, but that's no longer the case. Thanks!

mbrandonw added 5 commits December 1, 2025 15:06

Update mock database to respect atomic modifications and zones.

a9635c8

Merge remote-tracking branch 'origin/main' into batch-record-failed

cb8eb54

wip

154e529

Merge remote-tracking branch 'origin/main' into batch-record-failed

796cf28

clean up

b13a938

mbrandonw requested a review from stephencelis December 12, 2025 17:30

wip

177f45f

mbrandonw mentioned this pull request Dec 12, 2025

Inserted child record silently dropped when batched with a conflicting parent #315

Closed

4 tasks

lukaskubanek approved these changes Dec 13, 2025

View reviewed changes

wip

59966cc

mbrandonw added 2 commits December 14, 2025 17:01

clean up

dfc6772

wip

db63845

lukaskubanek approved these changes Dec 15, 2025

View reviewed changes

mbrandonw added 3 commits December 15, 2025 07:35

wip

c3d2b7f

wip

9b4ef7c

clean up

6f005fb

stephencelis approved these changes Dec 15, 2025

View reviewed changes

mbrandonw merged commit 341468a into main Dec 15, 2025
5 checks passed

mbrandonw deleted the batch-record-failed branch December 15, 2025 18:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Handle 'batchRequestFailed' errors #328

Handle 'batchRequestFailed' errors #328

Uh oh!

mbrandonw commented Dec 12, 2025

Uh oh!

lukaskubanek left a comment •

edited

Loading

Uh oh!

lukaskubanek Dec 13, 2025 •

edited

Loading

Uh oh!

mbrandonw Dec 14, 2025

Uh oh!

mbrandonw commented Dec 14, 2025

Uh oh!

lukaskubanek left a comment

Uh oh!

lukaskubanek Dec 15, 2025

Uh oh!

mbrandonw Dec 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Handle 'batchRequestFailed' errors #328

Handle 'batchRequestFailed' errors #328

Uh oh!

Conversation

mbrandonw commented Dec 12, 2025

Uh oh!

lukaskubanek left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lukaskubanek Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mbrandonw Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

mbrandonw commented Dec 14, 2025

Uh oh!

lukaskubanek left a comment

Choose a reason for hiding this comment

Uh oh!

lukaskubanek Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

mbrandonw Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lukaskubanek left a comment •

edited

Loading

lukaskubanek Dec 13, 2025 •

edited

Loading