From d297c0c5827daa3ceb41792f0cb6a684bb826669 Mon Sep 17 00:00:00 2001 From: Leo Dion Date: Mon, 11 May 2026 15:38:45 -0400 Subject: [PATCH 1/2] Resolve #313: paginationLimitExceeded carries accumulated records When `queryAllRecords` and `fetchAllRecordChanges` hit their `maxPages` cap, they previously threw an error that dropped the partial result on the floor (either `paginationLimitExceeded(recordsCollected: Int)` or a bare `.invalidResponse`). Callers had to retry from scratch or accept silent data loss. The case now carries `records: [RecordInfo]` so callers can pattern- match the error and recover everything collected before the cap. * `CloudKitError.paginationLimitExceeded(maxPages:records:)` replaces the count-only payload. `errorDescription` keeps the same human-readable shape via `records.count`. * `queryAllRecords` throws with `records: allRecords`. Docstring corrected (it used to claim the throw was `.invalidResponse`). * `fetchAllRecordChanges` gains an explicit `maxPages:` parameter (default 1 000, was a hard-coded local) and throws the same case on overflow. * New tests under `CloudKitServiceQueryPagination/+ErrorCases.swift` and `CloudKitServiceFetchChanges/+ErrorHandling.swift` drive overflow with `maxPages: 2` and assert the carried records. * ReleaseNotes flags the breaking case-shape change under v1.0.0-beta.1. Co-Authored-By: Claude Opus 4.7 (1M context) --- ReleaseNotes.md | 1 + .../CloudKitService+QueryPagination.swift | 11 +-- .../CloudKitService+SyncOperations.swift | 15 ++-- .../ResponseProcessing/CloudKitError.swift | 6 +- ...viceTests.FetchChanges+ErrorHandling.swift | 50 +++++++++++++ ...viceTests.QueryPagination+ErrorCases.swift | 71 +++++++++++++++++++ 6 files changed, 143 insertions(+), 11 deletions(-) create mode 100644 Tests/MistKitTests/Service/CloudKitServiceQueryPagination/CloudKitServiceTests.QueryPagination+ErrorCases.swift diff --git a/ReleaseNotes.md b/ReleaseNotes.md index 2f5045ee..ea672400 100644 --- a/ReleaseNotes.md +++ b/ReleaseNotes.md @@ -11,6 +11,7 @@ ### Error Handling * Improve error handling: typed TokenManagerError and safe RecordOperation conversion by @leogdion in https://github.com/brightdigit/MistKit/pull/305 * Move CloudKitResponseType default implementations to protocol extension by @leogdion in https://github.com/brightdigit/MistKit/pull/292 +* **Breaking:** `CloudKitError.paginationLimitExceeded` now carries `records: [RecordInfo]` instead of `recordsCollected: Int`. When `queryAllRecords` / `fetchAllRecordChanges` exceed `maxPages`, callers can pattern-match the error to recover every record collected before the cap. `fetchAllRecordChanges` gains an explicit `maxPages:` parameter and no longer throws the generic `.invalidResponse` on overflow. Resolves [#313](https://github.com/brightdigit/MistKit/issues/313). ### Concurrency * Replace custom AsyncChannel with swift-async-algorithms by @leogdion in https://github.com/brightdigit/MistKit/pull/280 diff --git a/Sources/MistKit/Service/Extensions/CloudKitService+QueryPagination.swift b/Sources/MistKit/Service/Extensions/CloudKitService+QueryPagination.swift index d4c11b41..0ce61153 100644 --- a/Sources/MistKit/Service/Extensions/CloudKitService+QueryPagination.swift +++ b/Sources/MistKit/Service/Extensions/CloudKitService+QueryPagination.swift @@ -45,13 +45,16 @@ extension CloudKitService { /// (1-200, defaults to `defaultQueryLimit`) /// - desiredKeys: Optional array of field names to fetch /// - maxPages: Maximum number of pages to fetch before throwing - /// `CloudKitError.invalidResponse` (defaults to 1,000) + /// `CloudKitError.paginationLimitExceeded` (defaults to 1,000) /// - Returns: Array of all matching records across all pages - /// - Throws: CloudKitError if any page request fails + /// - Throws: `CloudKitError`. When `maxPages` is exceeded, throws + /// `.paginationLimitExceeded(maxPages:records:)` whose `records` + /// payload contains every record collected before the cap was hit, + /// so callers can resume or surface partial results. /// /// - Warning: Stops early if the server returns the same /// continuation marker with no new records (stuck-marker - /// scenario), or if the page count exceeds `maxPages`. + /// scenario). public func queryAllRecords( recordType: String, filters: [QueryFilter]? = nil, @@ -69,7 +72,7 @@ extension CloudKitService { guard pageCount < maxPages else { throw CloudKitError.paginationLimitExceeded( maxPages: maxPages, - recordsCollected: allRecords.count + records: allRecords ) } diff --git a/Sources/MistKit/Service/Extensions/CloudKitService+SyncOperations.swift b/Sources/MistKit/Service/Extensions/CloudKitService+SyncOperations.swift index 7ddbe01c..6f55b73f 100644 --- a/Sources/MistKit/Service/Extensions/CloudKitService+SyncOperations.swift +++ b/Sources/MistKit/Service/Extensions/CloudKitService+SyncOperations.swift @@ -135,8 +135,12 @@ extension CloudKitService { /// (defaults to _defaultZone) /// - syncToken: Optional token from previous fetch (nil = initial fetch) /// - resultsLimit: Optional maximum records per request (1-200) + /// - maxPages: Maximum number of pages to fetch before throwing + /// `CloudKitError.paginationLimitExceeded` (defaults to 1,000) /// - Returns: Array of all changed records and final sync token - /// - Throws: CloudKitError if any fetch fails + /// - Throws: `CloudKitError`. When `maxPages` is exceeded, throws + /// `.paginationLimitExceeded(maxPages:records:)` whose `records` + /// payload contains every record collected before the cap was hit. /// /// Example: /// ```swift @@ -153,7 +157,7 @@ extension CloudKitService { /// with manual pagination for better memory control. /// - Warning: This method will stop early if the server repeatedly returns /// `moreComing: true` with no records and the same sync token - /// (stuck-token scenario), or if the page count exceeds 1000. + /// (stuck-token scenario). /// - Note: Makes sequential requests with no backoff or cooperative /// cancellation between pages. For fine-grained control, /// use `fetchRecordChanges(syncToken:)` directly. @@ -161,17 +165,20 @@ extension CloudKitService { zoneID: ZoneID? = nil, syncToken: String? = nil, resultsLimit: Int? = nil, + maxPages: Int = 1_000, database: Database = .public ) async throws(CloudKitError) -> (records: [RecordInfo], syncToken: String?) { var allRecords: [RecordInfo] = [] var currentToken = syncToken var moreComing = true var pageCount = 0 - let maxPages = 1_000 while moreComing { guard pageCount < maxPages else { - throw CloudKitError.invalidResponse + throw CloudKitError.paginationLimitExceeded( + maxPages: maxPages, + records: allRecords + ) } do { diff --git a/Sources/MistKit/Service/ResponseProcessing/CloudKitError.swift b/Sources/MistKit/Service/ResponseProcessing/CloudKitError.swift index c05c7c9e..ef57a1e4 100644 --- a/Sources/MistKit/Service/ResponseProcessing/CloudKitError.swift +++ b/Sources/MistKit/Service/ResponseProcessing/CloudKitError.swift @@ -44,7 +44,7 @@ public enum CloudKitError: LocalizedError, Sendable { case decodingError(DecodingError) case networkError(URLError) case unsupportedOperationType(String) - case paginationLimitExceeded(maxPages: Int, recordsCollected: Int) + case paginationLimitExceeded(maxPages: Int, records: [RecordInfo]) case missingCredentials(database: Database, reason: String) case invalidPrivateKey(path: String?, underlying: any Error) @@ -123,10 +123,10 @@ public enum CloudKitError: LocalizedError, Sendable { return message case .unsupportedOperationType(let type): return "Unsupported record operation type: \(type)" - case .paginationLimitExceeded(let maxPages, let recordsCollected): + case .paginationLimitExceeded(let maxPages, let records): return "CloudKit query exceeded pagination limit of \(maxPages) pages " - + "(collected \(recordsCollected) records)" + + "(collected \(records.count) records)" case .missingCredentials(let database, let reason): return "Missing credentials for database '\(database.rawValue)': \(reason)" diff --git a/Tests/MistKitTests/Service/CloudKitServiceFetchChanges/CloudKitServiceTests.FetchChanges+ErrorHandling.swift b/Tests/MistKitTests/Service/CloudKitServiceFetchChanges/CloudKitServiceTests.FetchChanges+ErrorHandling.swift index 53643789..4815a7c1 100644 --- a/Tests/MistKitTests/Service/CloudKitServiceFetchChanges/CloudKitServiceTests.FetchChanges+ErrorHandling.swift +++ b/Tests/MistKitTests/Service/CloudKitServiceFetchChanges/CloudKitServiceTests.FetchChanges+ErrorHandling.swift @@ -73,6 +73,56 @@ extension CloudKitServiceTests.FetchChanges { } } + @Test("fetchAllRecordChanges() throws paginationLimitExceeded carrying collected records") + internal func fetchAllOverflowReturnsAccumulatedRecords() async throws { + guard #available(macOS 11.0, iOS 14.0, tvOS 14.0, watchOS 7.0, *) else { + Issue.record("CloudKitService is not available on this operating system.") + return + } + // Three pages, each still indicating moreComing:true, so the loop would + // keep going. With maxPages:2 the third page triggers the cap and we + // expect the records from pages 1 and 2 to come back inside the error. + let provider = ResponseProvider( + defaultResponse: try .successfulFetchChangesResponse( + recordCount: 0, + moreComing: true, + syncToken: "default-token" + ) + ) + await provider.enqueue( + try .successfulFetchChangesResponse( + recordCount: 3, + moreComing: true, + syncToken: "token-1" + ), + for: "fetchRecordChanges" + ) + await provider.enqueue( + try .successfulFetchChangesResponse( + recordCount: 2, + moreComing: true, + syncToken: "token-2" + ), + for: "fetchRecordChanges" + ) + let service = try CloudKitServiceTests.makeService(provider: provider) + + do { + _ = try await service.fetchAllRecordChanges(maxPages: 2) + Issue.record("Expected paginationLimitExceeded to be thrown") + } catch CloudKitError.paginationLimitExceeded(let maxPages, let records) { + #expect(maxPages == 2) + #expect(records.count == 5) + #expect( + records.map(\.recordName) == [ + "record-0", "record-1", "record-2", + "record-0", "record-1", + ]) + } catch { + Issue.record("Expected paginationLimitExceeded, got \(error)") + } + } + @Test("fetchAllRecordChanges() propagates a mid-pagination network failure") internal func fetchAllPropagatesMidPaginationFailure() async throws { guard #available(macOS 11.0, iOS 14.0, tvOS 14.0, watchOS 7.0, *) else { diff --git a/Tests/MistKitTests/Service/CloudKitServiceQueryPagination/CloudKitServiceTests.QueryPagination+ErrorCases.swift b/Tests/MistKitTests/Service/CloudKitServiceQueryPagination/CloudKitServiceTests.QueryPagination+ErrorCases.swift new file mode 100644 index 00000000..f7aed2a1 --- /dev/null +++ b/Tests/MistKitTests/Service/CloudKitServiceQueryPagination/CloudKitServiceTests.QueryPagination+ErrorCases.swift @@ -0,0 +1,71 @@ +// +// CloudKitServiceTests.QueryPagination+ErrorCases.swift +// MistKit +// +// Created by Leo Dion. +// Copyright © 2026 BrightDigit. +// +// Permission is hereby granted, free of charge, to any person +// obtaining a copy of this software and associated documentation +// files (the "Software"), to deal in the Software without +// restriction, including without limitation the rights to use, +// copy, modify, merge, publish, distribute, sublicense, and/or +// sell copies of the Software, and to permit persons to whom the +// Software is furnished to do so, subject to the following +// conditions: +// +// The above copyright notice and this permission notice shall be +// included in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES +// OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT +// HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, +// WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR +// OTHER DEALINGS IN THE SOFTWARE. +// + +import Foundation +import Testing + +@testable import MistKit + +extension CloudKitServiceTests.QueryPagination { + @Suite("Error Cases") + internal struct ErrorCases { + @Test("queryAllRecords() throws paginationLimitExceeded carrying collected records") + internal func queryAllRecordsOverflowReturnsAccumulatedRecords() async throws { + guard #available(macOS 11.0, iOS 14.0, tvOS 14.0, watchOS 7.0, *) else { + Issue.record("CloudKitService is not available on this operating system.") + return + } + let service = try await CloudKitServiceTests.QueryPagination.makePaginatedService( + pages: [ + (recordCount: 3, continuationMarker: "marker-1"), + (recordCount: 2, continuationMarker: "marker-2"), + (recordCount: 5, continuationMarker: "marker-3"), + ] + ) + + do { + _ = try await service.queryAllRecords( + recordType: "TestRecord", + maxPages: 2 + ) + Issue.record("Expected paginationLimitExceeded to be thrown") + } catch CloudKitError.paginationLimitExceeded(let maxPages, let records) { + #expect(maxPages == 2) + #expect(records.count == 5) + #expect( + records.map(\.recordName) == [ + "record-0", "record-1", "record-2", + "record-0", "record-1", + ]) + } catch { + Issue.record("Expected paginationLimitExceeded, got \(error)") + } + } + } +} From 53833339a603d2097787a54da66d9cd7b04dfa8a Mon Sep 17 00:00:00 2001 From: Leo Dion Date: Mon, 11 May 2026 15:47:35 -0400 Subject: [PATCH 2/2] Drop ReleaseNotes bullet, match repeat..while style in fetchAllRecordChanges * ReleaseNotes: the v1.0.0-beta.1 section is a list of auto-generated per-PR titles; the free-form "Breaking" bullet didn't fit the format. PR #326's description carries the breaking-change note. * `fetchAllRecordChanges`: switch from `while moreComing` (seeded with a throwaway `true`) to `repeat..while moreComing`, matching the sibling `queryAllRecords` in `CloudKitService+QueryPagination.swift`. Behaviorally identical. Co-Authored-By: Claude Opus 4.7 (1M context) --- ReleaseNotes.md | 1 - .../Extensions/CloudKitService+SyncOperations.swift | 7 ++++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ReleaseNotes.md b/ReleaseNotes.md index ea672400..2f5045ee 100644 --- a/ReleaseNotes.md +++ b/ReleaseNotes.md @@ -11,7 +11,6 @@ ### Error Handling * Improve error handling: typed TokenManagerError and safe RecordOperation conversion by @leogdion in https://github.com/brightdigit/MistKit/pull/305 * Move CloudKitResponseType default implementations to protocol extension by @leogdion in https://github.com/brightdigit/MistKit/pull/292 -* **Breaking:** `CloudKitError.paginationLimitExceeded` now carries `records: [RecordInfo]` instead of `recordsCollected: Int`. When `queryAllRecords` / `fetchAllRecordChanges` exceed `maxPages`, callers can pattern-match the error to recover every record collected before the cap. `fetchAllRecordChanges` gains an explicit `maxPages:` parameter and no longer throws the generic `.invalidResponse` on overflow. Resolves [#313](https://github.com/brightdigit/MistKit/issues/313). ### Concurrency * Replace custom AsyncChannel with swift-async-algorithms by @leogdion in https://github.com/brightdigit/MistKit/pull/280 diff --git a/Sources/MistKit/Service/Extensions/CloudKitService+SyncOperations.swift b/Sources/MistKit/Service/Extensions/CloudKitService+SyncOperations.swift index 6f55b73f..d7af0c32 100644 --- a/Sources/MistKit/Service/Extensions/CloudKitService+SyncOperations.swift +++ b/Sources/MistKit/Service/Extensions/CloudKitService+SyncOperations.swift @@ -170,10 +170,10 @@ extension CloudKitService { ) async throws(CloudKitError) -> (records: [RecordInfo], syncToken: String?) { var allRecords: [RecordInfo] = [] var currentToken = syncToken - var moreComing = true + var moreComing = false var pageCount = 0 - while moreComing { + repeat { guard pageCount < maxPages else { throw CloudKitError.paginationLimitExceeded( maxPages: maxPages, @@ -194,6 +194,7 @@ extension CloudKitService { database: database ) + // Stuck-token detection if result.records.isEmpty && result.moreComing && result.syncToken == currentToken { break } @@ -206,7 +207,7 @@ extension CloudKitService { currentToken = result.syncToken moreComing = result.moreComing pageCount += 1 - } + } while moreComing return (allRecords, currentToken) }