Network Cleanup: Isolate "persisted " queue from "main" queue

src/ONYXKEYS.js

@@ -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',

src/libs/API.js

@@ -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
@@ -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)
@@ -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;
     }
@@ -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,
@@ -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;
@@ -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

src/libs/Network/NetworkEvents.js

@@ -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,
+};

src/libs/Network/NetworkStore.js

@@ -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}
  */
@@ -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,
 };

src/libs/Network/PersistedRequestsQueue.js

@@ -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,
+};