diff --git a/packages/core/src/resources/ApplicationPlanLimits.ts b/packages/core/src/resources/ApplicationPlanLimits.ts index 2d815835..3000f68e 100644 --- a/packages/core/src/resources/ApplicationPlanLimits.ts +++ b/packages/core/src/resources/ApplicationPlanLimits.ts @@ -61,6 +61,7 @@ export class ApplicationPlanLimits extends BaseResour } = options; return RequestHelper.put()(this, 'application/plan_limits', { + ...opts, searchParams: { planName, ciPipelineSize, @@ -81,7 +82,6 @@ export class ApplicationPlanLimits extends BaseResour terraformModuleMaxFileSize, storageSizeLimit, }, - opts, }); } } diff --git a/packages/core/test/unit/resources/ApplicationPlanLimits.ts b/packages/core/test/unit/resources/ApplicationPlanLimits.ts new file mode 100644 index 00000000..9d65e909 --- /dev/null +++ b/packages/core/test/unit/resources/ApplicationPlanLimits.ts @@ -0,0 +1,36 @@ +import { RequestHelper } from '../../../src/infrastructure'; +import { ApplicationPlanLimits } from '../../../src'; + +jest.mock( + '../../../src/infrastructure/RequestHelper', + () => require('../../__mocks__/RequestHelper').default, +); + +let service: ApplicationPlanLimits; + +beforeEach(() => { + service = new ApplicationPlanLimits({ + requesterFn: jest.fn(), + token: 'abcdefg', + }); +}); + +describe('ApplicationPlanLimits.show', () => { + it('should request GET /application/plan_limits', async () => { + await service.show(); + + expect(RequestHelper.get()).toHaveBeenCalledWith(service, 'application/plan_limits', undefined); + }); +}); + +describe('ApplicationPlanLimits.edit', () => { + it('should request PUT /application/plan_limits with a terms property', async () => { + await service.edit('Plan name'); + + expect(RequestHelper.put()).toHaveBeenCalledWith(service, 'application/plan_limits', { + searchParams: { + planName: 'Plan name', + }, + }); + }); +}); diff --git a/packages/core/test/unit/resources/ApplicationStatistics.ts b/packages/core/test/unit/resources/ApplicationStatistics.ts new file mode 100644 index 00000000..b568ced5 --- /dev/null +++ b/packages/core/test/unit/resources/ApplicationStatistics.ts @@ -0,0 +1,24 @@ +import { RequestHelper } from '../../../src/infrastructure'; +import { ApplicationStatistics } from '../../../src'; + +jest.mock( + '../../../src/infrastructure/RequestHelper', + () => require('../../__mocks__/RequestHelper').default, +); + +let service: ApplicationStatistics; + +beforeEach(() => { + service = new ApplicationStatistics({ + requesterFn: jest.fn(), + token: 'abcdefg', + }); +}); + +describe('ApplicationStatistics.show', () => { + it('should request GET /application/statistics', async () => { + await service.show(); + + expect(RequestHelper.get()).toHaveBeenCalledWith(service, 'application/statistics', undefined); + }); +}); diff --git a/packages/core/test/unit/resources/Applications.ts b/packages/core/test/unit/resources/Applications.ts new file mode 100644 index 00000000..8df17bbe --- /dev/null +++ b/packages/core/test/unit/resources/Applications.ts @@ -0,0 +1,44 @@ +import { RequestHelper } from '../../../src/infrastructure'; +import { Applications } from '../../../src'; + +jest.mock( + '../../../src/infrastructure/RequestHelper', + () => require('../../__mocks__/RequestHelper').default, +); + +let service: Applications; + +beforeEach(() => { + service = new Applications({ + requesterFn: jest.fn(), + token: 'abcdefg', + }); +}); + +describe('Applications.all', () => { + it('should request GET /applications without options', async () => { + await service.all(); + + expect(RequestHelper.get()).toHaveBeenCalledWith(service, 'applications', undefined); + }); +}); + +describe('Applications.show', () => { + it('should request GET /applications', async () => { + await service.create('application', 'url', 'scope1'); + + expect(RequestHelper.get()).toHaveBeenCalledWith(service, 'applications', { + name: 'application', + redirectUri: 'url', + scopes: 'scope1', + }); + }); +}); + +describe('Applications.remove', () => { + it('should request GET /applications/:id', async () => { + await service.remove(12); + + expect(RequestHelper.get()).toHaveBeenCalledWith(service, 'applications/12', undefined); + }); +}); diff --git a/packages/requester-utils/package.json b/packages/requester-utils/package.json index 28d5d627..b53c6c94 100644 --- a/packages/requester-utils/package.json +++ b/packages/requester-utils/package.json @@ -51,6 +51,8 @@ "release": "auto shipit" }, "dependencies": { + "async-sema": "^3.1.1", + "micromatch": "^4.0.5", "qs": "^6.11.2", "xcase": "^2.0.1" }, diff --git a/packages/requester-utils/src/BaseResource.ts b/packages/requester-utils/src/BaseResource.ts index ed81b8f9..656b4cf1 100644 --- a/packages/requester-utils/src/BaseResource.ts +++ b/packages/requester-utils/src/BaseResource.ts @@ -1,4 +1,4 @@ -import { RequesterType, ResourceOptions } from './RequesterUtils'; +import { RateLimitOptions, RequesterType, ResourceOptions } from './RequesterUtils'; export interface RootResourceOptions { // TODO: Not actually optional - Need to fix wrapper typing in requestUtils.ts: @@ -11,6 +11,7 @@ export interface RootResourceOptions { sudo?: string | number; profileToken?: string; profileMode?: 'execution' | 'memory'; + rateLimits?: RateLimitOptions; } export type GitlabToken = string | (() => Promise); @@ -36,6 +37,48 @@ function getDynamicToken(tokenArgument: (() => Promise) | string): Promi return tokenArgument instanceof Function ? tokenArgument() : Promise.resolve(tokenArgument); } +// Default rate limits per minute +const DEFAULT_RATE_LIMITS = Object.freeze({ + // Default rate limit + '**': 3000, + + // Import/Export + 'projects/import': 6, + 'projects/*/export': 6, + 'projects/*/download': 1, + 'groups/import': 6, + 'groups/*/export': 6, + 'groups/*/download': 1, + + // Note creation + 'projects/*/issues/*/notes': { + method: 'post', + limit: 300, + }, + 'projects/*/snippets/*/notes': { + method: 'post', + limit: 300, + }, + 'projects/*/merge_requests/*/notes': { + method: 'post', + limit: 300, + }, + 'groups/*/epics/*/notes': { + method: 'post', + limit: 300, + }, + + // Repositories - get file archive + 'projects/*/repository/archive*': 5, + + // Project Jobs + 'projects/*/jobs': 600, + + // Member deletion + 'projects/*/members': 60, + 'groups/*/members': 60, +}); + export class BaseResource { public readonly url: string; @@ -61,6 +104,7 @@ export class BaseResource { prefixUrl = '', rejectUnauthorized = true, queryTimeout = 300000, + rateLimits = DEFAULT_RATE_LIMITS, ...tokens }: BaseResourceOptions) { if (!requesterFn) throw new ReferenceError('requesterFn must be passed'); @@ -97,6 +141,6 @@ export class BaseResource { if (sudo) this.headers.Sudo = `${sudo}`; // Set requester instance using this information - this.requester = requesterFn({ ...this }); + this.requester = requesterFn({ ...this, rateLimits }); } } diff --git a/packages/requester-utils/src/RequesterUtils.ts b/packages/requester-utils/src/RequesterUtils.ts index 94f93125..05d570e5 100644 --- a/packages/requester-utils/src/RequesterUtils.ts +++ b/packages/requester-utils/src/RequesterUtils.ts @@ -1,7 +1,15 @@ import { stringify } from 'qs'; import { decamelizeKeys } from 'xcase'; +import { RateLimit } from 'async-sema'; +import micromatch from 'micromatch'; // Types +export type RateLimiters = Record< + string, + ReturnType | { method: string; limit: ReturnType } +>; +export type RateLimitOptions = Record; + export type ResponseBodyTypes = | Record | Record[] @@ -28,6 +36,7 @@ export type ResourceOptions = { authHeaders: { [authHeader: string]: () => Promise }; url: string; rejectUnauthorized: boolean; + rateLimits?: RateLimitOptions; }; export type DefaultRequestOptions = { @@ -48,6 +57,7 @@ export type RequestOptions = { body?: string | FormData; asStream?: boolean; signal?: AbortSignal; + rateLimiters?: Record>; }; export interface RequesterType { @@ -73,6 +83,11 @@ export interface RequesterType { ): Promise>; } +export type RequestHandlerFn = ( + endpoint: string, + options?: Record, +) => Promise>; + // Utility methods export function formatQuery(params: Record = {}): string { const decamelized = decamelizeKeys(params); @@ -137,10 +152,20 @@ export async function defaultOptionsHandler( return Promise.resolve(defaultOptions); } -export type RequestHandlerFn = ( - endpoint: string, - options?: Record, -) => Promise>; +export function createRateLimiters(rateLimitOptions: RateLimitOptions = {}) { + const rateLimiters: RateLimiters = {}; + + Object.entries(rateLimitOptions).forEach(([key, config]) => { + if (typeof config === 'number') rateLimiters[key] = RateLimit(config, { timeUnit: 60000 }); + else + rateLimiters[key] = { + method: config.method.toUpperCase(), + limit: RateLimit(config.limit, { timeUnit: 60000 }), + }; + }); + + return rateLimiters; +} export function createRequesterFn( optionsHandler: OptionsHandlerFn, @@ -150,6 +175,7 @@ export function createRequesterFn( return (serviceOptions) => { const requester: RequesterType = {} as RequesterType; + const rateLimiters = createRateLimiters(serviceOptions.rateLimits); methods.forEach((m) => { requester[m] = async (endpoint: string, options: Record) => { @@ -159,7 +185,7 @@ export function createRequesterFn( }); const requestOptions = await optionsHandler(serviceOptions, defaultRequestOptions); - return requestHandler(endpoint, requestOptions); + return requestHandler(endpoint, { ...requestOptions, rateLimiters }); }; }); @@ -167,10 +193,7 @@ export function createRequesterFn( }; } -function extendClass( - Base: T, - customConfig: Record = {}, -): T { +function extendClass(Base: T, customConfig: Record): T { return class extends Base { constructor(...options: any[]) { // eslint-disable-line @@ -195,3 +218,21 @@ export function presetResourceArguments> return updated as T; } + +export function getMatchingRateLimiter( + endpoint: string, + rateLimiters: RateLimiters = {}, + method: string = 'GET', +): () => Promise { + const sortedEndpoints = Object.keys(rateLimiters).sort().reverse(); + const match = sortedEndpoints.find((ep) => micromatch.isMatch(endpoint, ep)); + const rateLimitConfig = match && rateLimiters[match]; + + if (rateLimitConfig && typeof rateLimitConfig !== 'object') { + return rateLimitConfig; + } + if (rateLimitConfig && rateLimitConfig.method.toUpperCase() === method.toUpperCase()) { + return rateLimitConfig.limit; + } + return RateLimit(3000, { timeUnit: 60000 }); +} diff --git a/packages/requester-utils/test/unit/BaseResource.ts b/packages/requester-utils/test/unit/BaseResource.ts index b39fa364..29a3b2b9 100644 --- a/packages/requester-utils/test/unit/BaseResource.ts +++ b/packages/requester-utils/test/unit/BaseResource.ts @@ -98,6 +98,14 @@ describe('Creation of BaseResource instance', () => { await expect(service.authHeaders['job-token']()).resolves.toBe('1234'); }); + it('should throw an error if a token, jobToken or oauthToken is not passed', () => { + expect(() => { + // eslint-disable-next-line + // @ts-ignore + new BaseResource({ requesterFn: jest.fn() }); // eslint-disable-line + }).toThrow(); + }); + it('should set the X-Profile-Token header if the profileToken option is given', () => { const service = new BaseResource({ token: '123', @@ -196,6 +204,12 @@ describe('Creation of BaseResource instance', () => { // @ts-ignore new BaseResource(); // eslint-disable-line }).toThrow(); + + expect(() => { + // eslint-disable-next-line + // @ts-ignore + new BaseResource({}); // eslint-disable-line + }).toThrow(); }); it('should set the internal requester based on the required requesterFn parameter', async () => { diff --git a/packages/requester-utils/test/unit/RequesterUtils.ts b/packages/requester-utils/test/unit/RequesterUtils.ts index 417209a0..6cdd3b7f 100644 --- a/packages/requester-utils/test/unit/RequesterUtils.ts +++ b/packages/requester-utils/test/unit/RequesterUtils.ts @@ -1,21 +1,26 @@ +import * as AsyncSema from 'async-sema'; import { RequestOptions, + ResourceOptions, + createRateLimiters, createRequesterFn, defaultOptionsHandler, formatQuery, + getMatchingRateLimiter, presetResourceArguments, } from '../../src/RequesterUtils'; const methods = ['get', 'put', 'patch', 'delete', 'post']; describe('defaultOptionsHandler', () => { - const serviceOptions = { + const serviceOptions: ResourceOptions = { headers: { test: '5' }, authHeaders: { token: () => Promise.resolve('1234'), }, url: 'testurl', rejectUnauthorized: false, + rateLimits: {}, }; it('should not use default request options if not passed', async () => { @@ -105,7 +110,7 @@ describe('defaultOptionsHandler', () => { describe('createInstance', () => { const requestHandler = jest.fn(); const optionsHandler = jest.fn(() => Promise.resolve({} as RequestOptions)); - const serviceOptions = { + const serviceOptions: ResourceOptions = { headers: { test: '5' }, authHeaders: { token: () => Promise.resolve('1234'), @@ -141,26 +146,28 @@ describe('createInstance', () => { serviceOptions, expect.objectContaining({ method: m.toUpperCase() }), ); - expect(requestHandler).toHaveBeenCalledWith(testEndpoint, {}); + expect(requestHandler).toHaveBeenCalledWith(testEndpoint, { rateLimiters: {} }); } }); it('should respect the closure variables', async () => { - const serviceOptions1 = { + const serviceOptions1: ResourceOptions = { headers: { test: '5' }, authHeaders: { token: () => Promise.resolve('1234'), }, url: 'testurl', rejectUnauthorized: false, + rateLimits: {}, }; - const serviceOptions2 = { + const serviceOptions2: ResourceOptions = { headers: { test: '5' }, authHeaders: { token: () => Promise.resolve('1234'), }, url: 'testurl2', rejectUnauthorized: true, + rateLimits: {}, }; const requesterFn = createRequesterFn(optionsHandler, requestHandler); @@ -181,6 +188,65 @@ describe('createInstance', () => { expect.objectContaining({ method: 'GET' }), ); }); + + 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, + requestHandler, + )({ + ...serviceOptions, + rateLimits: { + '*': 40, + 'projects/*/test': { + method: 'GET', + limit: 10, + }, + }, + }); + + await requester.get(testEndpoint, {}); + + expect(rateLimitSpy).toHaveBeenCalledWith(10, { timeUnit: 60000 }); + expect(rateLimitSpy).toHaveBeenCalledWith(40, { timeUnit: 60000 }); + + expect(requestHandler).toHaveBeenCalledWith(testEndpoint, { + rateLimiters: { + '*': expect.toBeFunction(), + 'projects/*/test': { + method: 'GET', + limit: expect.toBeFunction(), + }, + }, + }); + }); +}); + +describe('createRateLimiters', () => { + it('should create rate limiter functions when configured', () => { + const rateLimitSpy = jest.spyOn(AsyncSema, 'RateLimit'); + + const limiters = createRateLimiters({ + '*': 40, + 'projects/*/test': { + method: 'GET', + limit: 10, + }, + }); + + expect(rateLimitSpy).toHaveBeenCalledWith(10, { timeUnit: 60000 }); + expect(rateLimitSpy).toHaveBeenCalledWith(40, { timeUnit: 60000 }); + + expect(limiters).toStrictEqual({ + '*': expect.toBeFunction(), + 'projects/*/test': { + method: 'GET', + limit: expect.toBeFunction(), + }, + }); + }); }); describe('presetResourceArguments', () => { @@ -244,3 +310,75 @@ describe('formatQuery', () => { expect(string).toBe('test=6¬%5Btest%5D=7'); }); }); + +describe('getMatchingRateLimiter', () => { + it('should default the method to GET if not passed', async () => { + const rateLimiter = jest.fn(); + const matchingRateLimiter = getMatchingRateLimiter('endpoint', { + '*': { method: 'GET', limit: rateLimiter }, + }); + + await matchingRateLimiter(); + + expect(rateLimiter).toHaveBeenCalledTimes(1); + }); + + it('should uppercase method for matching', async () => { + const rateLimiter = jest.fn(); + const matchingRateLimiter = getMatchingRateLimiter('endpoint', { + '*': { method: 'get', limit: rateLimiter }, + }); + + await matchingRateLimiter(); + + expect(rateLimiter).toHaveBeenCalledTimes(1); + }); + + 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 }); + }); + + it('should return the most specific rate limit', async () => { + const rateLimiter = jest.fn(); + const matchingRateLimiter = getMatchingRateLimiter('endpoint/testing', { + '*': jest.fn(), + 'endpoint/testing*': rateLimiter, + }); + + await matchingRateLimiter(); + + expect(rateLimiter).toHaveBeenCalledTimes(1); + }); + + 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 }); + }); + + it('should handle expanded rate limit options with a particular method and limit', async () => { + const rateLimiter = jest.fn(); + const matchingRateLimiter = getMatchingRateLimiter('endpoint', { + '*': { method: 'get', limit: rateLimiter }, + }); + + await matchingRateLimiter(); + + expect(rateLimiter).toHaveBeenCalledTimes(1); + }); + + it('should handle simple rate limit options with a particular limit', async () => { + const rateLimiter = jest.fn(); + const matchingRateLimiter = getMatchingRateLimiter('endpoint/testing', { '**': rateLimiter }); + + await matchingRateLimiter(); + + expect(rateLimiter).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/rest/README.md b/packages/rest/README.md index ce64f94d..2ad89c17 100644 --- a/packages/rest/README.md +++ b/packages/rest/README.md @@ -76,6 +76,7 @@ - [API Client](#api-client) - [Expanded Payloads](#expanded-payloads) - [Pagination](#pagination) + - [Rate Limits](#rate-limits) - [Error Handling](#error-handling) - [Examples](#examples) - [Testing](../../docs/TESTING.md) @@ -155,6 +156,7 @@ Available instantiating options: | `queryTimeout` | Yes | `300000` | Query Timeout in ms | | `profileToken` | Yes | N/A | [Requests Profiles Token](https://docs.gitlab.com/ee/administration/monitoring/performance/request_profiling.html) | | `profileMode` | Yes | `execution` | [Requests Profiles Token](https://docs.gitlab.com/ee/administration/monitoring/performance/request_profiling.html) | +| `rateLimits` | No | [DEFAULT_RATE_LIMITS](#rate-limits) | Global and endpoint specific adjustable rate limits | > \*One of these options must be supplied. @@ -267,6 +269,72 @@ const { data } = await api.Projects.all({ }); ``` +### Rate Limits + +Rate limits are completely customizable, and are used to limit the request rate between consecutive API requests within the library. By default, all non-specified endpoints use a 3000 rps rate limit, while some endpoints have much smaller rates as dictated by the [Gitlab Docs](https://docs.gitlab.com/ee/security/rate_limits.html). See below for the default values: + +```js +const DEFAULT_RATE_LIMITS = Object.freeze({ + // Default rate limit + '**': 3000, + + // Import/Export + 'projects/import': 6, + 'projects/*/export': 6, + 'projects/*/download': 1, + 'groups/import': 6, + 'groups/*/export': 6, + 'groups/*/download': 1, + + // Note creation + 'projects/*/issues/*/notes': { + method: 'post', + limit: 300, + }, + 'projects/*/snippets/*/notes': { + method: 'post', + limit: 300, + }, + 'projects/*/merge_requests/*/notes': { + method: 'post', + limit: 300, + }, + 'groups/*/epics/*/notes': { + method: 'post', + limit: 300, + }, + + // Repositories - get file archive + 'projects/*/repository/archive*': 5, + + // Project Jobs + 'projects/*/jobs': 600, + + // Member deletion + 'projects/*/members': 60, + 'groups/*/members': 60, +}); +``` + +Rate limits can be overridden when instantiating a API wrapper. For ease of use, these limits are configured using glob patterns, and can be formatted in two ways. + +1. The glob for the endpoint with the corresponding rate per second +2. The glob for the endpoint, with an object specifying the specific method for the endpoint and the corresponding rate limit + +```js +const api = new Gitlab({ + token: 'token', + rateLimits: { + '**': 30, + 'projects/import/*': 40, + 'projects/*/issues/*/notes': { + method: 'post', + limit: 300, + }, + }, +}); +``` + ### Error Handling Request errors are returned back within a plain Error instance, using the cause to hold the original response and a text description of the error pulled from the response's error or message fields if JSON, or its plain text value: diff --git a/packages/rest/package.json b/packages/rest/package.json index 93320b02..91ff5111 100644 --- a/packages/rest/package.json +++ b/packages/rest/package.json @@ -40,7 +40,7 @@ "test:types": "tsc", "test:e2e:browser": "playwright test", "test:e2e": "yarn test:e2e:browser", - "test:integration:nodejs": "jest --maxWorkers=50% test/integration/nodejs", + "test:integration:nodejs": "jest --maxWorkers=50% test/integration/nodejs/resources/Issues.ts", "test:integration": "yarn test:integration:nodejs", "test:unit": "jest --maxWorkers=50% test/unit", "format:docs": "prettier '**/(*.json|.yml|.js|.md)' --ignore-path ../../.prettierignore", diff --git a/packages/rest/src/Requester.ts b/packages/rest/src/Requester.ts index 8c05c63e..17d952d4 100644 --- a/packages/rest/src/Requester.ts +++ b/packages/rest/src/Requester.ts @@ -3,7 +3,7 @@ import type { ResourceOptions, ResponseBodyTypes, } from '@gitbeaker/requester-utils'; -import { createRequesterFn } from '@gitbeaker/requester-utils'; +import { createRequesterFn, getMatchingRateLimiter } from '@gitbeaker/requester-utils'; export async function defaultOptionsHandler( resourceOptions: ResourceOptions, @@ -93,7 +93,8 @@ function getConditionalMode(endpoint: string) { export async function defaultRequestHandler(endpoint: string, options?: RequestOptions) { const retryCodes = [429, 502]; const maxRetries = 10; - const { prefixUrl, asStream, searchParams, ...opts } = options || {}; + const { prefixUrl, asStream, searchParams, rateLimiters, method, ...opts } = options || {}; + const endpointRateLimit = getMatchingRateLimiter(endpoint, rateLimiters, method); let baseUrl: string | undefined; if (prefixUrl) baseUrl = prefixUrl.endsWith('/') ? prefixUrl : `${prefixUrl}/`; @@ -107,7 +108,10 @@ export async function defaultRequestHandler(endpoint: string, options?: RequestO /* eslint-disable no-await-in-loop */ for (let i = 0; i < maxRetries; i += 1) { - const request = new Request(url, { ...opts, mode }); + const request = new Request(url, { ...opts, method, mode }); + + await endpointRateLimit(); + const response = await fetch(request).catch((e) => { if (e.name === 'TimeoutError' || e.name === 'AbortError') { throw new Error('Query timeout was reached'); diff --git a/packages/rest/test/e2e/browser/assets/test-import.html b/packages/rest/test/e2e/browser/assets/test-import.html index 74c00624..58b5d498 100644 --- a/packages/rest/test/e2e/browser/assets/test-import.html +++ b/packages/rest/test/e2e/browser/assets/test-import.html @@ -11,6 +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", "@gitbeaker/requester-utils": "../../../../../requester-utils/dist/index.mjs", "@gitbeaker/core": "../../../../../core/dist/index.mjs" } diff --git a/packages/rest/test/integration/nodejs/resources/Issues.ts b/packages/rest/test/integration/nodejs/resources/Issues.ts index 2d2b1112..22acd6aa 100644 --- a/packages/rest/test/integration/nodejs/resources/Issues.ts +++ b/packages/rest/test/integration/nodejs/resources/Issues.ts @@ -26,6 +26,7 @@ describe('Issues.all', () => { const newIssues: ReturnType>[] = []; for (let i = 0; i < 10; i += 1) { + console.log(`start ${i}`); newIssues.push( issueAPI.create(project.id, `Issue.all Test - NoteJS ${TEST_ID} ${i}`, { description: 'A test issue', @@ -34,7 +35,7 @@ describe('Issues.all', () => { } await Promise.all(newIssues); - }, 20000); + }, 60000); it('should get 10 issues using keyset pagination', async () => { const projects = await issueAPI.all({ diff --git a/packages/rest/test/integration/nodejs/resources/Projects.ts b/packages/rest/test/integration/nodejs/resources/Projects.ts index 3bae1576..d599f70f 100644 --- a/packages/rest/test/integration/nodejs/resources/Projects.ts +++ b/packages/rest/test/integration/nodejs/resources/Projects.ts @@ -32,7 +32,7 @@ describe('Projects.all', () => { } await Promise.all(newProjects); - }, 20000); + }, 60000); it('should get 10 projects using offset pagination', async () => { const projects = await service.all({ diff --git a/packages/rest/test/unit/Requester.ts b/packages/rest/test/unit/Requester.ts index 20a972cb..e7a9ecda 100644 --- a/packages/rest/test/unit/Requester.ts +++ b/packages/rest/test/unit/Requester.ts @@ -21,7 +21,7 @@ describe('processBody', () => { it('should return a blob if type is octet-stream, binary, or gzip', async () => { const blobData = new Blob(['test'], { - type: 'plain/text', + type: 'text/plain', }); const output = [ @@ -123,6 +123,176 @@ describe('defaultRequestHandler', () => { }); }); + it('should return an error the content of the error message if response is not JSON', async () => { + const stringBody = 'Bad things happened'; + + MockFetch.mockReturnValueOnce( + Promise.resolve({ + ok: false, + status: 501, + statusText: 'Really Bad Error', + headers: new Headers({ + 'content-type': 'text/plain', + }), + json: () => Promise.resolve(stringBody), + text: () => Promise.resolve(stringBody), + }), + ); + + await expect(defaultRequestHandler('http://test.com', {} as RequestOptions)).rejects.toThrow({ + message: 'Really Bad Error', + name: 'Error', + cause: { + description: stringBody, + }, + }); + }); + + it('should return an error with a message "Query timeout was reached" if fetch throws a TimeoutError', async () => { + class TimeoutError extends Error { + constructor(message: string) { + super(message); + this.name = 'TimeoutError'; + } + } + + MockFetch.mockRejectedValueOnce(new TimeoutError('Hit timeout')); + + await expect(defaultRequestHandler('http://test.com', {} as RequestOptions)).rejects.toThrow({ + message: 'Query timeout was reached', + name: 'Error', + }); + }); + + it('should return an error with a message "Query timeout was reached" if fetch throws a AbortError', async () => { + class AbortError extends Error { + constructor(message: string) { + super(message); + this.name = 'AbortError'; + } + } + + MockFetch.mockRejectedValueOnce(new AbortError('Abort signal triggered')); + + await expect(defaultRequestHandler('http://test.com', {} as RequestOptions)).rejects.toThrow({ + message: 'Query timeout was reached', + name: 'Error', + }); + }); + + it('should return an unchanged error if fetch throws an error thats not an AbortError or TimeoutError', async () => { + class RandomError extends Error { + constructor(message: string) { + super(message); + this.name = 'RandomError'; + } + } + + MockFetch.mockRejectedValueOnce(new RandomError('Random Error')); + + await expect(defaultRequestHandler('http://test.com', {} as RequestOptions)).rejects.toThrow({ + message: 'Random Error', + name: 'RandomError', + }); + }); + + it('should retry request if a 429 retry code is returned', async () => { + const stringBody = { error: 'msg' }; + const fakeFailedReturnValue = Promise.resolve({ + ok: false, + status: 429, + statusText: 'Retry Code', + headers: new Headers({ + 'content-type': 'application/json', + }), + json: () => Promise.resolve(stringBody), + text: () => Promise.resolve(JSON.stringify(stringBody)), + }); + + const fakeSuccessfulReturnValue = Promise.resolve({ + json: () => Promise.resolve({}), + text: () => Promise.resolve(JSON.stringify({})), + ok: true, + status: 200, + headers: new Headers({ + 'content-type': 'application/json', + }), + }); + + // Mock return 10 times + MockFetch.mockReturnValue(fakeFailedReturnValue); + MockFetch.mockReturnValue(fakeSuccessfulReturnValue); + + const output = await defaultRequestHandler('http://test.com', {} as RequestOptions); + + expect(output).toMatchObject({ + body: {}, + headers: { 'content-type': 'application/json' }, + status: 200, + }); + }); + + it('should retry request if a 502 retry code is returned', async () => { + const stringBody = { error: 'msg' }; + const fakeFailedReturnValue = Promise.resolve({ + ok: false, + status: 502, + statusText: 'Retry Code', + headers: new Headers({ + 'content-type': 'application/json', + }), + json: () => Promise.resolve(stringBody), + text: () => Promise.resolve(JSON.stringify(stringBody)), + }); + + const fakeSuccessfulReturnValue = Promise.resolve({ + json: () => Promise.resolve({}), + text: () => Promise.resolve(JSON.stringify({})), + ok: true, + status: 200, + headers: new Headers({ + 'content-type': 'application/json', + }), + }); + + // Mock return 10 times + MockFetch.mockReturnValue(fakeFailedReturnValue); + MockFetch.mockReturnValue(fakeSuccessfulReturnValue); + + const output = await defaultRequestHandler('http://test.com', {} as RequestOptions); + + expect(output).toMatchObject({ + body: {}, + headers: { 'content-type': 'application/json' }, + status: 200, + }); + }); + + it('should return a default error if retries are unsuccessful', async () => { + const stringBody = { error: 'msg' }; + const fakeReturnValue = Promise.resolve({ + ok: false, + status: 429, + statusText: 'Retry Code', + headers: new Headers({ + 'content-type': 'application/json', + }), + json: () => Promise.resolve(stringBody), + text: () => Promise.resolve(JSON.stringify(stringBody)), + }); + + // Mock return 10 times + MockFetch.mockReturnValue(fakeReturnValue); + + await expect(defaultRequestHandler('http://test.com', {} as RequestOptions)).rejects.toThrow({ + message: + 'Could not successfully complete this request due to Error 429. Check the applicable rate limits for this endpoint.', + name: 'Error', + }); + + MockFetch.mockRestore(); + }); + it('should return correct properties if request is valid', async () => { MockFetch.mockReturnValueOnce( Promise.resolve({ @@ -140,7 +310,32 @@ describe('defaultRequestHandler', () => { expect(output).toMatchObject({ body: {}, - headers: {}, + headers: { 'content-type': 'application/json' }, + status: 200, + }); + }); + + it('should return correct properties as stream if request is valid', async () => { + MockFetch.mockReturnValueOnce( + Promise.resolve({ + body: 'text', + json: () => Promise.resolve({}), + text: () => Promise.resolve(JSON.stringify({})), + ok: true, + status: 200, + headers: new Headers({ + 'content-type': 'application/json', + }), + }), + ); + + const output = await defaultRequestHandler('http://test.com', { + asStream: true, + } as RequestOptions); + + expect(output).toMatchObject({ + body: 'text', + headers: { 'content-type': 'application/json' }, status: 200, }); }); diff --git a/yarn.lock b/yarn.lock index fd96021a..01393ad4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1057,6 +1057,8 @@ __metadata: resolution: "@gitbeaker/requester-utils@workspace:packages/requester-utils" dependencies: "@types/node": ^20.4.0 + async-sema: ^3.1.1 + micromatch: ^4.0.5 qs: ^6.11.2 tsup: ^7.1.0 typescript: ^5.1.6 @@ -3225,6 +3227,13 @@ __metadata: languageName: node linkType: hard +"async-sema@npm:^3.1.1": + version: 3.1.1 + resolution: "async-sema@npm:3.1.1" + checksum: 07b8c51f6cab107417ecdd8126b7a9fe5a75151b7f69fdd420dcc8ee08f9e37c473a217247e894b56e999b088b32e902dbe41637e4e9b594d3f8dfcdddfadc5e + languageName: node + linkType: hard + "async@npm:^3.1.0, async@npm:^3.2.3": version: 3.2.4 resolution: "async@npm:3.2.4" @@ -7972,7 +7981,7 @@ __metadata: languageName: node linkType: hard -"micromatch@npm:4.0.5, micromatch@npm:^4.0.4": +"micromatch@npm:4.0.5, micromatch@npm:^4.0.4, micromatch@npm:^4.0.5": version: 4.0.5 resolution: "micromatch@npm:4.0.5" dependencies: