From 1ca48b7701b48dfc1a3c3f335690eab6ba26669f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Tue, 20 Oct 2026 03:56:56 +0000 Subject: [PATCH 1/2] fix(mobile): settle profile and invoice reads on a flaky connection The Profile credits card, the Linked accounts row, and the invoices first page ran their reads with TanStack Query's default network mode, so a request that started while the app reported offline paused: it stayed pending without ever fetching. The credits slot and the linked-accounts section kept their skeleton placeholders, and the invoices list held its skeleton rows with no error or retry to leave them. Those reads now run with no network-mode gate and an explicit reconnect refetch, so each settles on content or on a retryable error and recovers when connectivity returns. The identity read settles the same way, so an unresolved getMe can no longer keep the credits read disabled. A read that fails while the device is confirmed offline now presents the offline error variant. --- .../invoices-screen.mounted.test.tsx | 8 ++ .../organization/invoices-screen.tsx | 13 ++- .../profile-credits-card.mounted.test.tsx | 34 +++++++- .../src/components/profile-credits-card.tsx | 23 +++++- .../profile-screen.queries.mounted.test.tsx | 59 +++++++++++++ .../profile-screen.signout.mounted.test.tsx | 6 ++ .../profile-screen.test-helpers.tsx | 10 +++ apps/mobile/src/components/profile-screen.tsx | 19 ++++- .../src/lib/hooks/use-current-user-id.test.ts | 6 -- .../src/lib/hooks/use-current-user-id.ts | 23 ++++-- .../hooks/use-organization-queries.test.ts | 82 ++++++++++++++++++- .../src/lib/hooks/use-organization-queries.ts | 11 +++ ...organization-invoice-error-variant.test.ts | 21 +++++ .../lib/organization-invoice-error-variant.ts | 25 ++++++ 14 files changed, 312 insertions(+), 28 deletions(-) create mode 100644 apps/mobile/src/lib/organization-invoice-error-variant.test.ts create mode 100644 apps/mobile/src/lib/organization-invoice-error-variant.ts diff --git a/apps/mobile/src/components/organization/invoices-screen.mounted.test.tsx b/apps/mobile/src/components/organization/invoices-screen.mounted.test.tsx index 23dfb47da19..7f1ab839fc7 100644 --- a/apps/mobile/src/components/organization/invoices-screen.mounted.test.tsx +++ b/apps/mobile/src/components/organization/invoices-screen.mounted.test.tsx @@ -36,6 +36,14 @@ const queryErrors = vi.hoisted(() => ({ errors: [] as { variant?: string; onRetry?: () => void }[], })); +const connectivity = vi.hoisted(() => ({ value: 'online' as 'online' | 'offline' | 'unknown' })); + +vi.mock('@/lib/hooks/use-offline-banner-state', () => ({ + getCommittedConnectivityStatus: () => connectivity.value, + useCommittedConnectivityStatus: () => connectivity.value, + useOfflineBannerState: () => connectivity.value === 'offline', +})); + const buttons = vi.hoisted(() => ({ rendered: [] as { children?: unknown; diff --git a/apps/mobile/src/components/organization/invoices-screen.tsx b/apps/mobile/src/components/organization/invoices-screen.tsx index 535c4986d00..e1bd08bf13d 100644 --- a/apps/mobile/src/components/organization/invoices-screen.tsx +++ b/apps/mobile/src/components/organization/invoices-screen.tsx @@ -22,6 +22,7 @@ import { useOrgBoundary, useOrgInvoicesPage, } from '@/lib/hooks/use-organization-queries'; +import { useCommittedConnectivityStatus } from '@/lib/hooks/use-offline-banner-state'; import { useRouteForegroundRefresh } from '@/lib/hooks/use-route-foreground-refresh'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; import { @@ -29,6 +30,7 @@ import { selectInvoiceRowState, shareOrganizationInvoicePdf, } from '@/lib/organization-invoice-download'; +import { selectInvoiceErrorVariant } from '@/lib/organization-invoice-error-variant'; import { cn, firstNonEmpty } from '@/lib/utils'; // FlashList takes `contentContainerStyle` (no className), so the FlatList's @@ -206,6 +208,7 @@ export function OrganizationInvoicesScreen() { const { query, entries: invoices, hasMore } = useOrgInvoicesPage(organizationId); const paddingBottom = useTabBarBottomPadding(); useRouteForegroundRefresh([[['organizations']]]); + const committedConnectivity = useCommittedConnectivityStatus(); // `InvoiceRow` is memoized at module scope, so the renderer keeps one identity // across data updates and only the changed rows re-render. @@ -229,13 +232,9 @@ export function OrganizationInvoicesScreen() { errorCode === 'NOT_FOUND' || errorCode === 'FORBIDDEN' || errorCode === 'UNAUTHORIZED'; // NOT_FOUND maps to the not-found state; FORBIDDEN/UNAUTHORIZED map to the - // permission state. Any other error stays the retryable neutral state. - let errorVariant: 'neutral' | 'server' | 'not-found' | 'permission' = 'neutral'; - if (errorCode === 'NOT_FOUND') { - errorVariant = 'not-found'; - } else if (errorCode === 'FORBIDDEN' || errorCode === 'UNAUTHORIZED') { - errorVariant = 'permission'; - } + // permission state. A read that settled by failing on a confirmed-offline + // device is a connectivity failure; `unknown` keeps the retryable neutral. + const errorVariant = selectInvoiceErrorVariant(errorCode, committedConnectivity); // A later-page failure must keep the already-loaded rows and offer an inline // retry instead of replacing the list. Read `isFetchNextPageError`, not diff --git a/apps/mobile/src/components/profile-credits-card.mounted.test.tsx b/apps/mobile/src/components/profile-credits-card.mounted.test.tsx index ed402ec0621..3f370730964 100644 --- a/apps/mobile/src/components/profile-credits-card.mounted.test.tsx +++ b/apps/mobile/src/components/profile-credits-card.mounted.test.tsx @@ -9,7 +9,7 @@ import { createElement, type ElementType, type ReactNode } from 'react'; import type * as ReactModule from 'react'; import { Platform, Pressable } from 'react-native'; import { act, type ReactTestRenderer } from '@/test/renderer'; -import { type QueryClient } from '@tanstack/react-query'; +import { onlineManager, type QueryClient } from '@tanstack/react-query'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import '@/i18n'; @@ -253,6 +253,8 @@ function headerClasses(renderer: ReactTestRenderer): string[] { beforeEach(() => { Platform.OS = 'ios'; windowDims.width = 390; + // A test that simulates the app reporting offline must not leak that state. + onlineManager.setOnline(true); savedMetadata.clear(); saveCompletion = undefined; addCreditsRowProps.latest = undefined; @@ -523,6 +525,36 @@ describe('CreditsCard add-credits entry point', () => { } ); + it('settles the balance read while the app reports offline', async () => { + // NetInfo can hold the device offline while the backend is reachable. A + // read that pauses there is pending without fetching, so the slot never + // leaves its skeleton: the card must ask Query to settle regardless. + currentUser.userId = 'user-1'; + getContextBalanceQueryFn.mockResolvedValue({ balance: 12.5 }); + onlineManager.setOnline(false); + + const { texts, unmount } = await mountCard(); + await waitFor(() => texts().includes('$12.50')); + + expect(texts()).not.toContain('SKELETON'); + unmount(); + onlineManager.setOnline(true); + }); + + it('settles an unreachable balance read to the retry surface while offline', async () => { + currentUser.userId = 'user-1'; + getContextBalanceQueryFn.mockRejectedValue(new Error('Network request failed')); + onlineManager.setOnline(false); + + const { texts, unmount } = await mountCard(); + await waitFor(() => texts().includes('Failed to load balance. Tap to retry.')); + + expect(texts()).not.toContain('SKELETON'); + expect(texts()).not.toContain('$0.00'); + unmount(); + onlineManager.setOnline(true); + }); + it('keeps personal Buy credits actionable when the balance request fails', async () => { currentUser.userId = 'user-1'; getContextBalanceQueryFn.mockRejectedValue(new Error('Balance unavailable')); diff --git a/apps/mobile/src/components/profile-credits-card.tsx b/apps/mobile/src/components/profile-credits-card.tsx index f9611efb0e7..b432aa0f00d 100644 --- a/apps/mobile/src/components/profile-credits-card.tsx +++ b/apps/mobile/src/components/profile-credits-card.tsx @@ -82,17 +82,30 @@ export function CreditsCard({ enabled, orgs }: Readonly) { ...balanceOptions, enabled: enabled && hasUserId, placeholderData: keepPreviousData, + // The card must stop waiting while the online manager reports offline: a + // paused read is pending without fetching, so the skeleton below had no + // exit. This one either answers or fails once the control-plane deadline + // lands, and `balanceFailed` then offers the retry. + networkMode: 'always', + // `networkMode: 'always'` also disables the derived reconnect refetch + // (`@tanstack/query-core` sets `refetchOnReconnect = networkMode !== + // 'always'` when it is undefined), so ask for it explicitly. + refetchOnReconnect: true, }); const { data: personalCreditData, isLoading: personalCreditsLoading } = useQuery({ ...personalCreditOptions, enabled: enabled && hasUserId && !selectedOrgId, + networkMode: 'always', + refetchOnReconnect: true, }); const { data: orgCreditData, isLoading: orgCreditsLoading } = useQuery({ ...orgCreditOptions, enabled: enabled && hasUserId && Boolean(selectedOrgId), placeholderData: keepPreviousData, + networkMode: 'always', + refetchOnReconnect: true, }); // A failed getMe (no userId) can never render a trusted balance, so it shares @@ -103,10 +116,12 @@ export function CreditsCard({ enabled, orgs }: Readonly) { const creditsLoading = selectedOrgId ? orgCreditsLoading : personalCreditsLoading; const balanceDollars = balance?.balance ?? 0; - // A paused query (offline/unknown connectivity, empty cache) is pending but - // not fetching, so `balanceLoading` (isLoading) is false while `balance` - // is still undefined. Treat "no data yet" as loading so the card shows a - // skeleton instead of `$0` on a cold launch before NetInfo settles. + // A read that has not answered yet is pending without fetching while the + // owner is still unresolved (the balance query is gated on the identity), so + // `balanceLoading` (isLoading) is false while `balance` is still undefined. + // Treat "no data yet" as loading so the card shows a skeleton instead of `$0` + // on a cold launch. The read itself settles regardless of the online manager, + // and an unreachable one lands on `balanceFailed` instead of this branch. const balancePending = balance === undefined && !balanceFailed; const showBalanceSkeleton = (balanceLoading || balancePending || !hideBalanceLoaded) && !balanceFailed; diff --git a/apps/mobile/src/components/profile-screen.queries.mounted.test.tsx b/apps/mobile/src/components/profile-screen.queries.mounted.test.tsx index e4c123d1d48..455b38f5cef 100644 --- a/apps/mobile/src/components/profile-screen.queries.mounted.test.tsx +++ b/apps/mobile/src/components/profile-screen.queries.mounted.test.tsx @@ -1,3 +1,4 @@ +import { onlineManager } from '@tanstack/react-query'; import { beforeEach, describe, expect, it, vi } from 'vitest'; // The mock harness is imported before any product module so its `vi.mock` @@ -5,6 +6,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { advanceUntil, authState, + connectivityState, findConfigureRows, findNode, flushInteractions, @@ -27,6 +29,9 @@ import { createTestQueryClient, waitFor } from '@/test/render-with-providers'; describe('ProfileScreen deferred queries', () => { beforeEach(() => { vi.useRealTimers(); + // A test that simulates the app reporting offline must not leak that state. + onlineManager.setOnline(true); + connectivityState.value = 'online'; providersQueryFn.mockReset(); organizationsQueryFn.mockReset(); signOutFn.mockReset(); @@ -206,6 +211,60 @@ describe('ProfileScreen deferred queries', () => { unmount(); }); + it('settles the linked-accounts read while the app reports offline', async () => { + // NetInfo can hold the device offline while the backend is reachable. A + // paused read is pending without fetching, so neither the row nor the error + // surface renders: the screen must ask Query to settle regardless. + providersQueryFn.mockResolvedValue({ + providers: [{ provider: 'github', email: 'dev@kilo.ai' }], + }); + organizationsQueryFn.mockResolvedValue([]); + onlineManager.setOnline(false); + + const { renderer, unmount } = await mountProfile(); + + flushInteractions(); + await waitFor(() => findConfigureRows(renderer.root, 'GitHub').length === 1); + + expect(findConfigureRows(renderer.root, 'GitHub')[0]?.props.subtitle).toBe('dev@kilo.ai'); + expect(nodeCount(renderer.root, 'Skeleton')).toBe(0); + + unmount(); + }); + + it('settles an unreachable linked-accounts read to its retry surface while offline', async () => { + providersQueryFn.mockRejectedValue(new Error('Network request failed')); + organizationsQueryFn.mockResolvedValue([]); + onlineManager.setOnline(false); + + const { renderer, unmount } = await mountProfile(); + + flushInteractions(); + await waitFor(() => nodeCount(renderer.root, 'QueryError') > 0); + + const queryError = findNode(renderer.root, 'QueryError'); + expect(queryError?.props.title).toBe('Could not load accounts'); + expect(typeof queryError?.props.onRetry).toBe('function'); + expect(nodeCount(renderer.root, 'Skeleton')).toBe(0); + + unmount(); + }); + + it('names connectivity when the failed read lands on a confirmed-offline device', async () => { + providersQueryFn.mockRejectedValue(new Error('Network request failed')); + organizationsQueryFn.mockResolvedValue([]); + connectivityState.value = 'offline'; + + const { renderer, unmount } = await mountProfile(); + + flushInteractions(); + await waitFor(() => nodeCount(renderer.root, 'QueryError') > 0); + + expect(findNode(renderer.root, 'QueryError')?.props.variant).toBe('offline'); + + unmount(); + }); + it('does not fire the queries when unauthenticated, even after the flush', async () => { authState.token = null; diff --git a/apps/mobile/src/components/profile-screen.signout.mounted.test.tsx b/apps/mobile/src/components/profile-screen.signout.mounted.test.tsx index da18563ab34..af7e9be10a2 100644 --- a/apps/mobile/src/components/profile-screen.signout.mounted.test.tsx +++ b/apps/mobile/src/components/profile-screen.signout.mounted.test.tsx @@ -90,6 +90,12 @@ vi.mock('@/lib/hooks/use-current-user-id', () => ({ useCurrentUserId: () => ({ userId: 'user-1' }), })); +vi.mock('@/lib/hooks/use-offline-banner-state', () => ({ + getCommittedConnectivityStatus: () => 'online', + useCommittedConnectivityStatus: () => 'online', + useOfflineBannerState: () => false, +})); + vi.mock('@/lib/hooks/use-theme-colors', () => ({ useThemeColors: () => ({ destructive: '#B0483A', mutedForeground: '#000000' }), })); diff --git a/apps/mobile/src/components/profile-screen.test-helpers.tsx b/apps/mobile/src/components/profile-screen.test-helpers.tsx index c89027eee75..38324cd0995 100644 --- a/apps/mobile/src/components/profile-screen.test-helpers.tsx +++ b/apps/mobile/src/components/profile-screen.test-helpers.tsx @@ -29,9 +29,13 @@ const captureInteraction = vi.hoisted(() => (cb: () => void) => { }); const getProfileAgentScopeMock = vi.hoisted(() => vi.fn()); const openExternalUrlFn = vi.hoisted(() => vi.fn()); +const connectivityState = vi.hoisted(() => ({ + value: 'online' as 'online' | 'offline' | 'unknown', +})); export { authState, + connectivityState, getProfileAgentScopeMock, interactionState, keys, @@ -104,6 +108,12 @@ vi.mock('@/lib/hooks/use-current-user-id', () => ({ useCurrentUserId: () => ({ userId: 'user-1' }), })); +vi.mock('@/lib/hooks/use-offline-banner-state', () => ({ + getCommittedConnectivityStatus: () => connectivityState.value, + useCommittedConnectivityStatus: () => connectivityState.value, + useOfflineBannerState: () => connectivityState.value === 'offline', +})); + vi.mock('@/lib/hooks/use-theme-colors', () => ({ useThemeColors: () => ({ mutedForeground: '#000000' }), })); diff --git a/apps/mobile/src/components/profile-screen.tsx b/apps/mobile/src/components/profile-screen.tsx index 762c881b8c8..aef776f08cd 100644 --- a/apps/mobile/src/components/profile-screen.tsx +++ b/apps/mobile/src/components/profile-screen.tsx @@ -39,6 +39,7 @@ import { useAuth } from '@/lib/auth/auth-context'; import { openExternalUrl } from '@/lib/external-link'; import { useAfterInteractions } from '@/lib/hooks/use-after-interactions'; import { useCurrentUserId } from '@/lib/hooks/use-current-user-id'; +import { useCommittedConnectivityStatus } from '@/lib/hooks/use-offline-banner-state'; import { useOrganization } from '@/lib/organization-context'; import { getCodeReviewerProfilePath, @@ -103,6 +104,16 @@ export function ProfileScreen() { } = useQuery({ ...trpc.user.getAuthProviders.queryOptions(), enabled: isAuthenticated && afterInteractions, + // The Linked accounts row must settle while the online manager reports + // offline: a paused read is pending without fetching, so `isLoading` is + // false and the section neither paints a row nor an error. This one either + // answers or fails once the control-plane deadline lands, and the error + // surface below then offers Retry. + networkMode: 'always', + // `networkMode: 'always'` also disables the derived reconnect refetch + // (`@tanstack/query-core` sets `refetchOnReconnect = networkMode !== + // 'always'` when it is undefined), so ask for it explicitly. + refetchOnReconnect: true, }); const { data: orgs, @@ -126,6 +137,10 @@ export function ProfileScreen() { const feedbackPrompt = useFeedbackPrompt(); const { t } = useTranslation(); + // Reactive: a connectivity commit re-renders the error-variant selection, so + // a read that settled by failing while the device is confirmed offline says + // so instead of presenting itself as a server fault. + const committedConnectivity = useCommittedConnectivityStatus(); const requestSignOut = () => { confirm({ @@ -283,7 +298,7 @@ export function ProfileScreen() { {organizationsError ? ( void refetchProviders()} diff --git a/apps/mobile/src/lib/hooks/use-current-user-id.test.ts b/apps/mobile/src/lib/hooks/use-current-user-id.test.ts index bfb631fa87c..12aa16789e3 100644 --- a/apps/mobile/src/lib/hooks/use-current-user-id.test.ts +++ b/apps/mobile/src/lib/hooks/use-current-user-id.test.ts @@ -91,12 +91,6 @@ describe('useCurrentUserId', () => { }); describe('isUnreachable', () => { - it('holds while the device is offline and the request waits for a connection', () => { - Object.assign(query, { fetchStatus: 'paused' }); - - expect(useCurrentUserId().isUnreachable).toBe(true); - }); - it('holds after a failure a retry can fix, and through that retry', () => { Object.assign(query, { error: new Error('Network request failed'), diff --git a/apps/mobile/src/lib/hooks/use-current-user-id.ts b/apps/mobile/src/lib/hooks/use-current-user-id.ts index 11ae9af0694..880bbd4f410 100644 --- a/apps/mobile/src/lib/hooks/use-current-user-id.ts +++ b/apps/mobile/src/lib/hooks/use-current-user-id.ts @@ -9,9 +9,20 @@ type UseCurrentUserIdOptions = { export function useCurrentUserId(options: UseCurrentUserIdOptions = {}) { const trpc = useTRPC(); - const { data, error, fetchStatus, isLoading, isError, isFetched, refetch } = useQuery({ + const { data, error, isLoading, isError, isFetched, refetch } = useQuery({ ...trpc.user.getMe.queryOptions(), enabled: options.enabled ?? true, + // The identity read must settle even while the online manager reports + // offline: NetInfo can hold the device offline while the backend is + // reachable, and a paused read leaves every surface gated on the identity + // (the Profile credits card among them) waiting on a request that never + // starts. The attempt either answers or fails once the control-plane + // deadline lands. + networkMode: 'always', + // `networkMode: 'always'` also disables the derived reconnect refetch + // (`@tanstack/query-core` sets `refetchOnReconnect = networkMode !== + // 'always'` when it is undefined), so ask for it explicitly. + refetchOnReconnect: true, }); const refused = error !== null && isTerminalTrpcCode(readTrpcErrorField(error, 'code')); @@ -29,11 +40,11 @@ export function useCurrentUserId(options: UseCurrentUserIdOptions = {}) { // 2026-09-15). isFetchError: isError || (isLoading && isFetched), // True while the account is unconfirmed because the API was not reached: - // the device is offline (Query pauses the request), or an attempt failed - // in a way a retry can fix. It holds through the retry that follows a - // failure, which v5 runs as a no-data refetch with `error` cleared. An - // answer that refuses the credentials is not this. - isUnreachable: data === undefined && !refused && (fetchStatus === 'paused' || isFetched), + // the offline attempt failed, or an attempt failed in a way a retry can + // fix. It holds through the retry that follows a failure, which v5 runs as + // a no-data refetch with `error` cleared. An answer that refuses the + // credentials is not this. + isUnreachable: data === undefined && !refused && isFetched, refetch: () => { void refetch(); }, diff --git a/apps/mobile/src/lib/hooks/use-organization-queries.test.ts b/apps/mobile/src/lib/hooks/use-organization-queries.test.ts index 454224a29f4..b3de1382a43 100644 --- a/apps/mobile/src/lib/hooks/use-organization-queries.test.ts +++ b/apps/mobile/src/lib/hooks/use-organization-queries.test.ts @@ -1,5 +1,5 @@ -import { InfiniteQueryObserver, QueryClient } from '@tanstack/react-query'; -import { describe, expect, it, vi } from 'vitest'; +import { InfiniteQueryObserver, onlineManager, QueryClient } from '@tanstack/react-query'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { buildOrgCreditTransactionsPageQueryOptions, @@ -148,6 +148,84 @@ describe('buildOrgInvoicesPageQueryOptions', () => { }); }); +/** + * Drive the builder's real options through React Query's infinite-query + * behavior while the online manager reports offline: a read that pauses there + * is pending without fetching, which is the screen's skeleton branch with no + * exit, so the builder has to let the request go out and settle. + */ +describe('org invoices first page on a flaky connection', () => { + afterEach(() => { + onlineManager.setOnline(true); + }); + + /** + * Mount the builder's options on a real query client with a failing first + * page. `QueryClient` subscribes to the online manager on `mount()` (React's + * provider does this for the app); without it the online edge reaches no + * cache and the reconnect refetch cannot happen. + */ + function observeInvoicesPage(queryFn: () => Promise) { + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + queryClient.mount(); + const observer = new InfiniteQueryObserver(queryClient, { + ...buildOrgInvoicesPageQueryOptions(createTrpcStub(), 'org-1'), + queryKey: ['org-invoices-flaky-connection'], + queryFn, + initialPageParam: undefined, + retry: false, + }); + // eslint-disable-next-line no-empty-function -- a real listener is required to activate the observer; the body is intentionally empty. + const unsubscribe = observer.subscribe(() => {}); + return { + observer, + stop: () => { + unsubscribe(); + queryClient.unmount(); + }, + }; + } + + function failingPage() { + return vi.fn<() => Promise>().mockRejectedValue(new Error('Network request failed')); + } + + it('settles to the error state while the manager reports offline', async () => { + onlineManager.setOnline(false); + const queryFn = failingPage(); + const { observer, stop } = observeInvoicesPage(queryFn); + + const settled = observer.refetch(); + await vi.waitFor(() => { + expect(observer.getCurrentResult().isError).toBe(true); + }); + await settled; + + expect(queryFn).toHaveBeenCalledTimes(1); + expect(observer.getCurrentResult().isPending).toBe(false); + stop(); + }); + + it('retries the failed first page when connectivity returns', async () => { + onlineManager.setOnline(false); + const queryFn = failingPage(); + const { observer, stop } = observeInvoicesPage(queryFn); + + const settled = observer.refetch(); + await vi.waitFor(() => { + expect(observer.getCurrentResult().isError).toBe(true); + }); + await settled; + + onlineManager.setOnline(true); + + await vi.waitFor(() => { + expect(queryFn).toHaveBeenCalledTimes(2); + }); + stop(); + }); +}); + /** * Drive the builder's real `getNextPageParam` and `maxPages` through React * Query's infinite-query behavior, so the front-trim hazard the finding diff --git a/apps/mobile/src/lib/hooks/use-organization-queries.ts b/apps/mobile/src/lib/hooks/use-organization-queries.ts index 7c2a5e8446d..607e448ed01 100644 --- a/apps/mobile/src/lib/hooks/use-organization-queries.ts +++ b/apps/mobile/src/lib/hooks/use-organization-queries.ts @@ -201,6 +201,12 @@ export type OrgInvoice = RouterOutputs['organizations']['invoicesPage']['entries * Build the invoices infinite-query options. Kept as a pure builder so the * retention bound, the fixed `period: 'year'` input, and the cursor * passthrough are testable without mounting the hook. + * + * The first page must settle while the online manager reports offline: a + * paused read leaves the screen's `isLoading` branch (its skeleton rows) with + * no exit, because a paused query is pending but never fetching, so it neither + * errors nor loads. This read either answers or fails once the control-plane + * deadline lands, and the screen's error state then offers Retry. */ export function buildOrgInvoicesPageQueryOptions( trpc: ReturnType, @@ -215,6 +221,11 @@ export function buildOrgInvoicesPageQueryOptions( lastPage.hasMore && !hasReachedRetentionBound(pages.length) ? (lastPage.nextCursor ?? undefined) : undefined, + networkMode: 'always', + // `networkMode: 'always'` also disables the derived reconnect refetch + // (`@tanstack/query-core` sets `refetchOnReconnect = networkMode !== + // 'always'` when it is undefined), so ask for it explicitly. + refetchOnReconnect: true, } ) ); diff --git a/apps/mobile/src/lib/organization-invoice-error-variant.test.ts b/apps/mobile/src/lib/organization-invoice-error-variant.test.ts new file mode 100644 index 00000000000..33265325742 --- /dev/null +++ b/apps/mobile/src/lib/organization-invoice-error-variant.test.ts @@ -0,0 +1,21 @@ +import { describe, expect, it } from 'vitest'; + +import { selectInvoiceErrorVariant } from '@/lib/organization-invoice-error-variant'; + +describe('selectInvoiceErrorVariant', () => { + it('keeps a permanent code ahead of connectivity', () => { + expect(selectInvoiceErrorVariant('NOT_FOUND', 'offline')).toBe('not-found'); + expect(selectInvoiceErrorVariant('FORBIDDEN', 'offline')).toBe('permission'); + expect(selectInvoiceErrorVariant('UNAUTHORIZED', 'online')).toBe('permission'); + }); + + it('presents a confirmed-offline failure as connectivity', () => { + expect(selectInvoiceErrorVariant(undefined, 'offline')).toBe('offline'); + }); + + it('keeps the retryable neutral state when connectivity is unknown or online', () => { + expect(selectInvoiceErrorVariant(undefined, 'unknown')).toBe('neutral'); + expect(selectInvoiceErrorVariant(undefined, 'online')).toBe('neutral'); + expect(selectInvoiceErrorVariant('INTERNAL_SERVER_ERROR', 'online')).toBe('neutral'); + }); +}); diff --git a/apps/mobile/src/lib/organization-invoice-error-variant.ts b/apps/mobile/src/lib/organization-invoice-error-variant.ts new file mode 100644 index 00000000000..947c0e34ec9 --- /dev/null +++ b/apps/mobile/src/lib/organization-invoice-error-variant.ts @@ -0,0 +1,25 @@ +import { type QueryErrorVariant } from '@/components/query-error'; + +/** The connectivity states the offline banner commits, as a reader sees them. */ +export type CommittedConnectivity = 'online' | 'offline' | 'unknown'; + +/** + * The error state the invoices screen presents for a failed first page. + * + * A thrown `NOT_FOUND` or a permission loss can never be fixed by retrying, so + * those keep their own permanent state. Any other failure is retryable, and a + * device the connectivity probe has confirmed offline presents that cause + * rather than a generic server fault; `unknown` keeps the neutral copy. + */ +export function selectInvoiceErrorVariant( + errorCode: string | undefined, + connectivity: CommittedConnectivity +): QueryErrorVariant { + if (errorCode === 'NOT_FOUND') { + return 'not-found'; + } + if (errorCode === 'FORBIDDEN' || errorCode === 'UNAUTHORIZED') { + return 'permission'; + } + return connectivity === 'offline' ? 'offline' : 'neutral'; +} From 95e853292d6a7f2e57b46f30ac31658adc822c9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Tue, 20 Oct 2026 04:56:35 +0000 Subject: [PATCH 2/2] fix(mobile): keep the offline identity fast path while the read settles The identity read now starts regardless of the online manager, so it is no longer paused while the app reports offline. That removed the immediate signal the root layout used to open on the restored account on a cold start: the bootstrap would wait for the attempt to fail instead, up to the control-plane deadline on a hanging connection. The hook subscribes to the online manager and reports the identity as unreachable as soon as the manager reports offline, so an offline launch still resolves the restored account at once while the attempt settles on its own. Also annotate the invoices observer in its test so the assertion follows the router's page types instead of a fixed one. --- .../src/lib/hooks/use-current-user-id.test.ts | 23 ++++++++++++++++++- .../src/lib/hooks/use-current-user-id.ts | 23 ++++++++++++++----- .../hooks/use-organization-queries.test.ts | 20 ++++++++++++---- .../use-restored-account-id.mounted.test.tsx | 6 ++++- 4 files changed, 60 insertions(+), 12 deletions(-) diff --git a/apps/mobile/src/lib/hooks/use-current-user-id.test.ts b/apps/mobile/src/lib/hooks/use-current-user-id.test.ts index 12aa16789e3..a1ff9997749 100644 --- a/apps/mobile/src/lib/hooks/use-current-user-id.test.ts +++ b/apps/mobile/src/lib/hooks/use-current-user-id.test.ts @@ -12,13 +12,27 @@ const query = vi.hoisted(() => ({ refetch: vi.fn(), })); -vi.mock('@tanstack/react-query', () => ({ useQuery: () => query })); +const manager = vi.hoisted(() => ({ online: true })); + +vi.mock('@tanstack/react-query', () => ({ + useQuery: () => query, + onlineManager: { + isOnline: () => manager.online, + subscribe: () => () => undefined, + }, +})); +// The hook subscribes to the manager through `useSyncExternalStore`; this suite +// calls hooks directly, so the mock reads the current snapshot. +vi.mock('react', () => ({ + useSyncExternalStore: (_subscribe: unknown, getSnapshot: () => unknown): unknown => getSnapshot(), +})); vi.mock('@/lib/trpc', () => ({ useTRPC: () => ({ user: { getMe: { queryOptions: () => ({}) } } }), })); describe('useCurrentUserId', () => { beforeEach(() => { + manager.online = true; Object.assign(query, { data: undefined, error: null, @@ -91,6 +105,13 @@ describe('useCurrentUserId', () => { }); describe('isUnreachable', () => { + it('holds at once when the manager reports offline, before the attempt settles', () => { + manager.online = false; + Object.assign(query, { isLoading: true, fetchStatus: 'fetching' }); + + expect(useCurrentUserId().isUnreachable).toBe(true); + }); + it('holds after a failure a retry can fix, and through that retry', () => { Object.assign(query, { error: new Error('Network request failed'), diff --git a/apps/mobile/src/lib/hooks/use-current-user-id.ts b/apps/mobile/src/lib/hooks/use-current-user-id.ts index 880bbd4f410..dc12009db37 100644 --- a/apps/mobile/src/lib/hooks/use-current-user-id.ts +++ b/apps/mobile/src/lib/hooks/use-current-user-id.ts @@ -1,4 +1,5 @@ -import { useQuery } from '@tanstack/react-query'; +import { onlineManager, useQuery } from '@tanstack/react-query'; +import { useSyncExternalStore } from 'react'; import { useTRPC } from '@/lib/trpc'; import { isTerminalTrpcCode, readTrpcErrorField } from '@/lib/trpc-error'; @@ -9,6 +10,13 @@ type UseCurrentUserIdOptions = { export function useCurrentUserId(options: UseCurrentUserIdOptions = {}) { const trpc = useTRPC(); + // The manager's own reachability signal. It re-renders this hook on the + // online/offline edge, so a cold start that is already offline resolves the + // restored account at once instead of waiting for an attempt to fail. + const managerOnline = useSyncExternalStore( + listener => onlineManager.subscribe(listener), + () => onlineManager.isOnline() + ); const { data, error, isLoading, isError, isFetched, refetch } = useQuery({ ...trpc.user.getMe.queryOptions(), enabled: options.enabled ?? true, @@ -40,11 +48,14 @@ export function useCurrentUserId(options: UseCurrentUserIdOptions = {}) { // 2026-09-15). isFetchError: isError || (isLoading && isFetched), // True while the account is unconfirmed because the API was not reached: - // the offline attempt failed, or an attempt failed in a way a retry can - // fix. It holds through the retry that follows a failure, which v5 runs as - // a no-data refetch with `error` cleared. An answer that refuses the - // credentials is not this. - isUnreachable: data === undefined && !refused && isFetched, + // the manager already reports offline (so the attempt running under + // `networkMode: 'always'` has not answered yet), or an attempt failed in a + // way a retry can fix. It holds through the retry that follows a failure, + // which v5 runs as a no-data refetch with `error` cleared. An answer that + // refuses the credentials is not this. The caller resolves the restored + // account from this signal on a cold start, so an offline launch must not + // wait for the attempt to fail. + isUnreachable: data === undefined && !refused && (!managerOnline || isFetched), refetch: () => { void refetch(); }, diff --git a/apps/mobile/src/lib/hooks/use-organization-queries.test.ts b/apps/mobile/src/lib/hooks/use-organization-queries.test.ts index b3de1382a43..a2fb08c7221 100644 --- a/apps/mobile/src/lib/hooks/use-organization-queries.test.ts +++ b/apps/mobile/src/lib/hooks/use-organization-queries.test.ts @@ -1,4 +1,9 @@ -import { InfiniteQueryObserver, onlineManager, QueryClient } from '@tanstack/react-query'; +import { + InfiniteQueryObserver, + type InfiniteQueryObserverOptions, + onlineManager, + QueryClient, +} from '@tanstack/react-query'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { @@ -168,13 +173,20 @@ describe('org invoices first page on a flaky connection', () => { function observeInvoicesPage(queryFn: () => Promise) { const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); queryClient.mount(); - const observer = new InfiniteQueryObserver(queryClient, { - ...buildOrgInvoicesPageQueryOptions(createTrpcStub(), 'org-1'), + const options = { + // The builder is driven by a `never`-typed stub, so its declared page + // types come from the real router; the observer only needs the contract + // the behavior under test exercises. + ...(buildOrgInvoicesPageQueryOptions( + createTrpcStub(), + 'org-1' + ) as unknown as InfiniteQueryObserverOptions), queryKey: ['org-invoices-flaky-connection'], queryFn, initialPageParam: undefined, retry: false, - }); + }; + const observer = new InfiniteQueryObserver(queryClient, options); // eslint-disable-next-line no-empty-function -- a real listener is required to activate the observer; the body is intentionally empty. const unsubscribe = observer.subscribe(() => {}); return { diff --git a/apps/mobile/src/lib/hooks/use-restored-account-id.mounted.test.tsx b/apps/mobile/src/lib/hooks/use-restored-account-id.mounted.test.tsx index f538ec3b8ae..f3f0bf55097 100644 --- a/apps/mobile/src/lib/hooks/use-restored-account-id.mounted.test.tsx +++ b/apps/mobile/src/lib/hooks/use-restored-account-id.mounted.test.tsx @@ -1,4 +1,5 @@ import { createElement } from 'react'; +import type * as ReactQueryModule from '@tanstack/react-query'; import { act, TestRenderer } from '@/test/renderer'; import { beforeEach, describe, expect, it, onTestFinished, vi } from 'vitest'; @@ -7,7 +8,10 @@ import { useRestoredAccountId } from './use-restored-account-id'; const getMe = vi.hoisted(() => ({ data: undefined as { id?: string } | undefined })); const getItemAsync = vi.hoisted(() => vi.fn<(key: string) => Promise>()); -vi.mock('@tanstack/react-query', () => ({ useQuery: () => getMe })); +vi.mock('@tanstack/react-query', async importOriginal => ({ + ...(await importOriginal()), + useQuery: () => getMe, +})); vi.mock('@/lib/trpc', () => ({ useTRPC: () => ({ user: { getMe: { queryOptions: () => ({}) } } }), }));