Recover from hung Google token refreshes#951
Conversation
WalkthroughGoogle authentication operations now have bounded async timeouts, explicit cancellation behavior, and late-callback isolation. Google calendar token refresh uses serialized retries, error classification, and persisted authentication-state recovery. ChangesGoogle authentication refresh resilience
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GoogleCalendarEventStore
participant GoogleAuthTimeout
participant OIDAuthState
GoogleCalendarEventStore->>GoogleAuthTimeout: Start bounded token refresh
GoogleAuthTimeout->>OIDAuthState: Perform authentication action
OIDAuthState-->>GoogleAuthTimeout: Return token or error
GoogleAuthTimeout-->>GoogleCalendarEventStore: Return result, timeout, or cancellation
GoogleCalendarEventStore->>GoogleCalendarEventStore: Recover auth state and retry when applicable
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #951 +/- ##
==========================================
- Coverage 52.26% 52.19% -0.08%
==========================================
Files 74 75 +1
Lines 8779 8835 +56
Branches 3367 3396 +29
==========================================
+ Hits 4588 4611 +23
- Misses 4053 4088 +35
+ Partials 138 136 -2 ☔ View full report in Codecov by Harness. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 324-325: Update the refresh task lifecycle around the Task created
in the Google calendar event store so deferred cleanup is associated with a
unique generation or UUID. When the task finishes, set refreshTask to nil only
if it still owns the recorded generation, preventing a canceled older task from
clearing a newer task; apply the same ownership check to the corresponding
cleanup at the other indicated location.
In `@MeetingBarLogicTests/GoogleAuthTimeoutTests.swift`:
- Line 16: Update both closure declarations in GoogleAuthTimeoutTests, including
the locations around the Result<String, Error> parameter and the second reported
occurrence, so each typed closure parameter appears on the line preceding its
opening brace. Preserve the existing parameter types and closure behavior.
🪄 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: e4882365-4cf8-4396-b31a-95e6000a09a9
📒 Files selected for processing (9)
MeetingBar/Calendar/CalendarSync.swiftMeetingBar/Calendar/ProviderHealth.swiftMeetingBar/Calendar/Providers/Google/GoogleAuthTimeout.swiftMeetingBar/Calendar/Providers/Google/GoogleAuthTransport.swiftMeetingBar/Calendar/Providers/Google/GoogleCalendarEventStore.swiftMeetingBar/Calendar/Providers/Google/GoogleCalendarPolicy.swiftMeetingBarLogicTests/GoogleAuthTimeoutTests.swiftMeetingBarLogicTests/GoogleCalendarPolicyTests.swiftPackage.swift
leits
changed the title
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
MeetingBar/Calendar/Providers/Google/GoogleCalendarEventStore.swift (1)
314-405: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for
validAccessToken's retry/recovery state machine
GoogleAuthTimeoutTestscover the timeout helper, but not thevalidAccessTokenflow itself. Add cases for timeout→recover→retry→success/refreshFailedand cancellation→recover→rethrow, ideally through a stubbedOIDAuthStateor a small seam around the refresh action.🤖 Prompt for 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. In `@MeetingBar/Calendar/Providers/Google/GoogleCalendarEventStore.swift` around lines 314 - 405, Add tests covering the validAccessToken refresh state machine: verify timeout recovery restores auth state and retries to success, repeated timeouts end in AuthError.refreshFailed, and cancellation recovers state before rethrowing CancellationError. Exercise the flow through a stubbed OIDAuthState or a focused seam around performTokenRefresh, and assert the relevant retry and recovery outcomes.
🤖 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.
Nitpick comments:
In `@MeetingBar/Calendar/Providers/Google/GoogleCalendarEventStore.swift`:
- Around line 314-405: Add tests covering the validAccessToken refresh state
machine: verify timeout recovery restores auth state and retries to success,
repeated timeouts end in AuthError.refreshFailed, and cancellation recovers
state before rethrowing CancellationError. Exercise the flow through a stubbed
OIDAuthState or a focused seam around performTokenRefresh, and assert the
relevant retry and recovery outcomes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2023f0f8-0204-4768-bed5-c87d99064881
📒 Files selected for processing (3)
MeetingBar/Calendar/Providers/Google/GoogleAuthTimeout.swiftMeetingBar/Calendar/Providers/Google/GoogleCalendarEventStore.swiftMeetingBarTests/GoogleAuthTimeoutTests.swift
1 participant
Status
DRAFT — CI is green; awaiting manual sleep/wake validation and maintainer review
Description
Fixes a Google Calendar reliability failure where one hung AppAuth token request can permanently block every later calendar refresh.
This is a smaller alternative to #950 and is limited to token refresh. Interactive sign-in is unchanged.
Root cause
OIDAuthState.performActioncoalesces refresh callers in an internal_pendingActionsqueue. If its token-request callback never arrives, timing out only MeetingBar's await is insufficient: the next call joins the same stuck AppAuth queue.Changes
AsyncThrowingStreambridge.OIDAuthStateand reopen the state already persisted in Keychain. AppAuth does not archive_pendingActions, so the replacement can start a real request.AuthError.refreshFailed.Scope and trade-off
The PR changes three files and adds no transport class, no new shared auth error, no package-target wiring, and no downstream provider-health cases.
AppAuth does not expose the underlying token-request task, so a timed-out request may still finish later. Its old auth state has no delegates, however, so it cannot overwrite the current persisted state or keep future refreshes in its pending queue.
Automated validation
git diff --checkScripts/validate_localizations.shManual verification before ready-for-review
The local Linux workspace has no Swift/Xcode toolchain, so GitHub Actions is the build authority and the live Google sleep/wake case remains the manual gate.