From d0d7bcb0112c99adac9d4b3616150b47eb74cc82 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 15 Aug 2023 13:58:20 -0600 Subject: [PATCH 1/6] Add skeleton methods for pausing/unpausing --- src/libs/Network/SequentialQueue.js | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/libs/Network/SequentialQueue.js b/src/libs/Network/SequentialQueue.js index ac71cec554b..6f6a144dbd7 100644 --- a/src/libs/Network/SequentialQueue.js +++ b/src/libs/Network/SequentialQueue.js @@ -20,6 +20,23 @@ resolveIsReadyPromise(); let isSequentialQueueRunning = false; let currentRequest = null; +let isQueuePaused = false; +function pause() { + if (isQueuePaused) { + return; + } + + isQueuePaused = true; +} + +function unpause() { + if (!isQueuePaused) { + return; + } + + isQueuePaused = false; +} + /** * Process any persisted requests, when online, one at a time until the queue is empty. * @@ -139,4 +156,4 @@ function waitForIdle() { return isReadyPromise; } -export {flush, getCurrentRequest, isRunning, push, waitForIdle}; +export {flush, getCurrentRequest, isRunning, push, waitForIdle, pause, unpause}; From e644798421aa137f6ef8c4f19db921288a9332b3 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 15 Aug 2023 14:05:20 -0600 Subject: [PATCH 2/6] Build out queue paused logic --- src/libs/Network/SequentialQueue.js | 49 +++++++++++++++++++---------- 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/src/libs/Network/SequentialQueue.js b/src/libs/Network/SequentialQueue.js index 6f6a144dbd7..4752590defe 100644 --- a/src/libs/Network/SequentialQueue.js +++ b/src/libs/Network/SequentialQueue.js @@ -19,23 +19,7 @@ resolveIsReadyPromise(); let isSequentialQueueRunning = false; let currentRequest = null; - let isQueuePaused = false; -function pause() { - if (isQueuePaused) { - return; - } - - isQueuePaused = true; -} - -function unpause() { - if (!isQueuePaused) { - return; - } - - isQueuePaused = false; -} /** * Process any persisted requests, when online, one at a time until the queue is empty. @@ -47,6 +31,11 @@ function unpause() { * @returns {Promise} */ function process() { + // When the queue is paused, return early. This prevents any new requests from happening. The queue will be flushed again when the queue is unpaused. + if (isQueuePaused) { + return Promise.resolve(); + } + const persistedRequests = PersistedRequests.getAll(); if (_.isEmpty(persistedRequests) || NetworkStore.isOffline()) { return Promise.resolve(); @@ -74,6 +63,11 @@ function process() { } function flush() { + // When the queue is paused, return early. This will keep an requests in the queue and they will get flushed again when the queue is unpaused + if (isQueuePaused) { + return; + } + if (isSequentialQueueRunning || _.isEmpty(PersistedRequests.getAll())) { return; } @@ -156,4 +150,27 @@ function waitForIdle() { return isReadyPromise; } +/** + * Puts the queue into a paused state so that no requests will be processed + */ +function pause() { + if (isQueuePaused) { + return; + } + + isQueuePaused = true; +} + +/** + * Unpauses the queue and flushes all the requests that were in it or were added to it while paused + */ +function unpause() { + if (!isQueuePaused) { + return; + } + + isQueuePaused = false; + flush(); +} + export {flush, getCurrentRequest, isRunning, push, waitForIdle, pause, unpause}; From 3b150bb2d57a4f6ce3454d323a320662e895d916 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 15 Aug 2023 14:08:59 -0600 Subject: [PATCH 3/6] Add some debugging --- src/libs/Network/SequentialQueue.js | 3 +++ src/libs/actions/App.js | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/libs/Network/SequentialQueue.js b/src/libs/Network/SequentialQueue.js index 4752590defe..23570809db2 100644 --- a/src/libs/Network/SequentialQueue.js +++ b/src/libs/Network/SequentialQueue.js @@ -158,6 +158,7 @@ function pause() { return; } + console.debug('[SequentialQueue] Pausing the queue'); isQueuePaused = true; } @@ -169,6 +170,8 @@ function unpause() { return; } + const numberOfPersistedRequests = PersistedRequests.getAll().length || 0; + console.debug(`[SequentialQueue] Unpausing the queue and flushing ${numberOfPersistedRequests} requests`); isQueuePaused = false; flush(); } diff --git a/src/libs/actions/App.js b/src/libs/actions/App.js index 4556f08eebd..2a824a064eb 100644 --- a/src/libs/actions/App.js +++ b/src/libs/actions/App.js @@ -18,6 +18,7 @@ import * as Session from './Session'; import * as ReportActionsUtils from '../ReportActionsUtils'; import Timing from './Timing'; import * as Browser from '../Browser'; +import * as SequentialQueue from '../Network/SequentialQueue'; let currentUserAccountID; let currentUserEmail; @@ -259,7 +260,8 @@ Onyx.connect({ previousUpdateIDFromServer, lastUpdateIDAppliedToClient, }); - getMissingOnyxUpdates(lastUpdateIDAppliedToClient, lastUpdateIDFromServer); + SequentialQueue.pause(); + getMissingOnyxUpdates(lastUpdateIDAppliedToClient, lastUpdateIDFromServer).then(SequentialQueue.unpause); } if (lastUpdateIDFromServer > lastUpdateIDAppliedToClient) { From 79646c30dc9b3d67dfaa1200203c2f0292aae4d6 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 15 Aug 2023 14:34:59 -0600 Subject: [PATCH 4/6] Return a promise when getting missing onyx updates --- src/libs/actions/App.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libs/actions/App.js b/src/libs/actions/App.js index 2a824a064eb..157f40a47b7 100644 --- a/src/libs/actions/App.js +++ b/src/libs/actions/App.js @@ -212,20 +212,22 @@ function reconnectApp(updateIDFrom = 0) { * Fetches data when the client has discovered it missed some Onyx updates from the server * @param {Number} [updateIDFrom] the ID of the Onyx update that we want to start fetching from * @param {Number} [updateIDTo] the ID of the Onyx update that we want to fetch up to + * @return {Promise} */ function getMissingOnyxUpdates(updateIDFrom = 0, updateIDTo = 0) { console.debug(`[OnyxUpdates] Fetching missing updates updateIDFrom: ${updateIDFrom} and updateIDTo: ${updateIDTo}`); - API.write( + // It is SUPER BAD FORM to return promises from action methods. + // DO NOT FOLLOW THIS PATTERN!!!!! + // It was absolutely necessary in order to block OnyxUpdates while fetching the missing updates from the server or else the udpates aren't applied in the proper order. + // eslint-disable-next-line rulesdir/no-api-side-effects-method + return API.makeRequestWithSideEffects( 'GetMissingOnyxMessages', { updateIDFrom, updateIDTo, }, getOnyxDataForOpenOrReconnect(), - - // Set this to true so that the request will be prioritized at the front of the sequential queue - true, ); } From 9dbd78b1b5ac1a101d301a852112e6caf8eabe56 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 15 Aug 2023 14:36:03 -0600 Subject: [PATCH 5/6] Remove code for priority requests --- src/libs/API.js | 5 ++--- src/libs/Network/SequentialQueue.js | 5 ++--- src/libs/actions/PersistedRequests.js | 5 ++--- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/libs/API.js b/src/libs/API.js index d1c88f93b66..9405fb8f3a5 100644 --- a/src/libs/API.js +++ b/src/libs/API.js @@ -36,9 +36,8 @@ Request.use(Middleware.SaveResponseInOnyx); * @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 {Boolean} [prioritizeRequest] Whether or not the request should be prioritized at the front of the queue or placed onto the back of the queue */ -function write(command, apiCommandParameters = {}, onyxData = {}, prioritizeRequest = false) { +function write(command, apiCommandParameters = {}, onyxData = {}) { Log.info('Called API write', false, {command, ...apiCommandParameters}); // Optimistically update Onyx @@ -71,7 +70,7 @@ function write(command, apiCommandParameters = {}, onyxData = {}, prioritizeRequ }; // Write commands can be saved and retried, so push it to the SequentialQueue - SequentialQueue.push(request, prioritizeRequest); + SequentialQueue.push(request); } /** diff --git a/src/libs/Network/SequentialQueue.js b/src/libs/Network/SequentialQueue.js index 23570809db2..f8ea396663a 100644 --- a/src/libs/Network/SequentialQueue.js +++ b/src/libs/Network/SequentialQueue.js @@ -112,11 +112,10 @@ NetworkStore.onReconnection(flush); /** * @param {Object} request - * @param {Boolean} [front] whether or not the request should be placed in the front of the queue */ -function push(request, front = false) { +function push(request) { // Add request to Persisted Requests so that it can be retried if it fails - PersistedRequests.save([request], front); + PersistedRequests.save([request]); // If we are offline we don't need to trigger the queue to empty as it will happen when we come back online if (NetworkStore.isOffline()) { diff --git a/src/libs/actions/PersistedRequests.js b/src/libs/actions/PersistedRequests.js index a71d034c83e..be30e6b3c8e 100644 --- a/src/libs/actions/PersistedRequests.js +++ b/src/libs/actions/PersistedRequests.js @@ -15,11 +15,10 @@ function clear() { /** * @param {Array} requestsToPersist - * @param {Boolean} [front] whether or not the request should go in the front of the queue */ -function save(requestsToPersist, front = false) { +function save(requestsToPersist) { if (persistedRequests.length) { - persistedRequests = front ? persistedRequests.unshift(...requestsToPersist) : persistedRequests.concat(requestsToPersist); + persistedRequests = persistedRequests.concat(requestsToPersist); } else { persistedRequests = requestsToPersist; } From b7a862eb9851b6cda351d99f86b628ed9702a39f Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Wed, 16 Aug 2023 14:37:08 -0600 Subject: [PATCH 6/6] Change promise resolution so that queue can't get stuck in a paused state --- src/libs/actions/App.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/App.js b/src/libs/actions/App.js index 157f40a47b7..709ad2133e4 100644 --- a/src/libs/actions/App.js +++ b/src/libs/actions/App.js @@ -263,7 +263,7 @@ Onyx.connect({ lastUpdateIDAppliedToClient, }); SequentialQueue.pause(); - getMissingOnyxUpdates(lastUpdateIDAppliedToClient, lastUpdateIDFromServer).then(SequentialQueue.unpause); + getMissingOnyxUpdates(lastUpdateIDAppliedToClient, lastUpdateIDFromServer).finally(SequentialQueue.unpause); } if (lastUpdateIDFromServer > lastUpdateIDAppliedToClient) {