Simpler offline checks (#1492)

* Fix sign message not returning

* Use shepherd offline state for offline checks

* Use hasCheckedOnline

* Fix tests

* Fix tsc err

common/actions/config/actionCreators.ts

@@ -1,10 +1,15 @@
 import * as interfaces from './actionTypes';
 import { TypeKeys } from './constants';
 
-export type TToggleOffline = typeof toggleOffline;
-export function toggleOffline(): interfaces.ToggleOfflineAction {
+export function setOnline(): interfaces.SetOnlineAction {
   return {
-    type: TypeKeys.CONFIG_TOGGLE_OFFLINE
+    type: TypeKeys.CONFIG_SET_ONLINE
+  };
+}
+
+export function setOffline(): interfaces.SetOfflineAction {
+  return {
+    type: TypeKeys.CONFIG_SET_OFFLINE
   };
 }
 

common/actions/config/actionTypes.ts

@@ -2,9 +2,12 @@ import { TypeKeys } from './constants';
 import { CustomNodeConfig, StaticNodeConfig } from 'types/node';
 import { CustomNetworkConfig } from 'types/network';
 
-/*** Toggle Offline ***/
-export interface ToggleOfflineAction {
-  type: TypeKeys.CONFIG_TOGGLE_OFFLINE;
+export interface SetOnlineAction {
+  type: TypeKeys.CONFIG_SET_ONLINE;
+}
+
+export interface SetOfflineAction {
+  type: TypeKeys.CONFIG_SET_OFFLINE;
 }
 
 export interface ToggleAutoGasLimitAction {
@@ -95,7 +98,8 @@ export type NodeAction =
 
 export type MetaAction =
   | ChangeLanguageAction
-  | ToggleOfflineAction
+  | SetOnlineAction
+  | SetOfflineAction
   | ToggleAutoGasLimitAction
   | PollOfflineStatus
   | SetLatestBlockAction;

common/actions/config/constants.ts

@@ -1,6 +1,9 @@
 export enum TypeKeys {
   CONFIG_LANGUAGE_CHANGE = 'CONFIG_LANGUAGE_CHANGE',
 
+  CONFIG_SET_ONLINE = 'CONFIG_SET_ONLINE',
+  CONFIG_SET_OFFLINE = 'CONFIG_SET_OFFLINE',
+
   CONFIG_TOGGLE_OFFLINE = 'CONFIG_TOGGLE_OFFLINE',
   CONFIG_TOGGLE_AUTO_GAS_LIMIT = 'CONFIG_TOGGLE_AUTO_GAS_LIMIT',
   CONFIG_POLL_OFFLINE_STATUS = 'CONFIG_POLL_OFFLINE_STATUS',

common/reducers/config/meta/meta.ts

@@ -22,10 +22,17 @@ function changeLanguage(state: State, action: ChangeLanguageAction): State {
   };
 }
 
-function toggleOffline(state: State): State {
+function setOnline(state: State): State {
   return {
     ...state,
-    offline: !state.offline
+    offline: false
+  };
+}
+
+function setOffline(state: State): State {
+  return {
+    ...state,
+    offline: true
   };
 }
 
@@ -48,8 +55,11 @@ export function meta(state: State = INITIAL_STATE, action: MetaAction): State {
     case TypeKeys.CONFIG_LANGUAGE_CHANGE:
       return changeLanguage(state, action);
 
-    case TypeKeys.CONFIG_TOGGLE_OFFLINE:
-      return toggleOffline(state);
+    case TypeKeys.CONFIG_SET_ONLINE:
+      return setOnline(state);
+    case TypeKeys.CONFIG_SET_OFFLINE:
+      return setOffline(state);
+
     case TypeKeys.CONFIG_TOGGLE_AUTO_GAS_LIMIT:
       return toggleAutoGasLimitEstimation(state);
 

common/sagas/config/node.ts

@@ -21,7 +21,8 @@ import {
 } from 'selectors/config';
 import { TypeKeys } from 'actions/config/constants';
 import {
-  toggleOffline,
+  setOnline,
+  setOffline,
   changeNode,
   changeNodeIntent,
   setLatestBlock,
@@ -46,52 +47,46 @@ import {
 
 export function* pollOfflineStatus(): SagaIterator {
   let hasCheckedOnline = false;
-  while (true) {
-    const isOffline: boolean = yield select(getOffline);
 
-    // If our offline state disagrees with the browser, run a check
-    // Don't check if the user is in another tab or window
-    const shouldPing = !hasCheckedOnline || navigator.onLine === isOffline;
-    if (shouldPing && !document.hidden) {
-      const pingSucceeded = yield call(getShepherdOffline);
-      if (pingSucceeded && isOffline) {
-        // If we were able to ping but redux says we're offline, mark online
-        yield put(
-          showNotification('success', 'Your connection to the network has been restored!', 3000)
+  const restoreNotif = showNotification(
+    'success',
+    'Your connection to the network has been restored!',
+    3000
   );
-        yield put(toggleOffline());
-      } else if (!pingSucceeded && !isOffline) {
-        // If we were unable to ping but redux says we're online, mark offline
-        // If they had been online, show an error.
-        // If they hadn't been online, just inform them with a warning.
-        if (hasCheckedOnline) {
-          yield put(
-            showNotification(
+  const lostNetworkNotif = showNotification(
     'danger',
     `You’ve lost your connection to the network, check your internet
       connection or try changing networks from the dropdown at the
       top right of the page.`,
     Infinity
-            )
   );
-        } else {
-          yield put(
-            showNotification(
+  const offlineNotif = showNotification(
     'info',
     'You are currently offline. Some features will be unavailable.',
     5000
-            )
   );
-        }
-        yield put(toggleOffline());
+
+  while (true) {
+    yield call(delay, 2500);
+
+    const isOffline: boolean = yield select(getOffline);
+    const balancerOffline = yield call(getShepherdOffline);
+    if (!balancerOffline && isOffline) {
+      // If we were able to ping but redux says we're offline, mark online
+      yield put(restoreNotif);
+      yield put(setOnline());
+    } else if (balancerOffline && !isOffline) {
+      // If we were unable to ping but redux says we're online, mark offline
+      // If they had been online, show an error.
+      // If they hadn't been online, just inform them with a warning.
+      yield put(setOffline());
+      if (hasCheckedOnline) {
+        yield put(lostNetworkNotif);
       } else {
-        // If neither case was true, try again in 5s
-        yield call(delay, 5000);
+        yield put(offlineNotif);
+      }
     }
     hasCheckedOnline = true;
-    } else {
-      yield call(delay, 1000);
-    }
   }
 }
 

spec/reducers/config/config.spec.ts

@@ -3,7 +3,8 @@ import { delay, SagaIterator } from 'redux-saga';
 import { call, cancel, fork, put, take, select, apply } from 'redux-saga/effects';
 import { cloneableGenerator, createMockTask } from 'redux-saga/utils';
 import {
-  toggleOffline,
+  setOffline,
+  setOnline,
   changeNode,
   changeNodeIntent,
   changeNodeForce,
@@ -29,89 +30,63 @@ import { showNotification } from 'actions/notifications';
 import { translateRaw } from 'translations';
 import { StaticNodeConfig } from 'types/node';
 import { staticNodesExpectedState } from './nodes/staticNodes.spec';
-import { metaExpectedState } from './meta/meta.spec';
 import { selectedNodeExpectedState } from './nodes/selectedNode.spec';
 import { customNodesExpectedState, firstCustomNodeId } from './nodes/customNodes.spec';
 import { unsetWeb3Node, unsetWeb3NodeOnWalletEvent } from 'sagas/config/web3';
 import { shepherd } from 'mycrypto-shepherd';
+import { getShepherdOffline } from 'libs/nodes';
 
 // init module
 configuredStore.getState();
 
 describe('pollOfflineStatus*', () => {
-  const { togglingToOffline, togglingToOnline } = metaExpectedState;
-  const nav = navigator as any;
-  const doc = document as any;
-  const data = {} as any;
-  data.gen = cloneableGenerator(pollOfflineStatus)();
-  const node = {
-    lib: {
-      ping: jest.fn()
-    }
-  };
-  const raceSuccess = {
-    pingSucceeded: true,
-    timeout: false
-  };
+  const restoreNotif = 'Your connection to the network has been restored!';
 
-  let originalHidden: any;
-  let originalOnLine: any;
-  let originalRandom: any;
+  const lostNetworkNotif = `You’ve lost your connection to the network, check your internet
+      connection or try changing networks from the dropdown at the
+      top right of the page.`;
 
-  beforeAll(() => {
-    // backup global config
-    originalHidden = document.hidden;
-    originalOnLine = navigator.onLine;
-    originalRandom = Math.random;
+  const offlineNotif = 'You are currently offline. Some features will be unavailable.';
 
-    // mock config
-    Object.defineProperty(document, 'hidden', { value: false, writable: true });
-    Object.defineProperty(navigator, 'onLine', { value: true, writable: true });
-    Math.random = () => 0.001;
+  const offlineOnFirstTimeCase = pollOfflineStatus();
+  it('should delay by 2.5 seconds', () => {
+    expect(offlineOnFirstTimeCase.next().value).toEqual(call(delay, 2500));
   });
 
-  afterAll(() => {
-    // restore global config
-    Object.defineProperty(document, 'hidden', {
-      value: originalHidden,
-      writable: false
-    });
-    Object.defineProperty(navigator, 'onLine', {
-      value: originalOnLine,
-      writable: false
-    });
-    Math.random = originalRandom;
+  it('should select offline', () => {
+    expect(offlineOnFirstTimeCase.next().value).toEqual(select(getOffline));
   });
 
-  it('should select getOffline', () => {
-    expect(data.gen.next(node).value).toEqual(select(getOffline));
+  it('should select shepherd"s offline', () => {
+    expect(offlineOnFirstTimeCase.next(false).value).toEqual(call(getShepherdOffline));
   });
 
-  it('should call delay if document is hidden', () => {
-    data.hiddenDoc = data.gen.clone();
-    doc.hidden = true;
-    data.isOfflineClone = data.gen.clone();
-    data.shouldDelayClone = data.gen.clone();
-    expect(data.hiddenDoc.next(togglingToOnline.offline).value).toEqual(call(delay, 1000));
-
-    doc.hidden = false;
-  });
-
-  it('should toggle offline and show notification if navigator disagrees with isOffline and ping succeeds', () => {
-    data.gen.next(raceSuccess);
-
-    expect(data.gen.next(raceSuccess).value).toEqual(
-      put(showNotification('success', 'Your connection to the network has been restored!', 3000))
+  // .PUT.action.payload.msg is used because the action creator uses an random ID, cant to a showNotif comparision
+  it('should put a different notif if online for the first time ', () => {
+    expect(offlineOnFirstTimeCase.next(true).value).toEqual(put(setOffline()));
+    expect((offlineOnFirstTimeCase.next().value as any).PUT.action.payload.msg).toEqual(
+      offlineNotif
     );
-    expect(data.gen.next().value).toEqual(put(toggleOffline()));
   });
 
-  it('should toggle offline and show notification if navigator agrees with isOffline and ping fails', () => {
-    nav.onLine = togglingToOffline.offline;
+  it('should loop around then go back online, putting a restore msg', () => {
+    expect(offlineOnFirstTimeCase.next().value).toEqual(call(delay, 2500));
+    expect(offlineOnFirstTimeCase.next().value).toEqual(select(getOffline));
+    expect(offlineOnFirstTimeCase.next(true).value).toEqual(call(getShepherdOffline));
+    expect((offlineOnFirstTimeCase.next().value as any).PUT.action.payload.msg).toEqual(
+      restoreNotif
+    );
+    expect(offlineOnFirstTimeCase.next(false).value).toEqual(put(setOnline()));
+  });
 
-    data.isOfflineClone.next(false);
-    data.isOfflineClone.next(false);
-    expect(data.isOfflineClone.next().value).toEqual(put(toggleOffline()));
+  it('should put a generic lost connection notif on every time afterwards', () => {
+    expect(offlineOnFirstTimeCase.next().value).toEqual(call(delay, 2500));
+    expect(offlineOnFirstTimeCase.next().value).toEqual(select(getOffline));
+    expect(offlineOnFirstTimeCase.next(false).value).toEqual(call(getShepherdOffline));
+    expect(offlineOnFirstTimeCase.next(true).value).toEqual(put(setOffline()));
+    expect((offlineOnFirstTimeCase.next().value as any).PUT.action.payload.msg).toEqual(
+      lostNetworkNotif
+    );
   });
 });
 

spec/reducers/config/meta/meta.spec.ts

@@ -1,5 +1,11 @@
 import { meta } from 'reducers/config/meta';
-import { changeLanguage, toggleOffline, toggleAutoGasLimit, setLatestBlock } from 'actions/config';
+import {
+  changeLanguage,
+  setOnline,
+  setOffline,
+  toggleAutoGasLimit,
+  setLatestBlock
+} from 'actions/config';
 
 const expectedInitialState = {
   languageSelection: 'en',
@@ -38,7 +44,8 @@ const expectedState = {
 
 const actions = {
   changeLangauge: changeLanguage('langaugeToChange'),
-  toggleOffline: toggleOffline(),
+  setOnline: setOnline(),
+  setOffline: setOffline(),
   toggleAutoGasLimit: toggleAutoGasLimit(),
   setLatestBlock: setLatestBlock('12345')
 };
@@ -48,12 +55,12 @@ describe('meta reducer', () => {
     expect(meta(undefined, {} as any)).toEqual(expectedState.initialState));
 
   it('should handle toggling to offline', () =>
-    expect(meta(expectedState.initialState, actions.toggleOffline)).toEqual(
+    expect(meta(expectedState.initialState, actions.setOffline)).toEqual(
       expectedState.togglingToOffline
     ));
 
   it('should handle toggling back to online', () =>
-    expect(meta(expectedState.togglingToOffline, actions.toggleOffline)).toEqual(
+    expect(meta(expectedState.togglingToOffline, actions.setOnline)).toEqual(
       expectedState.togglingToOnline
     ));