Skip to content

fix: prevent route crash when subscription or permissions prefetch fails #6047

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
devin-ai-integration wants to merge 1 commit into
base: main
Choose a base branch
from
+13 −8
Open

fix: prevent route crash when subscription or permissions prefetch fails #6047

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions frontend/src/pages/middlewares/inject-org-details.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,20 @@
queryFn: () => fetchOrganizationById(organizationId)
});

await context.queryClient.ensureQueryData({
queryKey: subscriptionQueryKeys.getOrgSubsription(organizationId),
queryFn: () => fetchOrgSubscription(organizationId)
});
// 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 })
})

Check warning on line 79 in frontend/src/pages/middlewares/inject-org-details.tsx

View check run for this annotation

Claude / Claude Code Review

Promise.allSettled: silent rejection swallowing and inaccurate fallback 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 t
Comment on lines +68 to +79
Copy link
Copy Markdown

@claude claude Bot Apr 16, 2026

Choose a reason for hiding this comment

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

🟡 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.

]);

await context.queryClient.ensureQueryData({
queryKey: roleQueryKeys.getUserOrgPermissions({ orgId: organizationId }),
queryFn: () => fetchUserOrgPermissions({ orgId: organizationId })
});
return { organizationId };
}
});
Loading