fix(auth): prevent session deletion on proactive refresh failure when token still valid

[7ttp]

What this PR introduces

Removes the _removeSession() call from _callRefreshToken. This prevents sessions from being deleted from storage on proactive refresh failure when the access token is still valid (e.g., within the 90s EXPIRY_MARGIN_MS window).

No additional logic, just a fix to ensure the session remains recoverable.

Why is this change needed

Previously, if a proactive token refresh failed with a non-retryable error (such as invalid_grant), the session was permanently deleted from storage even though the access token was still technically valid. This caused users to be silently logged out until they re-authenticated, and made recovery impossible until a full app reload.

_callRefreshToken is a low-level helper — it shouldn't own session lifecycle decisions. Callers that need to clean up already do so independently (_recoverAndRefresh has its own _removeSession() call for non-retryable errors). Removing the call here lets __loadSession (the getSession() path) degrade gracefully — returning the error without nuking a still-valid session.

Changes

• Removed the _removeSession() call from _callRefreshToken.
• Added a test verifying sessions are not deleted from storage on non-retryable refresh errors.
• Existing cleanup in other callers (_recoverAndRefresh) remains unchanged.

Related

• Closes Bug: _callRefreshToken permanently deletes session on non-retryable refresh failure, even when access token is still valid #2145
• Related to [Bug] iOS Safari/Chrome: supabase.auth.getSession() intermittently returns null until app is backgrounded/foregrounded #1560
• Adds on fix: don't call removeSession prematurely  auth-js#915

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes
  • Enhanced authentication error handling during token refresh. Sessions now persist when encountering non-retryable errors instead of being automatically invalidated, preventing unexpected logouts and improving user experience.

Walkthrough

The PR removes the code path that unconditionally deleted the stored session when a non-retryable error occurred during _callRefreshToken. On non-retryable refresh failures the client now surfaces the error but does not call this._removeSession(), leaving session data in storage intact. Tests were added to verify that a non-retryable AuthError from _refreshAccessToken does not remove the stored session.

Sequence Diagram(s)

(omitted)

Assessment against linked issues

Objective	Addressed	Explanation
Prevent permanent deletion of session on non-retryable refresh failures when access token is still valid [#2145]	✅
Only remove session if expires_at*1000 < Date.now(); preserve non-expired sessions [#2145]	❌	No new expiry check was added; the removal call was removed entirely rather than conditionally deleting based on expires_at.
Ensure getSession()/__loadSession() returns existing valid session instead of null [#2145]	❌	Tests show stored session remains but getSession response may still surface null session with an error; change does not demonstrate getSession returning the preserved session in all error paths.
Preserve retry semantics for retryable errors [#2145]	✅

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[pkg-pr-new]

Open in StackBlitz

@supabase/auth-js

npm i https://pkg.pr.new/@supabase/auth-js@2146

@supabase/functions-js

npm i https://pkg.pr.new/@supabase/functions-js@2146

@supabase/postgrest-js

npm i https://pkg.pr.new/@supabase/postgrest-js@2146

@supabase/realtime-js

npm i https://pkg.pr.new/@supabase/realtime-js@2146

@supabase/storage-js

npm i https://pkg.pr.new/@supabase/storage-js@2146

@supabase/supabase-js

npm i https://pkg.pr.new/@supabase/supabase-js@2146

commit: 65e9fb9

[mandarini]

Thank you @7ttp ! I removed the additional logic and added a test!!

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/core/auth-js/test/GoTrueClient.test.ts`:
- Around line 494-499: The test currently asserts that client.getSession()
returns data.session === null on proactive-refresh failure; update the
assertions to verify that the existing seeded session is returned instead and
that storage remains intact. Specifically, in the GoTrueClient.test where
getSession() is called, replace the expect(data.session).toBeNull() with an
assertion that data.session matches the seeded session object (or at least has
the expected access_token/uid/expires_at values) and add an assertion that the
stored refresh token (or session data in whatever storage mock you used) is
still present, ensuring client.getSession() returns the valid session even when
proactive refresh errors.

---

ℹ️ Review info

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Cache: Disabled due to Reviews > Disable Cache setting

Disabled knowledge base sources:

• Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between f3e0ff3 and 51b4fb1.

📒 Files selected for processing (2)

• packages/core/auth-js/src/GoTrueClient.ts
• packages/core/auth-js/test/GoTrueClient.test.ts

💤 Files with no reviewable changes (1)

• packages/core/auth-js/src/GoTrueClient.ts