Skip to content

Commit

Permalink
Merge pull request #43888 from tienifr/fix/43536
Browse files Browse the repository at this point in the history
Revert: Backend unreachability message
  • Loading branch information
srikarparsi authored Jun 19, 2024
2 parents 0ea9791 + 9c060fb commit 792df1a
Show file tree
Hide file tree
Showing 15 changed files with 80 additions and 165 deletions.
2 changes: 1 addition & 1 deletion .storybook/preview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import './fonts.css';
Onyx.init({
keys: ONYXKEYS,
initialKeyStates: {
[ONYXKEYS.NETWORK]: {isOffline: false, isBackendReachable: true},
[ONYXKEYS.NETWORK]: {isOffline: false},
},
});

Expand Down
6 changes: 2 additions & 4 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -561,10 +561,8 @@ const CONST = {
EMPTY_ARRAY,
EMPTY_OBJECT,
USE_EXPENSIFY_URL,
STATUS_EXPENSIFY_URL: 'https://status.expensify.com',
GOOGLE_MEET_URL_ANDROID: 'https://meet.google.com',
GOOGLE_DOC_IMAGE_LINK_MATCH: 'googleusercontent.com',
GOOGLE_CLOUD_URL: 'https://clients3.google.com/generate_204',
IMAGE_BASE64_MATCH: 'base64',
DEEPLINK_BASE_URL: 'new-expensify://',
PDF_VIEWER_URL: '/pdf/web/viewer.html',
Expand Down Expand Up @@ -1061,7 +1059,7 @@ const CONST = {
MAX_RETRY_WAIT_TIME_MS: 10 * 1000,
PROCESS_REQUEST_DELAY_MS: 1000,
MAX_PENDING_TIME_MS: 10 * 1000,
BACKEND_CHECK_INTERVAL_MS: 60 * 1000,
RECHECK_INTERVAL_MS: 60 * 1000,
MAX_REQUEST_RETRIES: 10,
NETWORK_STATUS: {
ONLINE: 'online',
Expand All @@ -1073,7 +1071,7 @@ const CONST = {
DEFAULT_TIME_ZONE: {automatic: true, selected: 'America/Los_Angeles'},
DEFAULT_ACCOUNT_DATA: {errors: null, success: '', isLoading: false},
DEFAULT_CLOSE_ACCOUNT_DATA: {errors: null, success: '', isLoading: false},
DEFAULT_NETWORK_DATA: {isOffline: false, isBackendReachable: true},
DEFAULT_NETWORK_DATA: {isOffline: false},
FORMS: {
LOGIN_FORM: 'LoginForm',
VALIDATE_CODE_FORM: 'ValidateCodeForm',
Expand Down
6 changes: 3 additions & 3 deletions src/Expensify.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,10 @@ function Expensify({
// Initialize this client as being an active client
ActiveClientManager.init();

// Used for the offline indicator appearing when someone is offline or backend is unreachable
const unsubscribeNetworkStatus = NetworkConnection.subscribeToNetworkStatus();
// Used for the offline indicator appearing when someone is offline
const unsubscribeNetInfo = NetworkConnection.subscribeToNetInfo();

return () => unsubscribeNetworkStatus();
return unsubscribeNetInfo;
}, []);

useEffect(() => {
Expand Down
23 changes: 3 additions & 20 deletions src/components/OfflineIndicator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,9 @@ import useResponsiveLayout from '@hooks/useResponsiveLayout';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import variables from '@styles/variables';
import CONST from '@src/CONST';
import Icon from './Icon';
import * as Expensicons from './Icon/Expensicons';
import Text from './Text';
import TextLink from './TextLink';

type OfflineIndicatorProps = {
/** Optional styles for container element that will override the default styling for the offline indicator */
Expand All @@ -25,7 +23,7 @@ function OfflineIndicator({style, containerStyles}: OfflineIndicatorProps) {
const theme = useTheme();
const styles = useThemeStyles();
const {translate} = useLocalize();
const {isOffline, isBackendReachable} = useNetwork();
const {isOffline} = useNetwork();
const {shouldUseNarrowLayout} = useResponsiveLayout();

const computedStyles = useMemo((): StyleProp<ViewStyle> => {
Expand All @@ -36,7 +34,7 @@ function OfflineIndicator({style, containerStyles}: OfflineIndicatorProps) {
return shouldUseNarrowLayout ? styles.offlineIndicatorMobile : styles.offlineIndicator;
}, [containerStyles, shouldUseNarrowLayout, styles.offlineIndicatorMobile, styles.offlineIndicator]);

if (!isOffline && isBackendReachable) {
if (!isOffline) {
return null;
}

Expand All @@ -48,22 +46,7 @@ function OfflineIndicator({style, containerStyles}: OfflineIndicatorProps) {
width={variables.iconSizeSmall}
height={variables.iconSizeSmall}
/>
<Text style={[styles.ml3, styles.chatItemComposeSecondaryRowSubText]}>
{isOffline ? (
translate('common.youAppearToBeOffline')
) : (
<>
{translate('common.weMightHaveProblem')}
<TextLink
href={CONST.STATUS_EXPENSIFY_URL}
style={[styles.chatItemComposeSecondaryRowSubText, styles.link]}
>
{new URL(CONST.STATUS_EXPENSIFY_URL).host}
</TextLink>
.
</>
)}
</Text>
<Text style={[styles.ml3, styles.chatItemComposeSecondaryRowSubText]}>{translate('common.youAppearToBeOffline')}</Text>
</View>
);
}
Expand Down
9 changes: 4 additions & 5 deletions src/hooks/useNetwork.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,13 @@ type UseNetworkProps = {
onReconnect?: () => void;
};

type UseNetwork = {isOffline: boolean; isBackendReachable: boolean};
type UseNetwork = {isOffline: boolean};

export default function useNetwork({onReconnect = () => {}}: UseNetworkProps = {}): UseNetwork {
const callback = useRef(onReconnect);
callback.current = onReconnect;

const {isOffline, networkStatus, isBackendReachable} = useContext(NetworkContext) ?? {...CONST.DEFAULT_NETWORK_DATA, networkStatus: CONST.NETWORK.NETWORK_STATUS.UNKNOWN};
const isNetworkStatusUnknown = networkStatus === CONST.NETWORK.NETWORK_STATUS.UNKNOWN;
const {isOffline, networkStatus} = useContext(NetworkContext) ?? {...CONST.DEFAULT_NETWORK_DATA, networkStatus: CONST.NETWORK.NETWORK_STATUS.UNKNOWN};
const prevOfflineStatusRef = useRef(isOffline);
useEffect(() => {
// If we were offline before and now we are not offline then we just reconnected
Expand All @@ -30,6 +29,6 @@ export default function useNetwork({onReconnect = () => {}}: UseNetworkProps = {
prevOfflineStatusRef.current = isOffline;
}, [isOffline]);

// If the network status is unknown, we fallback to default state, i.e. we're online and backend is reachable.
return isNetworkStatusUnknown ? CONST.DEFAULT_NETWORK_DATA : {isOffline, isBackendReachable};
// If the network status is undefined, we don't treat it as offline. Otherwise, we utilize the isOffline prop.
return {isOffline: networkStatus === CONST.NETWORK.NETWORK_STATUS.UNKNOWN ? false : isOffline};
}
1 change: 0 additions & 1 deletion src/languages/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,6 @@ export default {
your: 'your',
conciergeHelp: 'Please reach out to Concierge for help.',
youAppearToBeOffline: 'You appear to be offline.',
weMightHaveProblem: 'We might have a problem. Check out ',
thisFeatureRequiresInternet: 'This feature requires an active internet connection to be used.',
attachementWillBeAvailableOnceBackOnline: 'Attachment will become available once back online.',
areYouSure: 'Are you sure?',
Expand Down
1 change: 0 additions & 1 deletion src/languages/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,6 @@ export default {
your: 'tu',
conciergeHelp: 'Por favor, contacta con Concierge para obtener ayuda.',
youAppearToBeOffline: 'Parece que estás desconectado.',
weMightHaveProblem: 'Peude que te tengamos un problema. Echa un vistazo a ',
thisFeatureRequiresInternet: 'Esta función requiere una conexión a Internet activa para ser utilizada.',
attachementWillBeAvailableOnceBackOnline: 'El archivo adjunto estará disponible cuando vuelvas a estar en línea.',
areYouSure: '¿Estás seguro?',
Expand Down
112 changes: 42 additions & 70 deletions src/libs/NetworkConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ 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;
Expand Down Expand Up @@ -43,23 +42,12 @@ function setOfflineStatus(isCurrentlyOffline: boolean, reason = ''): 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, 'moved from offline to online');
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({
Expand Down Expand Up @@ -103,71 +91,39 @@ Onyx.connect({
});

/**
* 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
* 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.
* @returns unsubscribe method
*/
function subscribeToBackendAndInternetReachability(): () => void {
const intervalID = setInterval(() => {
// Offline status also implies backend unreachability
if (isOffline) {
// Periodically recheck the network connection
// More info: https://github.com/Expensify/App/issues/42988
recheckNetworkConnection();
Log.info(`[NetworkStatus] Rechecking the network connection with "isOffline" set to "true" to double-check internet reachability.`);
return;
}
// Using the API url ensures reachability is tested over internet
fetch(`${CONFIG.EXPENSIFY.DEFAULT_API_ROOT}api/Ping?accountID=${accountID || 'unknown'}`, {
method: 'GET',
cache: 'no-cache',
})
.then((response) => {
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?accountID=${accountID || 'unknown'}`,
reachabilityMethod: 'GET',
reachabilityTest: (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, 'successfully completed API request');
return;
}
NetworkActions.setIsBackendReachable(false, 'request succeeded, but internet reachability test failed');
checkInternetReachability().then((isInternetReachable: boolean) => {
setOfflineStatus(!isInternetReachable, 'checkInternetReachability was called after api/ping returned a non-200 jsonCode');
setNetWorkStatus(isInternetReachable);
});
})
.catch(() => {
NetworkActions.setIsBackendReachable(false, 'request failed and internet reachability test failed');
checkInternetReachability().then((isInternetReachable: boolean) => {
setOfflineStatus(!isInternetReachable, 'checkInternetReachability was called after api/ping request failed');
setNetWorkStatus(isInternetReachable);
});
});
}, CONST.NETWORK.BACKEND_CHECK_INTERVAL_MS);

return () => {
clearInterval(intervalID);
};
}
},

/**
* 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;
// If a check is taking longer than this time we're considered offline
reachabilityRequestTimeout: CONST.NETWORK.MAX_PENDING_TIME_MS,
});
}

// 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.
// Subscribe to the state change event via NetInfo so we can update
// whether a user has internet connectivity or not.
const unsubscribeNetInfo = NetInfo.addEventListener((state) => {
Log.info('[NetworkConnection] NetInfo state change', false, {...state});
if (shouldForceOffline) {
Expand All @@ -176,11 +132,27 @@ function subscribeToNetworkStatus(): () => void {
}
setOfflineStatus(state.isInternetReachable === false, 'NetInfo received a state change event');
Log.info(`[NetworkStatus] NetInfo.addEventListener event coming, setting "offlineStatus" to ${!!state.isInternetReachable} with network state: ${JSON.stringify(state)}`);
setNetWorkStatus(state.isInternetReachable);
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);
});

// Periodically recheck the network connection
// More info: https://github.com/Expensify/App/issues/42988
const recheckIntervalID = setInterval(() => {
if (!isOffline) {
return;
}
recheckNetworkConnection();
Log.info(`[NetworkStatus] Rechecking the network connection with "isOffline" set to "true" to double-check internet reachability.`);
}, CONST.NETWORK.RECHECK_INTERVAL_MS);

return () => {
unsubscribeFromBackendReachability?.();
clearInterval(recheckIntervalID);
unsubscribeNetInfo();
};
}
Expand Down Expand Up @@ -231,6 +203,6 @@ export default {
onReconnect,
triggerReconnectionCallbacks,
recheckNetworkConnection,
subscribeToNetworkStatus,
subscribeToNetInfo,
};
export type {NetworkStatus};
11 changes: 1 addition & 10 deletions src/libs/actions/Network.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,6 @@ import Log from '@libs/Log';
import type {NetworkStatus} from '@libs/NetworkConnection';
import ONYXKEYS from '@src/ONYXKEYS';

function setIsBackendReachable(isBackendReachable: boolean, reason: string) {
if (isBackendReachable) {
Log.info(`[Network] Backend is reachable because: ${reason}`);
} else {
Log.info(`[Network] Backend is not reachable because: ${reason}`);
}
Onyx.merge(ONYXKEYS.NETWORK, {isBackendReachable});
}

function setIsOffline(isOffline: boolean, reason = '') {
if (reason) {
let textToLog = '[Network] Client is';
Expand Down Expand Up @@ -41,4 +32,4 @@ function setShouldFailAllRequests(shouldFailAllRequests: boolean) {
Onyx.merge(ONYXKEYS.NETWORK, {shouldFailAllRequests});
}

export {setIsBackendReachable, setIsOffline, setShouldForceOffline, setShouldFailAllRequests, setTimeSkew, setNetWorkStatus};
export {setIsOffline, setShouldForceOffline, setShouldFailAllRequests, setTimeSkew, setNetWorkStatus};
15 changes: 0 additions & 15 deletions src/libs/checkInternetReachability/index.android.ts

This file was deleted.

5 changes: 0 additions & 5 deletions src/libs/checkInternetReachability/index.ts

This file was deleted.

3 changes: 0 additions & 3 deletions src/libs/checkInternetReachability/types.ts

This file was deleted.

3 changes: 0 additions & 3 deletions src/types/onyx/Network.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@ type Network = {
/** Is the network currently offline or not */
isOffline: boolean;

/** Is the backend reachable when online */
isBackendReachable: boolean;

/** Should the network be forced offline */
shouldForceOffline?: boolean;

Expand Down
Loading

0 comments on commit 792df1a

Please sign in to comment.