diff --git a/src/libs/NetworkState.ts b/src/libs/NetworkState.ts index b94e45a31d96..6ea480ae3946 100644 --- a/src/libs/NetworkState.ts +++ b/src/libs/NetworkState.ts @@ -20,6 +20,7 @@ let unsubscribeNetInfo: (() => void) | null = null; let prevIsInternetReachable: boolean | null | undefined; let isPoorConnectionSimulated: boolean | undefined; let networkTimeSkew = 0; +let suppressNextReachabilityRestored = false; // Subscriber sets const listeners = new Set<() => void>(); @@ -268,11 +269,24 @@ function setRandomNetworkStatus(initialCall = false) { * Must unsubscribe before calling configure() — configure tears down NetInfo internal state. */ function configureAndSubscribe() { + // Treat this as a reconfigure (not an initial subscription) when there's already a listener. + // Reconfigure tears down NetInfo internal state, so the new subscription emits a synthetic + // null→true transition that would look like a recovery — suppress the next would-be recovery + // until reachability settles. Initial subscription is left untouched so boot behavior is + // unchanged (prev=undefined boot guard already covers it). + // Skip suppression when prev was already false: the app was genuinely offline before + // reconfigure, so the next true is a real recovery we must not drop (otherwise + // internetUnreachable stays set and the app is stuck offline until a new outage cycle). + const isReconfigure = unsubscribeNetInfo !== null; if (unsubscribeNetInfo) { unsubscribeNetInfo(); unsubscribeNetInfo = null; } + if (isReconfigure && prevIsInternetReachable !== false) { + suppressNextReachabilityRestored = true; + } + if (!CONFIG.IS_USING_LOCAL_WEB) { NetInfo.configure({ reachabilityUrl: `${CONFIG.EXPENSIFY.DEFAULT_API_ROOT}api/Ping?accountID=${accountID ?? 'unknown'}`, @@ -315,8 +329,17 @@ function configureAndSubscribe() { // NetInfo event on subscribe which delivers current state, not a recovery. Firing // onReachabilityRestored() on boot would duplicate openApp()/reconnectApp(). if (!shouldForceOffline && state.isInternetReachable === true && prevIsInternetReachable !== true && prevIsInternetReachable !== undefined) { - Log.info(`[NetworkState] Internet reachability restored (${prevIsInternetReachable}→true)`); - onReachabilityRestored(); + if (suppressNextReachabilityRestored) { + Log.info(`[NetworkState] Suppressing recovery on first stable state after reconfigure (${prevIsInternetReachable}→true)`); + } else { + Log.info(`[NetworkState] Internet reachability restored (${prevIsInternetReachable}→true)`); + onReachabilityRestored(); + } + } + // End the post-reconfigure suppression window once reachability settles into a definitive + // state. Null/undefined are transient and should not end the window. + if (state.isInternetReachable === true || state.isInternetReachable === false) { + suppressNextReachabilityRestored = false; } prevIsInternetReachable = state.isInternetReachable; }); diff --git a/src/libs/actions/Delegate.ts b/src/libs/actions/Delegate.ts index 4cbacabd6034..3da4ee1b8602 100644 --- a/src/libs/actions/Delegate.ts +++ b/src/libs/actions/Delegate.ts @@ -41,6 +41,17 @@ const KEYS_TO_PRESERVE_DELEGATE_ACCESS = [ ONYXKEYS.COLLECTION.DEVICE_BIOMETRICS, ]; +/** + * Atomically reset Onyx for a delegate-access transition while keeping IS_LOADING_APP=true + * so consumers never observe the post-clear state with HAS_LOADED_APP=true and + * IS_LOADING_APP=undefined. That combination falsely looks like a stuck app and triggers + * DelegateAccessHandler's recovery effect, producing a duplicate openApp queued behind the + * explicit openApp the caller is about to make. + */ +function clearOnyxForDelegateTransition(): Promise { + return Onyx.merge(ONYXKEYS.IS_LOADING_APP, true).then(() => Onyx.clear([...KEYS_TO_PRESERVE_DELEGATE_ACCESS, ONYXKEYS.IS_LOADING_APP])); +} + type WithDelegatedAccess = { // Optional keeps call sites clean, but still encourages passing `account?.delegatedAccess`. delegatedAccess: DelegatedAccess | undefined; @@ -195,7 +206,7 @@ function connect({email, delegatedAccess, credentials, session, activePolicyID, }) .then(() => { NetworkStore.setAuthToken(response?.restrictedToken ?? null); - return Onyx.clear(KEYS_TO_PRESERVE_DELEGATE_ACCESS); + return clearOnyxForDelegateTransition(); }) .then(() => { confirmReadyToOpenApp(); @@ -294,7 +305,7 @@ function disconnect({stashedCredentials, stashedSession}: DisconnectParams) { }) .then(() => { NetworkStore.setAuthToken(response?.authToken ?? null); - return Onyx.clear(KEYS_TO_PRESERVE_DELEGATE_ACCESS); + return clearOnyxForDelegateTransition(); }) .then(() => { Onyx.set(ONYXKEYS.CREDENTIALS, { @@ -663,7 +674,7 @@ function updateDelegateRole({email, role, validateCode, delegatedAccess}: Update } function restoreDelegateSession(authenticateResponse: Response) { - Onyx.clear(KEYS_TO_PRESERVE_DELEGATE_ACCESS).then(() => { + clearOnyxForDelegateTransition().then(() => { updateSessionAuthTokens(authenticateResponse?.authToken, authenticateResponse?.encryptedAuthToken); updateSessionUser(authenticateResponse?.accountID, authenticateResponse?.email); @@ -690,5 +701,6 @@ export { updateDelegateRole, removeDelegate, openSecuritySettingsPage, + clearOnyxForDelegateTransition, KEYS_TO_PRESERVE_DELEGATE_ACCESS, }; diff --git a/src/libs/actions/Session/index.ts b/src/libs/actions/Session/index.ts index 90da86594b56..a5a44a83e43d 100644 --- a/src/libs/actions/Session/index.ts +++ b/src/libs/actions/Session/index.ts @@ -51,7 +51,7 @@ import Timers from '@libs/Timers'; import {hideContextMenu} from '@pages/inbox/report/ContextMenu/ReportActionContextMenu'; import {confirmReadyToOpenApp, KEYS_TO_PRESERVE, openApp} from '@userActions/App'; import {clearCachedAttachments} from '@userActions/Attachment'; -import {KEYS_TO_PRESERVE_DELEGATE_ACCESS} from '@userActions/Delegate'; +import {clearOnyxForDelegateTransition} from '@userActions/Delegate'; import * as Device from '@userActions/Device'; import type HybridAppSettings from '@userActions/HybridApp/types'; import {close} from '@userActions/Modal'; @@ -718,7 +718,7 @@ function setupNewDotAfterTransitionFromOldDot(hybridAppSettings: HybridAppSettin } Log.info('[HybridApp] User switched account on OldDot side. Clearing onyx and applying delegate data'); - return Onyx.clear(KEYS_TO_PRESERVE_DELEGATE_ACCESS) + return clearOnyxForDelegateTransition() .then(() => Onyx.multiSet({ ...stashedData, diff --git a/tests/actions/DelegateTest.ts b/tests/actions/DelegateTest.ts index 0081ba0d192f..e9e30aa3e31e 100644 --- a/tests/actions/DelegateTest.ts +++ b/tests/actions/DelegateTest.ts @@ -1,5 +1,13 @@ import Onyx from 'react-native-onyx'; -import {addDelegate, clearDelegateErrorsByField, clearDelegatorErrors, isConnectedAsDelegate, removeDelegate, updateDelegateRole} from '@libs/actions/Delegate'; +import { + addDelegate, + clearDelegateErrorsByField, + clearDelegatorErrors, + clearOnyxForDelegateTransition, + isConnectedAsDelegate, + removeDelegate, + updateDelegateRole, +} from '@libs/actions/Delegate'; import {pause, resetQueue} from '@libs/Network/SequentialQueue'; import CONST from '@src/CONST'; import OnyxUpdateManager from '@src/libs/actions/OnyxUpdateManager'; @@ -236,4 +244,54 @@ describe('actions/Delegate', () => { }); }); }); + + describe('clearOnyxForDelegateTransition', () => { + it('keeps IS_LOADING_APP=true after the clear so DelegateAccessHandler does not see HAS_LOADED_APP=true && IS_LOADING_APP=undefined and fire a duplicate openApp', async () => { + // Simulate the pre-switch state: app is fully loaded with the previous account. + await Onyx.multiSet({ + [ONYXKEYS.HAS_LOADED_APP]: true, + [ONYXKEYS.IS_LOADING_APP]: false, + [ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.DEFAULT, + }); + await waitForBatchedUpdates(); + + await clearOnyxForDelegateTransition(); + await waitForBatchedUpdates(); + + await new Promise((resolve) => { + const conn = Onyx.connect({ + key: ONYXKEYS.IS_LOADING_APP, + callback: (value) => { + expect(value).toBe(true); + Onyx.disconnect(conn); + resolve(); + }, + }); + }); + + // HAS_LOADED_APP is in the preserve list and should remain true. + await new Promise((resolve) => { + const conn = Onyx.connect({ + key: ONYXKEYS.HAS_LOADED_APP, + callback: (value) => { + expect(value).toBe(true); + Onyx.disconnect(conn); + resolve(); + }, + }); + }); + + // A non-preserved key should have been cleared. + await new Promise((resolve) => { + const conn = Onyx.connect({ + key: ONYXKEYS.NVP_PRIORITY_MODE, + callback: (value) => { + expect(value).toBeUndefined(); + Onyx.disconnect(conn); + resolve(); + }, + }); + }); + }); + }); }); diff --git a/tests/unit/NetworkStateReachabilityTest.ts b/tests/unit/NetworkStateReachabilityTest.ts index 53f510189e3c..2fbae232f391 100644 --- a/tests/unit/NetworkStateReachabilityTest.ts +++ b/tests/unit/NetworkStateReachabilityTest.ts @@ -2,6 +2,7 @@ import type {NetInfoState} from '@react-native-community/netinfo'; import type * as NetworkState from '@src/libs/NetworkState'; let netInfoListener: ((state: NetInfoState) => void) | null = null; +const mockOnyxCallbacks = new Map void>(); jest.mock('@react-native-community/netinfo', () => ({ addEventListener: jest.fn((cb: (state: NetInfoState) => void) => { @@ -17,7 +18,9 @@ jest.mock('@react-native-community/netinfo', () => ({ jest.mock('@src/libs/Log'); jest.mock('react-native-onyx', () => ({ - connectWithoutView: jest.fn(), + connectWithoutView: jest.fn(({key, callback}: {key: string; callback: (value: unknown) => void}) => { + mockOnyxCallbacks.set(key, callback); + }), })); function fireNetInfoState(overrides: Partial) { @@ -33,12 +36,21 @@ function fireNetInfoState(overrides: Partial) { } as NetInfoState); } +function fireSessionChange(accountID: number) { + const cb = mockOnyxCallbacks.get('session'); + if (!cb) { + throw new Error('SESSION callback not registered'); + } + cb({accountID}); +} + describe('NetworkState — internetUnreachable hard stop via NetInfo', () => { let getIsOffline: typeof NetworkState.getIsOffline; beforeEach(() => { jest.resetModules(); netInfoListener = null; + mockOnyxCallbacks.clear(); const mod = require('@src/libs/NetworkState'); getIsOffline = mod.getIsOffline; @@ -81,6 +93,7 @@ describe('NetworkState — reachability recovery triggers reconnect', () => { beforeEach(() => { jest.resetModules(); netInfoListener = null; + mockOnyxCallbacks.clear(); // Fresh import each test so prevIsInternetReachable resets const mod = require('@src/libs/NetworkState'); @@ -143,6 +156,69 @@ describe('NetworkState — reachability recovery triggers reconnect', () => { expect(reconnectListener).not.toHaveBeenCalled(); }); + test('SESSION accountID change does NOT fire reconnect listener via the post-reconfigure synthetic transition', () => { + // Repro for the doubled-OpenApp bug on delegate switch: the SESSION accountID change + // re-runs configureAndSubscribe(), which tears down and re-subscribes to NetInfo. The + // new subscription emits null then true, which would look like a recovery. The fix + // resets prev to undefined on reconfigure so the new subscription's first transitions + // are treated like boot, not recovery. + const reconnectListener = jest.fn(); + onReachabilityConfirmed(reconnectListener); + + // Establish a baseline reachable state (boot) + fireNetInfoState({isInternetReachable: true}); + expect(reconnectListener).not.toHaveBeenCalled(); + + // Delegate switch: SESSION accountID changes → reconfigure → new NetInfo subscription + fireSessionChange(42); + + // New subscription's initial events: null while the first Ping is in flight, then true + fireNetInfoState({isInternetReachable: null}); + fireNetInfoState({isInternetReachable: true}); + + expect(reconnectListener).not.toHaveBeenCalled(); + }); + + test('genuine offline→online after a SESSION reconfigure still fires reconnect listener', () => { + // Make sure the reconfigure suppression doesn't swallow real recoveries that happen + // afterwards — only the synthetic post-reconfigure transition should be ignored. + const reconnectListener = jest.fn(); + onReachabilityConfirmed(reconnectListener); + + fireNetInfoState({isInternetReachable: true}); + fireSessionChange(42); + + // Settle on the reconfigured subscription + fireNetInfoState({isInternetReachable: true}); + expect(reconnectListener).not.toHaveBeenCalled(); + + // Now a real outage and recovery + fireNetInfoState({isInternetReachable: false}); + fireNetInfoState({isInternetReachable: true}); + + expect(reconnectListener).toHaveBeenCalledTimes(1); + }); + + test('genuine offline before reconfigure still recovers on the next true', () => { + // Boot offline scenario: NetInfo confirms unreachable BEFORE SESSION hydrates and triggers + // a reconfigure. The post-reconfigure true must NOT be suppressed — otherwise the app would + // remain stuck with internetUnreachable=true until a brand new outage cycle. + const reconnectListener = jest.fn(); + onReachabilityConfirmed(reconnectListener); + + // Cold boot: null then false → app is genuinely offline, prev=false + fireNetInfoState({isInternetReachable: null}); + fireNetInfoState({isInternetReachable: false}); + + // SESSION hydrates → reconfigure happens while we are still offline + fireSessionChange(42); + + // New subscription's first definitive event recovers + fireNetInfoState({isInternetReachable: true}); + + expect(reconnectListener).toHaveBeenCalledTimes(1); + }); + test('turning off force-offline resets prevIsInternetReachable so next refresh triggers reconnect', () => { const reconnectListener = jest.fn(); onReachabilityConfirmed(reconnectListener);