Skip to content

Commit

Permalink
Merge pull request #8312 from Expensify/marcaaron-networkCleanup2
Browse files Browse the repository at this point in the history
Network Cleanup: Isolate "persisted " queue from "main" queue
  • Loading branch information
marcaaron authored Mar 28, 2022
2 parents 406599f + c39f2cf commit 52a7e05
Show file tree
Hide file tree
Showing 14 changed files with 407 additions and 342 deletions.
3 changes: 2 additions & 1 deletion src/ONYXKEYS.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ export default {
// Boolean flag set when workspace is being created
IS_CREATING_WORKSPACE: 'isCreatingWorkspace',

NETWORK_REQUEST_QUEUE: 'networkRequestQueue',
// Note: These are Persisted Requests - not all requests in the main queue as the key name might lead one to believe
PERSISTED_REQUESTS: 'networkRequestQueue',

// What the active route is for our navigator. Global route that determines what views to display.
CURRENT_URL: 'currentURL',
Expand Down
26 changes: 9 additions & 17 deletions src/libs/API.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ import updateSessionAuthTokens from './actions/Session/updateSessionAuthTokens';
import setSessionLoadingAndError from './actions/Session/setSessionLoadingAndError';
import * as NetworkStore from './Network/NetworkStore';
import enhanceParameters from './Network/enhanceParameters';

let isAuthenticating;
import * as NetworkEvents from './Network/NetworkEvents';

/**
* Function used to handle expired auth tokens. It re-authenticates with the API and
Expand All @@ -27,13 +26,12 @@ let isAuthenticating;
function handleExpiredAuthToken(originalCommand, originalParameters, originalType) {
// When the authentication process is running, and more API requests will be requeued and they will
// be performed after authentication is done.
if (isAuthenticating) {
if (NetworkStore.getIsAuthenticating()) {
return Network.post(originalCommand, originalParameters, originalType);
}

// Prevent any more requests from being processed while authentication happens
Network.pauseRequestQueue();
isAuthenticating = true;
NetworkStore.setIsAuthenticating(true);

// eslint-disable-next-line no-use-before-define
return reauthenticate(originalCommand)
Expand All @@ -50,9 +48,9 @@ function handleExpiredAuthToken(originalCommand, originalParameters, originalTyp
));
}

Network.registerLogHandler(() => Log);
NetworkEvents.registerLogHandler(() => Log);

Network.registerRequestHandler((queuedRequest, finalParameters) => {
NetworkEvents.onRequestMade((queuedRequest, finalParameters) => {
if (queuedRequest.command === 'Log') {
return;
}
Expand All @@ -65,11 +63,7 @@ Network.registerRequestHandler((queuedRequest, finalParameters) => {
});
});

Network.registerRequestSkippedHandler((parameters) => {
Log.hmmm('Trying to make a request when Network is not ready', parameters);
});

Network.registerResponseHandler((queuedRequest, response) => {
NetworkEvents.onResponse((queuedRequest, response) => {
if (queuedRequest.command !== 'Log') {
Log.info('Finished API request', false, {
command: queuedRequest.command,
Expand Down Expand Up @@ -114,7 +108,7 @@ Network.registerResponseHandler((queuedRequest, response) => {
queuedRequest.resolve(response);
});

Network.registerErrorHandler((queuedRequest, error) => {
NetworkEvents.onError((queuedRequest, error) => {
if (error.name === CONST.ERROR.REQUEST_CANCELLED) {
Log.info('[API] request canceled', false, queuedRequest);
return;
Expand Down Expand Up @@ -241,14 +235,12 @@ function reauthenticate(command = '') {

// The authentication process is finished so the network can be unpaused to continue
// processing requests
isAuthenticating = false;
Network.unpauseRequestQueue();
NetworkStore.setIsAuthenticating(false);
})

.catch((error) => {
// If authentication fails, then the network can be unpaused
Network.unpauseRequestQueue();
isAuthenticating = false;
NetworkStore.setIsAuthenticating(false);

// When a fetch() fails and the "API is offline" error is thrown we won't log the user out. Most likely they
// have a spotty connection and will need to try to reauthenticate when they come back online. We will
Expand Down
34 changes: 34 additions & 0 deletions src/libs/Network/NetworkEvents.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import CONST from '../../CONST';
import createCallback from '../createCallback';

const [getLogger, registerLogHandler] = createCallback();
const [triggerConnectivityResumed, onConnectivityResumed] = createCallback();
const [triggerRequestMade, onRequestMade] = createCallback();
const [triggerResponse, onResponse] = createCallback();
const [triggerError, onError] = createCallback();
const [triggerRecheckNeeded, onRecheckNeeded] = createCallback();

/**
* @returns {Function} cancel timer
*/
function startRecheckTimeoutTimer() {
// If request is still in processing after this time, we might be offline
const timerID = setTimeout(triggerRecheckNeeded, CONST.NETWORK.MAX_PENDING_TIME_MS);
return () => clearTimeout(timerID);
}

export {
registerLogHandler,
getLogger,
triggerConnectivityResumed,
onConnectivityResumed,
triggerRequestMade,
onRequestMade,
onResponse,
triggerResponse,
onError,
triggerError,
triggerRecheckNeeded,
onRecheckNeeded,
startRecheckTimeoutTimer,
};
69 changes: 57 additions & 12 deletions src/libs/Network/NetworkStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,48 +2,76 @@ import lodashGet from 'lodash/get';
import Onyx from 'react-native-onyx';
import _ from 'underscore';
import ONYXKEYS from '../../ONYXKEYS';
import * as NetworkEvents from './NetworkEvents';

let credentials;
let authToken;
let currentUserEmail;
let networkReady = false;
let hasReadRequiredData = false;
let isAuthenticating = false;
let isOffline = false;

/**
* @param {Boolean} ready
* @param {Boolean} val
*/
function setIsReady(ready) {
networkReady = ready;
function setHasReadRequiredDataFromStorage(val) {
hasReadRequiredData = val;
}

/**
* This is a hack to workaround the fact that Onyx may not yet have read these values from storage by the time Network starts processing requests.
* If the values are undefined we haven't read them yet. If they are null or have a value then we have and the network is "ready".
*/
function checkRequiredDataAndSetNetworkReady() {
function checkRequiredData() {
if (_.isUndefined(authToken) || _.isUndefined(credentials)) {
return;
}

setIsReady(true);
setHasReadRequiredDataFromStorage(true);
}

Onyx.connect({
key: ONYXKEYS.SESSION,
callback: (val) => {
authToken = lodashGet(val, 'authToken', null);
currentUserEmail = lodashGet(val, 'email', null);
checkRequiredDataAndSetNetworkReady();
checkRequiredData();
},
});

Onyx.connect({
key: ONYXKEYS.CREDENTIALS,
callback: (val) => {
credentials = val || null;
checkRequiredDataAndSetNetworkReady();
checkRequiredData();
},
});

// We subscribe to the online/offline status of the network to determine when we should fire off API calls
// vs queueing them for later.
Onyx.connect({
key: ONYXKEYS.NETWORK,
callback: (network) => {
if (!network) {
return;
}

// Client becomes online emit connectivity resumed event
if (isOffline && !network.isOffline) {
NetworkEvents.triggerConnectivityResumed();
}

isOffline = network.isOffline;
},
});

/**
* @returns {Boolean}
*/
function getIsOffline() {
return isOffline;
}

/**
* @returns {String}
*/
Expand Down Expand Up @@ -75,15 +103,32 @@ function getCurrentUserEmail() {
/**
* @returns {Boolean}
*/
function isReady() {
return networkReady;
function hasReadRequiredDataFromStorage() {
return hasReadRequiredData;
}

/**
* @param {Boolean} value
*/
function setIsAuthenticating(value) {
isAuthenticating = value;
}

/**
* @returns {Boolean}
*/
function getIsAuthenticating() {
return isAuthenticating;
}

export {
getAuthToken,
setAuthToken,
getCredentials,
getCurrentUserEmail,
isReady,
setIsReady,
hasReadRequiredDataFromStorage,
setHasReadRequiredDataFromStorage,
setIsAuthenticating,
getIsAuthenticating,
getIsOffline,
};
96 changes: 96 additions & 0 deletions src/libs/Network/PersistedRequestsQueue.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import _ from 'underscore';
import Onyx from 'react-native-onyx';
import * as PersistedRequests from '../actions/PersistedRequests';
import * as NetworkStore from './NetworkStore';
import * as NetworkEvents from './NetworkEvents';
import CONST from '../../CONST';
import ONYXKEYS from '../../ONYXKEYS';
import * as ActiveClientManager from '../ActiveClientManager';
import processRequest from './processRequest';

let isPersistedRequestsQueueRunning = false;

/**
* This method will get any persisted requests and fire them off in parallel to retry them.
* If we get any jsonCode besides 407 the request is a success. It doesn't make sense to
* continually retry things that have returned a response. However, we can retry any requests
* with known networking errors like "Failed to fetch".
*
* @returns {Promise}
*/
function process() {
const persistedRequests = PersistedRequests.getAll();

// This sanity check is also a recursion exit point
if (NetworkStore.getIsOffline() || _.isEmpty(persistedRequests)) {
return Promise.resolve();
}

const tasks = _.map(persistedRequests, request => processRequest(request)
.then((response) => {
if (response.jsonCode === CONST.JSON_CODE.NOT_AUTHENTICATED) {
NetworkEvents.getLogger().info('Persisted optimistic request needs authentication');
} else {
NetworkEvents.getLogger().info('Persisted optimistic request returned a valid jsonCode. Not retrying.');
}
NetworkEvents.triggerResponse(request, response);
PersistedRequests.remove(request);
})
.catch((error) => {
// Make this request if we are catching a known network error like "Failed to fetch" and have retries left
if (error.message === CONST.ERROR.FAILED_TO_FETCH) {
const retryCount = PersistedRequests.incrementRetries(request);
NetworkEvents.getLogger().info('Persisted request failed', false, {retryCount, command: request.command, error: error.message});
if (retryCount >= CONST.NETWORK.MAX_REQUEST_RETRIES) {
NetworkEvents.getLogger().info('Request failed too many times, removing from storage', false, {retryCount, command: request.command, error: error.message});
PersistedRequests.remove(request);
}
} else if (error.name === CONST.ERROR.REQUEST_CANCELLED) {
NetworkEvents.getLogger().info('Persisted request was cancelled. Not retrying.');
NetworkEvents.triggerError(request);
PersistedRequests.remove(request);
} else {
NetworkEvents.getLogger().alert(`${CONST.ERROR.ENSURE_BUGBOT} unknown error while retrying persisted request. Not retrying.`, {
command: request.command,
error: error.message,
});
PersistedRequests.remove(request);
}
}));

// Do a recursive call in case the queue is not empty after processing the current batch
return Promise.all(tasks)
.then(process);
}

function flush() {
if (isPersistedRequestsQueueRunning) {
return;
}

// ONYXKEYS.PERSISTED_REQUESTS is shared across clients, thus every client/tab will have a copy
// It is very important to only process the queue from leader client otherwise requests will be duplicated.
if (!ActiveClientManager.isClientTheLeader()) {
return;
}

isPersistedRequestsQueueRunning = true;

// Ensure persistedRequests are read from storage before proceeding with the queue
const connectionID = Onyx.connect({
key: ONYXKEYS.PERSISTED_REQUESTS,
callback: () => {
Onyx.disconnect(connectionID);
process()
.finally(() => isPersistedRequestsQueueRunning = false);
},
});
}

// Flush the queue when the connection resumes
NetworkEvents.onConnectivityResumed(flush);

export {
// eslint-disable-next-line import/prefer-default-export
flush,
};
Loading

0 comments on commit 52a7e05

Please sign in to comment.