Fix hung Google token refresh permanently freezing calendar sync - #950
Fix hung Google token refresh permanently freezing calendar sync#950GeoffreyPlitt wants to merge 3 commits into
Conversation
…ever A hung OIDAuthState.performAction callback (observed parked across sleep/wake in v4.11.6) never resumed its continuation, so refreshTask stayed set and every subsequent fetch awaited it indefinitely with no error, no retry, and no user-visible signal — the menu showed stale, greyed-out events until restart. Add TimeoutGuardedCompletion, a small helper that races a completion-callback operation against a hard timeout and guarantees single resumption regardless of which side wins. Wire it into validAccessToken (30s timeout) and ensureSignedIn (5min timeout, since interactive sign-in can legitimately take a while), so a stuck refresh or sign-in now throws instead of hanging, refreshTask/signInTask are always cleared, and the existing ProviderHealth machinery correctly marks data stale. Also drops the guard-let-self path inside the performAction callback that could silently leak the continuation if self were ever nil.
The app target (built without -strict-concurrency=complete, unlike the MeetingBarLogic SwiftPM package where this compiled fine) couldn't infer TimeoutGuardedCompletion's T through the nested Task/do-catch in ensureSignedIn's operation closure. CI caught this at build time.
The app target builds under Swift 6 language mode with strict concurrency complete. GCEventStore is @mainactor, so the operation closures built at its two call sites are MainActor-isolated; passing them into the nonisolated TimeoutGuardedCompletion.run tripped "sending main actor-isolated value ... risks causing data races". Marking the parameter `sending` tells the compiler the closure's isolation region transfers safely into run(), which only ever invokes it synchronously and once.
WalkthroughGoogle authentication now bounds interactive sign-in and token refresh operations with timeout guards. A single-resume helper handles late callbacks safely, and tests cover timeout, success, failure, late completion, and independent subsequent calls. ChangesGoogle authentication timeout handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GCEventStore
participant TimeoutGuardedCompletion
participant AppAuth
GCEventStore->>TimeoutGuardedCompletion: guard sign-in or token refresh
TimeoutGuardedCompletion->>AppAuth: start completion-based operation
AppAuth-->>TimeoutGuardedCompletion: return callback result or timeout
TimeoutGuardedCompletion-->>GCEventStore: return authentication result
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
063decb to
04f1a19
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@MeetingBar/Calendar/Providers/Google/GoogleCalendarEventStore.swift`:
- Around line 298-313: The sign-in timeout in ensureSignedIn should be converted
to AuthError.refreshFailed, matching validAccessToken. Update the surrounding
Task/TimeoutGuardedCompletion error handling to catch OperationTimedOut and
rethrow AuthError.refreshFailed, while preserving other errors unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d509964a-1e24-43e9-b24c-cb18122103c5
📒 Files selected for processing (3)
MeetingBar/Calendar/Providers/Google/GoogleCalendarEventStore.swiftMeetingBar/Calendar/Providers/Google/GoogleCalendarPolicy.swiftMeetingBarLogicTests/TimeoutGuardedCompletionTests.swift
Status
READY
Description
Fixes a reliability bug in the Google Calendar provider where a single hung OAuth token refresh permanently freezes all calendar refreshes with no error, no retry, and no user-visible signal.
This caused a real user-facing failure (observed on v4.11.6; the same pattern exists on master): token refreshes stopped silently after a sleep/wake cycle, the menu kept showing the previous day's events all greyed out (past-event styling), the status bar lost its countdown, and no notifications fired until the app was manually restarted.
Root cause (
GCEventStore.validAccessToken): token refreshes are serialized through a storedrefreshTask, and the task body wraps AppAuth'sOIDAuthState.performActioncallback in a continuation. IfperformActionnever invokes its callback (observed in the wild — e.g. its internal token-endpoint request gets parked across sleep/wake), the continuation never resumes,defer { refreshTask = nil }never runs, and every subsequent fetch awaits the dead task forever. There was no timeout anywhere in this path. Additionally, theguard let self else { return }inside the callback could leak the continuation outright, and the same serialize-through-a-stored-task pattern inensureSignedIn/signInTaskhad the same wedge risk.Fix:
TimeoutGuardedCompletionhelper (inGoogleCalendarPolicy.swift, part of theMeetingBarLogicpackage) that races a completion-callback-based operation against a hard timeout and guarantees the continuation resumes exactly once, even when the timeout and a late callback race.validAccessTokenwrapsperformActionwith a 30s timeout; on timeout it throwsAuthError.refreshFailed,refreshTaskis always cleared, and the existingProviderHealthmachinery surfaces the stale-data warning as it already does for thrown errors.ensureSignedIngets the same protection with a 5-minute timeout (interactive sign-in can legitimately take a while), so a never-completing sign-in can no longer block all future refreshes.guard let selfcontinuation leak is gone — the AppAuth callback no longer touchesself.No public API changes; EventKit provider untouched.
Checklist
Steps to Test or Reproduce
make test— new unit tests inMeetingBarLogicTests/TimeoutGuardedCompletionTests.swiftcover: a callback that never fires throws within the timeout instead of hanging; a late callback after timeout is safely ignored (no double-resume crash); a subsequent call is not blocked by an earlier hung operation; fast success/failure paths are unaffected.🤖 Generated with Claude Code
Summary by CodeRabbit