diff --git a/OutlineKit/Sources/OutlineKit/Caching/CachingOutlineAPIClient.swift b/OutlineKit/Sources/OutlineKit/Caching/CachingOutlineAPIClient.swift index 3c9bf20..0342b49 100644 --- a/OutlineKit/Sources/OutlineKit/Caching/CachingOutlineAPIClient.swift +++ b/OutlineKit/Sources/OutlineKit/Caching/CachingOutlineAPIClient.swift @@ -37,6 +37,16 @@ public actor CachingOutlineAPIClient: OutlineAPIClient { private let decoder: JSONDecoder private let keyEncoder: JSONEncoder + /// Recent failure timestamps per category — see `recordFailure` / + /// `repeatedFailureSummaries()`. A category only shows up there once it's + /// failed `failureThreshold` times within `failureWindow`; a single + /// transient blip (which `RetryPolicy` already tries to absorb) never + /// reaches this at all. + private var failureLog: [String: [Date]] = [:] + private var lastFailureMessage: [String: String] = [:] + private let failureWindow: TimeInterval = 300 + private let failureThreshold: Int = 3 + public init(live: OutlineAPIClient, cache: OfflineCacheStore, defaults: UserDefaults = .standard) { self.live = live self.cache = cache @@ -62,7 +72,7 @@ public actor CachingOutlineAPIClient: OutlineAPIClient { // MARK: - Cached reads public func documentInfo(id: String) async throws -> OutlineDocument { - try await cachedFetch(key: "document:\(id)") { try await self.live.documentInfo(id: id) } + try await cachedFetch(key: "document:\(id)", category: "document") { try await self.live.documentInfo(id: id) } } public func listDocuments( @@ -72,7 +82,7 @@ public actor CachingOutlineAPIClient: OutlineAPIClient { limit: Int ) async throws -> [OutlineDocument] { let key = "documents:\(collectionId ?? "-"):\(parentDocumentId ?? "-"):\(offset):\(limit)" - return try await cachedFetch(key: key) { + return try await cachedFetch(key: key, category: "documents") { try await self.live.listDocuments( collectionId: collectionId, parentDocumentId: parentDocumentId, @@ -83,29 +93,29 @@ public actor CachingOutlineAPIClient: OutlineAPIClient { } public func documentsList(_ request: DocumentsListRequest) async throws -> [OutlineDocument] { - try await cachedFetch(key: requestKey("documentsList", request)) { try await self.live.documentsList(request) } + try await cachedFetch(key: requestKey("documentsList", request), category: "documents") { try await self.live.documentsList(request) } } public func listViewedDocuments(offset: Int, limit: Int) async throws -> [OutlineDocument] { - try await cachedFetch(key: "documentsViewed:\(offset):\(limit)") { + try await cachedFetch(key: "documentsViewed:\(offset):\(limit)", category: "documents-viewed") { try await self.live.listViewedDocuments(offset: offset, limit: limit) } } public func listDrafts(_ request: ListDraftsRequest) async throws -> [OutlineDocument] { - try await cachedFetch(key: "documentsDrafts:\(request.offset):\(request.limit)") { + try await cachedFetch(key: "documentsDrafts:\(request.offset):\(request.limit)", category: "drafts") { try await self.live.listDrafts(request) } } public func listCollections(offset: Int, limit: Int) async throws -> [OutlineCollection] { - try await cachedFetch(key: "collections:\(offset):\(limit)") { + try await cachedFetch(key: "collections:\(offset):\(limit)", category: "collections") { try await self.live.listCollections(offset: offset, limit: limit) } } public func collectionInfo(id: String) async throws -> OutlineCollection { - try await cachedFetch(key: "collection:\(id)") { try await self.live.collectionInfo(id: id) } + try await cachedFetch(key: "collection:\(id)", category: "collections") { try await self.live.collectionInfo(id: id) } } // MARK: - Queueable writes @@ -121,10 +131,12 @@ public actor CachingOutlineAPIClient: OutlineAPIClient { public func createDocument(_ request: CreateDocumentRequest) async throws -> OutlineDocument { if !isManualOfflineModeEnabled { do { - let result = try await live.createDocument(request) + let result = try await RetryPolicy.withRetry { try await self.live.createDocument(request) } + recordSuccess(category: "documents-write") await cacheDocument(result) return result } catch { + recordWriteFailureIfStructural(category: "documents-write", error) return await queueDocumentCreate(request) } } @@ -134,10 +146,12 @@ public actor CachingOutlineAPIClient: OutlineAPIClient { public func updateDocument(_ request: UpdateDocumentRequest) async throws -> OutlineDocument { if !isManualOfflineModeEnabled { do { - let result = try await live.updateDocument(request) + let result = try await RetryPolicy.withRetry { try await self.live.updateDocument(request) } + recordSuccess(category: "documents-write") await cacheDocument(result) return result } catch { + recordWriteFailureIfStructural(category: "documents-write", error) return try await queueDocumentUpdate(request, dueTo: error) } } @@ -147,10 +161,12 @@ public actor CachingOutlineAPIClient: OutlineAPIClient { public func updateCollection(_ request: UpdateCollectionRequest) async throws -> OutlineCollection { if !isManualOfflineModeEnabled { do { - let result = try await live.updateCollection(request) + let result = try await RetryPolicy.withRetry { try await self.live.updateCollection(request) } + recordSuccess(category: "collections-write") await cacheCollection(result) return result } catch { + recordWriteFailureIfStructural(category: "collections-write", error) return try await queueCollectionUpdate(request, dueTo: error) } } @@ -159,7 +175,14 @@ public actor CachingOutlineAPIClient: OutlineAPIClient { public func createPin(_ request: CreatePinRequest) async throws -> OutlinePin { if !isManualOfflineModeEnabled { - do { return try await live.createPin(request) } catch { return await queuePinCreate(request) } + do { + let result = try await RetryPolicy.withRetry { try await self.live.createPin(request) } + recordSuccess(category: "pins") + return result + } catch { + recordWriteFailureIfStructural(category: "pins", error) + return await queuePinCreate(request) + } } return await queuePinCreate(request) } @@ -168,9 +191,11 @@ public actor CachingOutlineAPIClient: OutlineAPIClient { if await cancelIfNeverSynced(id: id) { return } if !isManualOfflineModeEnabled { do { - try await live.deletePin(id: id) + try await RetryPolicy.withRetry { try await self.live.deletePin(id: id) } + recordSuccess(category: "pins") return } catch { + recordWriteFailureIfStructural(category: "pins", error) await enqueue(.deletePin, payload: IDPayload(id: id), id: "delete-pin-\(id)") return } @@ -180,7 +205,14 @@ public actor CachingOutlineAPIClient: OutlineAPIClient { public func createSubscription(_ request: CreateSubscriptionRequest) async throws -> OutlineSubscription { if !isManualOfflineModeEnabled { - do { return try await live.createSubscription(request) } catch { return await queueSubscriptionCreate(request) } + do { + let result = try await RetryPolicy.withRetry { try await self.live.createSubscription(request) } + recordSuccess(category: "subscriptions") + return result + } catch { + recordWriteFailureIfStructural(category: "subscriptions", error) + return await queueSubscriptionCreate(request) + } } return await queueSubscriptionCreate(request) } @@ -189,9 +221,11 @@ public actor CachingOutlineAPIClient: OutlineAPIClient { if await cancelIfNeverSynced(id: id) { return } if !isManualOfflineModeEnabled { do { - try await live.deleteSubscription(id: id) + try await RetryPolicy.withRetry { try await self.live.deleteSubscription(id: id) } + recordSuccess(category: "subscriptions") return } catch { + recordWriteFailureIfStructural(category: "subscriptions", error) await enqueue(.deleteSubscription, payload: IDPayload(id: id), id: "delete-subscription-\(id)") return } @@ -201,14 +235,28 @@ public actor CachingOutlineAPIClient: OutlineAPIClient { public func starDocument(_ request: StarDocumentRequest) async throws -> OutlineStar { if !isManualOfflineModeEnabled { - do { return try await live.starDocument(request) } catch { return await queueStarDocumentCreate(request) } + do { + let result = try await RetryPolicy.withRetry { try await self.live.starDocument(request) } + recordSuccess(category: "stars") + return result + } catch { + recordWriteFailureIfStructural(category: "stars", error) + return await queueStarDocumentCreate(request) + } } return await queueStarDocumentCreate(request) } public func starCollection(_ request: StarCollectionRequest) async throws -> OutlineStar { if !isManualOfflineModeEnabled { - do { return try await live.starCollection(request) } catch { return await queueStarCollectionCreate(request) } + do { + let result = try await RetryPolicy.withRetry { try await self.live.starCollection(request) } + recordSuccess(category: "stars") + return result + } catch { + recordWriteFailureIfStructural(category: "stars", error) + return await queueStarCollectionCreate(request) + } } return await queueStarCollectionCreate(request) } @@ -217,9 +265,11 @@ public actor CachingOutlineAPIClient: OutlineAPIClient { if await cancelIfNeverSynced(id: id) { return } if !isManualOfflineModeEnabled { do { - try await live.deleteStar(id: id) + try await RetryPolicy.withRetry { try await self.live.deleteStar(id: id) } + recordSuccess(category: "stars") return } catch { + recordWriteFailureIfStructural(category: "stars", error) await enqueue(.deleteStar, payload: IDPayload(id: id), id: "delete-star-\(id)") return } @@ -564,7 +614,7 @@ public actor CachingOutlineAPIClient: OutlineAPIClient { // MARK: - Helpers - private func cachedFetch(key: String, fetch: () async throws -> T) async throws -> T { + private func cachedFetch(key: String, category: String, fetch: () async throws -> T) async throws -> T { // Manual offline mode means "skip the network entirely," not just // "prefer it" — without this check, a read would still hit `live` // (and succeed, showing content beyond whatever's cached) any time @@ -577,12 +627,21 @@ public actor CachingOutlineAPIClient: OutlineAPIClient { throw OutlineAPIError.transport(URLError(.notConnectedToInternet)) } do { - let result = try await fetch() + let result = try await RetryPolicy.withRetry { try await fetch() } + recordSuccess(category: category) if let data = try? encoder.encode(result) { await cache.save(data, forKey: key) } return result } catch { + // Only a structural failure (decode/auth/server — the server + // answered, but something's actually wrong) counts toward the + // repeated-failure log. Plain connectivity loss already has its + // own offline UI elsewhere; logging it here too would just be a + // second banner for the same thing every time Wi-Fi drops. + if !RetryPolicy.isRetryable(error) { + recordFailure(category: category, message: errorDescription(error)) + } if let data = await cache.load(forKey: key), let cached = try? decoder.decode(T.self, from: data) { return cached } @@ -788,6 +847,46 @@ public actor CachingOutlineAPIClient: OutlineAPIClient { } } + /// Writes always queue on any failure (existing behavior, unchanged) — + /// this only decides whether the failure is worth logging. A structural + /// error (decode/auth/server) queuing for later replay will likely just + /// fail the same way again next sync; a transient one might not. Either + /// way the queue doesn't change, only whether it's counted toward + /// `repeatedFailureSummaries()`. + private func recordWriteFailureIfStructural(category: String, _ error: Error) { + guard !RetryPolicy.isRetryable(error) else { return } + recordFailure(category: category, message: errorDescription(error)) + } + + private func recordFailure(category: String, message: String) { + let now = Date() + var timestamps = (failureLog[category] ?? []).filter { now.timeIntervalSince($0) < failureWindow } + timestamps.append(now) + failureLog[category] = timestamps + lastFailureMessage[category] = message + } + + private func recordSuccess(category: String) { + failureLog[category] = nil + lastFailureMessage[category] = nil + } + + /// Categories that have failed `failureThreshold`+ times within the last + /// `failureWindow` seconds — meant to be polled periodically (see + /// `RootView`), not pushed, since this actor has no UI-facing dependency + /// of its own. A single blip never shows up here: `RetryPolicy` absorbs + /// transient failures before they're ever logged, and only structural + /// ones (decode/auth/server) get logged at all — see + /// `recordWriteFailureIfStructural` and `cachedFetch`. + public func repeatedFailureSummaries() async -> [RepeatedFailure] { + let now = Date() + return failureLog.compactMap { category, timestamps in + let recent = timestamps.filter { now.timeIntervalSince($0) < failureWindow } + guard recent.count >= failureThreshold, let message = lastFailureMessage[category] else { return nil } + return RepeatedFailure(category: category, message: message, count: recent.count) + } + } + private func errorDescription(_ error: Error) -> String { if let apiError = error as? OutlineAPIError { switch apiError { diff --git a/OutlineKit/Sources/OutlineKit/Models/RepeatedFailure.swift b/OutlineKit/Sources/OutlineKit/Models/RepeatedFailure.swift new file mode 100644 index 0000000..a942f93 --- /dev/null +++ b/OutlineKit/Sources/OutlineKit/Models/RepeatedFailure.swift @@ -0,0 +1,19 @@ +import Foundation + +/// A category of API call that's failed repeatedly within a short window — +/// see `CachingOutlineAPIClient.repeatedFailureSummaries()`. Deliberately +/// carries no request/response payload, document content, or server URL: +/// this is meant to be safe to show a user or attach to a bug report as-is. +public struct RepeatedFailure: Sendable, Identifiable, Equatable { + public let category: String + public let message: String + public let count: Int + + public var id: String { category } + + public init(category: String, message: String, count: Int) { + self.category = category + self.message = message + self.count = count + } +} diff --git a/OutlineKit/Sources/OutlineKit/Support/RetryPolicy.swift b/OutlineKit/Sources/OutlineKit/Support/RetryPolicy.swift new file mode 100644 index 0000000..dacbd9a --- /dev/null +++ b/OutlineKit/Sources/OutlineKit/Support/RetryPolicy.swift @@ -0,0 +1,38 @@ +import Foundation + +/// Automatic retry-with-backoff for API calls, so a single transient network +/// blip doesn't turn into a user-visible failure (or a silently swallowed +/// one) the way one `try?` used to. +/// +/// Only retries `OutlineAPIError.transport` — a dropped connection or +/// timeout might succeed a second later. Everything else (`.decoding`, +/// `.unauthorized`, `.notFound`, `.server`, `.tokenUnavailable`) is retried +/// zero times: a response-shape mismatch or a 404 will look exactly the same +/// on attempt two, so retrying just burns the cooldown window for nothing — +/// callers should treat those as immediate failures instead. +public enum RetryPolicy { + public static func withRetry( + maxAttempts: Int = 3, + initialDelay: Duration = .seconds(1), + operation: () async throws -> T + ) async throws -> T { + var attempt = 1 + var delay = initialDelay + while true { + do { + return try await operation() + } catch { + guard attempt < maxAttempts, isRetryable(error) else { throw error } + attempt += 1 + try? await Task.sleep(for: delay) + delay *= 2 + } + } + } + + static func isRetryable(_ error: Error) -> Bool { + guard let apiError = error as? OutlineAPIError else { return false } + if case .transport = apiError { return true } + return false + } +} diff --git a/OutlineKit/Tests/OutlineKitTests/CachingOutlineAPIClientTests.swift b/OutlineKit/Tests/OutlineKitTests/CachingOutlineAPIClientTests.swift index f0f3f67..8df1d93 100644 --- a/OutlineKit/Tests/OutlineKitTests/CachingOutlineAPIClientTests.swift +++ b/OutlineKit/Tests/OutlineKitTests/CachingOutlineAPIClientTests.swift @@ -553,4 +553,102 @@ final class CachingOutlineAPIClientTests: XCTestCase { // expected — nothing left under the old id } } + + // MARK: - Repeated-failure tracking + + /// A structural failure (decode/auth/server) isn't retried — it fails + /// the same way every time, so `RetryPolicy` gives up after one attempt + /// and it's logged immediately. No cache entry means no fallback either, + /// so every call rethrows. + func testRepeatedDecodingFailuresSurfaceAfterThreshold() async throws { + let stub = StubOutlineAPIClient() + stub.documentInfoHandler = { _ in throw OutlineAPIError.decoding(NotStubbed()) } + let sut = CachingOutlineAPIClient(live: stub, cache: try makeCache()) + + for _ in 0..<3 { + _ = try? await sut.documentInfo(id: "doc-1") + } + + let summaries = await sut.repeatedFailureSummaries() + XCTAssertEqual(summaries.count, 1) + XCTAssertEqual(summaries.first?.category, "document") + XCTAssertEqual(summaries.first?.count, 3) + } + + /// Two failures alone shouldn't trip the banner — only three or more + /// within the window counts as "repeated." + func testFewerThanThresholdFailuresDoNotSurface() async throws { + let stub = StubOutlineAPIClient() + stub.documentInfoHandler = { _ in throw OutlineAPIError.decoding(NotStubbed()) } + let sut = CachingOutlineAPIClient(live: stub, cache: try makeCache()) + + for _ in 0..<2 { + _ = try? await sut.documentInfo(id: "doc-1") + } + + let summaries = await sut.repeatedFailureSummaries() + XCTAssertTrue(summaries.isEmpty) + } + + /// A later success clears the category entirely — a transient run of + /// bad luck shouldn't leave a stale banner up after things recover. + func testSuccessAfterRepeatedFailuresClearsTheLog() async throws { + let stub = StubOutlineAPIClient() + let document = makeDocument() + var callCount = 0 + stub.documentInfoHandler = { _ in + callCount += 1 + if callCount <= 3 { throw OutlineAPIError.decoding(NotStubbed()) } + return document + } + let sut = CachingOutlineAPIClient(live: stub, cache: try makeCache()) + + for _ in 0..<3 { + _ = try? await sut.documentInfo(id: "doc-1") + } + let beforeRecovery = await sut.repeatedFailureSummaries() + XCTAssertEqual(beforeRecovery.count, 1) + + _ = try await sut.documentInfo(id: "doc-1") + + let afterRecovery = await sut.repeatedFailureSummaries() + XCTAssertTrue(afterRecovery.isEmpty) + } + + /// Plain connectivity loss (`OutlineAPIError.transport`) already has its + /// own offline UI elsewhere — it shouldn't also pile up in the repeated- + /// failure log and pop a second, redundant banner. + func testTransportFailuresDoNotCountTowardTheRepeatedFailureLog() async throws { + let stub = StubOutlineAPIClient() + stub.documentInfoHandler = { _ in throw OutlineAPIError.transport(URLError(.notConnectedToInternet)) } + let sut = CachingOutlineAPIClient(live: stub, cache: try makeCache()) + + _ = try? await sut.documentInfo(id: "doc-1") + + let summaries = await sut.repeatedFailureSummaries() + XCTAssertTrue(summaries.isEmpty) + } + + /// Different categories (document reads vs. pin writes) track + /// independently — a broken pins endpoint shouldn't mask, or be masked + /// by, unrelated document failures, and one crossing the threshold + /// shouldn't drag an unrelated one along with it. + func testFailuresInDifferentCategoriesDoNotMix() async throws { + let stub = StubOutlineAPIClient() + stub.documentInfoHandler = { _ in throw OutlineAPIError.decoding(NotStubbed()) } + stub.createPinHandler = { _ in throw OutlineAPIError.decoding(NotStubbed()) } + let sut = CachingOutlineAPIClient(live: stub, cache: try makeCache()) + + // Document reads cross the threshold... + for _ in 0..<3 { + _ = try? await sut.documentInfo(id: "doc-1") + } + // ...pin creates don't. + for _ in 0..<2 { + _ = try? await sut.createPin(CreatePinRequest(documentId: "doc-1", collectionId: nil)) + } + + let summaries = await sut.repeatedFailureSummaries() + XCTAssertEqual(summaries.map(\.category), ["document"]) + } } diff --git a/OutlineKit/Tests/OutlineKitTests/RetryPolicyTests.swift b/OutlineKit/Tests/OutlineKitTests/RetryPolicyTests.swift new file mode 100644 index 0000000..74fab28 --- /dev/null +++ b/OutlineKit/Tests/OutlineKitTests/RetryPolicyTests.swift @@ -0,0 +1,69 @@ +import XCTest +@testable import OutlineKit + +private struct PlainError: Error {} + +final class RetryPolicyTests: XCTestCase { + func testSucceedsOnFirstAttemptWithoutRetrying() async throws { + var callCount = 0 + let result = try await RetryPolicy.withRetry(initialDelay: .milliseconds(1)) { + callCount += 1 + return "ok" + } + XCTAssertEqual(result, "ok") + XCTAssertEqual(callCount, 1) + } + + func testRetriesTransportErrorsAndSucceedsOnceItStopsFailing() async throws { + var callCount = 0 + let result = try await RetryPolicy.withRetry(initialDelay: .milliseconds(1)) { () -> String in + callCount += 1 + if callCount < 3 { throw OutlineAPIError.transport(URLError(.timedOut)) } + return "ok" + } + XCTAssertEqual(result, "ok") + XCTAssertEqual(callCount, 3) + } + + func testGivesUpAfterMaxAttemptsAndRethrowsTheLastError() async throws { + var callCount = 0 + do { + _ = try await RetryPolicy.withRetry(maxAttempts: 3, initialDelay: .milliseconds(1)) { () -> String in + callCount += 1 + throw OutlineAPIError.transport(URLError(.timedOut)) + } + XCTFail("Expected the persistent failure to be rethrown") + } catch { + XCTAssertEqual(callCount, 3) + } + } + + /// A decode failure means the response is structurally wrong — trying + /// again gets the exact same wrong response, so it isn't worth the + /// cooldown window the way a network blip is. + func testDoesNotRetryNonTransportErrors() async throws { + var callCount = 0 + do { + _ = try await RetryPolicy.withRetry(initialDelay: .milliseconds(1)) { () -> String in + callCount += 1 + throw OutlineAPIError.decoding(PlainError()) + } + XCTFail("Expected the decoding error to be rethrown without retrying") + } catch { + XCTAssertEqual(callCount, 1) + } + } + + func testDoesNotRetryErrorsThatAreNotOutlineAPIErrors() async throws { + var callCount = 0 + do { + _ = try await RetryPolicy.withRetry(initialDelay: .milliseconds(1)) { () -> String in + callCount += 1 + throw PlainError() + } + XCTFail("Expected the error to be rethrown without retrying") + } catch { + XCTAssertEqual(callCount, 1) + } + } +}