From 192aa7fc15e2095df6aed025cf0864526ee4a425 Mon Sep 17 00:00:00 2001 From: Daniel Silva Date: Fri, 12 Apr 2024 10:20:11 +0200 Subject: [PATCH 1/7] unpausing the queue --- src/libs/actions/OnyxUpdateManager.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index 40f522e215b55..88276d127aee6 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -46,6 +46,7 @@ export default () => { if (isLoadingApp) { console.debug(`[OnyxUpdateManager] Ignoring Onyx updates while OpenApp hans't finished yet.`); } + SequentialQueue.unpause(); return; } // This key is shared across clients, thus every client/tab will have a copy and try to execute this method. From 16e6a3f677b422c54be8e5a5f52e6cdb3ddb8195 Mon Sep 17 00:00:00 2001 From: Daniel Silva Date: Fri, 12 Apr 2024 17:10:35 +0200 Subject: [PATCH 2/7] adding more comments --- src/libs/actions/OnyxUpdateManager.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index 88276d127aee6..bb6676ac5dc8d 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -41,12 +41,16 @@ export default () => { Onyx.connect({ key: ONYXKEYS.ONYX_UPDATES_FROM_SERVER, callback: (value) => { - // When the OpenApp command hasn't finished yet, we should not process any updates. + // When value is set to null (which we do once we finish processing this method), we can just return early. + // Alternatively, if we're isLoadingApp is positive, that means that OpenApp command hasn't finished yet, + // so we should not process any updates that are being set here. if (!value || isLoadingApp) { if (isLoadingApp) { + // Some of the places that set ONYX_UPDATES_FROM_SERVER also stop the sequential queue. If we reached + // this point, let's unpause it to guarantee that it's still possible to use the app. + SequentialQueue.unpause(); console.debug(`[OnyxUpdateManager] Ignoring Onyx updates while OpenApp hans't finished yet.`); } - SequentialQueue.unpause(); return; } // This key is shared across clients, thus every client/tab will have a copy and try to execute this method. From 15a1fcdd79a70d7950c752ac02bda3687d0211e9 Mon Sep 17 00:00:00 2001 From: Daniel Silva Date: Fri, 12 Apr 2024 17:12:29 +0200 Subject: [PATCH 3/7] prettier --- src/libs/actions/OnyxUpdateManager.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index bb6676ac5dc8d..4a5e1c5ea13b8 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -42,12 +42,12 @@ export default () => { key: ONYXKEYS.ONYX_UPDATES_FROM_SERVER, callback: (value) => { // When value is set to null (which we do once we finish processing this method), we can just return early. - // Alternatively, if we're isLoadingApp is positive, that means that OpenApp command hasn't finished yet, - // so we should not process any updates that are being set here. + // Alternatively, if isLoadingApp is positive, that means that OpenApp command hasn't finished yet, so we + // should not process any updates that are being set. if (!value || isLoadingApp) { if (isLoadingApp) { // Some of the places that set ONYX_UPDATES_FROM_SERVER also stop the sequential queue. If we reached - // this point, let's unpause it to guarantee that it's still possible to use the app. + // this point, let's unpause it to guarantee the app won't be stuck without executing the queue. SequentialQueue.unpause(); console.debug(`[OnyxUpdateManager] Ignoring Onyx updates while OpenApp hans't finished yet.`); } From f9f9a0969637475c270d11d3e6f5a898e3b34fbf Mon Sep 17 00:00:00 2001 From: Daniel Silva Date: Fri, 12 Apr 2024 17:40:06 +0200 Subject: [PATCH 4/7] making comments better --- src/libs/actions/OnyxUpdateManager.ts | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index 4a5e1c5ea13b8..81fc76a76e741 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -41,16 +41,18 @@ export default () => { Onyx.connect({ key: ONYXKEYS.ONYX_UPDATES_FROM_SERVER, callback: (value) => { - // When value is set to null (which we do once we finish processing this method), we can just return early. - // Alternatively, if isLoadingApp is positive, that means that OpenApp command hasn't finished yet, so we - // should not process any updates that are being set. - if (!value || isLoadingApp) { - if (isLoadingApp) { - // Some of the places that set ONYX_UPDATES_FROM_SERVER also stop the sequential queue. If we reached - // this point, let's unpause it to guarantee the app won't be stuck without executing the queue. - SequentialQueue.unpause(); - console.debug(`[OnyxUpdateManager] Ignoring Onyx updates while OpenApp hans't finished yet.`); - } + // When there's no value, there's nothing to process, so let's return early. + if (!value) { + return; + } + // If isLoadingApp is positive it means that OpenApp command hasn't finished yet, and in that case + // we don't have base state of the app (reports, policies, etc) setup. If we apply this update, + // we'll only have them overriten by the openApp response. So let's skip it and return. + if (isLoadingApp) { + // When ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT is set, we pause the queue. Let's unpause + // it so the app is not stuck forever without processing requests. + SequentialQueue.unpause(); + console.debug(`[OnyxUpdateManager] Ignoring Onyx updates while OpenApp hans't finished yet.`); return; } // This key is shared across clients, thus every client/tab will have a copy and try to execute this method. From 8a63405654c746b9cb8c573decd648e5df47c616 Mon Sep 17 00:00:00 2001 From: Daniel Silva Date: Fri, 12 Apr 2024 17:42:36 +0200 Subject: [PATCH 5/7] moving the place where we pause the queue --- src/libs/actions/OnyxUpdates.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libs/actions/OnyxUpdates.ts b/src/libs/actions/OnyxUpdates.ts index e093ce9a964be..2e33d776a3e9d 100644 --- a/src/libs/actions/OnyxUpdates.ts +++ b/src/libs/actions/OnyxUpdates.ts @@ -119,6 +119,9 @@ function apply({lastUpdateID, type, request, response, updates}: OnyxUpdatesFrom * @param [updateParams.updates] Exists if updateParams.type === 'pusher' */ function saveUpdateInformation(updateParams: OnyxUpdatesFromServer) { + // Pause the Sequential queue so we don't do any other requests while we're fetching missing onyxUpdates + SequentialQueue.unpause(); + // Always use set() here so that the updateParams are never merged and always unique to the request that came in Onyx.set(ONYXKEYS.ONYX_UPDATES_FROM_SERVER, updateParams); } @@ -148,9 +151,6 @@ function applyOnyxUpdatesReliably(updates: OnyxUpdatesFromServer) { apply(updates); return; } - - // If we reached this point, we need to pause the queue while we prepare to fetch older OnyxUpdates. - SequentialQueue.pause(); saveUpdateInformation(updates); } From 7b4887656d30434f1f1680a1ce15dd1e97628b16 Mon Sep 17 00:00:00 2001 From: Daniel Silva Date: Fri, 12 Apr 2024 17:42:47 +0200 Subject: [PATCH 6/7] removing unecessary pause inside callback --- src/libs/actions/OnyxUpdateManager.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index 81fc76a76e741..94ba18a4959db 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -87,11 +87,6 @@ export default () => { // fully migrating to the reliable updates mode. // 2. This client already has the reliable updates mode enabled, but it's missing some updates and it // needs to fetch those. - // - // For both of those, we need to pause the sequential queue. This is important so that the updates are - // applied in their correct and specific order. If this queue was not paused, then there would be a lot of - // onyx data being applied while we are fetching the missing updates and that would put them all out of order. - SequentialQueue.pause(); let canUnpauseQueuePromise; // The flow below is setting the promise to a reconnect app to address flow (1) explained above. From 1f3829c346d38f4b3d4d3a21db444af789099d80 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Fri, 12 Apr 2024 10:17:54 -0600 Subject: [PATCH 7/7] Correct the comments --- src/libs/actions/OnyxUpdateManager.ts | 2 +- src/libs/actions/OnyxUpdates.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index 94ba18a4959db..f1f26e259ab1f 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -49,7 +49,7 @@ export default () => { // we don't have base state of the app (reports, policies, etc) setup. If we apply this update, // we'll only have them overriten by the openApp response. So let's skip it and return. if (isLoadingApp) { - // When ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT is set, we pause the queue. Let's unpause + // When ONYX_UPDATES_FROM_SERVER is set, we pause the queue. Let's unpause // it so the app is not stuck forever without processing requests. SequentialQueue.unpause(); console.debug(`[OnyxUpdateManager] Ignoring Onyx updates while OpenApp hans't finished yet.`); diff --git a/src/libs/actions/OnyxUpdates.ts b/src/libs/actions/OnyxUpdates.ts index 2e33d776a3e9d..9f74ce47c9eee 100644 --- a/src/libs/actions/OnyxUpdates.ts +++ b/src/libs/actions/OnyxUpdates.ts @@ -119,7 +119,7 @@ function apply({lastUpdateID, type, request, response, updates}: OnyxUpdatesFrom * @param [updateParams.updates] Exists if updateParams.type === 'pusher' */ function saveUpdateInformation(updateParams: OnyxUpdatesFromServer) { - // Pause the Sequential queue so we don't do any other requests while we're fetching missing onyxUpdates + // Unpause the Sequential queue so we go back to processing requests to guarantee we don't make the app unusable SequentialQueue.unpause(); // Always use set() here so that the updateParams are never merged and always unique to the request that came in