diff --git a/src/CONST.js b/src/CONST.js index fcb7ac97db9b..cc0a92471183 100755 --- a/src/CONST.js +++ b/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}, diff --git a/src/libs/API.js b/src/libs/API.js index b44fdd345bba..9405fb8f3a51 100644 --- a/src/libs/API.js +++ b/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'), }; diff --git a/src/libs/Authentication.js b/src/libs/Authentication.js index 7f0b399b6ca3..9f1967ecf0d8 100644 --- a/src/libs/Authentication.js +++ b/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, diff --git a/src/libs/HttpUtils.js b/src/libs/HttpUtils.js index 6a43c7c56d19..5a8185a03038 100644 --- a/src/libs/HttpUtils.js +++ b/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() { diff --git a/src/libs/Log.js b/src/libs/Log.js index 479b6ffb46bf..e51fb74aedd5 100644 --- a/src/libs/Log.js +++ b/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); diff --git a/src/libs/Middleware/Logging.js b/src/libs/Middleware/Logging.js index 1ee49db41f17..fdc9f0083abb 100644 --- a/src/libs/Middleware/Logging.js +++ b/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; } diff --git a/src/libs/Middleware/Reauthentication.js b/src/libs/Middleware/Reauthentication.js index 21d3a8486e4c..dfe4e1b7fda8 100644 --- a/src/libs/Middleware/Reauthentication.js +++ b/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) { diff --git a/src/libs/Network/MainQueue.js b/src/libs/Network/MainQueue.js index 4f028f33de4a..5b5b928d3284 100644 --- a/src/libs/Network/MainQueue.js +++ b/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); } /** diff --git a/src/libs/Network/index.js b/src/libs/Network/index.js index a08d33e0fe77..2f5dc9460e60 100644 --- a/src/libs/Network/index.js +++ b/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, }; diff --git a/src/libs/actions/App.js b/src/libs/actions/App.js index 356a37970926..ca03380368c2 100644 --- a/src/libs/actions/App.js +++ b/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); }); } diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 9dedfc3c5950..216d71b3389e 100644 --- a/src/libs/actions/Report.js +++ b/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))) { diff --git a/src/libs/actions/Session/index.js b/src/libs/actions/Session/index.js index 34a85f3c350f..2cb76342f43a 100644 --- a/src/libs/actions/Session/index.js +++ b/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) => {