From 80d634b4a2f10c4063ea7f5002c7eedda210a12b Mon Sep 17 00:00:00 2001 From: Povilas Zirgulis Date: Mon, 18 Nov 2024 22:56:07 +0200 Subject: [PATCH] revert RequestThrottle to be function based --- src/libs/Middleware/Reauthentication.ts | 7 +-- src/libs/Network/SequentialQueue.ts | 12 ++--- src/libs/RequestThrottle.ts | 65 ++++++++++++------------- tests/unit/APITest.ts | 9 ++-- 4 files changed, 42 insertions(+), 51 deletions(-) diff --git a/src/libs/Middleware/Reauthentication.ts b/src/libs/Middleware/Reauthentication.ts index 859dfa01697a..87f1d3d68034 100644 --- a/src/libs/Middleware/Reauthentication.ts +++ b/src/libs/Middleware/Reauthentication.ts @@ -7,16 +7,14 @@ import * as NetworkStore from '@libs/Network/NetworkStore'; import type {RequestError} from '@libs/Network/SequentialQueue'; import NetworkConnection from '@libs/NetworkConnection'; import * as Request from '@libs/Request'; -import RequestThrottle from '@libs/RequestThrottle'; import CONST from '@src/CONST'; +import * as RequestThrottle from '@src/libs/RequestThrottle'; import ONYXKEYS from '@src/ONYXKEYS'; import type Middleware from './types'; // We store a reference to the active authentication request so that we are only ever making one request to authenticate at a time. let isAuthenticating: Promise | null = null; -const reauthThrottle = new RequestThrottle(); - function reauthenticate(commandName?: string): Promise { if (isAuthenticating) { return isAuthenticating; @@ -46,8 +44,7 @@ function reauthenticate(commandName?: string): Promise { function retryReauthenticate(commandName?: string): Promise { return Authentication.reauthenticate(commandName).catch((error: RequestError) => { - return reauthThrottle - .sleep(error, 'Authenticate') + return RequestThrottle.sleep(error, 'Authenticate') .then(() => retryReauthenticate(commandName)) .catch(() => { NetworkStore.setIsAuthenticating(false); diff --git a/src/libs/Network/SequentialQueue.ts b/src/libs/Network/SequentialQueue.ts index ec07d315a608..cdf9b075ec44 100644 --- a/src/libs/Network/SequentialQueue.ts +++ b/src/libs/Network/SequentialQueue.ts @@ -2,10 +2,10 @@ import Onyx from 'react-native-onyx'; import * as ActiveClientManager from '@libs/ActiveClientManager'; import Log from '@libs/Log'; import * as Request from '@libs/Request'; -import RequestThrottle from '@libs/RequestThrottle'; import * as PersistedRequests from '@userActions/PersistedRequests'; import * as QueuedOnyxUpdates from '@userActions/QueuedOnyxUpdates'; import CONST from '@src/CONST'; +import * as RequestThrottle from '@src/libs/RequestThrottle'; import ONYXKEYS from '@src/ONYXKEYS'; import type OnyxRequest from '@src/types/onyx/Request'; import type {ConflictData} from '@src/types/onyx/Request'; @@ -28,7 +28,6 @@ resolveIsReadyPromise?.(); let isSequentialQueueRunning = false; let currentRequestPromise: Promise | null = null; let isQueuePaused = false; -const requestThrottle = new RequestThrottle(); /** * Puts the queue into a paused state so that no requests will be processed @@ -100,7 +99,7 @@ function process(): Promise { Log.info('[SequentialQueue] Removing persisted request because it was processed successfully.', false, {request: requestToProcess}); PersistedRequests.endRequestAndRemoveFromQueue(requestToProcess); - requestThrottle.clear(); + RequestThrottle.clear(); return process(); }) .catch((error: RequestError) => { @@ -109,18 +108,17 @@ function process(): Promise { if (error.name === CONST.ERROR.REQUEST_CANCELLED || error.message === CONST.ERROR.DUPLICATE_RECORD) { Log.info("[SequentialQueue] Removing persisted request because it failed and doesn't need to be retried.", false, {error, request: requestToProcess}); PersistedRequests.endRequestAndRemoveFromQueue(requestToProcess); - requestThrottle.clear(); + RequestThrottle.clear(); return process(); } PersistedRequests.rollbackOngoingRequest(); - return requestThrottle - .sleep(error, requestToProcess.command) + return RequestThrottle.sleep(error, requestToProcess.command) .then(process) .catch(() => { Onyx.update(requestToProcess.failureData ?? []); Log.info('[SequentialQueue] Removing persisted request because it failed too many times.', false, {error, request: requestToProcess}); PersistedRequests.endRequestAndRemoveFromQueue(requestToProcess); - requestThrottle.clear(); + RequestThrottle.clear(); return process(); }); }); diff --git a/src/libs/RequestThrottle.ts b/src/libs/RequestThrottle.ts index 8a6673c22a92..3bbc82ff5b45 100644 --- a/src/libs/RequestThrottle.ts +++ b/src/libs/RequestThrottle.ts @@ -3,44 +3,41 @@ import Log from './Log'; import type {RequestError} from './Network/SequentialQueue'; import {generateRandomInt} from './NumberUtils'; -class RequestThrottle { - private requestWaitTime = 0; +let requestWaitTime = 0; +let requestRetryCount = 0; - private requestRetryCount = 0; +function clear() { + requestWaitTime = 0; + requestRetryCount = 0; + Log.info(`[RequestThrottle] in clear()`); +} - clear() { - this.requestWaitTime = 0; - this.requestRetryCount = 0; - Log.info(`[RequestThrottle] in clear()`); - } - - getRequestWaitTime() { - if (this.requestWaitTime) { - this.requestWaitTime = Math.min(this.requestWaitTime * 2, CONST.NETWORK.MAX_RETRY_WAIT_TIME_MS); - } else { - this.requestWaitTime = generateRandomInt(CONST.NETWORK.MIN_RETRY_WAIT_TIME_MS, CONST.NETWORK.MAX_RANDOM_RETRY_WAIT_TIME_MS); - } - return this.requestWaitTime; +function getRequestWaitTime() { + if (requestWaitTime) { + requestWaitTime = Math.min(requestWaitTime * 2, CONST.NETWORK.MAX_RETRY_WAIT_TIME_MS); + } else { + requestWaitTime = generateRandomInt(CONST.NETWORK.MIN_RETRY_WAIT_TIME_MS, CONST.NETWORK.MAX_RANDOM_RETRY_WAIT_TIME_MS); } + return requestWaitTime; +} - getLastRequestWaitTime() { - return this.requestWaitTime; - } +function getLastRequestWaitTime() { + return requestWaitTime; +} - sleep(error: RequestError, command: string): Promise { - this.requestRetryCount++; - return new Promise((resolve, reject) => { - if (this.requestRetryCount <= CONST.NETWORK.MAX_REQUEST_RETRIES) { - const currentRequestWaitTime = this.getRequestWaitTime(); - Log.info( - `[RequestThrottle] Retrying request after error: '${error.name}', '${error.message}', '${error.status}'. Command: ${command}. Retry count: ${this.requestRetryCount}. Wait time: ${currentRequestWaitTime}`, - ); - setTimeout(resolve, currentRequestWaitTime); - } else { - reject(); - } - }); - } +function sleep(error: RequestError, command: string): Promise { + requestRetryCount++; + return new Promise((resolve, reject) => { + if (requestRetryCount <= CONST.NETWORK.MAX_REQUEST_RETRIES) { + const currentRequestWaitTime = getRequestWaitTime(); + Log.info( + `[RequestThrottle] Retrying request after error: '${error.name}', '${error.message}', '${error.status}'. Command: ${command}. Retry count: ${requestRetryCount}. Wait time: ${currentRequestWaitTime}`, + ); + setTimeout(resolve, currentRequestWaitTime); + return; + } + reject(); + }); } -export default RequestThrottle; +export {clear, getRequestWaitTime, sleep, getLastRequestWaitTime}; diff --git a/tests/unit/APITest.ts b/tests/unit/APITest.ts index bc4b650fb6e5..5b6947861dbe 100644 --- a/tests/unit/APITest.ts +++ b/tests/unit/APITest.ts @@ -9,7 +9,7 @@ import * as MainQueue from '@src/libs/Network/MainQueue'; import * as NetworkStore from '@src/libs/Network/NetworkStore'; import * as SequentialQueue from '@src/libs/Network/SequentialQueue'; import * as Request from '@src/libs/Request'; -import RequestThrottle from '@src/libs/RequestThrottle'; +import * as RequestThrottle from '@src/libs/RequestThrottle'; import ONYXKEYS from '@src/ONYXKEYS'; import type ReactNativeOnyxMock from '../../__mocks__/react-native-onyx'; import * as TestHelper from '../utils/TestHelper'; @@ -39,7 +39,6 @@ type XhrCalls = Array<{ }>; const originalXHR = HttpUtils.xhr; -const requestThrottle = new RequestThrottle(); beforeEach(() => { global.fetch = TestHelper.getGlobalFetchMock(); @@ -48,7 +47,7 @@ beforeEach(() => { MainQueue.clear(); HttpUtils.cancelPendingRequests(); PersistedRequests.clear(); - requestThrottle.clear(); + RequestThrottle.clear(); NetworkStore.checkRequiredData(); // Wait for any Log command to finish and Onyx to fully clear @@ -244,7 +243,7 @@ describe('APITests', () => { // We let the SequentialQueue process again after its wait time return new Promise((resolve) => { - setTimeout(resolve, requestThrottle.getLastRequestWaitTime()); + setTimeout(resolve, RequestThrottle.getLastRequestWaitTime()); }); }) .then(() => { @@ -257,7 +256,7 @@ describe('APITests', () => { // We let the SequentialQueue process again after its wait time return new Promise((resolve) => { - setTimeout(resolve, requestThrottle.getLastRequestWaitTime()); + setTimeout(resolve, RequestThrottle.getLastRequestWaitTime()); }).then(waitForBatchedUpdates); }) .then(() => {