Skip to content

Commit

Permalink
OfflineNotice: Show an alert if we stay in an unknown state too long.
Browse files Browse the repository at this point in the history
And log what we know from NetInfo to Sentry.
  • Loading branch information
chrisbobbe authored and gnprice committed Aug 27, 2021
1 parent 5a5b0de commit 88b0a71
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 11 deletions.
9 changes: 8 additions & 1 deletion src/boot/AppEventHandlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ class AppEventHandlersInner extends PureComponent<Props> {
//
// 1. A request loop is started. A HEAD request is made to
// https://clients3.google.com/generate_204, with a timeout of
// 15s, to see if the Internet is reachable.
// 15s (`reachabilityRequestTimeout`), to see if the Internet is
// reachable.
// - If the `fetch` succeeds and a 204 is received, this will be
// made `true`. We'll then sleep for 60s before making the
// request again.
Expand Down Expand Up @@ -130,7 +131,13 @@ class AppEventHandlersInner extends PureComponent<Props> {
handleInitialNotification(dispatch);
handleInitialShare(dispatch);

NetInfo.configure({
// This is the default, as of 6.0.0, but `OfflineNotice` depends on this
// value being stable.
reachabilityRequestTimeout: 15 * 1000,
});
this.netInfoDisconnectCallback = NetInfo.addEventListener(this.handleConnectivityChange);

AppState.addEventListener('change', this.handleAppStateChange);
AppState.addEventListener('memoryWarning', this.handleMemoryWarning);
ScreenOrientation.addOrientationChangeListener(this.handleOrientationChange);
Expand Down
61 changes: 51 additions & 10 deletions src/common/OfflineNotice.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
/* @flow strict-local */

import React from 'react';
import React, { useEffect } from 'react';
import type { Node } from 'react';
import { View } from 'react-native';
import NetInfo from '@react-native-community/netinfo';

import * as logging from '../utils/logging';
import { createStyleSheet, HALF_COLOR } from '../styles';
import { useHasStayedTrueForMs } from '../reactUtils';
import { useSelector } from '../react-redux';
import { getSession } from '../selectors';
import Label from './Label';
Expand All @@ -25,18 +28,56 @@ const styles = createStyleSheet({
type Props = $ReadOnly<{||}>;

/**
* Displays a notice that the app is working in offline mode.
* Not rendered if state is 'online'.
* Shows a notice if the app is working in offline mode.
*
* Shows a different notice if we've taken longer than we expect to
* determine Internet reachability. IOW, if the user sees this, there's a
* bug.
*
* Shows nothing if the Internet is reachable.
*/
export default function OfflineNotice(props: Props): Node {
const isOnline = useSelector(state => getSession(state).isOnline);
if (isOnline === true || isOnline === null) {
return null;
}

return (
<View style={styles.block}>
<Label style={styles.text} text="No Internet connection" />
</View>
const shouldShowUncertaintyNotice = useHasStayedTrueForMs(
// See note in `SessionState` for what this means.
isOnline === null,

// A decently long time, much longer than it takes to send `true` or
// `false` over the RN bridge.
//
// Also, one second longer than what we set for
// `reachabilityRequestTimeout` in NetInfo's config (15s), which is the
// longest `isOnline` can be `null` on iOS in an expected case. For
// details, see the comment where we dispatch the action to change
// `isOnline`.
//
// If this time passes and `isOnline` is still `null`, we should treat
// it as a bug and investigate.
16 * 1000,
);

useEffect(() => {
if (shouldShowUncertaintyNotice) {
NetInfo.fetch().then(state => {
logging.warn('Failed to determine Internet reachability in a reasonable time', state);
});
}
}, [shouldShowUncertaintyNotice]);

if (shouldShowUncertaintyNotice) {
return (
<View style={styles.block}>
<Label style={styles.text} text="Please check your Internet connection" />
</View>
);
} else if (isOnline === false) {
return (
<View style={styles.block}>
<Label style={styles.text} text="No Internet connection" />
</View>
);
} else {
return null;
}
}
1 change: 1 addition & 0 deletions static/translations/messages_en.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
"Mute stream": "Mute stream",
"Unmute stream": "Unmute stream",
"No Internet connection": "No Internet connection",
"Please check your Internet connection": "Please check your Internet connection",
"{realm} is running Zulip Server {serverVersion}, which is unsupported. Please upgrade your server as soon as possible.": "{realm} is running Zulip Server {serverVersion}, which is unsupported. Please upgrade your server as soon as possible.",
"{realm} is running Zulip Server {serverVersion}, which is unsupported. Please contact your administrator about upgrading.": "{realm} is running Zulip Server {serverVersion}, which is unsupported. Please contact your administrator about upgrading.",
"Dismiss": "Dismiss",
Expand Down

0 comments on commit 88b0a71

Please sign in to comment.