From 243cc401fce6dc1a98b83590f0ea5627437832a8 Mon Sep 17 00:00:00 2001 From: Cosmin Stretea Date: Fri, 12 Dec 2025 10:58:58 +0200 Subject: [PATCH 1/7] Throttle reachability requests to avoid infinite loop of requests --- src/libs/NetworkConnection.ts | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/libs/NetworkConnection.ts b/src/libs/NetworkConnection.ts index ccd7fbf4e8b9..248c25ada9c9 100644 --- a/src/libs/NetworkConnection.ts +++ b/src/libs/NetworkConnection.ts @@ -14,6 +14,12 @@ import AppStateMonitor from './AppStateMonitor'; import DateUtils from './DateUtils'; import Log from './Log'; +// Timestamp of when the last reachability request was completed +let lastReachabilityCheckTimestamp = 0; + +// Minimum interval between reachability checks to avoid spamming requests +const MIN_REACHABILITY_CHECK_INTERVAL = CONST.NETWORK.MAX_PENDING_TIME_MS; + let isOffline = false; type NetworkStatus = ValueOf; @@ -315,8 +321,18 @@ function clearReconnectionCallbacks() { * Refresh NetInfo state. */ function recheckNetworkConnection() { + + // Throttle reachability checks to avoid spamming PING requests + const now = Date.now(); + if (now - lastReachabilityCheckTimestamp < MIN_REACHABILITY_CHECK_INTERVAL) { + Log.info(`[NetworkConnection] Throttled recheck; last check at ${lastReachabilityCheckTimestamp}`); + return; + } Log.info('[NetworkConnection] recheck NetInfo'); - NetInfo.refresh(); + Promise.resolve(NetInfo.refresh()).finally(() => { + lastReachabilityCheckTimestamp = Date.now(); + Log.info(`[NetworkConnection] Reachability (PING) finished at ${lastReachabilityCheckTimestamp}`); + }); } export default { From c4ac99430b300b03f3070420465f3be276585384 Mon Sep 17 00:00:00 2001 From: Cosmin Stretea Date: Fri, 12 Dec 2025 16:27:19 +0200 Subject: [PATCH 2/7] Improvement to use lodash throttle function --- src/libs/NetworkConnection.ts | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/src/libs/NetworkConnection.ts b/src/libs/NetworkConnection.ts index 248c25ada9c9..752e47cdb5fe 100644 --- a/src/libs/NetworkConnection.ts +++ b/src/libs/NetworkConnection.ts @@ -14,12 +14,6 @@ import AppStateMonitor from './AppStateMonitor'; import DateUtils from './DateUtils'; import Log from './Log'; -// Timestamp of when the last reachability request was completed -let lastReachabilityCheckTimestamp = 0; - -// Minimum interval between reachability checks to avoid spamming requests -const MIN_REACHABILITY_CHECK_INTERVAL = CONST.NETWORK.MAX_PENDING_TIME_MS; - let isOffline = false; type NetworkStatus = ValueOf; @@ -319,21 +313,16 @@ function clearReconnectionCallbacks() { /** * Refresh NetInfo state. + * Throttle reachability checks to avoid spamming PING requests */ -function recheckNetworkConnection() { - - // Throttle reachability checks to avoid spamming PING requests - const now = Date.now(); - if (now - lastReachabilityCheckTimestamp < MIN_REACHABILITY_CHECK_INTERVAL) { - Log.info(`[NetworkConnection] Throttled recheck; last check at ${lastReachabilityCheckTimestamp}`); - return; - } - Log.info('[NetworkConnection] recheck NetInfo'); - Promise.resolve(NetInfo.refresh()).finally(() => { - lastReachabilityCheckTimestamp = Date.now(); - Log.info(`[NetworkConnection] Reachability (PING) finished at ${lastReachabilityCheckTimestamp}`); - }); -} +const recheckNetworkConnection = throttle( + () => { + Log.info('[NetworkConnection] refresh NetInfo'); + NetInfo.refresh(); + }, + CONST.NETWORK.MAX_PENDING_TIME_MS, + { leading: true, trailing: false }, +); export default { clearReconnectionCallbacks, From 8a57afa7047b0a56d0a39fb28e5e26964e72f03e Mon Sep 17 00:00:00 2001 From: Cosmin Stretea Date: Mon, 15 Dec 2025 09:55:53 +0200 Subject: [PATCH 3/7] Fix for eslint error --- src/libs/NetworkConnection.ts | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/libs/NetworkConnection.ts b/src/libs/NetworkConnection.ts index 752e47cdb5fe..b338d2663d41 100644 --- a/src/libs/NetworkConnection.ts +++ b/src/libs/NetworkConnection.ts @@ -50,6 +50,19 @@ const triggerReconnectionCallbacks = throttle( {trailing: false}, ); +/** + * Refresh NetInfo state. + * Throttle reachability checks to avoid spamming PING requests + */ +const recheckNetworkConnection = throttle( + () => { + Log.info('[NetworkConnection] refresh NetInfo'); + NetInfo.refresh(); + }, + CONST.NETWORK.MAX_PENDING_TIME_MS, + {leading: true, trailing: false}, +); + /** * 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,19 +324,6 @@ function clearReconnectionCallbacks() { } } -/** - * Refresh NetInfo state. - * Throttle reachability checks to avoid spamming PING requests - */ -const recheckNetworkConnection = throttle( - () => { - Log.info('[NetworkConnection] refresh NetInfo'); - NetInfo.refresh(); - }, - CONST.NETWORK.MAX_PENDING_TIME_MS, - { leading: true, trailing: false }, -); - export default { clearReconnectionCallbacks, setOfflineStatus, From f95dd92e07fd1f4a896a76983a0ff79cf40593ad Mon Sep 17 00:00:00 2001 From: Cosmin Stretea Date: Tue, 16 Dec 2025 14:48:13 +0200 Subject: [PATCH 4/7] Integrate feedback to also check already pending recheck connection run --- src/libs/NetworkConnection.ts | 40 +++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/src/libs/NetworkConnection.ts b/src/libs/NetworkConnection.ts index b338d2663d41..414b03170cbc 100644 --- a/src/libs/NetworkConnection.ts +++ b/src/libs/NetworkConnection.ts @@ -50,18 +50,39 @@ const triggerReconnectionCallbacks = throttle( {trailing: false}, ); + +// Only allow one NetInfo.refresh at a time, and respect the interval +let isCheckPending = false; +let lastCheckTimestamp = 0; + /** * Refresh NetInfo state. - * Throttle reachability checks to avoid spamming PING requests */ -const recheckNetworkConnection = throttle( - () => { - Log.info('[NetworkConnection] refresh NetInfo'); - NetInfo.refresh(); - }, - CONST.NETWORK.MAX_PENDING_TIME_MS, - {leading: true, trailing: false}, -); +function recheckNetworkConnection() { + const now = Date.now(); + + if (isCheckPending) { + Log.info('[NetworkConnection] NetInfo.refresh already in progress, skipping new check.'); + return; + } + + if (now - lastCheckTimestamp < CONST.NETWORK.MAX_PENDING_TIME_MS) { + Log.info('[NetworkConnection] NetInfo.refresh called too soon, skipping to respect interval.'); + return; + } + + isCheckPending = true; + lastCheckTimestamp = now; + Log.info('[NetworkConnection] refresh NetInfo.'); + Promise.resolve(NetInfo.refresh()) + .catch((err) => { + Log.info('[NetworkConnection] NetInfo.refresh failed.', false, err); + }) + .finally(() => { + 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) @@ -325,6 +346,7 @@ function clearReconnectionCallbacks() { } export default { + clearReconnectionCallbacks, setOfflineStatus, listenForReconnect, From f887a1b3f25bbbeb0c8e5450ad0c3a97032fdda0 Mon Sep 17 00:00:00 2001 From: Cosmin Stretea Date: Tue, 16 Dec 2025 15:07:24 +0200 Subject: [PATCH 5/7] Remove unnecessary newline --- src/libs/NetworkConnection.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libs/NetworkConnection.ts b/src/libs/NetworkConnection.ts index 414b03170cbc..0d5c2a451409 100644 --- a/src/libs/NetworkConnection.ts +++ b/src/libs/NetworkConnection.ts @@ -346,7 +346,6 @@ function clearReconnectionCallbacks() { } export default { - clearReconnectionCallbacks, setOfflineStatus, listenForReconnect, From 16914f87d07dc7cb05a8220cc0577d9cefe2facc Mon Sep 17 00:00:00 2001 From: Cosmin Stretea Date: Wed, 17 Dec 2025 14:05:58 +0200 Subject: [PATCH 6/7] Add unit tests for recheck connection an small eslint fix --- src/libs/NetworkConnection.ts | 22 ++++++---- tests/unit/NetworkTest.tsx | 78 ++++++++++++++++++++++++++++++++++- 2 files changed, 90 insertions(+), 10 deletions(-) diff --git a/src/libs/NetworkConnection.ts b/src/libs/NetworkConnection.ts index 0d5c2a451409..84d1080d2da2 100644 --- a/src/libs/NetworkConnection.ts +++ b/src/libs/NetworkConnection.ts @@ -52,8 +52,11 @@ const triggerReconnectionCallbacks = throttle( // Only allow one NetInfo.refresh at a time, and respect the interval -let isCheckPending = false; -let lastCheckTimestamp = 0; +// Exported state object to allow unit tests to inspect and control internal state +const recheckNetworkState = { + isCheckPending: false, + lastCheckTimestamp: 0, +} /** * Refresh NetInfo state. @@ -61,25 +64,25 @@ let lastCheckTimestamp = 0; function recheckNetworkConnection() { const now = Date.now(); - if (isCheckPending) { + if (recheckNetworkState.isCheckPending) { Log.info('[NetworkConnection] NetInfo.refresh already in progress, skipping new check.'); return; } - if (now - lastCheckTimestamp < CONST.NETWORK.MAX_PENDING_TIME_MS) { + if (now - recheckNetworkState.lastCheckTimestamp < CONST.NETWORK.MAX_PENDING_TIME_MS) { Log.info('[NetworkConnection] NetInfo.refresh called too soon, skipping to respect interval.'); return; } - isCheckPending = true; - lastCheckTimestamp = now; + recheckNetworkState.isCheckPending = true; + recheckNetworkState.lastCheckTimestamp = now; Log.info('[NetworkConnection] refresh NetInfo.'); Promise.resolve(NetInfo.refresh()) - .catch((err) => { - Log.info('[NetworkConnection] NetInfo.refresh failed.', false, err); + .catch((err: unknown) => { + Log.info('[NetworkConnection] NetInfo.refresh failed.', false, String(err)); }) .finally(() => { - isCheckPending = false; + recheckNetworkState.isCheckPending = false; Log.info('[NetworkConnection] NetInfo.refresh finished.'); }); } @@ -354,5 +357,6 @@ export default { recheckNetworkConnection, subscribeToNetInfo, getDBTimeWithSkew, + recheckNetworkState }; export type {NetworkStatus}; diff --git a/tests/unit/NetworkTest.tsx b/tests/unit/NetworkTest.tsx index e383ed196b7b..e9fcb35a3c77 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 {fireEvent, render, screen, waitFor} from '@testing-library/react-native'; +import NetInfo from '@react-native-community/netinfo'; +import type {NetInfoState} from '@react-native-community/netinfo'; 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(); From c639305011f04af82a4b710695dd3cf7b1779a8b Mon Sep 17 00:00:00 2001 From: Cosmin Stretea Date: Wed, 17 Dec 2025 14:11:12 +0200 Subject: [PATCH 7/7] Fix prettier issues --- src/libs/NetworkConnection.ts | 5 ++--- tests/unit/NetworkTest.tsx | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/libs/NetworkConnection.ts b/src/libs/NetworkConnection.ts index 84d1080d2da2..8a982556f0b0 100644 --- a/src/libs/NetworkConnection.ts +++ b/src/libs/NetworkConnection.ts @@ -50,13 +50,12 @@ 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. @@ -357,6 +356,6 @@ export default { recheckNetworkConnection, subscribeToNetInfo, getDBTimeWithSkew, - recheckNetworkState + recheckNetworkState, }; export type {NetworkStatus}; diff --git a/tests/unit/NetworkTest.tsx b/tests/unit/NetworkTest.tsx index e9fcb35a3c77..deb12fce73d9 100644 --- a/tests/unit/NetworkTest.tsx +++ b/tests/unit/NetworkTest.tsx @@ -1,6 +1,6 @@ -import {fireEvent, render, screen, waitFor} 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';