Skip to content

Commit

Permalink
online state: Switch to isInternetReachable, from connectionType.
Browse files Browse the repository at this point in the history
This is hopefully more accurate about what we actually want to know:
"can the app expect to reach the Internet?" Not just "is there some
network connection?".

Getting this right is already important for a nice user experience.
But it'll be more critical if we adapt our request retry-and-timeout
logic to be aware of the Internet reachability state, which is
reasonable to want to do.

In 6.0.0, `isInternetReachable` started being `boolean | null`; see
the comment about when it's `null`.

Since `null` is a meaningful third state, adjust the Redux plumbing
to allow `null` for `state.session.isOnline` so we can do something
distinct in the UI.
  • Loading branch information
chrisbobbe committed Aug 12, 2021
1 parent 9b5b527 commit 0e7a33c
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 9 deletions.
2 changes: 1 addition & 1 deletion src/actionTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ type RehydrateAction = {|

type AppOnlineAction = {|
type: typeof APP_ONLINE,
isOnline: boolean,
isOnline: boolean | null,
|};

type DeadQueueAction = {|
Expand Down
46 changes: 43 additions & 3 deletions src/boot/AppEventHandlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,49 @@ class AppEventHandlersInner extends PureComponent<Props> {

handleConnectivityChange = netInfoState => {
const { dispatch } = this.props;
const { type: connectionType } = netInfoState;
const isConnected = connectionType !== 'none' && connectionType !== 'unknown';
dispatch(appOnline(isConnected));

dispatch(
appOnline(
// From reading code at @react-native-community/net-info v6.0.0 (the
// docs and types don't really give these answers):
//
// This will be `null` on both platforms while the first known value
// of `true` or `false` is being shipped across the asynchronous RN
// bridge.
//
// On Android, it shouldn't otherwise be `null`. The value is set to the
// result of an Android function that only returns a boolean:
// https://developer.android.com/reference/android/net/NetworkInfo#isConnected()
//
// On iOS, this can also be `null` while the app asynchronously
// evaluates whether a network change should cause this to go from
// `false` to `true`. Read on for details (gathered from
// src/internal/internetReachability.ts in the library).
//
// 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.
// - 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.
// - If the `fetch` succeeds and a 204 is not received, or if the
// fetch fails, or if the timeout expires, this will be made
// `false`. We'll then sleep for only 5s before making the
// request again.
// 2. The request loop is interrupted if we get a
// 'netInfo.networkStatusDidChange' event from the library's
// native code, signaling a change in the network state. If that
// change would make `netInfoState.type` become or remain
// something good (i.e., not 'none' or 'unknown'), and this
// (`.isInternetReachable`) is currently `false`, then this will
// be made `null`, and the request loop described above will
// start again.
//
// (Several of those parameters are configurable -- timeout durations,
// URL, etc.)
netInfoState.isInternetReachable,
),
);
};

/** For the type, see docs: https://reactnative.dev/docs/appstate */
Expand Down
2 changes: 1 addition & 1 deletion src/common/OfflineNotice.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ type Props = $ReadOnly<{||}>;
*/
export default function OfflineNotice(props: Props): Node {
const isOnline = useSelector(state => getSession(state).isOnline);
if (isOnline) {
if (isOnline === true || isOnline === null) {
return null;
}

Expand Down
2 changes: 1 addition & 1 deletion src/directSelectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export const getAccounts = (state: GlobalState): Account[] => state.accounts;

export const getSession = (state: GlobalState): SessionState => state.session;

export const getIsOnline = (state: GlobalState): boolean => state.session.isOnline;
export const getIsOnline = (state: GlobalState): boolean | null => state.session.isOnline;
export const getDebug = (state: GlobalState): Debug => state.session.debug;
export const getIsHydrated = (state: GlobalState): boolean => state.session.isHydrated;

Expand Down
2 changes: 1 addition & 1 deletion src/session/sessionActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
DISMISS_SERVER_COMPAT_NOTICE,
} from '../actionConstants';

export const appOnline = (isOnline: boolean): Action => ({
export const appOnline = (isOnline: boolean | null): Action => ({
type: APP_ONLINE,
isOnline,
});
Expand Down
12 changes: 10 additions & 2 deletions src/session/sessionReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ import { getHasAuth } from '../account/accountsSelectors';
*/
export type SessionState = {|
eventQueueId: number,
isOnline: boolean,

// `null` if we don't know. See the place where we set this, for what that
// means.
isOnline: boolean | null,

isHydrated: boolean,

/**
Expand Down Expand Up @@ -79,7 +83,11 @@ export type SessionState = {|

const initialState: SessionState = {
eventQueueId: -1,
isOnline: true,

// This will be `null` on startup, while we wait to hear `true` or `false`
// from the native module over the RN bridge; so, have it start as `null`.
isOnline: null,

isHydrated: false,
loading: false,
needsInitialFetch: false,
Expand Down

0 comments on commit 0e7a33c

Please sign in to comment.