fix: prevent route crash when subscription or permissions prefetch fails#6047
Conversation
Wrap the subscription and permissions ensureQueryData calls in
Promise.allSettled so that a failure in either prefetch does not crash
the entire route tree. The components using this data already have their
own useSuspenseQuery hooks that will re-fetch independently if the
prefetch cache is empty.
This fixes a TypeError ('Cannot read properties of undefined (reading
component)') that occurs on self-hosted instances without a pro license
when accessing the access management pages.
Co-Authored-By: jake <jake@infisical.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — the core fix is correct and the nits are already covered by inline comments.
Extended reasoning...
Overview\nSingle-file change to that wraps the subscription and permissions calls in instead of hard-awaiting them sequentially. The org-details fetch (critical path) remains a hard await.\n\n### Security risks\nNo security-sensitive code touched. No auth, crypto, or permissions logic changed — only the route loading behavior for non-critical prefetches.\n\n### Level of scrutiny\nLow: this is a focused, mechanical fix for a real crash on self-hosted instances without a pro license. The change is minimal and the intent is clear. is the standard idiom for fire-and-forget parallel prefetches.\n\n### Other factors\nThe two reported bugs are minor nits (missing and an inaccurate comment about vs ), both already noted by the PR author in the description itself. Neither affects correctness of the fix. The inline comments will prompt the author to address them. The overall behavior change — preventing a hard crash from a non-critical prefetch failure — is strictly an improvement.
| // Prefetch subscription and permissions data but don't block route | ||
| // loading if they fail — the components using this data have their | ||
| // own useQuery hooks that will retry independently. | ||
| await Promise.allSettled([ | ||
| context.queryClient.ensureQueryData({ | ||
| queryKey: subscriptionQueryKeys.getOrgSubsription(organizationId), | ||
| queryFn: () => fetchOrgSubscription(organizationId) | ||
| }), | ||
| context.queryClient.ensureQueryData({ | ||
| queryKey: roleQueryKeys.getUserOrgPermissions({ orgId: organizationId }), | ||
| queryFn: () => fetchUserOrgPermissions({ orgId: organizationId }) | ||
| }) |
There was a problem hiding this comment.
🟡 The Promise.allSettled result is never inspected, so rejections from fetchOrgSubscription or fetchUserOrgPermissions are completely silent — no logging, no alerting — despite an analogous console.warn already existing at line 58 for similar non-fatal failures. Additionally, the comment on lines 68–70 says these components use "useQuery hooks that will retry independently", but both SubscriptionContext.tsx and OrgPermissionContext.tsx actually use useSuspenseQuery, which throws to the nearest React error boundary on persistent failure rather than silently retrying; users who hit a persistent error land on ErrorPage, which has only a "Back To Home" link with no retry button.
Extended reasoning...
Issue 1: Silent rejection swallowing
The await Promise.allSettled([...]) on line 71 discards the returned PromiseSettledResult[] array entirely — the result is never bound to a variable and never inspected. This means if fetchOrgSubscription or fetchUserOrgPermissions rejects (e.g. due to a network error, a broken license endpoint on a self-hosted instance, or an auth hiccup), the failure is completely invisible: no console output, no metrics, no alerting. The PR description itself calls this out: 'Promise.allSettled swallows rejections without logging. Consider whether a console.warn on rejected results would aid future debugging.'
Why existing code does not prevent it: The same file already has an established pattern for non-fatal failures. At line 58, the token-exchange catch block emits console.warn("Failed to automatically exchange token for organization:", error). The Promise.allSettled block is directly analogous — intentionally non-fatal — yet omits any equivalent logging. The inconsistency means subscription/permission prefetch failures are invisible while token-exchange failures are surfaced.
Concrete proof of the gap: Suppose fetchOrgSubscription rejects with a 500 from the license endpoint. Promise.allSettled catches the rejection internally and stores { status: 'rejected', reason: Error(...) } in the results array. Because that array is immediately discarded, the error is gone. The route continues loading. No developer — in local dev, staging, or production — has any signal that the prefetch failed. Filtering rejected results and emitting a console.warn (matching the pattern at line 58) would surface these failures for free.
Issue 2: Inaccurate comment — useSuspenseQuery vs useQuery
The comment on lines 68–70 reads: "the components using this data have their own useQuery hooks that will retry independently." Both downstream context files use useSuspenseQuery, not useQuery. These hooks have fundamentally different failure semantics: useQuery returns { status: 'error' } and lets the component decide how to render; useSuspenseQuery throws the error to the nearest React error boundary on persistent failure. With retry: 1 configured on the QueryClient, if both the prefetch and a single component-level retry fail, useSuspenseQuery throws to the router's defaultErrorComponent: ErrorPage. The ErrorPage component only renders a "Back To Home" link — no retry button — leaving users stuck.
Why this matters beyond a nit: The phrase "retry independently" implies graceful, silent recovery. A future maintainer relying on this comment might not add proper error handling (e.g. an error boundary with a retry button at the subscription/permission level), assuming the hooks silently recover. The comment should accurately describe that useSuspenseQuery is in use and that persistent failures surface to the error boundary.
Suggested fix: (1) Capture and inspect the Promise.allSettled results, logging any rejections via console.warn consistent with line 58. (2) Update the comment to accurately state useSuspenseQuery is used and that persistent failures throw to the error boundary rather than silently retrying.
1 participant
Context
On self-hosted instances (particularly those without a pro license), navigating to the organization or project access management pages can crash with:
The
_inject-org-detailsroute middleware runs three sequentialensureQueryDatacalls in itsbeforeLoadhook. If either the subscription or permissions prefetch throws (e.g. due to a network issue or an edge case in the license/plan endpoint), the entirebeforeLoadrejects. This prevents TanStack Router from initializing the route tree, which surfaces as the cryptic'component'TypeError inMatch.js.Before: All three prefetches were hard-awaited — any single failure crashed the route.
After: The org-details fetch remains a hard await (it's critical — without it nothing can render). The subscription and permissions fetches are wrapped in
Promise.allSettledso a failure in either is non-fatal. The downstream components already useuseSuspenseQuerywith the same query keys, so if the prefetch cache is empty they will independently re-fetch and suspend gracefully.Important things for reviewers to verify
ensureQueryDatais skipped (rejected), the consuminguseSuspenseQueryinuseOrgPermission/useOrganizationwill suspend. Confirm there is an adequate<Suspense>boundary in the route tree (TanStack Router'sMatchcomponent provides one whenPendingComponentorwrapInSuspenseis configured, and the router hasdefaultErrorComponent: ErrorPage).Promise.allSettledswallows rejections without logging. Consider whether aconsole.warnon rejected results would aid future debugging.fetchOrganizationByIdcall is still a hard await — if that fails, the route will still error as before. Only subscription + permissions are made resilient.Steps to verify the change
TypeErrorcrashType
Checklist
type(scope): short descriptionLink to Devin session: https://app.devin.ai/sessions/32d726ea8334497a8349cdc2d126ddf2