fix(desktop): show an error state instead of a blank window when organization.list fails (#6794) - #6810
fix(desktop): show an error state instead of a blank window when organization.list fails (#6794)#6810Kitenite wants to merge 1 commit into
Conversation
…nization.list fails A returning window with a registry org rendered nothing when organization.list errored while auth stayed healthy: CollectionsProvider returned null both while loading and on error. Extract the three-way decision into resolveWindowOrg and render OrgResolutionScreen (spinner, then Retry once errored or after 15s) instead of null. The guard is unchanged: an unverified registry org is never adopted on failure. Fixes #6794 Claude-Session: https://claude.ai/code/session_011XQusXtWEwq2nxN4jfMUpv
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe change adds organization resolution for persisted windows. The provider now distinguishes pending, resolved, and failed states. It adopts only verified organizations and renders a retryable loading or error screen when resolution has no result. ChangesWindow organization resolution
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change replaces a blank desktop window with loading, timeout, error, and retry states when organization loading fails. No actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant CollectionsProvider
participant organization.list
participant resolveWindowOrg
participant OrgResolutionScreen
CollectionsProvider->>organization.list: Read organizations and errors
CollectionsProvider->>resolveWindowOrg: Pass window, memberships, loading, errors, and session organization
resolveWindowOrg-->>CollectionsProvider: Return wait, resolved, or unresolvable
CollectionsProvider->>OrgResolutionScreen: Render unresolved state with retry
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
ashbrener
left a comment
ashbrener
left a comment
There was a problem hiding this comment.
I filed #6794, so I went looking for the trap rather than the happy path. It isn't here — the invariant holds.
resolveWindowOrg can't return resolved while a registry org is present and the membership list isn't: line 29 returns unresolvable/wait before anything below it runs, and registryOrgIsStillMine needs organizations != null anyway. The effect then sets initializedRef only on resolved, so an unverified id is rendered and never adopted. That was the way to get this wrong — a transient failure pinning the window to a dead org permanently, with the one-shot ref meaning nothing re-evaluates when the list comes back.
isError is also the right signal rather than status/failureCount: TanStack holds pending through the retry sequence, so the three transport retries still show the spinner and only a settled failure reaches the error screen — which lines up with hostServiceQueryRetry giving tRPC-shaped 500s zero retries. And self-healing survives, since the provider stays mounted and focus/reconnect refetch still resolves.
Three things, none of them correctness:
-
The tests cover the decision function, not the line that regressed. Reverting the render site to
return nullreintroduces #6794 with all seven still green. Separately, "waits when nothing can supply an org" isn't reachable in the app —layout.tsx:277redirects before the provider mounts — while the case that is live and untested isorganizationspresent withisError: true. That's precisely what a later "hoist the errored check to the top" edit would break, and it would blank a working window on a failed background refetch. -
The spinner renders unconditionally (
OrgResolutionScreen.tsx:27, outside theshowRetryblock), so "Can't reach the Superset server" sits under an animation while nothing is in flight and nothing will retry until focus or reconnect. The screen this mirrors doesn't do that —layout.tsx:250swaps inHiOutlineWifi. -
The body copy doesn't branch. The heading does, so the 15s-pending case reads "Still loading your organizations" directly above "Superset couldn't load your organizations." Worth noting too that an errored query keeps
status: "error"acrossrefetch()— onlyfetchStatusflips — so clicking Retry changes nothing on screen.isFetchingfrom the same destructure would fix the button state and the spinner gating together.
Drag strip is correctly outside the conditional, so the error window is still movable — easy one to miss on a frameless window.
2 participants
Problem
A returning window (one with an org already in the window registry) rendered an empty, undraggable frame when
organization.listfailed but/api/auth/*still answered.CollectionsProviderrefuses to adopt a registry org it can't verify against the membership list (correct), but its render site treated "still loading" and "load failed" identically and returnednull, unmounting the whole authenticated tree. tRPC-shaped 500s get zero retries, so an API outage looked like a hang.Fix
resolveWindowOrg.ts: the wait / unresolvable / resolved decision extracted as a pure function. The one-shotinitializedRefsemantics and the "never adopt an unverified registry org" invariant are unchanged:unresolvableis rendered, never adopted.components/OrgResolutionScreen: replaces the barenull. Spinner with a drag strip while loading; once the list has errored, or has been pending for 15s, shows a message and a Retry wired to the query'srefetch(). Mirrors the blocking screens in_authenticated/layout.tsx.resolveWindowOrg.test.ts: table test covering the five cases from the issue plus two edges.Verification
bun teston the new test: 7 pass. With the pre-fix behaviour restored (errored →wait), the "unresolvable" case fails, so the test guards the regression.bun typecheckinapps/desktop: clean..env/dev stack, so I did not drive the real app to simulate a/api/trpcoutage. Manual repro is in the issue (break/api/trpconly, relaunch within the 5-minute cookie-cache window).Fixes #6794
https://claude.ai/code/session_011XQusXtWEwq2nxN4jfMUpv
Summary by cubic
Show an error screen with Retry instead of a blank window when
organization.listfails in the desktop app. PreviouslyCollectionsProviderreturned null on both load and error, blanking the frameless window; now a spinner is shown while loading and an error with Retry appears after a failure or 15s.resolveWindowOrgwith three outcomes (wait, unresolvable, resolved); covered by a table test for core and edge cases.OrgResolutionScreenwith spinner, drag strip, and Retry wired toorganization.listrefetch; used only when the window’s org is unresolved.organization.listoutage to see spinner → error → Retry recovery; new tests pass.Written for commit 053e49c. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
New Features