Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 429 errors on OVSX requests #14030

Merged
merged 5 commits into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions dev-packages/cli/src/download-plugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ export interface DownloadPluginsOptions {
* Fetch plugins in parallel
*/
parallel?: boolean;

rateLimit?: number;
}

interface PluginDownload {
Expand All @@ -65,16 +63,19 @@ interface PluginDownload {
version?: string | undefined
}

export default async function downloadPlugins(ovsxClient: OVSXClient, requestService: RequestService, options: DownloadPluginsOptions = {}): Promise<void> {
export default async function downloadPlugins(
ovsxClient: OVSXClient,
rateLimiter: RateLimiter,
requestService: RequestService,
options: DownloadPluginsOptions = {}
): Promise<void> {
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.
Expand Down
12 changes: 7 additions & 5 deletions dev-packages/cli/src/theia.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -389,7 +390,7 @@ async function theiaCli(): Promise<void> {
'rate-limit': {
describe: 'Amount of maximum open-vsx requests per second',
number: true,
default: 15
default: OVSX_RATE_LIMIT
},
'proxy-url': {
describe: 'Proxy URL'
Expand All @@ -415,22 +416,23 @@ async function theiaCli(): Promise<void> {
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);
});
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<{
Expand Down
1 change: 1 addition & 0 deletions dev-packages/ovsx-client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
},
"dependencies": {
"@theia/request": "1.52.0",
"limiter": "^2.1.0",
"semver": "^7.5.4",
"tslib": "^2.6.2"
}
Expand Down
2 changes: 1 addition & 1 deletion dev-packages/ovsx-client/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
5 changes: 4 additions & 1 deletion dev-packages/ovsx-client/src/ovsx-api-filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,13 @@ export class OVSXApiFilterImpl implements OVSXApiFilter {

protected async queryLatestCompatibleExtension(query: VSXQueryOptions): Promise<VSXExtensionRaw | undefined> {
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);
Expand All @@ -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;
}
Expand Down
35 changes: 28 additions & 7 deletions dev-packages/ovsx-client/src/ovsx-http-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,26 @@

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 {

/**
* @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<string, OVSXClient> = 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<VSXSearchResult> {
Expand All @@ -43,10 +47,27 @@ export class OVSXHttpClient implements OVSXClient {
}

protected async requestJson<R>(url: string): Promise<R> {
return RequestContext.asJson<R>(await this.requestService.request({
url,
headers: { 'Accept': 'application/json' }
}));
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) {
tsmaeder marked this conversation as resolved.
Show resolved Hide resolved
// If there are still more attempts left, retry the request with a higher token count
if (i < attempts - 1) {
continue;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a last nitpick, can we get a warning if we ran into 429's even if we eventually succeed? IMO it's important info also for open-vsx.

} else {
console.warn('OVSX rate limit exceeded. Consider reducing the rate limit.');
}
}
return RequestContext.asJson<R>(context);
}
throw new Error('Failed to fetch data from OVSX.');
}

protected buildUrl(url: string, query?: object): string {
Expand Down
1 change: 1 addition & 0 deletions packages/vsx-registry/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions packages/vsx-registry/src/common/vsx-environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export const VSX_ENVIRONMENT_PATH = '/services/vsx-environment';

export const VSXEnvironment = Symbol('VSXEnvironment');
export interface VSXEnvironment {
getRateLimit(): Promise<number>;
getRegistryUri(): Promise<string>;
getRegistryApiUri(): Promise<string>;
getVscodeApiVersion(): Promise<string>;
Expand Down
13 changes: 10 additions & 3 deletions packages/vsx-registry/src/common/vsx-registry-common-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -34,10 +35,15 @@ export default new ContainerModule(bind => {
.all([
vsxEnvironment.getRegistryApiUri(),
vsxEnvironment.getOvsxRouterConfig?.(),
vsxEnvironment.getRateLimit()
])
.then<OVSXClient>(async ([apiUrl, ovsxRouterConfig]) => {
.then<OVSXClient>(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)),
Expand All @@ -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
Expand Down
6 changes: 5 additions & 1 deletion packages/vsx-registry/src/node/vsx-cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
}
}
4 changes: 4 additions & 0 deletions packages/vsx-registry/src/node/vsx-environment-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ export class VSXEnvironmentImpl implements VSXEnvironment {
@inject(VsxCli)
protected vsxCli: VsxCli;

async getRateLimit(): Promise<number> {
return this.vsxCli.ovsxRateLimit;
}

async getRegistryUri(): Promise<string> {
return this._registryUri.toString(true);
}
Expand Down
Loading