From 8f4bcd6541b5f0164084f0cbb3ddb25c04638c27 Mon Sep 17 00:00:00 2001 From: OpenCow Date: Sun, 26 Jul 2026 13:34:08 +0200 Subject: [PATCH] feat: retry throttled kdrive reads --- CHANGELOG.md | 17 + .../KDriveRateLimitRetry.swift | 165 +++++++++ .../KDriveRemoteService.swift | 186 ++++++++-- doc/FILE_PROVIDER_LIFECYCLE.md | 11 + doc/KDRIVE_API_MAPPING.md | 30 ++ doc/LOGGING.md | 7 + doc/TESTING_AND_DEVELOPMENT.md | 12 +- potassiumProvider.xcodeproj/project.pbxproj | 4 +- .../xcshareddata/swiftpm/Package.resolved | 5 +- .../KDriveRateLimitRetryTests.swift | 342 ++++++++++++++++++ .../potassiumProviderTests.swift | 9 + 11 files changed, 744 insertions(+), 44 deletions(-) create mode 100644 PotassiumProviderCore/KDriveRateLimitRetry.swift create mode 100644 potassiumProviderTests/KDriveRateLimitRetryTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index d38f400..e075dd5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,22 @@ # Changelog +## Unreleased + +### Changed + +- Read-style kDrive operations now recognize HTTP 429 responses and retry with + bounded, cancellation-aware backoff. Server-provided `Retry-After` values are + preferred; otherwise the provider uses jittered exponential delays. +- Mutation requests remain single-attempt so an ambiguous response cannot + replay a server-side change. +- Exhausted rate limits map to File Provider's recoverable + `.serverUnreachable` state and retain only sanitized diagnostics. + +### Dependency state + +- Requires the coordinated potassiumChannel 0.3.0 response-metadata API, which + preserves `Retry-After` without exposing arbitrary response headers. + ## 0.3.0 ### Added diff --git a/PotassiumProviderCore/KDriveRateLimitRetry.swift b/PotassiumProviderCore/KDriveRateLimitRetry.swift new file mode 100644 index 0000000..956b50d --- /dev/null +++ b/PotassiumProviderCore/KDriveRateLimitRetry.swift @@ -0,0 +1,165 @@ +import Foundation + +struct KDriveRateLimitRetryPolicy: Equatable, Sendable { + static let `default` = KDriveRateLimitRetryPolicy( + maximumRetryCount: 3, + initialBackoff: 1, + maximumDelay: 60, + jitterRange: 0.8...1.2 + ) + + let maximumRetryCount: Int + let initialBackoff: TimeInterval + let maximumDelay: TimeInterval + let jitterRange: ClosedRange + + func delay( + retryNumber: Int, + retryAfter: String?, + now: Date, + jitterUnitValue: Double + ) -> KDriveRateLimitRetryDelay { + if let retryAfter, + let requestedDelay = Self.retryAfterDelay(retryAfter, relativeTo: now) { + return KDriveRateLimitRetryDelay( + seconds: min(requestedDelay, maximumDelay), + source: .server + ) + } + + let exponent = max(retryNumber - 1, 0) + let exponentialDelay = initialBackoff * pow(2, Double(exponent)) + let clampedJitter = min(max(jitterUnitValue, 0), 1) + let jitterMultiplier = jitterRange.lowerBound + + ((jitterRange.upperBound - jitterRange.lowerBound) * clampedJitter) + return KDriveRateLimitRetryDelay( + seconds: min(exponentialDelay * jitterMultiplier, maximumDelay), + source: .exponential + ) + } + + private static func retryAfterDelay( + _ rawValue: String, + relativeTo now: Date + ) -> TimeInterval? { + let value = rawValue.trimmingCharacters(in: .whitespacesAndNewlines) + if let seconds = Int(value), seconds >= 0 { + return TimeInterval(seconds) + } + + for format in httpDateFormats { + let formatter = DateFormatter() + formatter.locale = Locale(identifier: "en_US_POSIX") + formatter.calendar = Calendar(identifier: .gregorian) + formatter.timeZone = TimeZone(secondsFromGMT: 0) + formatter.dateFormat = format + if let date = formatter.date(from: value) { + return max(0, date.timeIntervalSince(now)) + } + } + return nil + } + + private static let httpDateFormats = [ + "EEE',' dd MMM yyyy HH':'mm':'ss z", + "EEEE',' dd-MMM-yy HH':'mm':'ss z", + "EEE MMM d HH':'mm':'ss yyyy", + ] +} + +struct KDriveRateLimitRetryDelay: Equatable, Sendable { + enum Source: String, Equatable, Sendable { + case server + case exponential + } + + let seconds: TimeInterval + let source: Source +} + +protocol KDriveRetrySleeping: Sendable { + func sleep(for seconds: TimeInterval) async throws +} + +struct TaskKDriveRetrySleeper: KDriveRetrySleeping { + func sleep(for seconds: TimeInterval) async throws { + guard seconds > 0 else { + try Task.checkCancellation() + return + } + try await Task.sleep(for: .seconds(seconds)) + } +} + +protocol KDriveRetryClock: Sendable { + func now() -> Date +} + +struct SystemKDriveRetryClock: KDriveRetryClock { + func now() -> Date { + Date() + } +} + +protocol KDriveRetryJitterProviding: Sendable { + func unitValue() -> Double +} + +struct SystemKDriveRetryJitter: KDriveRetryJitterProviding { + func unitValue() -> Double { + Double.random(in: 0...1) + } +} + +struct KDriveRateLimitRetryExecutor: Sendable { + static let live = KDriveRateLimitRetryExecutor( + policy: .default, + sleeper: TaskKDriveRetrySleeper(), + clock: SystemKDriveRetryClock(), + jitter: SystemKDriveRetryJitter() + ) + + let policy: KDriveRateLimitRetryPolicy + let sleeper: any KDriveRetrySleeping + let clock: any KDriveRetryClock + let jitter: any KDriveRetryJitterProviding + + func perform( + _ work: () async throws -> Value, + onRetry: (Int, KDriveRateLimitRetryDelay) -> Void + ) async throws -> Value { + var retryCount = 0 + + while true { + try Task.checkCancellation() + do { + return try await work() + } catch is CancellationError { + throw CancellationError() + } catch { + if Task.isCancelled { + throw CancellationError() + } + guard let throttling = KDriveRemoteErrorClassifier.throttling(from: error), + retryCount < policy.maximumRetryCount else { + throw error + } + + retryCount += 1 + let delay = policy.delay( + retryNumber: retryCount, + retryAfter: throttling.retryAfter, + now: clock.now(), + jitterUnitValue: jitter.unitValue() + ) + onRetry(retryCount, delay) + try await sleeper.sleep(for: delay.seconds) + } + } + } +} + +struct KDriveRemoteThrottling: Equatable, Sendable { + let statusCode: Int + let retryAfter: String? +} diff --git a/PotassiumProviderCore/KDriveRemoteService.swift b/PotassiumProviderCore/KDriveRemoteService.swift index 2cc73c6..2f48452 100644 --- a/PotassiumProviderCore/KDriveRemoteService.swift +++ b/PotassiumProviderCore/KDriveRemoteService.swift @@ -148,12 +148,29 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot private let apiClient: InfomaniakAPIClient private let driveClient: InfomaniakAPIClient private let service: KDriveService + private let retryExecutor: KDriveRateLimitRetryExecutor public init( bearerToken: String, apiBaseURL: URL = ProviderConstants.apiBaseURL, driveBaseURL: URL = ProviderConstants.driveBaseURL, session: URLSession = .shared + ) { + self.init( + bearerToken: bearerToken, + apiBaseURL: apiBaseURL, + driveBaseURL: driveBaseURL, + session: session, + retryExecutor: .live + ) + } + + init( + bearerToken: String, + apiBaseURL: URL = ProviderConstants.apiBaseURL, + driveBaseURL: URL = ProviderConstants.driveBaseURL, + session: URLSession = .shared, + retryExecutor: KDriveRateLimitRetryExecutor ) { self.apiClient = InfomaniakAPIClient( configuration: APIClientConfiguration(baseURL: apiBaseURL, bearerToken: bearerToken), @@ -164,10 +181,11 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot session: session ) self.service = KDriveService(client: apiClient) + self.retryExecutor = retryExecutor } public func listDrives() async throws -> [KDriveDriveSummary] { - try await performNetworkOperation("listDrives") { + try await performNetworkOperation("listDrives", retryMode: .rateLimit) { let response = try await driveClient.send(APIRequest>( method: .get, path: "/2/drive/init", @@ -187,13 +205,13 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot } public func item(driveID: Int, fileID: Int) async throws -> KDriveRemoteItem { - try await performNetworkOperation("item") { + try await performNetworkOperation("item", retryMode: .rateLimit) { try await service.getFile(driveId: driveID, fileId: fileID).data.remoteItem } } public func listDirectory(driveID: Int, folderID: Int, cursor: String?, limit: Int) async throws -> KDriveItemPage { - try await performNetworkOperation("listDirectory") { + try await performNetworkOperation("listDirectory", retryMode: .rateLimit) { let response = try await service.listDirectoryFiles( driveId: driveID, fileId: folderID, @@ -207,7 +225,7 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot let orderBy = ["type", "name"] let orderFor = ["type": "asc", "name": "asc"] - return try await performNetworkOperation("listAdvancedDirectory") { + return try await performNetworkOperation("listAdvancedDirectory", retryMode: .rateLimit) { let response: CursorPaginatedInfomaniakResponse if let cursor { response = try await service.continueAdvancedDirectoryListing( @@ -245,7 +263,7 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot } public func listTrash(driveID: Int, cursor: String?, limit: Int) async throws -> KDriveItemPage { - try await performNetworkOperation("listTrash") { + try await performNetworkOperation("listTrash", retryMode: .rateLimit) { let response = try await service.listTrashFiles( driveId: driveID, options: ListKDriveTrashOptions(cursor: cursor, limit: limit, orderBy: ["name"], order: "asc") @@ -255,19 +273,25 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot } public func listWorkingSetRelevantItems(driveID: Int, latestLimit: Int) async throws -> [KDriveRemoteItem] { - try await performNetworkOperation("listWorkingSetRelevantItems") { - let latest = try await service.listLastModifiedFiles(driveId: driveID, limit: latestLimit).data - let favorites = try await service.listFavoriteFiles(driveId: driveID, limit: latestLimit).data - let myShared = try await service.listMySharedFiles(driveId: driveID, limit: latestLimit).data - let sharedWithMe = try await service.listSharedWithMeFiles(driveId: driveID, limit: latestLimit).data - var itemsByID: [Int: KDriveRemoteItem] = [:] - for item in latest + favorites + myShared + sharedWithMe { - itemsByID[item.id] = item.remoteItem - } - return itemsByID.values.sorted { - if $0.updatedAt != $1.updatedAt { return $0.updatedAt > $1.updatedAt } - return $0.id < $1.id - } + let latest = try await performNetworkOperation("listLastModifiedFiles", retryMode: .rateLimit) { + try await service.listLastModifiedFiles(driveId: driveID, limit: latestLimit).data + } + let favorites = try await performNetworkOperation("listFavoriteFiles", retryMode: .rateLimit) { + try await service.listFavoriteFiles(driveId: driveID, limit: latestLimit).data + } + let myShared = try await performNetworkOperation("listMySharedFiles", retryMode: .rateLimit) { + try await service.listMySharedFiles(driveId: driveID, limit: latestLimit).data + } + let sharedWithMe = try await performNetworkOperation("listSharedWithMeFiles", retryMode: .rateLimit) { + try await service.listSharedWithMeFiles(driveId: driveID, limit: latestLimit).data + } + var itemsByID: [Int: KDriveRemoteItem] = [:] + for item in latest + favorites + myShared + sharedWithMe { + itemsByID[item.id] = item.remoteItem + } + return itemsByID.values.sorted { + if $0.updatedAt != $1.updatedAt { return $0.updatedAt > $1.updatedAt } + return $0.id < $1.id } } @@ -277,7 +301,7 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot since: Date ) async throws -> [KDrivePartialActivityResult] { guard fileIDs.isEmpty == false else { return [] } - return try await performNetworkOperation("listPartialActivities") { + return try await performNetworkOperation("listPartialActivities", retryMode: .rateLimit) { let response = try await service.listPartialFileActivities( driveId: driveID, options: ListKDrivePartialFileActivitiesOptions( @@ -311,20 +335,30 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot } public func downloadFileOperation(driveID: Int, fileID: Int) throws -> KDriveTransferOperation { - let operation = try service.downloadFile(driveId: driveID, fileId: fileID) + let progress = Progress(totalUnitCount: -1) + let controller = KDriveRetryingTransferController() return KDriveTransferOperation( - progress: operation.progress, + progress: progress, value: { - try await performNetworkOperation("downloadFile") { - try await operation.value + try await performNetworkOperation("downloadFile", retryMode: .rateLimit) { + let operation = try service.downloadFile(driveId: driveID, fileId: fileID) + let transfer = KDriveTransferOperation( + progress: operation.progress, + value: { try await operation.value }, + cancellation: operation.cancel + ) + try controller.install(transfer) + progress.attachRetryAttempt(transfer.progress) + defer { controller.clear(transfer) } + return try await transfer.value } }, - cancellation: operation.cancel + cancellation: controller.cancel ) } public func thumbnail(driveID: Int, fileID: Int, width: Int?, height: Int?) async throws -> Data { - try await performNetworkOperation("thumbnail") { + try await performNetworkOperation("thumbnail", retryMode: .rateLimit) { try await service.getFileThumbnail( driveId: driveID, fileId: fileID, @@ -480,14 +514,14 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot } public func trashedItem(driveID: Int, fileID: Int) async throws -> KDriveRemoteItem { - try await performNetworkOperation("trashedItem") { + try await performNetworkOperation("trashedItem", retryMode: .rateLimit) { try await service.getTrashedFile(driveId: driveID, fileId: fileID).data.remoteItem } } public func existingFileIDs(driveID: Int, fileIDs: [Int]) async throws -> Set { guard fileIDs.isEmpty == false else { return [] } - return try await performNetworkOperation("existingFileIDs") { + return try await performNetworkOperation("existingFileIDs", retryMode: .rateLimit) { Set(try await service.checkFilesExistence(driveId: driveID, fileIds: fileIDs).data.lazy .filter(\.result) .map(\.id)) @@ -506,12 +540,12 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot public func shareLink(driveID: Int, fileID: Int) async throws -> KDriveShareLinkSummary? { do { - return try await performNetworkOperation("shareLink") { + return try await performNetworkOperation("shareLink", retryMode: .rateLimit) { try Self.shareLinkSummary( try await service.getFileShareLink(driveId: driveID, fileId: fileID).data ) } - } catch APIClientError.unacceptableStatusCode(let statusCode, _) where statusCode == 404 { + } catch APIClientError.unacceptableStatusCode(let statusCode, _, _) where statusCode == 404 { return nil } } @@ -564,7 +598,7 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot page: Int, pageSize: Int ) async throws -> KDriveFileVersionPage { - try await performNetworkOperation("fileVersions") { + try await performNetworkOperation("fileVersions", retryMode: .rateLimit) { let response = try await service.listFileVersions( driveId: driveID, fileId: fileID, @@ -673,6 +707,7 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot private func performNetworkOperation( _ operation: String, + retryMode: KDriveNetworkRetryMode = .none, _ work: () async throws -> Value ) async throws -> Value { let correlationID = UUID().uuidString @@ -680,7 +715,16 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot ProviderLog.network.debug("network start operation(\(operation, privacy: .public)) correlationID(\(correlationID, privacy: .public))") do { - let value = try await work() + let value: Value + switch retryMode { + case .none: + value = try await work() + case .rateLimit: + value = try await retryExecutor.perform(work) { retryNumber, delay in + let delayMilliseconds = Int(delay.seconds * 1_000) + ProviderLog.network.info("network throttled operation(\(operation, privacy: .public)) correlationID(\(correlationID, privacy: .public)) retryNumber(\(retryNumber, privacy: .public)) delayMilliseconds(\(delayMilliseconds, privacy: .public)) delaySource(\(delay.source.rawValue, privacy: .public))") + } + } let durationMilliseconds = Int(Date().timeIntervalSince(startedAt) * 1_000) ProviderLog.network.info("network success operation(\(operation, privacy: .public)) correlationID(\(correlationID, privacy: .public)) durationMilliseconds(\(durationMilliseconds, privacy: .public))") return value @@ -700,15 +744,29 @@ public struct PotassiumKDriveService: KDriveFileProviding, KDriveWorkingSetRemot public enum KDriveRemoteErrorClassifier { public static func apiRejection(from error: Error) -> KDriveRemoteAPIRejection? { - guard case let APIClientError.unacceptableStatusCode(statusCode, body) = error else { + guard case let APIClientError.unacceptableStatusCode(statusCode, body, metadata) = error else { return nil } - return KDriveRemoteAPIRejection(statusCode: statusCode, responseBody: body) + return KDriveRemoteAPIRejection( + statusCode: statusCode, + responseBody: body, + retryAfter: metadata.retryAfter + ) + } + + static func throttling(from error: Error) -> KDriveRemoteThrottling? { + guard let rejection = apiRejection(from: error), rejection.statusCode == 429 else { + return nil + } + return KDriveRemoteThrottling( + statusCode: rejection.statusCode, + retryAfter: rejection.retryAfter + ) } public static func isInvalidCursor(_ error: Error) -> Bool { - guard case let APIClientError.unacceptableStatusCode(_, body) = error else { + guard case let APIClientError.unacceptableStatusCode(_, body, _) = error else { return false } @@ -720,16 +778,21 @@ public enum KDriveRemoteErrorClassifier { public struct KDriveRemoteAPIRejection: Equatable, Sendable { public let statusCode: Int public let responseBody: String + public let retryAfter: String? - public init(statusCode: Int, responseBody: String) { + public init(statusCode: Int, responseBody: String, retryAfter: String? = nil) { self.statusCode = statusCode self.responseBody = responseBody + self.retryAfter = retryAfter } public var recovery: KDriveRemoteAPIRejectionRecovery { if statusCode == 401 { return .notAuthenticated } + if statusCode == 429 { + return .serverUnreachable + } if isInsufficientQuota { return .insufficientQuota } @@ -766,6 +829,59 @@ public struct KDriveRemoteAPIRejection: Equatable, Sendable { } } +private enum KDriveNetworkRetryMode { + case none + case rateLimit +} + +private final class KDriveRetryingTransferController: @unchecked Sendable { + private let lock = NSLock() + private var activeOperation: KDriveTransferOperation? + private var isCancelled = false + + func install(_ operation: KDriveTransferOperation) throws { + let shouldCancel = lock.withLock { + if isCancelled { + return true + } + activeOperation = operation + return false + } + if shouldCancel { + operation.cancel() + throw CancellationError() + } + } + + func clear(_ operation: KDriveTransferOperation) { + lock.withLock { + if activeOperation === operation { + activeOperation = nil + } + } + } + + func cancel() { + let operation = lock.withLock { + isCancelled = true + let operation = activeOperation + activeOperation = nil + return operation + } + operation?.cancel() + } +} + +private extension Progress { + func attachRetryAttempt(_ child: Progress) { + if totalUnitCount < 0, child.totalUnitCount >= 0 { + totalUnitCount = max(child.totalUnitCount, 1) + } + let pendingUnits = max(totalUnitCount, 1) + addChild(child, withPendingUnitCount: pendingUnits) + } +} + public enum KDriveRemoteAPIRejectionRecovery: Equatable, Sendable { case notAuthenticated case serverUnreachable diff --git a/doc/FILE_PROVIDER_LIFECYCLE.md b/doc/FILE_PROVIDER_LIFECYCLE.md index 59c1976..d3451af 100644 --- a/doc/FILE_PROVIDER_LIFECYCLE.md +++ b/doc/FILE_PROVIDER_LIFECYCLE.md @@ -210,6 +210,14 @@ updates late when the containing app and extension are suspended. errors to `.serverUnreachable`, preserves Cocoa/File Provider errors, and wraps unexpected errors as an XPC reply invalid error. +Read-style kDrive calls retry HTTP 429 responses inside +`PotassiumKDriveService` before an error reaches a File Provider callback. +`Retry-After` is preferred and bounded; otherwise the service uses jittered +exponential backoff for at most three retries. The sleep and any active +download are cancelled with the callback task. If the budget is exhausted, +429 maps to `.serverUnreachable`; mutation requests are never automatically +replayed. + The extension uses the same mapping decision to create a sanitized activity diagnostic. File Provider callback behavior stays unchanged: recording failures is best-effort, database write errors are sent only to `OSLog`, and the original @@ -227,3 +235,6 @@ Generic failure activity is recorded at File Provider callback boundaries: Cancellations are not recorded as failures. Conflict-specific failures that already have `conflict_events` rows, such as stale mutation blocks and failed conflict-copy uploads, are not duplicated as generic failure activity. +Intermediate 429 attempts are unified-log events only. A read that later +succeeds does not create durable failure activity; exhausted retries create one +sanitized terminal failure at the existing callback boundary. diff --git a/doc/KDRIVE_API_MAPPING.md b/doc/KDRIVE_API_MAPPING.md index 3e5c467..7108638 100644 --- a/doc/KDRIVE_API_MAPPING.md +++ b/doc/KDRIVE_API_MAPPING.md @@ -52,6 +52,30 @@ It preserves potassiumChannel's live Foundation progress, shared async result, and cancellation of the underlying URL session task. Async convenience methods remain available for callers that do not need to observe the transfer. +## Rate-Limit Recovery + +potassiumChannel preserves the `Retry-After` response field as safe typed +metadata on rejected HTTP responses. `PotassiumKDriveService` classifies HTTP +429 as throttling and retries read-style work at the network-operation +boundary: + +- metadata, drive, directory, trash, and working-set reads +- read-only partial-activity and existence queries, even though those endpoints + use POST +- thumbnails and downloads +- trashed-item metadata, share-link lookup, and file-version listing + +The default budget is three retries after the initial request. A valid +`Retry-After` delay or HTTP date is capped at 60 seconds and used without +jitter. Missing or malformed values use 1, 2, and 4 second exponential delays +with uniform ±20% jitter. Each download retry creates a fresh lazy transfer; +the returned parent progress and cancellation boundary remain stable. + +Upload, create, replace, rename, move, trash, delete, favorite, duplicate, +restore, share-link mutation, and version-restore requests are never +automatically replayed. Safe metadata reads surrounding those mutations still +use the read policy. + ## Listing Options Legacy directory listing uses: @@ -111,6 +135,12 @@ Directory create does not currently pass an explicit conflict policy. listing cursors from `APIClientError.unacceptableStatusCode` bodies containing both "invalid" and "cursor". +`KDriveRemoteErrorClassifier.apiRejection(...)` maps exhausted HTTP 429 +responses to `.serverUnreachable`. Existing mappings remain unchanged: 401 is +`.notAuthenticated`, quota responses including 507 are `.insufficientQuota`, +5xx is `.serverUnreachable`, and other API rejections are +`.cannotSynchronize`. + The partial-activity request is batched at 200 identifiers and uses the last durable successful-poll watermark. It includes create, delete, trash, restore, update, rename, move, favorite, and share actions relevant to working-set state. diff --git a/doc/LOGGING.md b/doc/LOGGING.md index ea40386..7277479 100644 --- a/doc/LOGGING.md +++ b/doc/LOGGING.md @@ -31,6 +31,13 @@ or file bytes. The service does not currently expose a kDrive request ID, so the optional durable `remoteRequestID` field remains empty unless a future typed API surface provides one safely. +Rate-limit retries keep the original network correlation ID. Unified logs +include the retry number, numeric delay in milliseconds, and whether the delay +came from `Retry-After` or exponential fallback. They never include the raw +header value or response body. Intermediate throttling is not persisted in the +Activities database; only terminal exhaustion reaches the existing sanitized +failure recorder. + ## Durable Activity Data `KDriveProviderActivityEvent` records the operation, scope, outcome, severity, diff --git a/doc/TESTING_AND_DEVELOPMENT.md b/doc/TESTING_AND_DEVELOPMENT.md index 76bddfa..2ef108b 100644 --- a/doc/TESTING_AND_DEVELOPMENT.md +++ b/doc/TESTING_AND_DEVELOPMENT.md @@ -35,10 +35,10 @@ Swift package dependencies are resolved by Xcode: The app imports split potassiumChannel modules directly. It should not import an old monolithic `potassiumChannel` module name. -The project requires the published potassiumChannel 0.2 release line. -`Package.resolved` must stay locked to the validated 0.2.0 release unless a -later compatible package version is adopted and the full validation matrix is -rerun. +Rate-limit recovery requires potassiumChannel 0.3.0 or later so rejected +responses preserve safe `Retry-After` metadata. `Package.resolved` must stay +locked to the validated release and the full validation matrix must be rerun +when that package pin changes. ## Commands @@ -129,6 +129,10 @@ Provider release gates remain outside its scope. - UI automation uses XCTest. - Existing URLProtocol-based tests use shared capture helpers, so the unit suite is serialized. +- Rate-limit tests inject their clock, jitter source, and sleeper. They validate + server-directed and fallback delays without real waiting, and use a blocking + test sleeper only to prove cancellation interrupts backoff before another + request starts. - Live network checks should not be part of the default test path. ## Local State Caveats diff --git a/potassiumProvider.xcodeproj/project.pbxproj b/potassiumProvider.xcodeproj/project.pbxproj index 1bf7d17..5b1aa08 100644 --- a/potassiumProvider.xcodeproj/project.pbxproj +++ b/potassiumProvider.xcodeproj/project.pbxproj @@ -1255,8 +1255,8 @@ isa = XCRemoteSwiftPackageReference; repositoryURL = "https://github.com/OpenCow42/potassiumChannel.git"; requirement = { - kind = upToNextMajorVersion; - minimumVersion = 0.2.0; + kind = revision; + revision = fc41c4213060af2cf735d6a7381431487987097e; }; }; C0DCD8ED2FFA44AB00520215 /* XCRemoteSwiftPackageReference "swift-concurrency" */ = { diff --git a/potassiumProvider.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/potassiumProvider.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 34cbefa..db30785 100644 --- a/potassiumProvider.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/potassiumProvider.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -1,13 +1,12 @@ { - "originHash" : "613cbb0d4ce50189d97b72e2dc228bda845d46b491781fb0143dbeb79a057a19", + "originHash" : "4ca0a296a28bfdc2857c81a1f70f11cc9891a6c43de2f84a68c780c40bcf7fe3", "pins" : [ { "identity" : "potassiumchannel", "kind" : "remoteSourceControl", "location" : "https://github.com/OpenCow42/potassiumChannel.git", "state" : { - "revision" : "8a6d236d69c381c17f334b66dd4075ef2e0b7d89", - "version" : "0.2.0" + "revision" : "fc41c4213060af2cf735d6a7381431487987097e" } }, { diff --git a/potassiumProviderTests/KDriveRateLimitRetryTests.swift b/potassiumProviderTests/KDriveRateLimitRetryTests.swift new file mode 100644 index 0000000..7a6ae26 --- /dev/null +++ b/potassiumProviderTests/KDriveRateLimitRetryTests.swift @@ -0,0 +1,342 @@ +import Foundation +import PotassiumChannelCore +@testable import PotassiumProviderCore +import Testing + +@Suite("kDrive rate-limit retry", .serialized) +struct KDriveRateLimitRetryTests { + @Test func policyPrefersAndCapsRetryAfterSeconds() { + let decision = KDriveRateLimitRetryPolicy.default.delay( + retryNumber: 1, + retryAfter: "120", + now: Date(timeIntervalSince1970: 0), + jitterUnitValue: 0 + ) + + #expect(decision == KDriveRateLimitRetryDelay(seconds: 60, source: .server)) + } + + @Test func policyParsesStandardAndLegacyHTTPDates() { + let now = Date(timeIntervalSince1970: 784_111_747) + let values = [ + "Sun, 06 Nov 1994 08:49:37 GMT", + "Sunday, 06-Nov-94 08:49:37 GMT", + "Sun Nov 6 08:49:37 1994", + ] + + for value in values { + let decision = KDriveRateLimitRetryPolicy.default.delay( + retryNumber: 1, + retryAfter: value, + now: now, + jitterUnitValue: 0 + ) + #expect(decision.source == .server) + #expect(decision.seconds == 30) + } + } + + @Test func policyUsesDeterministicBoundedExponentialFallback() { + let policy = KDriveRateLimitRetryPolicy.default + + #expect(policy.delay( + retryNumber: 1, + retryAfter: nil, + now: .distantPast, + jitterUnitValue: 0 + ).seconds == 0.8) + #expect(policy.delay( + retryNumber: 2, + retryAfter: "invalid", + now: .distantPast, + jitterUnitValue: 0.5 + ).seconds == 2) + #expect(policy.delay( + retryNumber: 7, + retryAfter: nil, + now: .distantPast, + jitterUnitValue: 1 + ).seconds == 60) + } + + @Test func metadataReadHonorsRetryAfterThenSucceeds() async throws { + let sleeper = RecordingRetrySleeper() + RateLimitURLProtocol.reset(responses: [ + .init(statusCode: 429, headers: ["Retry-After": "7"], data: Data("limited".utf8)), + .init(statusCode: 200, data: Self.itemResponseData), + ]) + let service = makeService(sleeper: sleeper) + + let item = try await service.item(driveID: 100, fileID: 42) + + #expect(item.id == 42) + #expect(RateLimitURLProtocol.requestCount == 2) + #expect(sleeper.delays == [7]) + } + + @Test func missingRetryAfterUsesBoundedBackoffAndExhaustsBudget() async { + let sleeper = RecordingRetrySleeper() + RateLimitURLProtocol.reset(responses: Array( + repeating: .init(statusCode: 429, data: Data("limited".utf8)), + count: 4 + )) + let service = makeService(sleeper: sleeper) + + do { + _ = try await service.item(driveID: 100, fileID: 42) + Issue.record("Expected the retry budget to be exhausted.") + } catch { + let rejection = KDriveRemoteErrorClassifier.apiRejection(from: error) + #expect(rejection?.statusCode == 429) + #expect(rejection?.recovery == .serverUnreachable) + } + + #expect(RateLimitURLProtocol.requestCount == 4) + #expect(sleeper.delays == [1, 2, 4]) + } + + @Test func nonThrottlingFailuresAreNotRetried() async { + let sleeper = RecordingRetrySleeper() + RateLimitURLProtocol.reset(responses: [ + .init(statusCode: 503, data: Data("unavailable".utf8)), + .init(statusCode: 200, data: Self.itemResponseData), + ]) + let service = makeService(sleeper: sleeper) + + do { + _ = try await service.item(driveID: 100, fileID: 42) + Issue.record("Expected a server failure.") + } catch { + #expect(KDriveRemoteErrorClassifier.apiRejection(from: error)?.statusCode == 503) + } + + #expect(RateLimitURLProtocol.requestCount == 1) + #expect(sleeper.delays.isEmpty) + } + + @Test func mutationRequestsRemainSingleAttempt() async { + let sleeper = RecordingRetrySleeper() + RateLimitURLProtocol.reset(responses: [ + .init(statusCode: 429, headers: ["Retry-After": "1"], data: Data("limited".utf8)), + .init(statusCode: 200, data: Self.itemResponseData), + ]) + let service = makeService(sleeper: sleeper) + + do { + _ = try await service.createDirectory(driveID: 100, parentID: 7, name: "Folder") + Issue.record("Expected the mutation to fail without retry.") + } catch { + #expect(KDriveRemoteErrorClassifier.apiRejection(from: error)?.statusCode == 429) + } + + #expect(RateLimitURLProtocol.requestCount == 1) + #expect(sleeper.delays.isEmpty) + } + + @Test func downloadCreatesFreshTransferForRetry() async throws { + let sleeper = RecordingRetrySleeper() + let expectedData = Data([0x89, 0x50, 0x4E, 0x47]) + RateLimitURLProtocol.reset(responses: [ + .init(statusCode: 429, headers: ["Retry-After": "2"], data: Data("limited".utf8)), + .init(statusCode: 200, data: expectedData), + ]) + let service = makeService(sleeper: sleeper) + + let operation = try service.downloadFileOperation(driveID: 100, fileID: 42) + let data = try await operation.value + + #expect(data == expectedData) + #expect(RateLimitURLProtocol.requestCount == 2) + #expect(sleeper.delays == [2]) + } + + @Test func cancellationDuringBackoffStopsBeforeAnotherRequest() async throws { + let sleeper = RecordingRetrySleeper(blocks: true) + RateLimitURLProtocol.reset(responses: [ + .init(statusCode: 429, headers: ["Retry-After": "30"], data: Data("limited".utf8)), + .init(statusCode: 200, data: Self.itemResponseData), + ]) + let service = makeService(sleeper: sleeper) + let task = Task { + try await service.item(driveID: 100, fileID: 42) + } + + try await waitUntil { sleeper.delays.count == 1 } + task.cancel() + + do { + _ = try await task.value + Issue.record("Expected cancellation during backoff.") + } catch is CancellationError { + // Expected. + } catch { + Issue.record("Unexpected error: \(error)") + } + + #expect(RateLimitURLProtocol.requestCount == 1) + } + + private func makeService(sleeper: RecordingRetrySleeper) -> PotassiumKDriveService { + let configuration = URLSessionConfiguration.ephemeral + configuration.protocolClasses = [RateLimitURLProtocol.self] + let session = URLSession(configuration: configuration) + return PotassiumKDriveService( + bearerToken: "redacted-token", + apiBaseURL: URL(string: "https://api.example.test")!, + driveBaseURL: URL(string: "https://drive.example.test")!, + session: session, + retryExecutor: KDriveRateLimitRetryExecutor( + policy: .default, + sleeper: sleeper, + clock: FixedRetryClock(date: Date(timeIntervalSince1970: 0)), + jitter: FixedRetryJitter(value: 0.5) + ) + ) + } + + private func waitUntil( + _ predicate: @escaping @Sendable () -> Bool + ) async throws { + let clock = ContinuousClock() + let deadline = clock.now.advanced(by: .seconds(1)) + while predicate() == false { + guard clock.now < deadline else { + Issue.record("Timed out waiting for retry state.") + return + } + try await Task.sleep(for: .milliseconds(5)) + } + } + + private static let itemResponseData = """ + { + "result": "success", + "data": { + "id": 42, + "name": "Report.txt", + "path": "/Report.txt", + "type": "file", + "status": "active", + "visibility": "is_private_space", + "drive_id": 100, + "parent_id": 7, + "depth": 1, + "created_at": 1700000000, + "last_modified_at": 1700000001, + "updated_at": 1700000002, + "size": 4, + "mime_type": "text/plain", + "is_favorite": false + }, + "response_at": 1700000003 + } + """.data(using: .utf8)! +} + +private struct FixedRetryClock: KDriveRetryClock { + let date: Date + + func now() -> Date { + date + } +} + +private struct FixedRetryJitter: KDriveRetryJitterProviding { + let value: Double + + func unitValue() -> Double { + value + } +} + +private final class RecordingRetrySleeper: KDriveRetrySleeping, @unchecked Sendable { + private let lock = NSLock() + private let blocks: Bool + private var storedDelays: [TimeInterval] = [] + + init(blocks: Bool = false) { + self.blocks = blocks + } + + var delays: [TimeInterval] { + lock.withLock { storedDelays } + } + + func sleep(for seconds: TimeInterval) async throws { + lock.withLock { + storedDelays.append(seconds) + } + if blocks { + try await Task.sleep(for: .seconds(30)) + } else { + try Task.checkCancellation() + } + } +} + +private final class RateLimitURLProtocol: URLProtocol, @unchecked Sendable { + struct Stub: Sendable { + let statusCode: Int + let headers: [String: String] + let data: Data + + init(statusCode: Int, headers: [String: String] = [:], data: Data) { + self.statusCode = statusCode + self.headers = headers + self.data = data + } + } + + private struct State { + var responses: [Stub] = [] + var requestCount = 0 + } + + private final class Storage: @unchecked Sendable { + let lock = NSLock() + var state = State() + } + + static var requestCount: Int { + storage.lock.withLock { storage.state.requestCount } + } + + static func reset(responses: [Stub]) { + storage.lock.withLock { + storage.state = State(responses: responses) + } + } + + private static let storage = Storage() + + override class func canInit(with request: URLRequest) -> Bool { + true + } + + override class func canonicalRequest(for request: URLRequest) -> URLRequest { + request + } + + override func startLoading() { + let stub = Self.storage.lock.withLock { + Self.storage.state.requestCount += 1 + guard Self.storage.state.responses.isEmpty == false else { + return Stub(statusCode: 500, data: Data("missing stub".utf8)) + } + return Self.storage.state.responses.removeFirst() + } + var headers = stub.headers + headers["Content-Length"] = String(stub.data.count) + let response = HTTPURLResponse( + url: request.url!, + statusCode: stub.statusCode, + httpVersion: "HTTP/1.1", + headerFields: headers + )! + client?.urlProtocol(self, didReceive: response, cacheStoragePolicy: .notAllowed) + client?.urlProtocol(self, didLoad: stub.data) + client?.urlProtocolDidFinishLoading(self) + } + + override func stopLoading() {} +} diff --git a/potassiumProviderTests/potassiumProviderTests.swift b/potassiumProviderTests/potassiumProviderTests.swift index 6bfd33f..2132c7e 100644 --- a/potassiumProviderTests/potassiumProviderTests.swift +++ b/potassiumProviderTests/potassiumProviderTests.swift @@ -1381,6 +1381,15 @@ struct PotassiumProviderCoreTests { #expect(authRejection.recovery == .notAuthenticated) #expect(serverRejection.recovery == .serverUnreachable) #expect(validationRejection.recovery == .cannotSynchronize) + let rateLimitRejection = try #require(KDriveRemoteErrorClassifier.apiRejection( + from: APIClientError.unacceptableStatusCode( + 429, + body: "Too Many Requests", + metadata: APIResponseMetadata(retryAfter: "30") + ) + )) + #expect(rateLimitRejection.retryAfter == "30") + #expect(rateLimitRejection.recovery == .serverUnreachable) #expect(KDriveRemoteErrorClassifier.apiRejection(from: NSError(domain: NSURLErrorDomain, code: -1009)) == nil) }