Skip to content

Commit

Permalink
Make it very obvious when the rate limit has been hit
Browse files Browse the repository at this point in the history
Fixes #4546
  • Loading branch information
alexr00 committed Mar 28, 2023
1 parent 3787394 commit 54691eb
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 3 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2358,6 +2358,7 @@
"@vscode/extension-telemetry": "0.7.5",
"apollo-boost": "^0.4.9",
"apollo-link-context": "1.0.20",
"cockatiel": "^3.1.1",
"cross-fetch": "3.1.5",
"dayjs": "1.10.4",
"events": "3.2.0",
Expand Down
33 changes: 30 additions & 3 deletions src/github/loggingOctokit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

import { Octokit } from '@octokit/rest';
import { ApolloClient, ApolloQueryResult, FetchResult, MutationOptions, NormalizedCacheObject, OperationVariables, QueryOptions } from 'apollo-boost';
import { bulkhead, BulkheadPolicy } from 'cockatiel';
import * as vscode from 'vscode';
import Logger from '../common/logger';
import { ITelemetry } from '../common/telemetry';
import { RateLimit } from './graphql';
Expand All @@ -17,11 +19,27 @@ interface RestResponse {
}

export class RateLogger {
private bulkhead: BulkheadPolicy = bulkhead(140);
private static ID = 'RateLimit';
private hasLoggedLowRateLimit: boolean = false;

constructor(private readonly telemetry: ITelemetry) { }

public logAndLimit(apiRequest: () => Promise<any>): Promise<any> | undefined {
if (this.bulkhead.executionSlots === 0) {
Logger.error('API call count has exceeded 140 concurrent calls.', RateLogger.ID);
// We have hit more than 140 concurrent API requests.
/* __GDPR__
"pr.highApiCallRate" : {}
*/
this.telemetry.sendTelemetryErrorEvent('pr.highApiCallRate');
vscode.window.showErrorMessage(vscode.l10n.t('The GitHub Pull Requests extension is making too many requests to GitHub. This indicates a bug in the extension. Please file an issue on GitHub and include the output from "GitHub Pull Request".'));
return undefined;
}
Logger.debug(`Extension rate limit remaining: ${this.bulkhead.executionSlots}`, RateLogger.ID);
return this.bulkhead.execute(() => apiRequest());
}

public async logRateLimit(info: string | undefined, result: Promise<{ data: { rateLimit: RateLimit | undefined } | undefined } | undefined>, isRest: boolean = false) {
let rateLimitInfo;
try {
Expand Down Expand Up @@ -70,13 +88,19 @@ export class LoggingApolloClient {
constructor(private readonly _graphql: ApolloClient<NormalizedCacheObject>, private _rateLogger: RateLogger) { };

query<T = any, TVariables = OperationVariables>(options: QueryOptions<TVariables>): Promise<ApolloQueryResult<T>> {
const result = this._graphql.query(options);
const result = this._rateLogger.logAndLimit(() => this._graphql.query(options));
if (result === undefined) {
throw new Error('API call count has exceeded a rate limit.');
}
this._rateLogger.logRateLimit((options.query.definitions[0] as { name: { value: string } | undefined }).name?.value, result as any);
return result;
}

mutate<T = any, TVariables = OperationVariables>(options: MutationOptions<T, TVariables>): Promise<FetchResult<T>> {
const result = this._graphql.mutate(options);
const result = this._rateLogger.logAndLimit(() => this._graphql.mutate(options));
if (result === undefined) {
throw new Error('API call count has exceeded a rate limit.');
}
this._rateLogger.logRateLimit(options.context, result as any);
return result;
}
Expand All @@ -86,7 +110,10 @@ export class LoggingOctokit {
constructor(public readonly api: Octokit, private _rateLogger: RateLogger) { };

async call<T, U>(api: (T) => Promise<U>, args: T): Promise<U> {
const result = api(args);
const result = this._rateLogger.logAndLimit(() => api(args));
if (result === undefined) {
throw new Error('API call count has exceeded a rate limit.');
}
this._rateLogger.logRestRateLimit((api as unknown as { endpoint: { DEFAULTS: { url: string } | undefined } | undefined }).endpoint?.DEFAULTS?.url, result as Promise<unknown> as Promise<RestResponse>);
return result;
}
Expand Down
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1693,6 +1693,11 @@ co@^4.6.0:
resolved "https://registry.yarnpkg.com/co/-/co-4.6.0.tgz#6ea6bdf3d853ae54ccb8e47bfa0bf3f9031fb184"
integrity sha1-bqa989hTrlTMuOR7+gvz+QMfsYQ=

cockatiel@^3.1.1:
version "3.1.1"
resolved "https://registry.yarnpkg.com/cockatiel/-/cockatiel-3.1.1.tgz#82c95dcad673649c43c0a35c424c5d2ad59d4e6b"
integrity sha512-zHMqBGvkZLfMKkBMD+0U8X1nW8zYwMtymgJ8CTknWOmTDpvjEwygtFN4QR9A1iFQDwCbg8g8+B/zVBoxvj1feQ==

color-convert@^1.9.0:
version "1.9.3"
resolved "https://registry.yarnpkg.com/color-convert/-/color-convert-1.9.3.tgz#bb71850690e1f136567de629d2d5471deda4c1e8"
Expand Down

0 comments on commit 54691eb

Please sign in to comment.