diff --git a/src/libs/NetworkConnection.ts b/src/libs/NetworkConnection.ts index ccd7fbf4e8b9..8a982556f0b0 100644 --- a/src/libs/NetworkConnection.ts +++ b/src/libs/NetworkConnection.ts @@ -50,6 +50,42 @@ const triggerReconnectionCallbacks = throttle( {trailing: false}, ); +// Only allow one NetInfo.refresh at a time, and respect the interval +// Exported state object to allow unit tests to inspect and control internal state +const recheckNetworkState = { + isCheckPending: false, + lastCheckTimestamp: 0, +}; + +/** + * Refresh NetInfo state. + */ +function recheckNetworkConnection() { + const now = Date.now(); + + if (recheckNetworkState.isCheckPending) { + Log.info('[NetworkConnection] NetInfo.refresh already in progress, skipping new check.'); + return; + } + + if (now - recheckNetworkState.lastCheckTimestamp < CONST.NETWORK.MAX_PENDING_TIME_MS) { + Log.info('[NetworkConnection] NetInfo.refresh called too soon, skipping to respect interval.'); + return; + } + + recheckNetworkState.isCheckPending = true; + recheckNetworkState.lastCheckTimestamp = now; + Log.info('[NetworkConnection] refresh NetInfo.'); + Promise.resolve(NetInfo.refresh()) + .catch((err: unknown) => { + Log.info('[NetworkConnection] NetInfo.refresh failed.', false, String(err)); + }) + .finally(() => { + recheckNetworkState.isCheckPending = false; + Log.info('[NetworkConnection] NetInfo.refresh finished.'); + }); +} + /** * Called when the offline status of the app changes and if the network is "reconnecting" (going from offline to online) * then all of the reconnection callbacks are triggered @@ -311,14 +347,6 @@ function clearReconnectionCallbacks() { } } -/** - * Refresh NetInfo state. - */ -function recheckNetworkConnection() { - Log.info('[NetworkConnection] recheck NetInfo'); - NetInfo.refresh(); -} - export default { clearReconnectionCallbacks, setOfflineStatus, @@ -328,5 +356,6 @@ export default { recheckNetworkConnection, subscribeToNetInfo, getDBTimeWithSkew, + recheckNetworkState, }; export type {NetworkStatus}; diff --git a/tests/unit/NetworkTest.tsx b/tests/unit/NetworkTest.tsx index e383ed196b7b..deb12fce73d9 100644 --- a/tests/unit/NetworkTest.tsx +++ b/tests/unit/NetworkTest.tsx @@ -1,4 +1,6 @@ -import {fireEvent, render, screen} from '@testing-library/react-native'; +import NetInfo from '@react-native-community/netinfo'; +import type {NetInfoState} from '@react-native-community/netinfo'; +import {fireEvent, render, screen, waitFor} from '@testing-library/react-native'; import {sub as dateSubtract} from 'date-fns/sub'; import type {Mock} from 'jest-mock'; import type {OnyxEntry} from 'react-native-onyx'; @@ -66,6 +68,80 @@ afterEach(() => { }); describe('NetworkTests', () => { + test('should not perform a network check if one is already pending', () => { + // Given a check is already in progress (simulating concurrent recheck attempts) + const logInfoSpy = jest.spyOn(Log, 'info'); + const originalIsCheckPending = NetworkConnection.recheckNetworkState.isCheckPending; + NetworkConnection.recheckNetworkState.isCheckPending = true; + + // When another recheck is attempted + NetworkConnection.recheckNetworkConnection(); + + // Then the function should return early without making a new NetInfo call + expect(logInfoSpy).toHaveBeenCalledWith('[NetworkConnection] NetInfo.refresh already in progress, skipping new check.'); + expect(NetworkConnection.recheckNetworkState.isCheckPending).toBe(true); + + // Cleanup + NetworkConnection.recheckNetworkState.isCheckPending = originalIsCheckPending; + logInfoSpy.mockRestore(); + }); + + test('should not perform a network check if called before the minimum interval', () => { + // Given a check was recently performed (to enforce the minimum interval between checks) + const logInfoSpy = jest.spyOn(Log, 'info'); + const now = Date.now(); + const originalIsCheckPending = NetworkConnection.recheckNetworkState.isCheckPending; + const originalLastCheckTimestamp = NetworkConnection.recheckNetworkState.lastCheckTimestamp; + NetworkConnection.recheckNetworkState.isCheckPending = false; + NetworkConnection.recheckNetworkState.lastCheckTimestamp = now; + jest.spyOn(Date, 'now').mockReturnValue(now + CONST.NETWORK.MAX_PENDING_TIME_MS - 1); + + // When another recheck is requested before the minimum interval elapses + NetworkConnection.recheckNetworkConnection(); + + // Then the function should skip the check to respect the interval + expect(logInfoSpy).toHaveBeenCalledWith('[NetworkConnection] NetInfo.refresh called too soon, skipping to respect interval.'); + + // Cleanup + NetworkConnection.recheckNetworkState.isCheckPending = originalIsCheckPending; + NetworkConnection.recheckNetworkState.lastCheckTimestamp = originalLastCheckTimestamp; + (Date.now as jest.Mock).mockRestore(); + logInfoSpy.mockRestore(); + }); + + test('should perform a network check and reset pending state when conditions are met', async () => { + // Given sufficient time has passed since the last check (allowing a new check to proceed) + const logInfoSpy = jest.spyOn(Log, 'info'); + const refreshMock = jest.spyOn(NetInfo, 'refresh').mockResolvedValue(null as unknown as NetInfoState); + const now = Date.now(); + const originalIsCheckPending = NetworkConnection.recheckNetworkState.isCheckPending; + const originalLastCheckTimestamp = NetworkConnection.recheckNetworkState.lastCheckTimestamp; + NetworkConnection.recheckNetworkState.isCheckPending = false; + NetworkConnection.recheckNetworkState.lastCheckTimestamp = now - CONST.NETWORK.MAX_PENDING_TIME_MS - 1; + jest.spyOn(Date, 'now').mockReturnValue(now); + + // When a recheck is triggered + NetworkConnection.recheckNetworkConnection(); + + // Then the network refresh should be initiated and the pending state should be tracked + expect(logInfoSpy).toHaveBeenCalledWith('[NetworkConnection] refresh NetInfo.'); + expect(refreshMock).toHaveBeenCalled(); + expect(NetworkConnection.recheckNetworkState.isCheckPending).toBe(true); + + // And after the refresh completes, the pending state should be cleared + await waitFor(() => { + expect(logInfoSpy).toHaveBeenCalledWith('[NetworkConnection] NetInfo.refresh finished.'); + }); + expect(NetworkConnection.recheckNetworkState.isCheckPending).toBe(false); + + // Cleanup + NetworkConnection.recheckNetworkState.isCheckPending = originalIsCheckPending; + NetworkConnection.recheckNetworkState.lastCheckTimestamp = originalLastCheckTimestamp; + (Date.now as jest.Mock).mockRestore(); + logInfoSpy.mockRestore(); + refreshMock.mockRestore(); + }); + test('failing to reauthenticate should not log out user', () => { // Use fake timers to control timing in the test jest.useFakeTimers();