From 5c8d1fb7afdfd758eee3746621fa6781e85de06f Mon Sep 17 00:00:00 2001 From: Niels Kootstra <545768+nkootstra@users.noreply.github.com> Date: Fri, 24 Apr 2026 21:48:37 +0200 Subject: [PATCH] fix: surface fetch errors behind cached data and break 429 loop on dead refresh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the popover cache hydrates on launch, any subsequent fetch failure was silently hidden: PopoverView renders UsageDetailView whenever usage != nil, so the error branch was never reached. Combined with a dead refresh_token grant (400) and the usage endpoint returning 429 for stale access tokens, the app got stuck in a 120s backoff loop with no user-visible signal — the refresh button, interval changes, and the menu bar percentage all appeared frozen. - TokenRefreshingClient: when refresh_token grant returns 400/401, clear the stored credential so next read falls through to file-based sources or surfaces noCredential. - TokenRefreshingClient: if the credential is expired, refresh failed, and the re-read returns the same expired value, throw unauthorized instead of shipping a known-dead token to the API. - KeychainReader: add CredentialStore.clearStoredCredential() that removes the entry without setting isSignedOut (user did not sign out). - PopoverView: add StaleDataBanner rendered above UsageDetailView whenever usage != nil && error != nil, with contextual copy and a Sign in / Retry action. Reorder view selection so authFlow.isAwaitingCode takes priority over cached usage — otherwise the code entry field was hidden behind the detail view when sign-in was started from the banner. --- Sources/ClaudeUsageApp/PopoverView.swift | 70 ++++++++++++++++- Sources/ClaudeUsageCore/KeychainReader.swift | 7 ++ .../TokenRefreshingClient.swift | 19 ++++- .../TokenRefreshingClientTests.swift | 78 +++++++++++++++++-- 4 files changed, 162 insertions(+), 12 deletions(-) diff --git a/Sources/ClaudeUsageApp/PopoverView.swift b/Sources/ClaudeUsageApp/PopoverView.swift index 5b3c56e..523f6e2 100644 --- a/Sources/ClaudeUsageApp/PopoverView.swift +++ b/Sources/ClaudeUsageApp/PopoverView.swift @@ -20,10 +20,15 @@ struct MenuContentView: View { .background(Color.primary.opacity(0.06), in: Capsule()) } } - if let usage = viewModel.usage { - UsageDetailView(usage: usage, creditProjection: viewModel.creditProjection) - } else if authFlow.isAwaitingCode { + if authFlow.isAwaitingCode { OAuthCodeEntryView(authFlow: authFlow, viewModel: viewModel) + } else if let usage = viewModel.usage { + if let error = viewModel.error { + StaleDataBanner(error: error, authFlow: authFlow) { + Task { await viewModel.refresh() } + } + } + UsageDetailView(usage: usage, creditProjection: viewModel.creditProjection) } else if let error = viewModel.error { UsageErrorView(error: error, authFlow: authFlow) { Task { await viewModel.refresh() } @@ -91,3 +96,62 @@ struct MenuContentView: View { .frame(width: 260) } } + +/// Inline warning shown above cached usage when the most recent fetch failed. +/// Without this the detail view hides auth/rate-limit errors behind stale data. +private struct StaleDataBanner: View { + let error: UsageError + @ObservedObject var authFlow: AuthFlowState + let onRetry: () -> Void + + private var needsReauth: Bool { error.isAuthError } + + private var message: String { + switch error { + case .noCredential, .unauthorized: return "Session expired — data may be stale" + case .rateLimited: return "Rate limited — showing cached data" + case .networkError: return "Offline — showing cached data" + case .unknown: return "Can't refresh — showing cached data" + } + } + + private var tint: Color { + switch error { + case .noCredential, .unauthorized: return .orange + case .rateLimited: return .yellow + case .networkError, .unknown: return .red + } + } + + var body: some View { + HStack(spacing: 6) { + Image(systemName: "exclamationmark.triangle.fill") + .font(.system(size: 10)) + Text(message) + .font(.system(size: 10)) + .lineLimit(1) + .truncationMode(.tail) + Spacer(minLength: 4) + if needsReauth { + Button("Sign in") { authFlow.startFlow() } + .buttonStyle(.borderless) + .font(.system(size: 10, weight: .medium)) + .foregroundStyle(tint) + } else { + Button("Retry") { onRetry() } + .buttonStyle(.borderless) + .font(.system(size: 10, weight: .medium)) + .foregroundStyle(tint) + } + } + .foregroundStyle(tint) + .padding(.horizontal, 8) + .padding(.vertical, 5) + .background(tint.opacity(0.12), in: RoundedRectangle(cornerRadius: 6)) + .overlay( + RoundedRectangle(cornerRadius: 6) + .strokeBorder(tint.opacity(0.3), lineWidth: 0.5) + ) + .accessibilityElement(children: .combine) + } +} diff --git a/Sources/ClaudeUsageCore/KeychainReader.swift b/Sources/ClaudeUsageCore/KeychainReader.swift index 74d2897..15e9b50 100644 --- a/Sources/ClaudeUsageCore/KeychainReader.swift +++ b/Sources/ClaudeUsageCore/KeychainReader.swift @@ -113,4 +113,11 @@ public enum CredentialStore { try? keychain.remove("credentials") isSignedOut = true } + + /// Removes our stored credential without flagging the user as signed out. + /// Used when a refresh attempt proves the stored tokens are dead, so the + /// next read can fall through to the file-based credential sources. + public static func clearStoredCredential() { + try? keychain.remove("credentials") + } } diff --git a/Sources/ClaudeUsageCore/TokenRefreshingClient.swift b/Sources/ClaudeUsageCore/TokenRefreshingClient.swift index a5ab089..52970fd 100644 --- a/Sources/ClaudeUsageCore/TokenRefreshingClient.swift +++ b/Sources/ClaudeUsageCore/TokenRefreshingClient.swift @@ -32,7 +32,16 @@ public final class TokenRefreshingClient: Sendable { if let refreshed = await refreshOwnToken(credential) { credential = refreshed } else { - credential = try resolveCredential() + let reread = try resolveCredential() + // If re-read returns the same (still-expired) token, sending it + // to the API just burns a request on a known-dead credential — + // and the server often responds with 429, trapping us in a + // backoff loop. Surface as unauthorized so the UI can prompt + // re-auth instead. + if reread.accessToken == credential.accessToken && reread.isExpired { + throw TokenRefreshingClientError.unauthorized + } + credential = reread } } @@ -84,6 +93,14 @@ public final class TokenRefreshingClient: Sendable { ) // Re-read from store so the credential is fully formed return credentialProvider() + } catch APIError.unauthorized { + // Refresh token was rejected — drop it so we don't retry forever. + CredentialStore.clearStoredCredential() + return nil + } catch APIError.serverError(let code) where code == 400 { + // 400 on the refresh_token grant means the token itself is invalid. + CredentialStore.clearStoredCredential() + return nil } catch { return nil } diff --git a/Tests/ClaudeUsageTests/TokenRefreshingClientTests.swift b/Tests/ClaudeUsageTests/TokenRefreshingClientTests.swift index e126025..925c870 100644 --- a/Tests/ClaudeUsageTests/TokenRefreshingClientTests.swift +++ b/Tests/ClaudeUsageTests/TokenRefreshingClientTests.swift @@ -131,27 +131,38 @@ struct TokenRefreshingClientTests { // MARK: - Edge cases - @Test("Provider returns same expired token twice — still calls API") + @Test("Provider returns same expired token twice with no refresh token — fails fast") func sameExpiredTokenTwice() async throws { let expiredMs = Int64(Date().timeIntervalSince1970 * 1000) - 1000 + let apiCounter = FetchCounter() - let mockSession = MockURLSession { _ in + let mockSession = MockURLSession { request in + apiCounter.increment() return (self.fixture, HTTPURLResponse( - url: URL(string: "https://api.anthropic.com/api/oauth/usage")!, - statusCode: 200, httpVersion: nil, headerFields: nil)!) + url: request.url!, statusCode: 200, httpVersion: nil, headerFields: nil)!) } let client = TokenRefreshingClient( apiClient: AnthropicAPIClient(session: mockSession), credentialProvider: { - // Always returns expired token — still usable if API accepts it + // Always returns expired token — no refresh token means we can't + // even try to refresh, so the usage call would be wasted. OAuthCredential.mock(accessToken: "expired", expiresAt: expiredMs) } ) - // Should still attempt the API call with the expired token - let result = try await client.fetchUsage() - #expect(result.usage.fiveHour?.utilization == 42.0) + do { + _ = try await client.fetchUsage() + Issue.record("Expected unauthorized") + } catch let error as TokenRefreshingClientError { + if case .unauthorized = error { + // Correct — no refresh path and token is expired → bail. + } else { + Issue.record("Expected unauthorized, got \(error)") + } + } + + #expect(apiCounter.value == 0) } @Test("Fresh token also gets 401 — throws unauthorized") @@ -347,4 +358,55 @@ struct TokenRefreshingClientTests { #expect(apiCounter.value == 3) #expect(result.usage.fiveHour?.utilization == 42.0) } + + @Test("Expired token + failed refresh + same credential → fails fast as unauthorized") + func expiredTokenFailsFastWhenRefreshDead() async throws { + let expiredMs = Int64(Date().timeIntervalSince1970 * 1000) - 1000 + let apiCounter = FetchCounter() + + let mockSession = MockURLSession { request in + apiCounter.increment() + let urlPath = request.url?.path ?? "" + + // Refresh endpoint returns 400 (refresh token invalid) + if urlPath.contains("/oauth/token") { + return (Data(), HTTPURLResponse( + url: request.url!, statusCode: 400, + httpVersion: nil, headerFields: nil)!) + } + + // Should never reach the usage endpoint with a known-dead credential + Issue.record("Should not call usage endpoint when refresh is dead") + return (Data(), HTTPURLResponse( + url: request.url!, statusCode: 429, + httpVersion: nil, headerFields: ["Retry-After": "120"])!) + } + + let client = TokenRefreshingClient( + apiClient: AnthropicAPIClient(session: mockSession), + credentialProvider: { + // Always returns the same expired credential — simulates a keychain + // whose refresh token is dead and no file-based fallback exists. + OAuthCredential.mock( + accessToken: "expired", + refreshToken: "dead-refresh", + expiresAt: expiredMs + ) + } + ) + + do { + _ = try await client.fetchUsage() + Issue.record("Expected unauthorized") + } catch let error as TokenRefreshingClientError { + if case .unauthorized = error { + // Correct — we bailed out before hitting the usage endpoint. + } else { + Issue.record("Expected unauthorized, got \(error)") + } + } + + // Only the refresh attempt should have been made. + #expect(apiCounter.value == 1) + } }