Merge pull request #11151 from Expensify/marcaaron-reconnectOnReauthe…

…nticate

Fix `OpenApp` failures due to expired authToken

src/CONST.js

@@ -838,6 +838,11 @@ const CONST = {
             },
         },
     },
+    API_REQUEST_TYPE: {
+        READ: 'read',
+        WRITE: 'write',
+        MAKE_REQUEST_WITH_SIDE_EFFECTS: 'makeRequestWithSideEffects',
+    },
 };
 
 export default CONST;

src/libs/API.js

@@ -3,6 +3,7 @@ import Onyx from 'react-native-onyx';
 import * as Request from './Request';
 import * as SequentialQueue from './Network/SequentialQueue';
 import pkg from '../../package.json';
+import CONST from '../CONST';
 
 /**
  * All calls to API.write() will be persisted to disk as JSON with the params, successData, and failureData.
@@ -27,6 +28,7 @@ function write(command, apiCommandParameters = {}, onyxData = {}) {
     const data = {
         ...apiCommandParameters,
         appversion: pkg.version,
+        apiRequestType: CONST.API_REQUEST_TYPE.WRITE,
     };
 
     // Assemble all the request data we'll be storing in the queue
@@ -62,9 +64,11 @@ function write(command, apiCommandParameters = {}, onyxData = {}) {
  * @param {Object} [onyxData.optimisticData] - Onyx instructions that will be passed to Onyx.update() before the request is made.
  * @param {Object} [onyxData.successData] - Onyx instructions that will be passed to Onyx.update() when the response has jsonCode === 200.
  * @param {Object} [onyxData.failureData] - Onyx instructions that will be passed to Onyx.update() when the response has jsonCode !== 200.
+ * @param {String} [apiRequestType] - Can be either 'read', 'write', or 'makeRequestWithSideEffects'. We use this to either return the chained
+ *                                    response back to the caller or to trigger reconnection callbacks when re-authentication is required.
  * @returns {Promise}
  */
-function makeRequestWithSideEffects(command, apiCommandParameters = {}, onyxData = {}) {
+function makeRequestWithSideEffects(command, apiCommandParameters = {}, onyxData = {}, apiRequestType = CONST.API_REQUEST_TYPE.MAKE_REQUEST_WITH_SIDE_EFFECTS) {
     // Optimistically update Onyx
     if (onyxData.optimisticData) {
         Onyx.update(onyxData.optimisticData);
@@ -74,6 +78,7 @@ function makeRequestWithSideEffects(command, apiCommandParameters = {}, onyxData
     const data = {
         ...apiCommandParameters,
         appversion: pkg.version,
+        apiRequestType,
     };
 
     // Assemble all the request data we'll be storing
@@ -100,7 +105,7 @@ function makeRequestWithSideEffects(command, apiCommandParameters = {}, onyxData
  * @param {Object} [onyxData.failureData] - Onyx instructions that will be passed to Onyx.update() when the response has jsonCode !== 200.
  */
 function read(command, apiCommandParameters, onyxData) {
-    makeRequestWithSideEffects(command, apiCommandParameters, onyxData);
+    makeRequestWithSideEffects(command, apiCommandParameters, onyxData, CONST.API_REQUEST_TYPE.READ);
 }
 
 export {

src/libs/Middleware/Reauthentication.js

@@ -7,6 +7,32 @@ import * as Authentication from '../Authentication';
 import * as PersistedRequests from '../actions/PersistedRequests';
 import * as Request from '../Request';
 import Log from '../Log';
+import NetworkConnection from '../NetworkConnection';
+
+// We store a reference to the active authentication request so that we are only ever making one request to authenticate at a time.
+let isAuthenticating = null;
+
+/**
+ * @param {String} commandName
+ * @returns {Promise}
+ */
+function reauthenticate(commandName) {
+    if (isAuthenticating) {
+        return isAuthenticating;
+    }
+
+    isAuthenticating = Authentication.reauthenticate(commandName)
+        .then((response) => {
+            isAuthenticating = null;
+            return response;
+        })
+        .catch((error) => {
+            isAuthenticating = null;
+            throw error;
+        });
+
+    return isAuthenticating;
+}
 
 /**
  * Reauthentication middleware
@@ -35,7 +61,8 @@ function Reauthentication(response, request, isFromSequentialQueue) {
                 // creating and deleting logins. In those cases, they should handle the original response instead
                 // of the new response created by handleExpiredAuthToken.
                 const shouldRetry = lodashGet(request, 'data.shouldRetry');
-                if (!shouldRetry) {
+                const apiRequestType = lodashGet(request, 'data.apiRequestType');
+                if (!shouldRetry && !apiRequestType) {
                     if (isFromSequentialQueue) {
                         return data;
                     }
@@ -46,32 +73,29 @@ function Reauthentication(response, request, isFromSequentialQueue) {
                     return data;
                 }
 
-                // We are already authenticating
-                if (NetworkStore.isAuthenticating()) {
-                    if (isFromSequentialQueue) {
-                        // This should never happen in theory. If we go offline while we are Authenticating or handling a response with a 407 jsonCode then isAuthenticating should be
-                        // set to false. If we do somehow get here, we will log about it since we will never know it happened otherwise and it would be difficult to diagnose.
-                        const message = 'Cannot complete sequential request because we are already authenticating';
-                        Log.alert(`${CONST.ERROR.ENSURE_BUGBOT} ${message}`);
-                        throw new Error(message);
-                    }
-
+                // We are already authenticating and using the DeprecatedAPI so we will replay the request
+                if (!apiRequestType && NetworkStore.isAuthenticating()) {
                     MainQueue.replay(request);
                     return data;
                 }
 
-                return Authentication.reauthenticate(request.commandName)
+                return reauthenticate(request.commandName)
                     .then((authenticateResponse) => {
-                        if (isFromSequentialQueue) {
-                            return Request.processWithMiddleware(request, true);
+                        if (isFromSequentialQueue || apiRequestType === CONST.API_REQUEST_TYPE.MAKE_REQUEST_WITH_SIDE_EFFECTS) {
+                            return Request.processWithMiddleware(request, isFromSequentialQueue);
+                        }
+
+                        if (apiRequestType === CONST.API_REQUEST_TYPE.READ) {
+                            NetworkConnection.triggerReconnectionCallbacks('read request made with expired authToken');
+                            return Promise.resolve();
                         }
 
                         MainQueue.replay(request);
                         return authenticateResponse;
                     })
                     .catch(() => {
-                        if (isFromSequentialQueue) {
-                            throw new Error('Unable to reauthenticate sequential queue request because we failed to reauthenticate');
+                        if (isFromSequentialQueue || apiRequestType) {
+                            throw new Error('Failed to reauthenticate');
                         }
 
                         // If we make it here, then our reauthenticate request could not be made due to a networking issue. The original request can be retried safely.

src/libs/NetworkConnection.js

@@ -14,7 +14,8 @@ let isOffline = false;
 let hasPendingNetworkCheck = false;
 
 // Holds all of the callbacks that need to be triggered when the network reconnects
-const reconnectionCallbacks = [];
+let callbackID = 0;
+const reconnectionCallbacks = {};
 
 /**
  * Loop over all reconnection callbacks and fire each one
@@ -99,9 +100,13 @@ function stopListeningForReconnect() {
  * Register callback to fire when we reconnect
  *
  * @param {Function} callback - must return a Promise
+ * @returns {Function} unsubscribe method
  */
 function onReconnect(callback) {
-    reconnectionCallbacks.push(callback);
+    const currentID = callbackID;
+    callbackID++;
+    reconnectionCallbacks[currentID] = callback;
+    return () => delete reconnectionCallbacks[currentID];
 }
 
 /**