From ade76f12ff4154498152e9d1fa655209d139525d Mon Sep 17 00:00:00 2001 From: Mark Sujew Date: Fri, 9 Aug 2024 10:50:12 +0000 Subject: [PATCH 1/5] Fix 429 errors on OVSX requests --- .github/workflows/ci-cd.yml | 2 +- dev-packages/cli/src/download-plugins.ts | 11 ++++---- dev-packages/cli/src/theia.ts | 12 ++++---- dev-packages/ovsx-client/package.json | 1 + dev-packages/ovsx-client/src/index.ts | 2 +- .../ovsx-client/src/ovsx-http-client.ts | 28 ++++++++++++++----- packages/vsx-registry/package.json | 1 + .../src/common/vsx-environment.ts | 1 + .../src/common/vsx-registry-common-module.ts | 13 +++++++-- packages/vsx-registry/src/node/vsx-cli.ts | 6 +++- .../src/node/vsx-environment-impl.ts | 4 +++ 11 files changed, 58 insertions(+), 23 deletions(-) diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index e563ca511eb20..60732cd06ff46 100644 --- a/.github/workflows/ci-cd.yml +++ b/.github/workflows/ci-cd.yml @@ -86,7 +86,7 @@ jobs: if: runner.os == 'Linux' shell: bash run: | - yarn -s download:plugins --rate-limit 3 + yarn -s download:plugins --rate-limit 5 - name: Build shell: bash diff --git a/dev-packages/cli/src/download-plugins.ts b/dev-packages/cli/src/download-plugins.ts index dee69530e72c8..ca17aa88cc989 100644 --- a/dev-packages/cli/src/download-plugins.ts +++ b/dev-packages/cli/src/download-plugins.ts @@ -55,8 +55,6 @@ export interface DownloadPluginsOptions { * Fetch plugins in parallel */ parallel?: boolean; - - rateLimit?: number; } interface PluginDownload { @@ -65,16 +63,19 @@ interface PluginDownload { version?: string | undefined } -export default async function downloadPlugins(ovsxClient: OVSXClient, requestService: RequestService, options: DownloadPluginsOptions = {}): Promise { +export default async function downloadPlugins( + ovsxClient: OVSXClient, + rateLimiter: RateLimiter, + requestService: RequestService, + options: DownloadPluginsOptions = {} +): Promise { const { packed = false, ignoreErrors = false, apiVersion = DEFAULT_SUPPORTED_API_VERSION, - rateLimit = 15, parallel = true } = options; - const rateLimiter = new RateLimiter({ tokensPerInterval: rateLimit, interval: 'second' }); const apiFilter = new OVSXApiFilterImpl(ovsxClient, apiVersion); // Collect the list of failures to be appended at the end of the script. diff --git a/dev-packages/cli/src/theia.ts b/dev-packages/cli/src/theia.ts index d215c578de288..6da5e2c498b1f 100644 --- a/dev-packages/cli/src/theia.ts +++ b/dev-packages/cli/src/theia.ts @@ -24,9 +24,10 @@ import { ApplicationProps, DEFAULT_SUPPORTED_API_VERSION } from '@theia/applicat import checkDependencies from './check-dependencies'; import downloadPlugins from './download-plugins'; import runTest from './run-test'; +import { RateLimiter } from 'limiter'; import { LocalizationManager, extract } from '@theia/localization-manager'; import { NodeRequestService } from '@theia/request/lib/node-request-service'; -import { ExtensionIdMatchesFilterFactory, OVSXClient, OVSXHttpClient, OVSXRouterClient, RequestContainsFilterFactory } from '@theia/ovsx-client'; +import { ExtensionIdMatchesFilterFactory, OVSX_RATE_LIMIT, OVSXClient, OVSXHttpClient, OVSXRouterClient, RequestContainsFilterFactory } from '@theia/ovsx-client'; const { executablePath } = require('puppeteer'); @@ -389,7 +390,7 @@ async function theiaCli(): Promise { 'rate-limit': { describe: 'Amount of maximum open-vsx requests per second', number: true, - default: 15 + default: OVSX_RATE_LIMIT }, 'proxy-url': { describe: 'Proxy URL' @@ -415,6 +416,7 @@ async function theiaCli(): Promise { strictSSL: strictSsl }); let client: OVSXClient | undefined; + const rateLimiter = new RateLimiter({ tokensPerInterval: options.rateLimit, interval: 'second' }); if (ovsxRouterConfig) { const routerConfig = await fs.promises.readFile(ovsxRouterConfig, 'utf8').then(JSON.parse, error => { console.error(error); @@ -422,15 +424,15 @@ async function theiaCli(): Promise { if (routerConfig) { client = await OVSXRouterClient.FromConfig( routerConfig, - OVSXHttpClient.createClientFactory(requestService), + OVSXHttpClient.createClientFactory(requestService, rateLimiter), [RequestContainsFilterFactory, ExtensionIdMatchesFilterFactory] ); } } if (!client) { - client = new OVSXHttpClient(apiUrl, requestService); + client = new OVSXHttpClient(apiUrl, requestService, rateLimiter); } - await downloadPlugins(client, requestService, options); + await downloadPlugins(client, rateLimiter, requestService, options); }, }) .command<{ diff --git a/dev-packages/ovsx-client/package.json b/dev-packages/ovsx-client/package.json index edbc5bb930921..3ef7f91b6d1bd 100644 --- a/dev-packages/ovsx-client/package.json +++ b/dev-packages/ovsx-client/package.json @@ -30,6 +30,7 @@ }, "dependencies": { "@theia/request": "1.52.0", + "limiter": "^2.1.0", "semver": "^7.5.4", "tslib": "^2.6.2" } diff --git a/dev-packages/ovsx-client/src/index.ts b/dev-packages/ovsx-client/src/index.ts index 42455e7897de3..bc836bde8ce98 100644 --- a/dev-packages/ovsx-client/src/index.ts +++ b/dev-packages/ovsx-client/src/index.ts @@ -15,7 +15,7 @@ // ***************************************************************************** export { OVSXApiFilter, OVSXApiFilterImpl, OVSXApiFilterProvider } from './ovsx-api-filter'; -export { OVSXHttpClient } from './ovsx-http-client'; +export { OVSXHttpClient, OVSX_RATE_LIMIT } from './ovsx-http-client'; export { OVSXMockClient } from './ovsx-mock-client'; export { OVSXRouterClient, OVSXRouterConfig, OVSXRouterFilterFactory as FilterFactory } from './ovsx-router-client'; export * from './ovsx-router-filters'; diff --git a/dev-packages/ovsx-client/src/ovsx-http-client.ts b/dev-packages/ovsx-client/src/ovsx-http-client.ts index a810680711f30..726712040046e 100644 --- a/dev-packages/ovsx-client/src/ovsx-http-client.ts +++ b/dev-packages/ovsx-client/src/ovsx-http-client.ts @@ -16,6 +16,9 @@ import { OVSXClient, VSXQueryOptions, VSXQueryResult, VSXSearchOptions, VSXSearchResult } from './ovsx-types'; import { RequestContext, RequestService } from '@theia/request'; +import { RateLimiter } from 'limiter'; + +export const OVSX_RATE_LIMIT = 15; export class OVSXHttpClient implements OVSXClient { @@ -23,15 +26,16 @@ export class OVSXHttpClient implements OVSXClient { * @param requestService * @returns factory that will cache clients based on the requested input URL. */ - static createClientFactory(requestService: RequestService): (url: string) => OVSXClient { + static createClientFactory(requestService: RequestService, rateLimiter?: RateLimiter): (url: string) => OVSXClient { // eslint-disable-next-line no-null/no-null const cachedClients: Record = Object.create(null); - return url => cachedClients[url] ??= new this(url, requestService); + return url => cachedClients[url] ??= new this(url, requestService, rateLimiter); } constructor( protected vsxRegistryUrl: string, - protected requestService: RequestService + protected requestService: RequestService, + protected rateLimiter = new RateLimiter({ tokensPerInterval: OVSX_RATE_LIMIT, interval: 'second' }) ) { } search(searchOptions?: VSXSearchOptions): Promise { @@ -43,10 +47,20 @@ export class OVSXHttpClient implements OVSXClient { } protected async requestJson(url: string): Promise { - return RequestContext.asJson(await this.requestService.request({ - url, - headers: { 'Accept': 'application/json' } - })); + for (let i = 0; i < 10; i++) { + await this.rateLimiter.removeTokens(1); + const context = await this.requestService.request({ + url, + headers: { 'Accept': 'application/json' } + }); + if (context.res.statusCode === 429) { + // Wait a bit more, since we performed too many requests + await this.rateLimiter.removeTokens(1); + continue; + } + return RequestContext.asJson(context); + } + throw new Error('Request timed out due to too many requests'); } protected buildUrl(url: string, query?: object): string { diff --git a/packages/vsx-registry/package.json b/packages/vsx-registry/package.json index 25d2e4b2d37d5..fa9adcd5c5f2b 100644 --- a/packages/vsx-registry/package.json +++ b/packages/vsx-registry/package.json @@ -11,6 +11,7 @@ "@theia/plugin-ext-vscode": "1.52.0", "@theia/preferences": "1.52.0", "@theia/workspace": "1.52.0", + "limiter": "^2.1.0", "luxon": "^2.4.0", "p-debounce": "^2.1.0", "semver": "^7.5.4", diff --git a/packages/vsx-registry/src/common/vsx-environment.ts b/packages/vsx-registry/src/common/vsx-environment.ts index 4366d5ef56dc7..8b1ef42657bce 100644 --- a/packages/vsx-registry/src/common/vsx-environment.ts +++ b/packages/vsx-registry/src/common/vsx-environment.ts @@ -20,6 +20,7 @@ export const VSX_ENVIRONMENT_PATH = '/services/vsx-environment'; export const VSXEnvironment = Symbol('VSXEnvironment'); export interface VSXEnvironment { + getRateLimit(): Promise; getRegistryUri(): Promise; getRegistryApiUri(): Promise; getVscodeApiVersion(): Promise; diff --git a/packages/vsx-registry/src/common/vsx-registry-common-module.ts b/packages/vsx-registry/src/common/vsx-registry-common-module.ts index fc7fd7de4f3db..5a0940c903d9e 100644 --- a/packages/vsx-registry/src/common/vsx-registry-common-module.ts +++ b/packages/vsx-registry/src/common/vsx-registry-common-module.ts @@ -21,6 +21,7 @@ import { ExtensionIdMatchesFilterFactory, OVSXApiFilter, OVSXApiFilterImpl, OVSXApiFilterProvider, OVSXClient, OVSXHttpClient, OVSXRouterClient, RequestContainsFilterFactory } from '@theia/ovsx-client'; import { VSXEnvironment } from './vsx-environment'; +import { RateLimiter } from 'limiter'; export default new ContainerModule(bind => { bind(OVSXUrlResolver) @@ -34,10 +35,15 @@ export default new ContainerModule(bind => { .all([ vsxEnvironment.getRegistryApiUri(), vsxEnvironment.getOvsxRouterConfig?.(), + vsxEnvironment.getRateLimit() ]) - .then(async ([apiUrl, ovsxRouterConfig]) => { + .then(async ([apiUrl, ovsxRouterConfig, rateLimit]) => { + const rateLimiter = new RateLimiter({ + interval: 'second', + tokensPerInterval: rateLimit + }); if (ovsxRouterConfig) { - const clientFactory = OVSXHttpClient.createClientFactory(requestService); + const clientFactory = OVSXHttpClient.createClientFactory(requestService, rateLimiter); return OVSXRouterClient.FromConfig( ovsxRouterConfig, async url => clientFactory(await urlResolver(url)), @@ -46,7 +52,8 @@ export default new ContainerModule(bind => { } return new OVSXHttpClient( await urlResolver(apiUrl), - requestService + requestService, + rateLimiter ); }); // reuse the promise for subsequent calls to this provider diff --git a/packages/vsx-registry/src/node/vsx-cli.ts b/packages/vsx-registry/src/node/vsx-cli.ts index aeef403518eb6..2b3efab2a4cc8 100644 --- a/packages/vsx-registry/src/node/vsx-cli.ts +++ b/packages/vsx-registry/src/node/vsx-cli.ts @@ -17,17 +17,19 @@ import { CliContribution } from '@theia/core/lib/node'; import { injectable } from '@theia/core/shared/inversify'; import { Argv } from '@theia/core/shared/yargs'; -import { OVSXRouterConfig } from '@theia/ovsx-client'; +import { OVSX_RATE_LIMIT, OVSXRouterConfig } from '@theia/ovsx-client'; import * as fs from 'fs'; @injectable() export class VsxCli implements CliContribution { ovsxRouterConfig: OVSXRouterConfig | undefined; + ovsxRateLimit: number; pluginsToInstall: string[] = []; configure(conf: Argv<{}>): void { conf.option('ovsx-router-config', { description: 'JSON configuration file for the OVSX router client', type: 'string' }); + conf.option('ovsx-rate-limit', { description: 'Limits the number of requests to OVSX per second', type: 'number', default: OVSX_RATE_LIMIT }); conf.option('install-plugin', { alias: 'install-extension', nargs: 1, @@ -47,5 +49,7 @@ export class VsxCli implements CliContribution { if (Array.isArray(pluginsToInstall)) { this.pluginsToInstall = pluginsToInstall; } + const ovsxRateLimit = args.ovsxRateLimit; + this.ovsxRateLimit = typeof ovsxRateLimit === 'number' ? ovsxRateLimit : OVSX_RATE_LIMIT; } } diff --git a/packages/vsx-registry/src/node/vsx-environment-impl.ts b/packages/vsx-registry/src/node/vsx-environment-impl.ts index ff094b09c41f7..8515650c5d119 100644 --- a/packages/vsx-registry/src/node/vsx-environment-impl.ts +++ b/packages/vsx-registry/src/node/vsx-environment-impl.ts @@ -32,6 +32,10 @@ export class VSXEnvironmentImpl implements VSXEnvironment { @inject(VsxCli) protected vsxCli: VsxCli; + async getRateLimit(): Promise { + return this.vsxCli.ovsxRateLimit; + } + async getRegistryUri(): Promise { return this._registryUri.toString(true); } From e97839de3ece0b4630097636ca30aaa8daffe131 Mon Sep 17 00:00:00 2001 From: Mark Sujew Date: Mon, 12 Aug 2024 21:08:21 +0200 Subject: [PATCH 2/5] Remove retry-loop --- .../ovsx-client/src/ovsx-http-client.ts | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/dev-packages/ovsx-client/src/ovsx-http-client.ts b/dev-packages/ovsx-client/src/ovsx-http-client.ts index 726712040046e..73ea8cd4d8640 100644 --- a/dev-packages/ovsx-client/src/ovsx-http-client.ts +++ b/dev-packages/ovsx-client/src/ovsx-http-client.ts @@ -47,20 +47,15 @@ export class OVSXHttpClient implements OVSXClient { } protected async requestJson(url: string): Promise { - for (let i = 0; i < 10; i++) { - await this.rateLimiter.removeTokens(1); - const context = await this.requestService.request({ - url, - headers: { 'Accept': 'application/json' } - }); - if (context.res.statusCode === 429) { - // Wait a bit more, since we performed too many requests - await this.rateLimiter.removeTokens(1); - continue; - } - return RequestContext.asJson(context); + await this.rateLimiter.removeTokens(1); + const context = await this.requestService.request({ + url, + headers: { 'Accept': 'application/json' } + }); + if (context.res.statusCode === 429) { + console.warn('OVSX rate limit exceeded. Consider reducing the rate limit.'); } - throw new Error('Request timed out due to too many requests'); + return RequestContext.asJson(context); } protected buildUrl(url: string, query?: object): string { From c877cb899a4a8ee5ee04723c2d151de79bd84bb3 Mon Sep 17 00:00:00 2001 From: Mark Sujew Date: Mon, 12 Aug 2024 21:21:45 +0200 Subject: [PATCH 3/5] Reduce rate-limit again --- .github/workflows/ci-cd.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index 60732cd06ff46..e563ca511eb20 100644 --- a/.github/workflows/ci-cd.yml +++ b/.github/workflows/ci-cd.yml @@ -86,7 +86,7 @@ jobs: if: runner.os == 'Linux' shell: bash run: | - yarn -s download:plugins --rate-limit 5 + yarn -s download:plugins --rate-limit 3 - name: Build shell: bash From d3611ea2663f5e8ae99e92c079bf5aebf1b379d3 Mon Sep 17 00:00:00 2001 From: Mark Sujew Date: Tue, 13 Aug 2024 12:06:21 +0200 Subject: [PATCH 4/5] Re-introduce loop with exponential backoff --- .../ovsx-client/src/ovsx-api-filter.ts | 5 +++- .../ovsx-client/src/ovsx-http-client.ts | 28 +++++++++++++------ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/dev-packages/ovsx-client/src/ovsx-api-filter.ts b/dev-packages/ovsx-client/src/ovsx-api-filter.ts index a470a4ee3f989..17a882b1f9796 100644 --- a/dev-packages/ovsx-client/src/ovsx-api-filter.ts +++ b/dev-packages/ovsx-client/src/ovsx-api-filter.ts @@ -67,12 +67,13 @@ export class OVSXApiFilterImpl implements OVSXApiFilter { protected async queryLatestCompatibleExtension(query: VSXQueryOptions): Promise { let offset = 0; + let size = 5; let loop = true; while (loop) { const queryOptions: VSXQueryOptions = { ...query, offset, - size: 5 // there is a great chance that the newest version will work + size // there is a great chance that the newest version will work }; const results = await this.client.query(queryOptions); const compatibleExtension = this.getLatestCompatibleExtension(results.extensions); @@ -83,6 +84,8 @@ export class OVSXApiFilterImpl implements OVSXApiFilter { offset += results.extensions.length; // Continue querying if there are more extensions available loop = results.totalSize > offset; + // Adjust the size to fetch more extensions next time + size = Math.min(size * 2, 100); } return undefined; } diff --git a/dev-packages/ovsx-client/src/ovsx-http-client.ts b/dev-packages/ovsx-client/src/ovsx-http-client.ts index 73ea8cd4d8640..9f69c05611310 100644 --- a/dev-packages/ovsx-client/src/ovsx-http-client.ts +++ b/dev-packages/ovsx-client/src/ovsx-http-client.ts @@ -47,15 +47,27 @@ export class OVSXHttpClient implements OVSXClient { } protected async requestJson(url: string): Promise { - await this.rateLimiter.removeTokens(1); - const context = await this.requestService.request({ - url, - headers: { 'Accept': 'application/json' } - }); - if (context.res.statusCode === 429) { - console.warn('OVSX rate limit exceeded. Consider reducing the rate limit.'); + const attempts = 5; + for (let i = 0; i < attempts; i++) { + // Use 1, 2, 4, 8, 16 tokens for each attempt + const tokenCount = Math.pow(2, i); + await this.rateLimiter.removeTokens(tokenCount); + console.log('Sending request at: ' + Date.now()); + const context = await this.requestService.request({ + url, + headers: { 'Accept': 'application/json' } + }); + if (context.res.statusCode === 429) { + // If there are still more attempts left, retry the request with a higher token count + if (i < attempts - 1) { + continue; + } else { + console.warn('OVSX rate limit exceeded. Consider reducing the rate limit.'); + } + } + return RequestContext.asJson(context); } - return RequestContext.asJson(context); + throw new Error('Failed to fetch data from OVSX.'); } protected buildUrl(url: string, query?: object): string { From 0e9857de15f716da7bc79cb1b5b9696a64c148fd Mon Sep 17 00:00:00 2001 From: Mark Sujew Date: Tue, 13 Aug 2024 13:56:48 +0200 Subject: [PATCH 5/5] Always log rate limit warning --- dev-packages/ovsx-client/src/ovsx-http-client.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/dev-packages/ovsx-client/src/ovsx-http-client.ts b/dev-packages/ovsx-client/src/ovsx-http-client.ts index 9f69c05611310..e6e5c3298827b 100644 --- a/dev-packages/ovsx-client/src/ovsx-http-client.ts +++ b/dev-packages/ovsx-client/src/ovsx-http-client.ts @@ -52,17 +52,15 @@ export class OVSXHttpClient implements OVSXClient { // Use 1, 2, 4, 8, 16 tokens for each attempt const tokenCount = Math.pow(2, i); await this.rateLimiter.removeTokens(tokenCount); - console.log('Sending request at: ' + Date.now()); const context = await this.requestService.request({ url, headers: { 'Accept': 'application/json' } }); if (context.res.statusCode === 429) { + console.warn('OVSX rate limit exceeded. Consider reducing the rate limit.'); // If there are still more attempts left, retry the request with a higher token count if (i < attempts - 1) { continue; - } else { - console.warn('OVSX rate limit exceeded. Consider reducing the rate limit.'); } } return RequestContext.asJson(context);