Skip to content

Fix infinite spinner when session endpoint is unreachable - #2636

Open
NKoech123 wants to merge 4 commits into
mainfrom
ai_main_f05de42a5e5947ba82d5
Open

Fix infinite spinner when session endpoint is unreachable#2636
NKoech123 wants to merge 4 commits into
mainfrom
ai_main_f05de42a5e5947ba82d5

Conversation

@NKoech123

@NKoech123 NKoech123 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes useSession retrying a failed session request forever while stuck in a loading state, and adds a distinct unavailable status with a recoverable UI in RequireSession.

Problem

useSession retried a failed /_agent-native/auth/session request every second forever while holding isLoading true. A transient 5xx, network failure, or timeout produced a spinner that never resolved and surfaced no error, stranding users indefinitely. Additionally, DefaultSpinner's stall hint always told visitors to "check the terminal running the dev server," which is meaningless on hosted/production deployments.

Solution

useSession now caps retries at a bounded number of attempts with backoff and reports a distinct status: "unavailable" instead of retrying forever. RequireSession branches on this new status to show a recoverable notice (with "Try again" and "Reload" actions) rather than collapsing it into "signed out" (which would bounce a signed-in user to sign-in) or "loading" (which would strand them). DefaultSpinner's stall hint is also made environment-aware so it only mentions the dev server terminal in development.

Key Changes

  • use-session.ts: adds SessionStatus type (loading | authenticated | unauthenticated | unavailable), bounded retry count (SESSION_MAX_ATTEMPTS = 4) with increasing backoff, error and retry fields on the hook result, and keeps isLoading true for unavailable so legacy consumers aren't misled into thinking the user is signed out.
  • require-session.tsx: ResolvedSessionGate now reads status/retry from useSession, only redirects on unauthenticated, and renders a new SessionUnavailableNotice component (with "Try again" and "Reload page" buttons) when status === "unavailable".
  • DefaultSpinner.tsx: adds a stallHint() helper that returns dev-specific wording when import.meta.env indicates development, and generic "try reloading" wording otherwise (including when the Vite env is absent, e.g. in prebuilt dist consumption).
  • Updates specs in app-providers.spec.tsx, require-session.spec.tsx, and use-session.spec.tsx to cover the new status field and the "unavailable" retry/notice behavior.
  • Adds a changeset documenting the fix.

Edit in Builder  Preview


To clone this PR locally use the Github CLI with command gh pr checkout 2636

You can tag me at @BuilderIO for anything you want me to fix or change

@netlify

This comment has been minimized.

@builder-io-integration builder-io-integration Bot changed the title Update from the Builder.io agent Fix infinite spinner when session endpoint is unreachable Aug 4, 2026
@netlify

This comment has been minimized.

…f05de42a5e5947ba82d5

# Conflicts:
#	packages/core/src/client/DefaultSpinner.tsx
@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Here's a visual recap of what changed:

Visual recap

Open the full interactive recap

@netlify

This comment has been minimized.

@builder-io-integration builder-io-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Builder reviewed your changes and found 1 potential issue 🟡

Review Details

Code Review Summary

PR #2636 bounds retries for the shared session endpoint, introduces an explicit unavailable state, and gives RequireSession a recoverable notice instead of redirecting or spinning forever. The core distinction between unreadable session state and a confirmed signed-out state is sound, and the retry cancellation, backoff, and focused unit coverage are good. This is a high-risk review because the change affects authentication/session handling.

Key Findings

🟡 MEDIUMisLoading remains true after the retry budget is exhausted, which preserves compatibility for existing consumers but also permanently blocks public flows that use !sessionLoading as a gate for optional session resolution. In particular, public Clips share routes can fail to fetch their unauthenticated recording/meeting data when the session endpoint is down. Those consumers need to distinguish active loading from terminal unavailability, or the hook needs a compatibility signal that lets safe anonymous requests proceed.

🧪 Browser testing: Could not verify — the dev server was healthy, but all browser executor batches lacked browser automation tools and escalated every planned test case.

Comment on lines +150 to +157
// Callers that only read `isLoading`/`session` (most of the codebase, not
// yet migrated to `status`) must not see "unavailable" as "signed out" —
// that bounces an authenticated user through sign-in-only UI over a
// transient blip. Keeping `isLoading` true here reproduces this hook's
// pre-existing behavior for those callers (an indefinite "still resolving"
// instead of a wrong answer); only `status`-aware callers get the distinct
// "unavailable" treatment with a retry affordance.
const isLoading = status === "loading" || status === "unavailable";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Unavailable sessions still permanently block public session consumers

isLoading remains true after the retry budget is exhausted. Public Clips share routes such as templates/clips/app/routes/share.$shareId.tsx and share.meeting.$meetingId.tsx gate their public data requests on !sessionLoading, so four session-endpoint failures disable those requests indefinitely even though the share data does not require a session. Update safe anonymous consumers to distinguish terminal status === "unavailable" from active loading, and add regression coverage for a public share while the session endpoint is unreachable.

Additional Info
Reported by 1 of 2 independent code-review agents; confirmed against the current PR diff and consumer paths.

Fix in Builder

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.

3 participants