Skip to content

Commit

Permalink
feat: add ability to configure app to skip throttling on rate limits (#…
Browse files Browse the repository at this point in the history
…5517)

Sometimes we don't want to throttle all tasks in case rate limits from
one installation block other installations.
  • Loading branch information
chingor13 authored Sep 27, 2024
1 parent 67b62d9 commit fc03235
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 2 deletions.
3 changes: 3 additions & 0 deletions packages/gcf-utils/src/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ export interface WrapConfig {
// When batch scheduling cron requests, delay groups of requests by X seconds
// to avoid flooding
flowControlDelayInSeconds: number;
// Whether or not to throttle on rate limiting
throttleOnRateLimits: boolean;
}

// Default configuration options
Expand All @@ -42,4 +44,5 @@ export const DEFAULT_WRAP_CONFIG: WrapConfig = {
maxRetries: 10,
maxPubSubRetries: 0,
flowControlDelayInSeconds: DEFAULT_FLOW_CONTROL_DELAY_IN_SECOND,
throttleOnRateLimits: true,
};
8 changes: 6 additions & 2 deletions packages/gcf-utils/src/gcf-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@ export interface WrapOptions {

// Delay in task scheduling flow control
flowControlDelayInSeconds?: number;

// Whether or not to throttle on rate limiting. Defaults to `true`.
throttleOnRateLimits?: boolean;
}

// Default logger, in general, you will want to configure a local logger that
Expand Down Expand Up @@ -529,8 +532,9 @@ export class GCFBootstrapper {
requestLogger.warn('Rate limit exceeded', rateLimits);
// On GitHub quota issues, return a 503 to throttle our task queues
// https://cloud.google.com/tasks/docs/common-pitfalls#backoff_errors_and_enforced_rates
response.status(503).send({
statusCode: 503,
const statusCode = wrapConfig.throttleOnRateLimits ? 503 : 500;
response.status(statusCode).send({
statusCode: statusCode,
body: JSON.stringify({
...rateLimits,
message: 'Rate Limited',
Expand Down
48 changes: 48 additions & 0 deletions packages/gcf-utils/test/gcf-bootstrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,54 @@ describe('GCFBootstrapper', () => {
assert.strictEqual(response.statusCode, 503);
});

it('returns 500 on rate limit errors if configured to skip throttling', async () => {
await mockBootstrapper({throttleOnRateLimits: false}, async app => {
app.on('issues', async () => {
throw new RequestError(
'API rate limit exceeded for user ID 3456',
403,
{
response: {
headers: {
'x-ratelimit-remaining': '0',
'x-ratelimit-reset': '1653880306',
'x-ratelimit-limit': '5000',
'x-ratelimit-resource': 'core',
},
status: 403,
url: '',
data: '',
},
request: {
headers: {},
method: 'POST',
url: '',
},
}
);
});
});
req.body = {
installation: {id: 1},
};
req.headers = {};
req.headers['x-github-event'] = 'issues';
req.headers['x-github-delivery'] = '123';
req.headers['x-cloudtasks-taskname'] = 'my-task';

await handler(req, response);

sinon.assert.calledOnce(configStub);
sinon.assert.notCalled(issueSpy);
sinon.assert.notCalled(repositoryCronSpy);
sinon.assert.notCalled(installationCronSpy);
sinon.assert.notCalled(globalCronSpy);
sinon.assert.notCalled(sendStatusStub);
sinon.assert.called(sendStub);

assert.strictEqual(response.statusCode, 500);
});

it('returns 503 on secondary rate limit errors', async () => {
await mockBootstrapper(undefined, async app => {
app.on('issues', async () => {
Expand Down

0 comments on commit fc03235

Please sign in to comment.