diff --git a/package-lock.json b/package-lock.json index afce19ba..87143e96 100644 --- a/package-lock.json +++ b/package-lock.json @@ -91,6 +91,7 @@ "patch-package": "^8.0.1", "prom-client": "^15.1.3", "qs": "^6.15.2", + "rate-limiter-flexible": "^11.2.0", "react-use-timeout": "^1.0.0", "round-robin-js": "^3.0.10", "serialize-error": "^13.0.1", @@ -17795,6 +17796,12 @@ "node": "^14.13.1 || >=16.0.0" } }, + "node_modules/rate-limiter-flexible": { + "version": "11.2.0", + "resolved": "https://registry.npmjs.org/rate-limiter-flexible/-/rate-limiter-flexible-11.2.0.tgz", + "integrity": "sha512-L0eIK+BmFMi6NcvmtEg7RSswOFKi9MMD5RhBIypFfOve+G6Jl1Xbb8qEHgK3uRzNtkOFYF0L9f7P4rSf2PnUVw==", + "license": "ISC" + }, "node_modules/raw-body": { "version": "3.0.2", "resolved": "https://registry.npmjs.org/raw-body/-/raw-body-3.0.2.tgz", diff --git a/package.json b/package.json index e7563102..e8e7a44f 100644 --- a/package.json +++ b/package.json @@ -132,6 +132,7 @@ "patch-package": "^8.0.1", "prom-client": "^15.1.3", "qs": "^6.15.2", + "rate-limiter-flexible": "^11.2.0", "react-use-timeout": "^1.0.0", "round-robin-js": "^3.0.10", "serialize-error": "^13.0.1", diff --git a/src/backend/common/vendor/musicbrainz/MusicbrainzApiClient.ts b/src/backend/common/vendor/musicbrainz/MusicbrainzApiClient.ts index bd592069..5daf5882 100644 --- a/src/backend/common/vendor/musicbrainz/MusicbrainzApiClient.ts +++ b/src/backend/common/vendor/musicbrainz/MusicbrainzApiClient.ts @@ -21,7 +21,7 @@ import type {IRecordingMSList} from '../../transforms/MusicbrainzTransformer.ts' import dayjs, { type Dayjs } from 'dayjs'; import { artistCreditsToNames } from '../../../../core/StringUtils.ts'; import { isrcNoHyphens } from '../../../../core/PlayUtils.ts'; - +import { RateLimiterMemory, RateLimiterQueue } from 'rate-limiter-flexible'; export interface SubmitResponse { payload?: { ignored_listens: number @@ -33,8 +33,7 @@ export interface SubmitResponse { export interface MusicbrainzApiConfig extends MusicbrainzApiConfigData { api: MusicBrainzApi, hostname: string - minRequestIntervalDuration: number - lastRequest: Dayjs + reqQueue: RateLimiterQueue } export interface MusicbrainzApiClientConfig { @@ -58,7 +57,7 @@ export class MusicbrainzApiClient extends AbstractApiClient { cache: Cacheable; protected asyncStore: AsyncLocalStorage; - constructor(name: any, config: MusicbrainzApiClientConfig, options: AbstractApiOptions & {cache?: Cacheable, logUrl?: boolean}) { + constructor(name: any, config: MusicbrainzApiClientConfig, options: AbstractApiOptions & {cache?: Cacheable, logUrl?: boolean, reqQueueDuration?: number}) { super('Musicbrainz', name, config, options); this.asyncStore = new AsyncLocalStorage(); @@ -71,8 +70,7 @@ export class MusicbrainzApiClient extends AbstractApiClient { const mbApiConfig: Omit = { ...mbConfig, hostname: u.url.hostname, - minRequestIntervalDuration: 1000, - lastRequest: dayjs().subtract(1000 + 1000, 'ms') + reqQueue: new RateLimiterQueue(new RateLimiterMemory({points: 1, duration: options?.reqQueueDuration ?? 1}), {maxQueueSize: 20}) } if(mb === undefined) { const api = new MusicBrainzApi({ @@ -117,7 +115,6 @@ export class MusicbrainzApiClient extends AbstractApiClient { let apiConfig = this.rrApis.next().value; const { - timeout = 30000, cacheKey, useCachedResult = true } = options || {}; @@ -140,15 +137,8 @@ export class MusicbrainzApiClient extends AbstractApiClient { const triedHosts: string[] = []; while(!triedHosts.includes(apiConfig.hostname)) { - // keep track of last request init at and wait until at least 1 second since that - // to help prevent rate limiting - const sinceLast = dayjs().diff(apiConfig.lastRequest, 'ms'); - const waitTime = Math.max(0, apiConfig.minRequestIntervalDuration - sinceLast); - apiConfig.lastRequest = dayjs().add(waitTime, 'ms'); - //this.logger.trace(`Waiting ${waitTime}ms to call ${apiConfig.hostname} request at ${apiConfig.lastRequest.toISOString()}`) - if(waitTime > 0) { - await sleep(waitTime); - } + // rate limit at 1req/s + const res = await apiConfig.reqQueue.removeTokens(1); try { const res = await this.callApiEndpoint(apiConfig.api, func, options); @@ -183,7 +173,7 @@ export class MusicbrainzApiClient extends AbstractApiClient { } } - protected callApiEndpoint = async(mbApi: MusicBrainzApi, func: (mb: MusicBrainzApi) => Promise, options?: { timeout?: number, cacheKey?: string }): Promise => { + protected async callApiEndpoint(mbApi: MusicBrainzApi, func: (mb: MusicBrainzApi) => Promise, options?: { timeout?: number, cacheKey?: string }): Promise { const { timeout = 30000, cacheKey diff --git a/src/backend/tests/musicbrainz/musicbrainz.test.ts b/src/backend/tests/musicbrainz/musicbrainz.test.ts index aa1164b2..6395bcb4 100644 --- a/src/backend/tests/musicbrainz/musicbrainz.test.ts +++ b/src/backend/tests/musicbrainz/musicbrainz.test.ts @@ -13,10 +13,13 @@ import type {MusicbrainzApiConfigData} from '../../common/infrastructure/Atomic. import { MockNetworkError, withRequestInterception } from '../utils/networking.ts'; import { http, HttpResponse, delay } from "msw"; import { generatePlay, withBrainz } from '../../../core/tests/utils/PlayTestUtils.ts'; -import { intersect, missingMbidTypes } from '../../utils.ts'; +import { intersect, missingMbidTypes, sleep } from '../../utils.ts'; import { CoverArtApiClient, type CoverArtApiConfig } from '../../common/vendor/musicbrainz/CoverArtApiClient.ts'; import { artistNamesToCredits, artistNameToCredit } from '../../../core/StringUtils.ts'; import dayjs from 'dayjs'; +import { MusicbrainzApiClient } from '../../common/vendor/musicbrainz/MusicbrainzApiClient.ts'; +import {spy} from 'sinon'; +import type { MusicBrainzApi } from 'musicbrainz-api'; chai.use(asPromised); @@ -48,6 +51,12 @@ const createCoverArtApi = (config: CoverArtApiConfig = {}) => { return new CoverArtApiClient('test', config, {logger: loggerTest}); } +class MusicbrainzApiTestClient extends MusicbrainzApiClient { + callApiEndpoint = async(mbApi: MusicBrainzApi, func: (mb: MusicBrainzApi) => Promise, options?: { timeout?: number, cacheKey?: string }): Promise => { + return super.callApiEndpoint(mbApi, func,options); + } +} + describe('Musicbrainz API', function () { before(function () { @@ -567,6 +576,28 @@ describe('Musicbrainz API', function () { }); }); + it('rate limits to 1req per duration', async function () { + await withRequestInterception([ + http.get(/mbtest\.local\/?\/ws/, async () => { + return HttpResponse.json([], {status: 200}); + }) + ], async function () { + + const apiClient = new MusicbrainzApiTestClient('test', {apis: [{url: 'https://mbtest.local', contact: 'test@email.com'}]}, { + logger: loggerTest, + cache: memorycache(), + reqQueueDuration: 0.01 + }); + const sp = spy(apiClient, 'callApiEndpoint'); + + for(let i = 0; i < 5; i++) { + apiClient.callApi((mb) => mb.search('recording', {})).then(() => null) + } + await sleep(30); + expect(sp.callCount).to.eq(3); + })(); + }); + }); describe('#MB Missing Types', function() {