Merge pull request #19675 from Expensify/maxence-rracfa

Remove shouldRetry and canCancel now that deprecatedAPI is gone

Author: madmax330

src/CONST.js

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

src/libs/Log.js

@@ -6,6 +6,7 @@ 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;
 
@@ -16,12 +17,11 @@ let timeout = null;
  * @returns {Promise}
  */
 function LogCommand(parameters) {
-    const commandName = 'Log';
+    const commandName = CONST.NETWORK.COMMAND.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.
-    // 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});
+    return Network.post(commandName, {...parameters, forceNetworkRequest: true});
 }
 
 /**
@@ -36,7 +36,6 @@ 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 === 'Log') {
+    if (request.command === CONST.NETWORK.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 === 'Log') {
+                if (request.command === CONST.NETWORK.COMMAND.LOG) {
                     delete logParams.request;
                 }
 

src/libs/Middleware/Reauthentication.js

@@ -58,30 +58,18 @@ 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.
-                const shouldRetry = lodashGet(request, 'data.shouldRetry');
+                // 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 apiRequestType = lodashGet(request, 'data.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 isDeprecatedRequest = !apiRequestType;
                 const skipReauthentication = lodashGet(request, 'data.skipReauthentication');
-                if ((!shouldRetry && !apiRequestType) || skipReauthentication) {
-                    if (isFromSequentialQueue) {
-                        return data;
-                    }
-
+                if (!isFromSequentialQueue && (isDeprecatedRequest || skipReauthentication)) {
                     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,18 +56,12 @@ 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 or chuck it into the ether
+        // Check if we can make this request at all and if we can't see if we should save it for the next run
         if (!canMakeRequest(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});
-            }
+            requestsToProcessOnNextRun.push(queuedRequest);
             return;
         }
 
@@ -84,7 +78,7 @@ function process() {
  * Non-cancellable requests like Log would not be cleared
  */
 function clear() {
-    networkRequestQueue = _.filter(networkRequestQueue, (request) => !request.data.canCancel);
+    _.filter(networkRequestQueue, (request) => request.command !== CONST.NETWORK.COMMAND.LOG);
 }
 
 /**

src/libs/Network/index.js

@@ -35,8 +35,6 @@ 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

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

src/libs/actions/Report.js

@@ -424,10 +424,6 @@ 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,7 +71,6 @@ 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);
@@ -290,6 +289,9 @@ 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});
 }
@@ -545,7 +547,6 @@ function authenticatePusher(socketID, channelName, callback) {
     API.makeRequestWithSideEffects('AuthenticatePusher', {
         socket_id: socketID,
         channel_name: channelName,
-        shouldRetry: false,
         forceNetworkRequest: true,
     })
         .then((response) => {