diff --git a/src/DelegateAccessHandler.tsx b/src/DelegateAccessHandler.tsx index c0e3c44f729c..22eb3734f3f2 100644 --- a/src/DelegateAccessHandler.tsx +++ b/src/DelegateAccessHandler.tsx @@ -1,14 +1,12 @@ import {useEffect, useRef} from 'react'; import CONST from './CONST'; -import useNetwork from './hooks/useNetwork'; +import useIsLoadingAppRecovery from './hooks/useIsLoadingAppRecovery'; import useOnyx from './hooks/useOnyx'; -import {openApp} from './libs/actions/App'; import {disconnect} from './libs/actions/Delegate'; import Log from './libs/Log'; import ONYXKEYS from './ONYXKEYS'; import {accountIDSelector, emailSelector} from './selectors/Session'; -import isLoadingOnyxValue from './types/utils/isLoadingOnyxValue'; /** * Component that does not render anything but isolates delegate-access–related Onyx subscriptions @@ -18,16 +16,16 @@ import isLoadingOnyxValue from './types/utils/isLoadingOnyxValue'; */ function DelegateAccessHandler() { const hasLoggedDelegateMismatchRef = useRef(false); - const hasHandledMissingIsLoadingAppRef = useRef(false); const [account] = useOnyx(ONYXKEYS.ACCOUNT); const [stashedCredentials = CONST.EMPTY_OBJECT] = useOnyx(ONYXKEYS.STASHED_CREDENTIALS); const [stashedSession] = useOnyx(ONYXKEYS.STASHED_SESSION); const [hasLoadedApp] = useOnyx(ONYXKEYS.HAS_LOADED_APP); - const [isLoadingApp, isLoadingAppMetadata] = useOnyx(ONYXKEYS.IS_LOADING_APP); + const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP); const [sessionAccountID] = useOnyx(ONYXKEYS.SESSION, {selector: accountIDSelector}); const [sessionEmail] = useOnyx(ONYXKEYS.SESSION, {selector: emailSelector}); - const {isOffline} = useNetwork(); + + useIsLoadingAppRecovery(); // Disconnect delegate when the delegate is no longer in the delegates list useEffect(() => { @@ -59,19 +57,6 @@ function DelegateAccessHandler() { }); }, [account?.delegatedAccess?.delegate, account?.delegatedAccess?.delegators, account?.primaryLogin, hasLoadedApp, isLoadingApp, sessionAccountID, sessionEmail]); - // Recovery: if isLoadingApp is missing after the app is ready, re-open the app - useEffect(() => { - if (hasHandledMissingIsLoadingAppRef.current || !hasLoadedApp || isLoadingApp !== undefined || isOffline || isLoadingOnyxValue(isLoadingAppMetadata)) { - return; - } - hasHandledMissingIsLoadingAppRef.current = true; - Log.info('[Onyx] isLoadingApp missing after app is ready', false, { - sessionAccountID, - hasLoadedApp: !!hasLoadedApp, - }); - openApp(); - }, [hasLoadedApp, isLoadingApp, isOffline, sessionAccountID, isLoadingAppMetadata]); - return null; } diff --git a/src/hooks/useIsLoadingAppRecovery.ts b/src/hooks/useIsLoadingAppRecovery.ts new file mode 100644 index 000000000000..f10504d31fdc --- /dev/null +++ b/src/hooks/useIsLoadingAppRecovery.ts @@ -0,0 +1,83 @@ +import {WRITE_COMMANDS} from '@libs/API/types'; +import Log from '@libs/Log'; + +import {openApp} from '@userActions/App'; +import {getAll as getAllPersistedRequests, getOngoingRequest} from '@userActions/PersistedRequests'; + +import ONYXKEYS from '@src/ONYXKEYS'; +import isLoadingOnyxValue from '@src/types/utils/isLoadingOnyxValue'; + +import {accountIDSelector} from '@selectors/Session'; +import {useEffect, useRef} from 'react'; + +import useNetwork from './useNetwork'; +import useOnyx from './useOnyx'; + +// How often to check whether IS_LOADING_APP is stranded. This is a check cadence, not a timeout: +// a slow but legitimate OpenApp (large accounts can take 30s+) is visible as a pending request at +// every check, so the recovery waits for it instead of interfering. Only a `true` with no pending +// OpenApp/ReconnectApp anywhere is treated as stranded. +const STRANDED_IS_LOADING_APP_RECOVERY_DELAY_MS = 10000; + +/** + * Recovers IS_LOADING_APP when it can no longer resolve on its own. The key is only ever cleared by + * finallyData of the OpenApp/ReconnectApp family — the backend never writes it — and the optimistic `true` + * persists immediately while the clearing finallyData is held in memory until the sequential queue flushes. + * An interrupted session can therefore leave the key `undefined` or stranded at `true` across reloads, with + * nothing left to reset it. This hook re-opens the app in both cases. + */ +function useIsLoadingAppRecovery() { + const hasHandledMissingIsLoadingAppRef = useRef(false); + const hasHandledStrandedIsLoadingAppRef = useRef(false); + + const [hasLoadedApp] = useOnyx(ONYXKEYS.HAS_LOADED_APP); + const [isLoadingApp, isLoadingAppMetadata] = useOnyx(ONYXKEYS.IS_LOADING_APP); + const [sessionAccountID] = useOnyx(ONYXKEYS.SESSION, {selector: accountIDSelector}); + const {isOffline} = useNetwork(); + + // Recovery: if isLoadingApp is missing after the app is ready, re-open the app + useEffect(() => { + if (hasHandledMissingIsLoadingAppRef.current || !hasLoadedApp || isLoadingApp !== undefined || isOffline || isLoadingOnyxValue(isLoadingAppMetadata)) { + return; + } + hasHandledMissingIsLoadingAppRef.current = true; + Log.info('[Onyx] isLoadingApp missing after app is ready', false, { + sessionAccountID, + hasLoadedApp: !!hasLoadedApp, + }); + openApp(); + }, [hasLoadedApp, isLoadingApp, isOffline, sessionAccountID, isLoadingAppMetadata]); + + // Recovery: if isLoadingApp stays `true` while the app is loaded and online with no reconnect-family + // request pending, the clearing finallyData was lost — re-open the app. + useEffect(() => { + if (hasHandledStrandedIsLoadingAppRef.current || !hasLoadedApp || isLoadingApp !== true || isOffline || isLoadingOnyxValue(isLoadingAppMetadata)) { + return; + } + + let timeoutID: ReturnType; + const checkStranded = () => { + // A legitimate in-flight load still has a reconnect-family request pending; only the + // stranded case has none. In that case check again later instead of giving up — the effect + // dependencies won't change if that request is later removed without applying its finallyData. + const hasPendingReconnectRequest = [getOngoingRequest(), ...getAllPersistedRequests()].some( + (request) => request?.command === WRITE_COMMANDS.OPEN_APP || request?.command === WRITE_COMMANDS.RECONNECT_APP, + ); + if (hasPendingReconnectRequest) { + timeoutID = setTimeout(checkStranded, STRANDED_IS_LOADING_APP_RECOVERY_DELAY_MS); + return; + } + hasHandledStrandedIsLoadingAppRef.current = true; + Log.info('[Onyx] isLoadingApp stranded true with no pending OpenApp/ReconnectApp, re-opening', false, { + sessionAccountID, + hasLoadedApp: !!hasLoadedApp, + }); + openApp(); + }; + timeoutID = setTimeout(checkStranded, STRANDED_IS_LOADING_APP_RECOVERY_DELAY_MS); + + return () => clearTimeout(timeoutID); + }, [hasLoadedApp, isLoadingApp, isOffline, sessionAccountID, isLoadingAppMetadata]); +} + +export default useIsLoadingAppRecovery; diff --git a/src/libs/actions/App.ts b/src/libs/actions/App.ts index 5d5b92da8563..b34de34a4240 100644 --- a/src/libs/actions/App.ts +++ b/src/libs/actions/App.ts @@ -324,6 +324,7 @@ function getOnyxDataForOpenOrReconnect( isFullReconnect = false, shouldKeepPublicRooms = false, allReportsWithDraftComments?: Record, + shouldClearAppLoading = false, ): OnyxData { let commandName: string; if (isOpenApp) { @@ -372,7 +373,16 @@ function getOnyxDataForOpenOrReconnect( key: ONYXKEYS.IS_LOADING_APP, value: true, }); + } + // Clear IS_LOADING_APP in finallyData for OpenApp and ReconnectApp, not just OpenApp. The optimistic `true` + // above persists immediately, while finallyData for write commands is held in memory until the sequential + // queue flushes (see QueuedOnyxUpdates), so an interrupted session can leave `true` persisted with no OpenApp + // left to clear it — once HAS_LOADED_APP is set, the app only ever runs ReconnectApp. Having ReconnectApp also + // clear the flag makes it self-healing. Callers outside that family (GetMissingOnyxMessages) must not clear + // it: they run as side-effect requests outside the sequential queue, so they could race a legitimate in-flight + // OpenApp and drop the loading gate before its data lands. + if (isOpenApp || shouldClearAppLoading) { result.finallyData?.push({ onyxMethod: Onyx.METHOD.MERGE, key: ONYXKEYS.IS_LOADING_APP, @@ -506,7 +516,11 @@ function reconnectApp(updateIDFrom: OnyxEntry = 0) { } const isFullReconnect = !updateIDFrom; - return API.writeWithNoDuplicatesReconnectConflictAction(WRITE_COMMANDS.RECONNECT_APP, params, getOnyxDataForOpenOrReconnect(false, isFullReconnect, isSidebarLoaded)).finally(() => { + return API.writeWithNoDuplicatesReconnectConflictAction( + WRITE_COMMANDS.RECONNECT_APP, + params, + getOnyxDataForOpenOrReconnect(false, isFullReconnect, isSidebarLoaded, undefined, true), + ).finally(() => { if (!bootsplashSpan) { return; } diff --git a/tests/actions/IsLoadingAppStrandedTest.ts b/tests/actions/IsLoadingAppStrandedTest.ts new file mode 100644 index 000000000000..5d239511440f --- /dev/null +++ b/tests/actions/IsLoadingAppStrandedTest.ts @@ -0,0 +1,79 @@ +import * as App from '@libs/actions/App'; +import {WRITE_COMMANDS} from '@libs/API/types'; +import {resetQueue} from '@libs/Network/SequentialQueue'; + +import OnyxUpdateManager from '@src/libs/actions/OnyxUpdateManager'; +import * as PersistedRequests from '@src/libs/actions/PersistedRequests'; +import ONYXKEYS from '@src/ONYXKEYS'; + +import Onyx from 'react-native-onyx'; + +import getOnyxValue from '../utils/getOnyxValue'; +import * as TestHelper from '../utils/TestHelper'; +import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; + +OnyxUpdateManager(); + +describe('IS_LOADING_APP stranded true', () => { + beforeAll(() => { + Onyx.init({keys: ONYXKEYS}); + }); + + beforeEach(async () => { + global.fetch = TestHelper.getGlobalFetchMock(); + // Onyx.clear does not reset these module-level singletons, so reset them explicitly + // to keep the tests order-independent: the request queue, the sequential queue, and + // the NetworkState offline flag (forced offline by the stranding test below). + await PersistedRequests.clear(); + resetQueue(); + await Onyx.clear(); + await Onyx.merge(ONYXKEYS.NETWORK, {shouldForceOffline: false}); + await waitForBatchedUpdates(); + }); + + it('stays true when the OpenApp that should clear it never settles, and is stranded once that request is lost', async () => { + // The app was already loaded in a previous session and the client is offline. + await Onyx.set(ONYXKEYS.NETWORK, {shouldForceOffline: true}); + await Onyx.set(ONYXKEYS.HAS_LOADED_APP, true); + + // IS_LOADING_APP=true is persisted as a standalone write, decoupled from any request (this is how + // both openApp's optimisticData and clearOnyxForDelegateTransition write it). Only finallyData of + // the OpenApp/ReconnectApp family clears it. + await Onyx.set(ONYXKEYS.IS_LOADING_APP, true); + await waitForBatchedUpdates(); + expect(await getOnyxValue(ONYXKEYS.IS_LOADING_APP)).toBe(true); + + // An OpenApp is queued to clear the flag. Offline, so it persists but never flushes. + App.openApp(); + await waitForBatchedUpdates(); + + // OpenApp is queued (its finallyData would set the flag false on completion) ... + expect(PersistedRequests.getAll().some((request) => request.command === WRITE_COMMANDS.OPEN_APP)).toBe(true); + // ... but the flag is still true because finallyData has not run. + expect(await getOnyxValue(ONYXKEYS.IS_LOADING_APP)).toBe(true); + + // The queued OpenApp is lost before it ever settles (e.g. the session was interrupted after the + // request left the queue but before its finallyData was applied). Nothing is left to clear the flag. + await PersistedRequests.clear(); + resetQueue(); + await waitForBatchedUpdates(); + + // Stranded: no OpenApp left in the queue, the backend never sets this key, and the app is + // "loaded" so nothing re-fires OpenApp. + expect(PersistedRequests.getAll().some((request) => request.command === WRITE_COMMANDS.OPEN_APP)).toBe(false); + expect(await getOnyxValue(ONYXKEYS.IS_LOADING_APP)).toBe(true); + }); + + it('is healed by the next successful ReconnectApp', async () => { + // A previous session stranded the flag on disk and the app booted from it. + await Onyx.set(ONYXKEYS.HAS_LOADED_APP, true); + await Onyx.set(ONYXKEYS.IS_LOADING_APP, true); + await waitForBatchedUpdates(); + + // Boot with HAS_LOADED_APP set runs ReconnectApp, never OpenApp. + App.reconnectApp(); + await waitForBatchedUpdates(); + + expect(await getOnyxValue(ONYXKEYS.IS_LOADING_APP)).toBe(false); + }); +}); diff --git a/tests/ui/DelegateAccessHandlerTest.tsx b/tests/ui/DelegateAccessHandlerTest.tsx new file mode 100644 index 000000000000..440b7a4164d8 --- /dev/null +++ b/tests/ui/DelegateAccessHandlerTest.tsx @@ -0,0 +1,97 @@ +import {act, render} from '@testing-library/react-native'; + +import {openApp} from '@userActions/App'; + +import DelegateAccessHandler from '@src/DelegateAccessHandler'; +import * as PersistedRequests from '@src/libs/actions/PersistedRequests'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type Request from '@src/types/onyx/Request'; + +import React from 'react'; +import Onyx from 'react-native-onyx'; + +import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; +import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct'; + +jest.mock('@userActions/App', () => ({ + openApp: jest.fn(), +})); + +jest.mock('@userActions/Delegate', () => ({ + disconnect: jest.fn(), +})); + +const RECOVERY_DELAY_MS = 10000; + +describe('DelegateAccessHandler stuck isLoadingApp recovery', () => { + beforeAll(() => { + Onyx.init({keys: ONYXKEYS}); + }); + + beforeEach(async () => { + jest.clearAllMocks(); + await PersistedRequests.clear(); + await Onyx.clear(); + await waitForBatchedUpdates(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + async function renderWithStrandedState(isLoadingApp: boolean | undefined) { + await Onyx.multiSet({ + [ONYXKEYS.HAS_LOADED_APP]: true, + ...(isLoadingApp === undefined ? {} : {[ONYXKEYS.IS_LOADING_APP]: isLoadingApp}), + }); + await waitForBatchedUpdates(); + + // The recovery timeout must be scheduled under fake timers so the test controls when it fires. + jest.useFakeTimers({doNotFake: ['nextTick']}); + render(); + await waitForBatchedUpdatesWithAct(); + act(() => { + jest.advanceTimersByTime(RECOVERY_DELAY_MS); + }); + await waitForBatchedUpdatesWithAct(); + } + + it('re-fires openApp when isLoadingApp is stranded true with no pending OpenApp', async () => { + await renderWithStrandedState(true); + + expect(openApp).toHaveBeenCalledTimes(1); + }); + + it('does not re-fire while an OpenApp is still pending in the queue', async () => { + const pendingOpenApp: Request = {command: 'OpenApp'}; + await PersistedRequests.save(pendingOpenApp); + + await renderWithStrandedState(true); + + expect(openApp).not.toHaveBeenCalled(); + }); + + it('keeps checking and recovers once a pending OpenApp is dropped without settling', async () => { + const pendingOpenApp: Request = {command: 'OpenApp'}; + await PersistedRequests.save(pendingOpenApp); + + await renderWithStrandedState(true); + expect(openApp).not.toHaveBeenCalled(); + + // The pending request is removed without its finallyData ever applying, so the flag stays true. + // The effect dependencies do not change, so only the rescheduled check can catch this. + await PersistedRequests.clear(); + act(() => { + jest.advanceTimersByTime(RECOVERY_DELAY_MS); + }); + await waitForBatchedUpdatesWithAct(); + + expect(openApp).toHaveBeenCalledTimes(1); + }); + + it('does not re-fire when isLoadingApp already cleared to false', async () => { + await renderWithStrandedState(false); + + expect(openApp).not.toHaveBeenCalled(); + }); +});