Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Revert: Backend unreachability message #43888

Merged
merged 1 commit into from
Jun 19, 2024
Merged

Revert: Backend unreachability message #43888

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .storybook/preview.tsx
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -560,10 +560,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 @@ -1059,7 +1057,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 @@ -1071,7 +1069,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
View file
Open in desktop
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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tienifr Could you explain why we need to return unsubscribeNetInfo here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, see that. We need to clear the recheckNetwork interval

}, []);

useEffect(() => {
Expand Down
23 changes: 3 additions & 20 deletions src/components/OfflineIndicator.tsx
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop

This file was deleted.

5 changes: 0 additions & 5 deletions src/libs/checkInternetReachability/index.ts
View file
Open in desktop

This file was deleted.

3 changes: 0 additions & 3 deletions src/libs/checkInternetReachability/types.ts
View file
Open in desktop

This file was deleted.

3 changes: 0 additions & 3 deletions src/types/onyx/Network.ts
View file
Open in desktop
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
Loading