Revert "Remove shouldRetry and canCancel now that deprecatedAPI is gone"

src/CONST.js

@@ -748,9 +748,6 @@ const CONST = {
         MAX_RETRY_WAIT_TIME_MS: 10 * 1000,
         PROCESS_REQUEST_DELAY_MS: 1000,
         MAX_PENDING_TIME_MS: 10 * 1000,
-        COMMAND: {
-            LOG: 'Log',
-        },
     },
     DEFAULT_TIME_ZONE: {automatic: true, selected: 'America/Los_Angeles'},
     DEFAULT_ACCOUNT_DATA: {errors: null, success: '', isLoading: false},

src/libs/API.js

@@ -61,6 +61,10 @@ function write(command, apiCommandParameters = {}, onyxData = {}) {
         command,
         data: {
             ...data,
+
+            // This should be removed once we are no longer using deprecatedAPI https://github.com/Expensify/Expensify/issues/215650
+            shouldRetry: true,
+            canCancel: true,
         },
         ..._.omit(onyxData, 'optimisticData'),
     };

src/libs/Authentication.js

@@ -36,6 +36,7 @@ function Authenticate(parameters) {
         partnerUserSecret: parameters.partnerUserSecret,
         twoFactorAuthCode: parameters.twoFactorAuthCode,
         authToken: parameters.authToken,
+        shouldRetry: false,
 
         // Force this request to be made because the network queue is paused when re-authentication is happening
         forceNetworkRequest: true,

src/libs/HttpUtils.js

@@ -29,15 +29,13 @@ let cancellationController = new AbortController();
  * @param {String} url
  * @param {String} [method]
  * @param {Object} [body]
- * @param {String} [command]
+ * @param {Boolean} [canCancel]
  * @returns {Promise}
  */
-function processHTTPRequest(url, method = 'get', body = null, command = '') {
-    const signal = command === CONST.NETWORK.COMMAND.LOG ? undefined : cancellationController.signal;
-
+function processHTTPRequest(url, method = 'get', body = null, canCancel = true) {
     return fetch(url, {
         // We hook requests to the same Controller signal, so we can cancel them all at once
-        signal,
+        signal: canCancel ? cancellationController.signal : undefined,
         method,
         body,
     })
@@ -129,7 +127,7 @@ function xhr(command, data, type = CONST.NETWORK.METHOD.POST, shouldUseSecure =
     });
 
     const url = ApiUtils.getCommandURL({shouldUseSecure, command});
-    return processHTTPRequest(url, type, formData, command);
+    return processHTTPRequest(url, type, formData, data.canCancel);
 }
 
 function cancelPendingRequests() {

src/libs/Log.js

@@ -6,7 +6,6 @@ import getPlatform from './getPlatform';
 import pkg from '../../package.json';
 import requireParameters from './requireParameters';
 import * as Network from './Network';
-import CONST from '../CONST';
 
 let timeout = null;
 
@@ -17,11 +16,12 @@ let timeout = null;
  * @returns {Promise}
  */
 function LogCommand(parameters) {
-    const commandName = CONST.NETWORK.COMMAND.LOG;
+    const commandName = 'Log';
     requireParameters(['logPacket', 'expensifyCashAppVersion'], parameters, commandName);
 
     // Note: We are forcing Log to run since it requires no authToken and should only be queued when we are offline.
-    return Network.post(commandName, {...parameters, forceNetworkRequest: true});
+    // Non-cancellable request: during logout, when requests are cancelled, we don't want to cancel any remaining logs
+    return Network.post(commandName, {...parameters, forceNetworkRequest: true, canCancel: false});
 }
 
 /**
@@ -36,6 +36,7 @@ function LogCommand(parameters) {
 function serverLoggingCallback(logger, params) {
     const requestParams = params;
     requestParams.shouldProcessImmediately = false;
+    requestParams.shouldRetry = false;
     requestParams.expensifyCashAppVersion = `expensifyCash[${getPlatform()}]${pkg.version}`;
     if (requestParams.parameters) {
         requestParams.parameters = JSON.stringify(params.parameters);

src/libs/Middleware/Logging.js

@@ -10,7 +10,7 @@ import CONST from '../../CONST';
  */
 function logRequestDetails(message, request, response = {}) {
     // Don't log about log or else we'd cause an infinite loop
-    if (request.command === CONST.NETWORK.COMMAND.LOG) {
+    if (request.command === 'Log') {
         return;
     }
 
@@ -65,7 +65,7 @@ function Logging(response, request) {
             } else if (error.message === CONST.ERROR.FAILED_TO_FETCH) {
                 // If the command that failed is Log it's possible that the next call to Log may also fail.
                 // This will lead to infinitely complex log params that can eventually crash the app.
-                if (request.command === CONST.NETWORK.COMMAND.LOG) {
+                if (request.command === 'Log') {
                     delete logParams.request;
                 }
 

src/libs/Middleware/Reauthentication.js

@@ -58,18 +58,30 @@ function Reauthentication(response, request, isFromSequentialQueue) {
                 // There are some API requests that should not be retried when there is an auth failure like
                 // creating and deleting logins. In those cases, they should handle the original response instead
                 // of the new response created by handleExpiredAuthToken.
-                // If the request was from the sequential queue we don't want to return, we want to run the reauthentication callback and retry the request
+                const shouldRetry = lodashGet(request, 'data.shouldRetry');
                 const apiRequestType = lodashGet(request, 'data.apiRequestType');
-                const isDeprecatedRequest = !apiRequestType;
+
+                // For the SignInWithShortLivedAuthToken command, if the short token expires, the server returns a 407 error,
+                // and credentials are still empty at this time, which causes reauthenticate to throw an error (requireParameters),
+                // and the subsequent SaveResponseInOnyx also cannot be executed, so we need this parameter to skip the reauthentication logic.
                 const skipReauthentication = lodashGet(request, 'data.skipReauthentication');
-                if (!isFromSequentialQueue && (isDeprecatedRequest || skipReauthentication)) {
+                if ((!shouldRetry && !apiRequestType) || skipReauthentication) {
+                    if (isFromSequentialQueue) {
+                        return data;
+                    }
+
                     if (request.resolve) {
                         request.resolve(data);
-                        return;
                     }
                     return data;
                 }
 
+                // We are already authenticating and using the DeprecatedAPI so we will replay the request
+                if (!apiRequestType && NetworkStore.isAuthenticating()) {
+                    MainQueue.replay(request);
+                    return data;
+                }
+
                 return reauthenticate(request.commandName)
                     .then((authenticateResponse) => {
                         if (isFromSequentialQueue || apiRequestType === CONST.API_REQUEST_TYPE.MAKE_REQUEST_WITH_SIDE_EFFECTS) {

src/libs/Network/MainQueue.js

@@ -1,8 +1,8 @@
 import _ from 'underscore';
+import lodashGet from 'lodash/get';
 import * as NetworkStore from './NetworkStore';
 import * as SequentialQueue from './SequentialQueue';
 import * as Request from '../Request';
-import CONST from '../../CONST';
 
 // Queue for network requests so we don't lose actions done by the user while offline
 let networkRequestQueue = [];
@@ -56,12 +56,18 @@ function process() {
     // Some requests should be retried and will end up here if the following conditions are met:
     // - we are in the process of authenticating and the request is retryable (most are)
     // - the request does not have forceNetworkRequest === true (this will trigger it to process immediately)
+    // - the request does not have shouldRetry === false (specified when we do not want to retry, defaults to true)
     const requestsToProcessOnNextRun = [];
 
     _.each(networkRequestQueue, (queuedRequest) => {
-        // Check if we can make this request at all and if we can't see if we should save it for the next run
+        // Check if we can make this request at all and if we can't see if we should save it for the next run or chuck it into the ether
         if (!canMakeRequest(queuedRequest)) {
-            requestsToProcessOnNextRun.push(queuedRequest);
+            const shouldRetry = lodashGet(queuedRequest, 'data.shouldRetry');
+            if (shouldRetry) {
+                requestsToProcessOnNextRun.push(queuedRequest);
+            } else {
+                console.debug('Skipping request that should not be re-tried: ', {command: queuedRequest.command});
+            }
             return;
         }
 
@@ -78,7 +84,7 @@ function process() {
  * Non-cancellable requests like Log would not be cleared
  */
 function clear() {
-    _.filter(networkRequestQueue, (request) => request.command !== CONST.NETWORK.COMMAND.LOG);
+    networkRequestQueue = _.filter(networkRequestQueue, (request) => !request.data.canCancel);
 }
 
 /**

src/libs/Network/index.js

@@ -35,6 +35,8 @@ function post(command, data = {}, type = CONST.NETWORK.METHOD.POST, shouldUseSec
         // (e.g. any requests currently happening when the user logs out are cancelled)
         request.data = {
             ...data,
+            shouldRetry: lodashGet(data, 'shouldRetry', true),
+            canCancel: lodashGet(data, 'canCancel', true),
             appversion: pkg.version,
         };
 

src/libs/actions/App.js

@@ -371,7 +371,7 @@ function beginDeepLinkRedirect() {
     }
 
     // eslint-disable-next-line rulesdir/no-api-side-effects-method
-    API.makeRequestWithSideEffects('OpenOldDotLink', {}, {}).then((response) => {
+    API.makeRequestWithSideEffects('OpenOldDotLink', {shouldRetry: false}, {}).then((response) => {
         Browser.openRouteInDesktopApp(response.shortLivedAuthToken, currentUserEmail);
     });
 }

src/libs/actions/Report.js

@@ -424,6 +424,10 @@ function openReport(reportID, participantLoginList = [], newReportObject = {}, p
         parentReportActionID,
     };
 
+    if (isFromDeepLink) {
+        params.shouldRetry = false;
+    }
+
     // If we open an exist report, but it is not present in Onyx yet, we should change the method to set for this report
     // and we need data to be available when we navigate to the chat page
     if (_.isEmpty(ReportUtils.getReport(reportID))) {

src/libs/actions/Session/index.js

@@ -71,6 +71,7 @@ function signOut() {
         partnerUserID: lodashGet(credentials, 'autoGeneratedLogin', ''),
         partnerName: CONFIG.EXPENSIFY.PARTNER_NAME,
         partnerPassword: CONFIG.EXPENSIFY.PARTNER_PASSWORD,
+        shouldRetry: false,
     });
     clearCache().then(() => {
         Log.info('Cleared all chache data', true, {}, true);
@@ -289,9 +290,6 @@ function signInWithShortLivedAuthToken(email, authToken) {
     // If the user is signing in with a different account from the current app, should not pass the auto-generated login as it may be tied to the old account.
     // scene 1: the user is transitioning to newDot from a different account on oldDot.
     // scene 2: the user is transitioning to desktop app from a different account on web app.
-    // For the SignInWithShortLivedAuthToken command, if the short token expires, the server returns a 407 error,
-    // and credentials are still empty at this time, which causes reauthenticate to throw an error (requireParameters),
-    // and the subsequent SaveResponseInOnyx also cannot be executed, so we need this parameter to skip the reauthentication logic.
     const oldPartnerUserID = credentials.login === email ? credentials.autoGeneratedLogin : '';
     API.read('SignInWithShortLivedAuthToken', {authToken, oldPartnerUserID, skipReauthentication: true}, {optimisticData, successData, failureData});
 }
@@ -547,6 +545,7 @@ function authenticatePusher(socketID, channelName, callback) {
     API.makeRequestWithSideEffects('AuthenticatePusher', {
         socket_id: socketID,
         channel_name: channelName,
+        shouldRetry: false,
         forceNetworkRequest: true,
     })
         .then((response) => {