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.

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

[INV-3715] Use Network to determine online status v1 #3722

Closed
wants to merge 8 commits into from
2 changes: 1 addition & 1 deletion app/src/UI/Header/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ const LoginOrOutMemo = React.memo(() => {

const NetworkStateControl: React.FC = () => {
const handleNetworkStateChange = () => {
dispatch(connected ? NetworkActions.offline() : NetworkActions.online());
dispatch(connected ? NetworkActions.offline() : NetworkActions.manualReconnect());
};
const { connected } = useSelector((state) => state.Network);
const dispatch = useDispatch();
Expand Down
6 changes: 6 additions & 0 deletions app/src/constants/alerts/networkAlerts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ const networkAlertMessages: Record<string, AlertMessage> = {
severity: AlertSeverity.Error,
subject: AlertSubjects.Network,
autoClose: 10
},
automaticReconnectFailed: {
content: 'We were unable to bring you back online. Please try again later.',
severity: AlertSeverity.Error,
subject: AlertSubjects.Network,
autoClose: 10
}
};

Expand Down
2 changes: 2 additions & 0 deletions app/src/constants/misc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,5 @@ export const MediumDateTimeFormat = 'MMMM D, YYYY, H:mm a'; //January 5, 2020, 3
export const LongDateFormat = 'dddd, MMMM D, YYYY, H:mm a'; //Monday, January 5, 2020, 3:30 pm

export const LongDateTimeFormat = 'dddd, MMMM D, YYYY, H:mm a'; //Monday, January 5, 2020, 3:30 pm

export const HEALTH_ENDPOINT = '/api/misc/version';
9 changes: 8 additions & 1 deletion app/src/state/actions/network/NetworkActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,15 @@ import { createAction } from '@reduxjs/toolkit';

class NetworkActions {
private static readonly PREFIX = 'NetworkActions';

static readonly online = createAction(`${this.PREFIX}/online`);
static readonly offline = createAction(`${this.PREFIX}/offline`);
static readonly checkMobileNetworkStatus = createAction(
`${this.PREFIX}/checkMobileNetworkStatus`,
(cancel: boolean = false) => ({ payload: cancel })
);
static readonly userLostConnection = createAction(`${this.PREFIX}/userLostConnection`);
static readonly manualReconnect = createAction(`${this.PREFIX}/manualReconnect`);
static readonly automaticReconnectFailed = createAction(`${this.PREFIX}/automaticReconnectFailed`);
static readonly checkInitConnection = createAction(`${this.PREFIX}/checkInitConnection`);
}
export default NetworkActions;
16 changes: 11 additions & 5 deletions app/src/state/reducers/alertsAndPrompts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ const initialState: AlertsAndPromptsState = {
const filterDuplicates = (key: keyof AlertMessage, matchValue: any, state: AlertMessage[]): any[] =>
state.filter((entry) => entry[key] !== matchValue);

const addAlert = (state: AlertMessage[], alert: AlertMessage): AlertMessage[] => [...state, { ...alert, id: nanoid() }];
const addPrompt = (state: PromptAction[], prompt: PromptAction): PromptAction[] => [
...state,
{ ...prompt, id: nanoid() }
];

export function createAlertsAndPromptsReducer(
configuration: AppConfig
): (AlertsAndPromptsState, AnyAction) => AlertsAndPromptsState {
Expand All @@ -43,15 +49,15 @@ export function createAlertsAndPromptsReducer(
draftState.prompts = [];
} else if (RegExp(Prompt.NEW_PROMPT).exec(action.type)) {
const newPrompt: PromptAction = action.payload;
draftState.prompts = [...state.prompts, { ...newPrompt, id: nanoid() }];
draftState.prompts = addPrompt(state.prompts, newPrompt);
} else if (RecordCache.requestCaching.fulfilled.match(action)) {
draftState.alerts = [...state.alerts, { ...cacheAlertMessages.recordsetCacheSuccess, id: nanoid() }];
draftState.alerts = addAlert(state.alerts, cacheAlertMessages.recordsetCacheSuccess);
} else if (RecordCache.requestCaching.rejected.match(action)) {
draftState.alerts = [...state.alerts, { ...cacheAlertMessages.recordsetCacheFailed, id: nanoid() }];
draftState.alerts = addAlert(state.alerts, cacheAlertMessages.recordsetCacheFailed);
} else if (RecordCache.deleteCache.rejected.match(action)) {
draftState.alerts = [...state.alerts, { ...cacheAlertMessages.recordsetDeleteCacheFailed, id: nanoid() }];
draftState.alerts = addAlert(state.alerts, cacheAlertMessages.recordsetDeleteCacheFailed);
} else if (RecordCache.deleteCache.fulfilled.match(action)) {
draftState.alerts = [...state.alerts, { ...cacheAlertMessages.recordsetDeleteCacheSuccess, id: nanoid() }];
draftState.alerts = addAlert(state.alerts, cacheAlertMessages.recordsetDeleteCacheSuccess);
}
});
};
Expand Down
2 changes: 1 addition & 1 deletion app/src/state/reducers/network.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ function createNetworkReducer(initialStatus: Network) {
return createNextState(state, (draftState: Draft<Network>) => {
if (NetworkActions.online.match(action)) {
draftState.connected = true;
} else if (NetworkActions.offline.match(action)) {
} else if (NetworkActions.offline.match(action) || NetworkActions.userLostConnection.match(action)) {
draftState.connected = false;
}
});
Expand Down
129 changes: 127 additions & 2 deletions app/src/state/sagas/network.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,103 @@
import { PayloadAction } from '@reduxjs/toolkit';
import { all, cancelled, delay, put, select, takeEvery, takeLatest } from 'redux-saga/effects';
import networkAlertMessages from 'constants/alerts/networkAlerts';
import { all, put, select, takeEvery } from 'redux-saga/effects';
import { HEALTH_ENDPOINT } from 'constants/misc';
import Alerts from 'state/actions/alerts/Alerts';
import NetworkActions from 'state/actions/network/NetworkActions';
import { MOBILE } from 'state/build-time-config';
import { selectConfiguration } from 'state/reducers/configuration';
import { OfflineActivitySyncState, selectOfflineActivity } from 'state/reducers/offlineActivity';

/**
* @desc Handler for a Manual Reconnect attempt by user
*/
function* handle_MANUAL_RECONNECT() {
const configuration = yield select(selectConfiguration);
if (yield canConnectToNetwork(configuration.API_BASE + HEALTH_ENDPOINT)) {
yield put(NetworkActions.online());
} else {
yield put(Alerts.create(networkAlertMessages.attemptToReconnectFailed));
}
}
function* handle_AUTOMATIC_RECONNECT_FAILED() {
yield put(Alerts.create(networkAlertMessages.automaticReconnectFailed));
}

/**
* @desc Rolling function that targets the API to determine our online status.
* On failure to reach the API, attempts up to 5 times before determining we cannot proceed
* In event of this, disconnect the user and alert them of the incident.
*/
function* handle_CHECK_MOBILE_NETWORK_STATUS(cancel: PayloadAction<boolean>) {
if (!MOBILE || cancel.payload) {
return;
}
const MAX_ATTEMPTS = 5;
const SECONDS_BETWEEN_CHECKS = 20;
const SECONDS_BETWEEN_ATTEMPTS = 5;

while (true) {
const configuration = yield select(selectConfiguration);
let attempts: number = 0;
let canConnect: boolean = false;

do {
canConnect = yield canConnectToNetwork(configuration.API_BASE + HEALTH_ENDPOINT);
if (!canConnect) {
attempts++;
yield delay(SECONDS_BETWEEN_ATTEMPTS * 1000);
}

if (yield cancelled()) {
return;
}
} while (!canConnect && attempts < MAX_ATTEMPTS);

if (!canConnect) {
yield put(NetworkActions.userLostConnection());
return;
}
yield delay(SECONDS_BETWEEN_CHECKS * 1000);
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

a single do-while loop should suffice? You have about 5 attempts going on here with a delay of 5 seconds. You can increase the MAX_ATTEMPTS and remove the outer while loop or increase SECONDS_BETWEEN_ATTEMPTS.

Copy link
Collaborator Author

@LocalNewsTV LocalNewsTV Nov 29, 2024

Choose a reason for hiding this comment

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

@meghna0593
the while(true) loop is always pinging the API to ensure continued connection unless otherwise cancelled (by a user choosing to go offline, or being disconnected and unable to reconnect)

If that do-while fetch request fails and we are UNABLE to reach our API, we don't want to immediately panic, so we send a few more requests at a faster interval to see if anything will come back. If we still can't reach the API we will trigger Offline mode.

While in offline mode, we will sporadically try a few more attempts to come back online.

In the end this is just v1 functionality, it will be succeeded by WebSockets or some similar network equivalent

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Originally this PR was VERY different than it is now.
The previous code you reviewed still had a handle_CHECK_MOBILE_NETWORK_STATUS function, but the logic for it is black and white compared to the logic I explained it in call :)
Sorry for any confusion !

Copy link
Collaborator

Choose a reason for hiding this comment

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

if this was intended to be on an infinite loop when canConnect is true, then do you think this could be blocking in nature? Maybe making this a background task is worth looking at? @plasticviking thoughts?


/**
* @desc Targets the API and checks for successful response.
* @param url Path to API Health check
* @returns Connection to API Succeeded
*/
const canConnectToNetwork = async (url: string): Promise<boolean> => {
return await fetch(url)
.then((res) => res.ok)
.catch(() => false);
};

/**
* Initial Network Connectivity Check, determines to begin application online or offline
*/
function* handle_CHECK_INIT_CONNECTION() {
if (!MOBILE) {
return;
}
const configuration = yield select(selectConfiguration);
if (yield canConnectToNetwork(configuration.API_BASE + HEALTH_ENDPOINT)) {
yield put(NetworkActions.online());
} else {
yield put(NetworkActions.offline());
}
}
/**
* @desc Handler for User manually going offline. Fires a cancellation event to stop rolling api checks.
*/
function* handle_NETWORK_GO_OFFLINE() {
yield put(Alerts.create(networkAlertMessages.userWentOffline));
yield put(NetworkActions.checkMobileNetworkStatus(true));
meghna0593 marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* @desc When user comes online, check for any existing unsychronized Activities.
* Restart the rolling Network status checks.
*/
function* handle_NETWORK_GO_ONLINE() {
const { serializedActivities } = yield select(selectOfflineActivity);
const userHasUnsynchronizedActivities = Object.keys(serializedActivities).some(
Expand All @@ -18,12 +108,47 @@ function* handle_NETWORK_GO_ONLINE() {
} else {
yield put(Alerts.create(networkAlertMessages.userWentOnline));
}
yield put(NetworkActions.checkMobileNetworkStatus());
}

/**
* @desc Attempt to establish connection with the API. Abandons after ~3 minutes of disconnection.
* When this event fires, it cancels the rolling API checks
*/
function* handle_ATTEMPT_AUTOMATIC_RECONNECT() {
const MAX_RECONNECT_ATTEMPTS = 18;
const SECONDS_BETWEEN_ATTEMPTS = 10;

const configuration = yield select(selectConfiguration);
let attempts = 0;
let canReconnect: boolean;

yield put(Alerts.create(networkAlertMessages.userLostConnection));

do {
canReconnect = yield canConnectToNetwork(configuration.API_BASE + HEALTH_ENDPOINT);
if (!canReconnect) {
attempts++;
yield delay(SECONDS_BETWEEN_ATTEMPTS * 1000);
}
} while (!canReconnect && attempts < MAX_RECONNECT_ATTEMPTS);

if (canReconnect) {
yield put(NetworkActions.online());
} else {
yield put(NetworkActions.automaticReconnectFailed());
}
}

function* networkSaga() {
yield all([
takeEvery(NetworkActions.manualReconnect, handle_MANUAL_RECONNECT),
takeEvery(NetworkActions.automaticReconnectFailed, handle_AUTOMATIC_RECONNECT_FAILED),
takeEvery(NetworkActions.checkInitConnection, handle_CHECK_INIT_CONNECTION),
takeLatest(NetworkActions.checkMobileNetworkStatus, handle_CHECK_MOBILE_NETWORK_STATUS),
takeEvery(NetworkActions.offline, handle_NETWORK_GO_OFFLINE),
takeEvery(NetworkActions.online, handle_NETWORK_GO_ONLINE)
takeEvery(NetworkActions.online, handle_NETWORK_GO_ONLINE),
takeLatest(NetworkActions.userLostConnection, handle_ATTEMPT_AUTOMATIC_RECONNECT)
]);
}

Expand Down
4 changes: 3 additions & 1 deletion app/src/state/store.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { configureStore, ThunkDispatch } from '@reduxjs/toolkit';
import { configureStore } from '@reduxjs/toolkit';
import createSagaMiddleware from 'redux-saga';
import { createLogger } from 'redux-logger';
import { createBrowserHistory } from 'history';
Expand All @@ -19,6 +19,7 @@ import userSettingsSaga from './sagas/userSettings';
import { createSagaCrashHandler } from './sagas/error_handler';
import { AppConfig } from './config';
import { DEBUG } from './build-time-config';
import NetworkActions from './actions/network/NetworkActions';

const historySingleton = createBrowserHistory();

Expand Down Expand Up @@ -79,6 +80,7 @@ export function setupStore(configuration: AppConfig) {
sagaMiddleware.run(emailTemplatesSaga);
sagaMiddleware.run(networkSaga);

store.dispatch(NetworkActions.checkInitConnection());
store.dispatch({ type: AUTH_INITIALIZE_REQUEST });

historySingleton.listen((location) => {
Expand Down
Loading