Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 4 additions & 19 deletions src/DelegateAccessHandler.tsx
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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(() => {
Expand Down Expand Up @@ -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;
}

Expand Down
83 changes: 83 additions & 0 deletions src/hooks/useIsLoadingAppRecovery.ts
Original file line number Diff line number Diff line change
@@ -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<typeof setTimeout>;
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;
16 changes: 15 additions & 1 deletion src/libs/actions/App.ts
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,7 @@ function getOnyxDataForOpenOrReconnect(
isFullReconnect = false,
shouldKeepPublicRooms = false,
allReportsWithDraftComments?: Record<string, string | undefined>,
shouldClearAppLoading = false,
): OnyxData<OnyxDataForOpenOrReconnectKeys> {
let commandName: string;
if (isOpenApp) {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -506,7 +516,11 @@ function reconnectApp(updateIDFrom: OnyxEntry<number> = 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;
}
Expand Down
79 changes: 79 additions & 0 deletions tests/actions/IsLoadingAppStrandedTest.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
97 changes: 97 additions & 0 deletions tests/ui/DelegateAccessHandlerTest.tsx
Original file line number Diff line number Diff line change
@@ -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(<DelegateAccessHandler />);
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<never> = {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<never> = {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();
});
});
Loading