Skip to content

FCE-3061: SDK reconnect should handle invalid token#514

Draft
MiloszFilimowski wants to merge 7 commits intomainfrom
mfilimowski/FCE-3060-reconnection-fixes
Draft

FCE-3061: SDK reconnect should handle invalid token#514
MiloszFilimowski wants to merge 7 commits intomainfrom
mfilimowski/FCE-3060-reconnection-fixes

Conversation

@MiloszFilimowski
Copy link
Copy Markdown
Collaborator

Description

  • packages/ts-client/src/reconnection.ts: If a socketClose carries an auth or join error while a reconnect is in progress, clear the scheduled reconnect, set status to error, and stop retrying (still ignores those closes when idle).
  • packages/ts-client/src/auth.ts: isAuthError matches case-insensitively and treats known reasons as substrings so server messages like Invalid token are detected reliably.
  • packages/react-client/src/hooks/internal/useReconnection.ts: On authError / joinError, move reconnection UI to error only when status was already reconnecting.
  • packages/ts-client/src/tests/: Added auth.test.ts and reconnection.test.ts (Vitest) for the above behavior.

Motivation and Context

Reconnection could keep retrying or show the wrong status when the socket closed with auth/join failures, especially if the close reason casing did not match the client’s exact strings. This aligns core and React reconnection state with those terminal failures.

Documentation impact

  • Documentation update required
  • Documentation updated in another PR
  • No documentation update required

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to
    not work as expected)

@linear
Copy link
Copy Markdown

linear bot commented Mar 30, 2026

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts reconnection behavior in the TS client and React client so that terminal auth/join failures stop an in-progress reconnect cycle, and improves auth-error detection to be case-insensitive/substring-based, with accompanying Vitest coverage.

Changes:

  • TS client: stop scheduled reconnects and transition to an error state when an auth/join socket close happens during active reconnection.
  • TS client: make isAuthError detection case-insensitive and substring-based; add tests for auth and reconnection behavior.
  • React client: update useReconnection to move UI to "error" on authError/joinError only if already "reconnecting".

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/ts-client/src/reconnection.ts Cancels pending reconnect + sets error state when auth/join close occurs during reconnection.
packages/ts-client/src/auth.ts Updates auth error detection logic to be case-insensitive/substring-based.
packages/react-client/src/hooks/internal/useReconnection.ts Adjusts reconnection UI state transitions for auth/join errors.
packages/ts-client/src/tests/auth.test.ts Adds Vitest coverage for isAuthError matching behavior.
packages/ts-client/src/tests/reconnection.test.ts Adds Vitest coverage for stopping reconnection on auth/join errors mid-reconnect.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@MiloszFilimowski MiloszFilimowski changed the title FCE-3060: Reconnections should happen based on mobile internet status FCE-3061: SDK reconnect should handle invalid token Mar 31, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@MiloszFilimowski MiloszFilimowski marked this pull request as draft April 1, 2026 12:28
@MiloszFilimowski MiloszFilimowski removed the request for review from czerwiukk April 1, 2026 12:28
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.

2 participants