Skip to content

Commit

Permalink
revert RequestThrottle to be function based
Browse files Browse the repository at this point in the history
  • Loading branch information
zirgulis committed Nov 18, 2024
1 parent 876ff5a commit 80d634b
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 51 deletions.
7 changes: 2 additions & 5 deletions src/libs/Middleware/Reauthentication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> | null = null;

const reauthThrottle = new RequestThrottle();

function reauthenticate(commandName?: string): Promise<void> {
if (isAuthenticating) {
return isAuthenticating;
Expand Down Expand Up @@ -46,8 +44,7 @@ function reauthenticate(commandName?: string): Promise<void> {

function retryReauthenticate(commandName?: string): Promise<void> {
return Authentication.reauthenticate(commandName).catch((error: RequestError) => {
return reauthThrottle
.sleep(error, 'Authenticate')
return RequestThrottle.sleep(error, 'Authenticate')
.then(() => retryReauthenticate(commandName))
.catch(() => {
NetworkStore.setIsAuthenticating(false);
Expand Down
12 changes: 5 additions & 7 deletions src/libs/Network/SequentialQueue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -28,7 +28,6 @@ resolveIsReadyPromise?.();
let isSequentialQueueRunning = false;
let currentRequestPromise: Promise<void> | null = null;
let isQueuePaused = false;
const requestThrottle = new RequestThrottle();

/**
* Puts the queue into a paused state so that no requests will be processed
Expand Down Expand Up @@ -100,7 +99,7 @@ function process(): Promise<void> {

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) => {
Expand All @@ -109,18 +108,17 @@ function process(): Promise<void> {
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();
});
});
Expand Down
65 changes: 31 additions & 34 deletions src/libs/RequestThrottle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> {
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<void> {
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};
9 changes: 4 additions & 5 deletions tests/unit/APITest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -39,7 +39,6 @@ type XhrCalls = Array<{
}>;

const originalXHR = HttpUtils.xhr;
const requestThrottle = new RequestThrottle();

beforeEach(() => {
global.fetch = TestHelper.getGlobalFetchMock();
Expand All @@ -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
Expand Down Expand Up @@ -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(() => {
Expand All @@ -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(() => {
Expand Down

0 comments on commit 80d634b

Please sign in to comment.