-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature: Display backend unreachability message #38377
Changes from 32 commits
37cf997
4b24bd9
7e8cf7f
c7b627d
3da4432
f3be100
6fa6d52
d3faf92
8f84a24
aae3f37
eb5094d
4c84f89
0b5dec8
b7be2f2
292656e
ddc17c8
4967911
4213ccf
5c86046
5de949b
3f1cf05
2f8e345
bffd5f0
01aafb0
c851a49
045047f
17b6aad
3103489
ab835c9
3122072
17a5965
fb78adc
ce38e99
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ import CONST from '@src/CONST'; | |
import ONYXKEYS from '@src/ONYXKEYS'; | ||
import * as NetworkActions from './actions/Network'; | ||
import AppStateMonitor from './AppStateMonitor'; | ||
import checkInternetReachability from './checkInternetReachability'; | ||
import Log from './Log'; | ||
|
||
let isOffline = false; | ||
|
@@ -42,12 +43,23 @@ function setOfflineStatus(isCurrentlyOffline: boolean): void { | |
// When reconnecting, ie, going from offline to online, all the reconnection callbacks | ||
// are triggered (this is usually Actions that need to re-download data from the server) | ||
if (isOffline && !isCurrentlyOffline) { | ||
NetworkActions.setIsBackendReachable(true); | ||
triggerReconnectionCallbacks('offline status changed'); | ||
} | ||
|
||
isOffline = isCurrentlyOffline; | ||
} | ||
|
||
function setNetWorkStatus(isInternetReachable: boolean | null): void { | ||
let networkStatus; | ||
if (!isBoolean(isInternetReachable)) { | ||
networkStatus = CONST.NETWORK.NETWORK_STATUS.UNKNOWN; | ||
} else { | ||
networkStatus = isInternetReachable ? CONST.NETWORK.NETWORK_STATUS.ONLINE : CONST.NETWORK.NETWORK_STATUS.OFFLINE; | ||
} | ||
NetworkActions.setNetWorkStatus(networkStatus); | ||
} | ||
|
||
// Update the offline status in response to changes in shouldForceOffline | ||
let shouldForceOffline = false; | ||
Onyx.connect({ | ||
|
@@ -71,53 +83,81 @@ Onyx.connect({ | |
}); | ||
|
||
/** | ||
* Set up the event listener for NetInfo to tell whether the user has | ||
* internet connectivity or not. This is more reliable than the Pusher | ||
* `disconnected` event which takes about 10-15 seconds to emit. | ||
* Set interval to periodically (re)check backend status. | ||
* Because backend unreachability might imply lost internet connection, we need to check internet reachability. | ||
* @returns clearInterval cleanup | ||
*/ | ||
function subscribeToNetInfo(): void { | ||
// Note: We are disabling the configuration for NetInfo when using the local web API since requests can get stuck in a 'Pending' state and are not reliable indicators for "offline". | ||
// If you need to test the "recheck" feature then switch to the production API proxy server. | ||
if (!CONFIG.IS_USING_LOCAL_WEB) { | ||
// Calling NetInfo.configure (re)checks current state. We use it to force a recheck whenever we (re)subscribe | ||
NetInfo.configure({ | ||
// By default, NetInfo uses `/` for `reachabilityUrl` | ||
// When App is served locally (or from Electron) this address is always reachable - even offline | ||
// Using the API url ensures reachability is tested over internet | ||
reachabilityUrl: `${CONFIG.EXPENSIFY.DEFAULT_API_ROOT}api/Ping`, | ||
reachabilityMethod: 'GET', | ||
reachabilityTest: (response) => { | ||
function subscribeToBackendAndInternetReachability(): () => void { | ||
const intervalID = setInterval(() => { | ||
// Offline status also implies backend unreachability | ||
if (isOffline) { | ||
return; | ||
} | ||
// Using the API url ensures reachability is tested over internet | ||
fetch(`${CONFIG.EXPENSIFY.DEFAULT_API_ROOT}api/Ping`, { | ||
method: 'GET', | ||
cache: 'no-cache', | ||
}) | ||
.then((response) => { | ||
if (!response.ok) { | ||
return Promise.resolve(false); | ||
} | ||
return response | ||
.json() | ||
.then((json) => Promise.resolve(json.jsonCode === 200)) | ||
.catch(() => Promise.resolve(false)); | ||
}, | ||
}) | ||
.then((isBackendReachable: boolean) => { | ||
if (isBackendReachable) { | ||
NetworkActions.setIsBackendReachable(true); | ||
return; | ||
} | ||
checkInternetReachability().then((isInternetReachable: boolean) => { | ||
setOfflineStatus(!isInternetReachable); | ||
setNetWorkStatus(isInternetReachable); | ||
NetworkActions.setIsBackendReachable(false); | ||
}); | ||
}) | ||
.catch(() => { | ||
checkInternetReachability().then((isInternetReachable: boolean) => { | ||
setOfflineStatus(!isInternetReachable); | ||
setNetWorkStatus(isInternetReachable); | ||
NetworkActions.setIsBackendReachable(false); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this inside the promise for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You meant the Backend unreachability might mean internet unreachability so we need to check the internet first to clarify whether the root cause is internet or backend failure. If we move the |
||
}); | ||
}); | ||
}, CONST.NETWORK.BACKEND_CHECK_INTERVAL_MS); | ||
|
||
return () => { | ||
clearInterval(intervalID); | ||
}; | ||
} | ||
|
||
// If a check is taking longer than this time we're considered offline | ||
reachabilityRequestTimeout: CONST.NETWORK.MAX_PENDING_TIME_MS, | ||
}); | ||
} | ||
/** | ||
* Monitor internet connectivity and perform periodic backend reachability checks | ||
* @returns unsubscribe method | ||
*/ | ||
function subscribeToNetworkStatus(): () => void { | ||
// Note: We are disabling the reachability check when using the local web API since requests can get stuck in a 'Pending' state and are not reliable indicators for reachability. | ||
// If you need to test the "recheck" feature then switch to the production API proxy server. | ||
const unsubscribeFromBackendReachability = !CONFIG.IS_USING_LOCAL_WEB ? subscribeToBackendAndInternetReachability() : undefined; | ||
|
||
// Subscribe to the state change event via NetInfo so we can update | ||
// whether a user has internet connectivity or not. | ||
NetInfo.addEventListener((state) => { | ||
// Set up the event listener for NetInfo to tell whether the user has | ||
// internet connectivity or not. This is more reliable than the Pusher | ||
// `disconnected` event which takes about 10-15 seconds to emit. | ||
const unsubscribeNetInfo = NetInfo.addEventListener((state) => { | ||
tienifr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Log.info('[NetworkConnection] NetInfo state change', false, {...state}); | ||
if (shouldForceOffline) { | ||
Log.info('[NetworkConnection] Not setting offline status because shouldForceOffline = true'); | ||
return; | ||
} | ||
setOfflineStatus((state.isInternetReachable ?? false) === false); | ||
let networkStatus; | ||
if (!isBoolean(state.isInternetReachable)) { | ||
networkStatus = CONST.NETWORK.NETWORK_STATUS.UNKNOWN; | ||
} else { | ||
networkStatus = state.isInternetReachable ? CONST.NETWORK.NETWORK_STATUS.ONLINE : CONST.NETWORK.NETWORK_STATUS.OFFLINE; | ||
} | ||
NetworkActions.setNetWorkStatus(networkStatus); | ||
setNetWorkStatus(state.isInternetReachable); | ||
}); | ||
|
||
return () => { | ||
unsubscribeFromBackendReachability?.(); | ||
unsubscribeNetInfo(); | ||
}; | ||
} | ||
|
||
function listenForReconnect() { | ||
|
@@ -166,6 +206,6 @@ export default { | |
onReconnect, | ||
triggerReconnectionCallbacks, | ||
recheckNetworkConnection, | ||
subscribeToNetInfo, | ||
subscribeToNetworkStatus, | ||
}; | ||
export type {NetworkStatus}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
import CONST from '@src/CONST'; | ||
import type InternetReachabilityCheck from './types'; | ||
|
||
/** | ||
* Although Android supports internet reachability check, it only does on initiating the connection. | ||
* We need to implement a test for a highly-available endpoint in case of lost internet after initiation. | ||
*/ | ||
export default function checkInternetReachability(): InternetReachabilityCheck { | ||
return fetch(CONST.GOOGLE_CLOUD_URL, { | ||
method: 'GET', | ||
cache: 'no-cache', | ||
}) | ||
.then((response) => Promise.resolve(response.status === 204)) | ||
.catch(() => Promise.resolve(false)); | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,5 @@ | ||||||||||||
import type InternetReachabilityCheck from './types'; | ||||||||||||
|
||||||||||||
export default function checkInternetReachability(): InternetReachabilityCheck { | ||||||||||||
return Promise.resolve(true); | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this a no-op for all other platforms? Shouldn't this at least be doing a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We only need to manually check for internet reachability on Android. This is due to a limitation on Android OS: App/src/libs/checkInternetReachability/index.android.ts Lines 4 to 7 in e9dc2ef
Other platforms does not have that problem so we use App/src/libs/NetworkConnection.ts Line 147 in e9dc2ef
|
||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
type InternetReachabilityCheck = Promise<boolean>; | ||
|
||
export default InternetReachabilityCheck; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tienifr We implemented
subscribeToNetworkStatus
to detect the onl/off status by usingNetInfo.addEventListener
. Why do we need to callsetOfflineStatus
here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DylanDylann The reason for this check is here: #38377 (comment). Please raise questions if any.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that you might wonder whether
react-native-netinfo
's event listener collided with our own customcheckInternetReachability
. Note that we only run this check on Android and we do not trigger it if the system is already offline here.