Skip to content

Keep a logged-out Tidepool service and retract the re-login alert - #46

Open
ps2 wants to merge 1 commit into
next-devfrom
fix/tidepool-logged-out-session
Open

ps2 wants to merge 1 commit into
next-devfrom
fix/tidepool-logged-out-session

Conversation

@ps2

@ps2 ps2 commented Oct 31, 2026

Copy link
Copy Markdown

When the Tidepool session is dropped (for example when the refresh token is rejected; see tidepool-org/TidepoolKit#82), apiDidUpdateSession(nil) raises the "authentication-needed" alert. Two things went wrong after that:

  • The alert was never retracted after logging back in. It's now retracted when a session arrives after having none. Ordinary token refreshes don't retract it.
  • The service was discarded on the next launch. init?(rawState:) reads the session from the keychain, and getSession threw copy(errSecItemNotFound) when none was stored. Init then returned nil and Loop dropped the Tidepool service, instead of showing that a login is needed. A missing keychain item now means no session. SettingsView already shows "You are not logged in" with a Login button for that state.

Tests

  • New TidepoolServiceSessionTests (mock SessionStorage and ServiceDelegate): logging out issues the alert, logging back in retracts it, and a token refresh doesn't retract it.
  • All 69 TidepoolServiceKit tests pass. The log-back-in test fails without the source change.
  • The keychain path isn't unit tested: the test bundle has no host app, so keychain calls fail with -34018.

When the Tidepool session is dropped (for example after the refresh token
is rejected), the service raises an "authentication-needed" alert, but two
things went wrong afterwards:

- Nothing retracted that alert once the user logged in again. Retract it
  when a session arrives after having none.
- On the next launch, init?(rawState:) failed because the keychain had no
  session: getSession threw errSecItemNotFound and the whole service was
  discarded, so the user lost the Tidepool service instead of seeing that
  a login is needed. A missing keychain item now means no session; the
  settings screen already shows "You are not logged in" with a Login button.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant