Skip to content

Commit

Permalink
Update rate limit feature to be more browser freindly (#3428)
Browse files Browse the repository at this point in the history
  • Loading branch information
jdalrymple authored Jan 17, 2024
1 parent 29e36d2 commit 18d4123
Show file tree
Hide file tree
Showing 11 changed files with 368 additions and 308 deletions.
3 changes: 1 addition & 2 deletions .all-contributorsrc
Original file line number Diff line number Diff line change
Expand Up @@ -766,6 +766,5 @@
"contributions": ["code"]
}
],
"contributorsPerLine": 26
"contributorsPerLine": 300
}

16 changes: 8 additions & 8 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,28 +38,28 @@
"@auto-it/omit-commits": "^11.0.4",
"@auto-it/omit-release-notes": "^11.0.4",
"@auto-it/released": "^11.0.4",
"@swc/core": "^1.3.100",
"@swc/core": "^1.3.101",
"@swc/jest": "^0.2.29",
"@types/jest": "^29.5.11",
"@typescript-eslint/eslint-plugin": "^6.13.2",
"@typescript-eslint/parser": "^6.13.2",
"@typescript-eslint/eslint-plugin": "^6.15.0",
"@typescript-eslint/parser": "^6.15.0",
"auto": "^11.0.4",
"eslint": "^8.55.0",
"eslint": "^8.56.0",
"eslint-config-airbnb-base": "^15.0.0",
"eslint-config-prettier": "^9.1.0",
"eslint-import-resolver-typescript": "^3.6.1",
"eslint-plugin-import": "^2.29.0",
"eslint-plugin-import": "^2.29.1",
"eslint-plugin-jest": "^27.6.0",
"eslint-plugin-jest-extended": "^2.0.0",
"eslint-plugin-prettier": "^5.0.1",
"eslint-plugin-prettier": "^5.1.0",
"husky": "^8.0.3",
"jest": "^29.7.0",
"jest-extended": "^4.0.2",
"jest-junit": "^16.0.0",
"lerna": "^8.0.0",
"lerna": "^8.0.1",
"lint-staged": "^15.2.0",
"nx": "17.2.8",
"prettier": "^3.1.0",
"prettier": "^3.1.1",
"typescript": "^5.3.3"
},
"packageManager": "[email protected]"
Expand Down
15 changes: 10 additions & 5 deletions packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,14 @@
"exports": {
"./map.json": "./dist/map.json",
".": {
"types": "./dist/index.d.ts",
"import": "./dist/index.mjs",
"require": "./dist/index.js"
"import": {
"types": "./dist/index.d.mts",
"default": "./dist/index.mjs"
},
"require": {
"types": "./dist/index.d.ts",
"default": "./dist/index.js"
}
}
},
"files": [
Expand Down Expand Up @@ -60,10 +65,10 @@
"xcase": "^2.0.1"
},
"devDependencies": {
"@types/node": "^20.10.4",
"@types/node": "^20.10.5",
"get-param-names": "github:jdalrymple/get-param-names#1-improve-functionality",
"tsup": "^8.0.1",
"tsx": "^4.6.2",
"tsx": "^4.7.0",
"typescript": "^5.3.3"
}
}
18 changes: 8 additions & 10 deletions packages/core/src/infrastructure/RequestHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,22 +92,20 @@ export type GitlabAPIExpandedResponse<T, E extends boolean | void, P> = E extend
: ExpandedResponse<T>
: T;

export type GitlabAPISingleResponse<
T,
C extends boolean | void,
E extends boolean | void,
> = T extends Record<string, unknown>
? GitlabAPIExpandedResponse<CamelizedResponse<T, C>, E, undefined>
: GitlabAPIExpandedResponse<T, E, undefined>;
export type GitlabAPISingleResponse<T, C extends boolean | void, E extends boolean | void> =
T extends Record<string, unknown>
? GitlabAPIExpandedResponse<CamelizedResponse<T, C>, E, undefined>
: GitlabAPIExpandedResponse<T, E, undefined>;

export type GitlabAPIMultiResponse<
T,
C extends boolean | void,
E extends boolean | void,
P extends PaginationTypes | void,
> = T extends Record<string, unknown>
? GitlabAPIExpandedResponse<CamelizedResponse<T, C>[], E, P>
: GitlabAPIExpandedResponse<T[], E, P>;
> =
T extends Record<string, unknown>
? GitlabAPIExpandedResponse<CamelizedResponse<T, C>[], E, P>
: GitlabAPIExpandedResponse<T[], E, P>;

export type GitlabAPIResponse<
T,
Expand Down
17 changes: 11 additions & 6 deletions packages/requester-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,14 @@
"types": "./dist/index.d.ts",
"exports": {
".": {
"types": "./dist/index.d.ts",
"import": "./dist/index.mjs",
"require": "./dist/index.js"
"import": {
"types": "./dist/index.d.mts",
"default": "./dist/index.mjs"
},
"require": {
"types": "./dist/index.d.ts",
"default": "./dist/index.js"
}
}
},
"files": [
Expand All @@ -51,13 +56,13 @@
"release": "auto shipit"
},
"dependencies": {
"async-sema": "^3.1.1",
"micromatch": "^4.0.5",
"picomatch-browser": "^2.2.6",
"qs": "^6.11.2",
"rate-limiter-flexible": "^4.0.0",
"xcase": "^2.0.1"
},
"devDependencies": {
"@types/node": "^20.10.4",
"@types/node": "^20.10.5",
"tsup": "^8.0.1",
"typescript": "^5.3.3"
}
Expand Down
40 changes: 24 additions & 16 deletions packages/requester-utils/src/RequesterUtils.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { stringify } from 'qs';
import { decamelizeKeys } from 'xcase';
import { RateLimit } from 'async-sema';
import micromatch from 'micromatch';
import { RateLimiterMemory, RateLimiterQueue } from 'rate-limiter-flexible';
import Picomatch from 'picomatch-browser';

const { isMatch: isGlobMatch } = Picomatch;

// Types
export type RateLimiters = Record<
string,
ReturnType<typeof RateLimit> | { method: string; limit: ReturnType<typeof RateLimit> }
>;
export type RateLimiterFn = () => Promise<number>;
export type RateLimiters = Record<string, RateLimiterFn | { method: string; limit: RateLimiterFn }>;
export type RateLimitOptions = Record<string, number | { method: string; limit: number }>;

export type ResponseBodyTypes =
Expand Down Expand Up @@ -57,7 +57,7 @@ export type RequestOptions = {
body?: string | FormData;
asStream?: boolean;
signal?: AbortSignal;
rateLimiters?: Record<string, ReturnType<typeof RateLimit>>;
rateLimiters?: Record<string, RateLimiterFn>;
};

export interface RequesterType {
Expand Down Expand Up @@ -89,6 +89,14 @@ export type RequestHandlerFn<T extends ResponseBodyTypes = ResponseBodyTypes> =
) => Promise<FormattedResponse<T>>;

// Utility methods
export function generateRateLimiterFn(limit: number, interval: number) {
const limiter = new RateLimiterQueue(
new RateLimiterMemory({ points: limit, duration: interval }),
);

return () => limiter.removeTokens(1);
}

export function formatQuery(params: Record<string, unknown> = {}): string {
const decamelized = decamelizeKeys(params);

Expand Down Expand Up @@ -152,11 +160,11 @@ export function createRateLimiters(rateLimitOptions: RateLimitOptions = {}) {
const rateLimiters: RateLimiters = {};

Object.entries(rateLimitOptions).forEach(([key, config]) => {
if (typeof config === 'number') rateLimiters[key] = RateLimit(config, { timeUnit: 60000 });
if (typeof config === 'number') rateLimiters[key] = generateRateLimiterFn(config, 60);
else
rateLimiters[key] = {
method: config.method.toUpperCase(),
limit: RateLimit(config.limit, { timeUnit: 60000 }),
limit: generateRateLimiterFn(config.limit, 60),
};
});

Expand Down Expand Up @@ -219,16 +227,16 @@ export function getMatchingRateLimiter(
endpoint: string,
rateLimiters: RateLimiters = {},
method: string = 'GET',
): () => Promise<void> {
): RateLimiterFn {
const sortedEndpoints = Object.keys(rateLimiters).sort().reverse();
const match = sortedEndpoints.find((ep) => micromatch.isMatch(endpoint, ep));
const match = sortedEndpoints.find((ep) => isGlobMatch(endpoint, ep));
const rateLimitConfig = match && rateLimiters[match];

if (rateLimitConfig && typeof rateLimitConfig !== 'object') {
return rateLimitConfig;
}
if (rateLimitConfig && rateLimitConfig.method.toUpperCase() === method.toUpperCase()) {
if (typeof rateLimitConfig === 'function') return rateLimitConfig;

if (rateLimitConfig && rateLimitConfig?.method?.toUpperCase() === method.toUpperCase()) {
return rateLimitConfig.limit;
}
return RateLimit(3000, { timeUnit: 60000 });

return generateRateLimiterFn(3000, 60);
}
23 changes: 7 additions & 16 deletions packages/requester-utils/test/unit/RequesterUtils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as AsyncSema from 'async-sema';
import { RateLimiterMemory } from 'rate-limiter-flexible';
import {
RequestOptions,
ResourceOptions,
Expand All @@ -10,6 +10,8 @@ import {
presetResourceArguments,
} from '../../src/RequesterUtils';

jest.mock('rate-limiter-flexible');

const methods = ['get', 'put', 'patch', 'delete', 'post'];

describe('defaultOptionsHandler', () => {
Expand Down Expand Up @@ -193,8 +195,6 @@ describe('createInstance', () => {
});

it('should pass the rate limiters to the requestHandler function', async () => {
const rateLimitSpy = jest.spyOn(AsyncSema, 'RateLimit');

const testEndpoint = 'test endpoint';
const requester = createRequesterFn(
optionsHandler,
Expand All @@ -212,9 +212,6 @@ describe('createInstance', () => {

await requester.get(testEndpoint, {});

expect(rateLimitSpy).toHaveBeenCalledWith(10, { timeUnit: 60000 });
expect(rateLimitSpy).toHaveBeenCalledWith(40, { timeUnit: 60000 });

expect(requestHandler).toHaveBeenCalledWith(testEndpoint, {
rateLimiters: {
'*': expect.toBeFunction(),
Expand All @@ -229,8 +226,6 @@ describe('createInstance', () => {

describe('createRateLimiters', () => {
it('should create rate limiter functions when configured', () => {
const rateLimitSpy = jest.spyOn(AsyncSema, 'RateLimit');

const limiters = createRateLimiters({
'*': 40,
'projects/*/test': {
Expand All @@ -239,8 +234,8 @@ describe('createRateLimiters', () => {
},
});

expect(rateLimitSpy).toHaveBeenCalledWith(10, { timeUnit: 60000 });
expect(rateLimitSpy).toHaveBeenCalledWith(40, { timeUnit: 60000 });
expect(RateLimiterMemory).toHaveBeenCalledWith({ points: 10, duration: 60 });
expect(RateLimiterMemory).toHaveBeenCalledWith({ points: 40, duration: 60 });

expect(limiters).toStrictEqual({
'*': expect.toBeFunction(),
Expand Down Expand Up @@ -338,11 +333,9 @@ describe('getMatchingRateLimiter', () => {
});

it('should default the rateLimiters to an empty object if not passed and return the default rate of 3000 rpm', () => {
const rateLimitSpy = jest.spyOn(AsyncSema, 'RateLimit');

getMatchingRateLimiter('endpoint');

expect(rateLimitSpy).toHaveBeenCalledWith(3000, { timeUnit: 60000 });
expect(RateLimiterMemory).toHaveBeenCalledWith({ points: 3000, duration: 60 });
});

it('should return the most specific rate limit', async () => {
Expand All @@ -358,11 +351,9 @@ describe('getMatchingRateLimiter', () => {
});

it('should return a default rate limit of 3000 rpm if nothing matches', () => {
const rateLimitSpy = jest.spyOn(AsyncSema, 'RateLimit');

getMatchingRateLimiter('endpoint', { someurl: jest.fn() });

expect(rateLimitSpy).toHaveBeenCalledWith(3000, { timeUnit: 60000 });
expect(RateLimiterMemory).toHaveBeenCalledWith({ points: 3000, duration: 60 });
});

it('should handle expanded rate limit options with a particular method and limit', async () => {
Expand Down
13 changes: 9 additions & 4 deletions packages/rest/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,14 @@
"types": "./dist/index.d.ts",
"exports": {
".": {
"types": "./dist/index.d.ts",
"import": "./dist/index.mjs",
"require": "./dist/index.js"
"import": {
"types": "./dist/index.d.mts",
"default": "./dist/index.mjs"
},
"require": {
"types": "./dist/index.d.ts",
"default": "./dist/index.js"
}
}
},
"files": [
Expand Down Expand Up @@ -61,7 +66,7 @@
},
"devDependencies": {
"@playwright/test": "^1.40.1",
"@types/node": "^20.10.4",
"@types/node": "^20.10.5",
"tsup": "^8.0.1",
"typescript": "^5.3.3"
}
Expand Down
6 changes: 3 additions & 3 deletions packages/rest/src/Requester.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export async function defaultOptionsHandler(
resourceOptions.rejectUnauthorized != null &&
resourceOptions.rejectUnauthorized === false
) {
if (typeof window !== 'object') {
if (typeof window === 'undefined') {
const { Agent } = await import('https');

options.agent = new Agent({
Expand Down Expand Up @@ -103,7 +103,7 @@ export async function defaultRequestHandler(endpoint: string, options?: RequestO
const retryCodes = [429, 502];
const maxRetries = 10;
const { prefixUrl, asStream, searchParams, rateLimiters, method, ...opts } = options || {};
const endpointRateLimit = getMatchingRateLimiter(endpoint, rateLimiters, method);
const rateLimit = getMatchingRateLimiter(endpoint, rateLimiters, method);
let baseUrl: string | undefined;

if (prefixUrl) baseUrl = prefixUrl.endsWith('/') ? prefixUrl : `${prefixUrl}/`;
Expand All @@ -119,7 +119,7 @@ export async function defaultRequestHandler(endpoint: string, options?: RequestO
for (let i = 0; i < maxRetries; i += 1) {
const request = new Request(url, { ...opts, method, mode });

await endpointRateLimit();
await rateLimit();

const response = await fetch(request).catch((e) => {
if (e.name === 'TimeoutError' || e.name === 'AbortError') {
Expand Down
4 changes: 2 additions & 2 deletions packages/rest/test/e2e/browser/assets/test-import.html
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
"imports": {
"qs": "https://esm.sh/qs?min",
"xcase": "https://esm.sh/xcase?min",
"async-sema": "https://esm.sh/async-sema?min",
"micromatch": "https://esm.sh/micromatch?min",
"rate-limiter-flexible": "https://esm.sh/rate-limiter-flexible?min",
"picomatch-browser": "https://esm.sh/picomatch-browser?min",
"@gitbeaker/requester-utils": "../../../../../requester-utils/dist/index.mjs",
"@gitbeaker/core": "../../../../../core/dist/index.mjs"
}
Expand Down
Loading

0 comments on commit 18d4123

Please sign in to comment.