Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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: 9 additions & 2 deletions bin/gh-prs-merge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ Options:
--no-ready-drafts Ignore draft PRs instead of marking them ready
--fix Repair mechanical blockers, then re-evaluate
--fix-wait SECONDS How long --fix waits on running checks; default 600
--rate-wait SECONDS Longest wait for a spent GitHub API quota to reset;
default 900. A later reset fails the run, exit 1
-h, --help Show this help

Eligibility:
Expand Down Expand Up @@ -83,7 +85,7 @@ Fixing (--fix):
async function main(argv: string[]): Promise<number> {
const parsed = parseArgs(argv, {
boolean: ['--apply', '--allow-no-checks', '--ready-drafts', '--no-ready-drafts', '--fix', '--no-fix', '-h', '--help'],
string: ['--orgs', '--users', '--limit', '--fix-wait'],
string: ['--orgs', '--users', '--limit', '--fix-wait', '--rate-wait'],
});

if (parsed.flags.has('-h') || parsed.flags.has('--help')) {
Expand Down Expand Up @@ -115,7 +117,12 @@ async function main(argv: string[]): Promise<number> {
throw new UsageError('--fix requires --apply');
}

const summary = await sweep(options, new Gh(), (line) => {
const gh = new Gh({
rateWaitMs: integer(parsed.values, '--rate-wait', 900, { max: 3_600 }) * 1000,
onRateLimit: (text) => process.stderr.write(`WAIT: ${text}\n`),
});

const summary = await sweep(options, gh, (line) => {
const text = render(line);
if (line.kind === 'failed' || line.kind === 'fixme' || line.kind === 'warn') {
process.stderr.write(`${text}\n`);
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@profullstack/cli-tools",
"version": "0.68.2",
"version": "0.68.3",
"private": true,
"description": "Local command-line tools, in TypeScript, exposed on PATH.",
"type": "module",
Expand Down
98 changes: 95 additions & 3 deletions src/gh.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,52 @@ export class GhError extends Error {
}
}

/**
* GitHub refused the call because the token's quota is spent.
*
* Kept apart from GhError because it is not a verdict on the thing asked
* about. The sweep used to read a rate-limited search as "this org is
* inaccessible", warn, find no PRs anywhere, and print an all-zero summary
* with exit 0 — a run that did nothing, reported as a run with nothing to do.
*/
export class RateLimitError extends GhError {
readonly resetAt: Date;

constructor(message: string, resetAt: Date, result?: RunResult) {
super(message, result);
this.name = 'RateLimitError';
this.resetAt = resetAt;
}
}

/**
* Primary ("API rate limit exceeded", GraphQL and REST alike) and secondary
* ("You have exceeded a secondary rate limit") refusals. A rate-limited call
* was not executed, so retrying it — even a merge — cannot do anything twice.
*/
export function isRateLimited(stderr: string): boolean {
return /rate limit (already )?exceeded|exceeded a secondary rate limit|HTTP 429/i.test(stderr);
}

/**
* When the quota that ran out comes back, from `gh api rate_limit`.
*
* That endpoint is free: it does not count against any quota, so it still
* answers while everything else is refused. A secondary limit leaves every
* bucket with quota remaining and documents no reset, so it gets a minute.
*/
export function parseRateLimitReset(raw: unknown, now: number): Date {
const resources = (raw as { resources?: Record<string, { remaining?: unknown; reset?: unknown }> })
?.resources;
let latest = 0;
for (const bucket of Object.values(resources ?? {})) {
if (bucket?.remaining === 0 && typeof bucket.reset === 'number') {
latest = Math.max(latest, bucket.reset * 1000);
}
}
return new Date(latest > now ? latest : now + 60_000);
}

/** Values GitHub documents for `mergeable`, plus the honest fallback. */
export const MERGEABLE = ['MERGEABLE', 'CONFLICTING', 'UNKNOWN'] as const;
export type Mergeable = (typeof MERGEABLE)[number];
Expand Down Expand Up @@ -202,22 +248,68 @@ export function parseMergeAsync(stdout: string): MergeAsyncState | undefined {
export interface GhOptions {
/** Swap in a fake for tests. */
exec?: typeof run;
sleep?: typeof sleep;
/**
* Longest a single rate-limit wait may be. A reset further off than this
* throws RateLimitError instead of sitting silent for most of an hour.
* Default 15 minutes; 0 never waits.
*/
rateWaitMs?: number;
/** Told before each rate-limit wait, so the pause is not a silent hang. */
onRateLimit?: (text: string) => void;
}

export class Gh {
private readonly exec: typeof run;
private readonly sleep: typeof sleep;
private readonly rateWaitMs: number;
private readonly onRateLimit: (text: string) => void;

constructor(options: GhOptions = {}) {
this.exec = options.exec ?? run;
this.sleep = options.sleep ?? sleep;
this.rateWaitMs = options.rateWaitMs ?? 15 * 60_000;
this.onRateLimit = options.onRateLimit ?? (() => {});
}

private async call(args: readonly string[]): Promise<RunResult> {
// GH_PAGER=cat so a configured pager cannot block on a TTY that is not
// there. The bash version set this on every call site and missed none by
// luck rather than design.
return this.exec('gh', args, {
env: { ...process.env, GH_PAGER: 'cat', CLICOLOR: '0' },
});
const env = { ...process.env, GH_PAGER: 'cat', CLICOLOR: '0' };

for (;;) {
const result = await this.exec('gh', args, { env });
if (result.code === 0 || !isRateLimited(result.stderr)) return result;

const resetAt = await this.rateLimitReset(env);
// A few seconds past the reset: GitHub's clock and ours are not one clock.
const waitMs = resetAt.getTime() - Date.now() + 5_000;
const when = `${resetAt.toLocaleTimeString()} (in ${Math.ceil(waitMs / 60_000)}m)`;

if (waitMs > this.rateWaitMs) {
throw new RateLimitError(
`GitHub API rate limit exhausted; it resets at ${when}, past the ` +
`${Math.round(this.rateWaitMs / 60_000)}m --rate-wait budget — ${result.stderr.trim()}`,
resetAt,
result,
);
}

this.onRateLimit(`GitHub API rate limit exhausted; waiting until it resets at ${when}`);
await this.sleep(waitMs);
}
}

private async rateLimitReset(env: NodeJS.ProcessEnv): Promise<Date> {
const result = await this.exec('gh', ['api', 'rate_limit'], { env });
let raw: unknown;
try {
raw = result.code === 0 ? JSON.parse(result.stdout) : undefined;
} catch {
raw = undefined;
}
return parseRateLimitReset(raw, Date.now());
}

private async json<T>(
Expand Down
9 changes: 8 additions & 1 deletion src/prs-merge.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Gh, type Check, type MergeState, type PullRequest, type WorkflowRun } from './gh.ts';
import { Gh, RateLimitError, type Check, type MergeState, type PullRequest, type WorkflowRun } from './gh.ts';
import { sleep } from './exec.ts';

export interface MergeOptions {
Expand Down Expand Up @@ -174,6 +174,13 @@ export async function sweep(
if (!found.has(pr.url)) found.set(pr.url, pr.createdAt);
}
} catch (error) {
// Not a verdict on the scope: its PRs were never looked at, so the
// run has to fail rather than summarise zero of them as all there is.
if (error instanceof RateLimitError) {
emit({ kind: 'failed', url: `${qualifier}:${owner}`, reason: error.message });
summary.failed += 1;
continue;
}
emit({
kind: 'warn',
text: `skipped inaccessible or invalid scope ${qualifier}:${owner} — ${
Expand Down
90 changes: 90 additions & 0 deletions test/prs-merge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {
parsePullRequest,
parseRunsAwaitingApproval,
parsePullRequestUrl,
parseRateLimitReset,
isRateLimited,
GhError,
} from '../src/gh.ts';
import type { RunResult } from '../src/exec.ts';
Expand Down Expand Up @@ -631,6 +633,94 @@ describe('sweep', () => {
});
});

describe('rate limit', () => {
const LIMITED = 'GraphQL: API rate limit already exceeded for user ID 27381.';

/** `gh search` refused `failures` times, then answers; rate_limit says reset in `resetInS`. */
function limitedGh(failures: number, resetInS: number, rateWaitMs: number) {
const calls: string[] = [];
const waits: number[] = [];
const notices: string[] = [];
const gh = new Gh({
rateWaitMs,
sleep: async (ms) => {
waits.push(ms);
},
onRateLimit: (text) => notices.push(text),
exec: async (_f, args) => {
calls.push(args.join(' '));
if (args[0] === 'api' && args[1] === 'rate_limit') {
const reset = Math.floor(Date.now() / 1000) + resetInS;
return {
code: 0,
stdout: JSON.stringify({
resources: {
core: { remaining: 4000, reset: reset + 900 },
graphql: { remaining: 0, reset },
},
}),
stderr: '',
};
}
if (args[0] === 'search') {
if (failures > 0) {
failures -= 1;
return { code: 1, stdout: '', stderr: LIMITED };
}
return { code: 0, stdout: '[]', stderr: '' };
}
return { code: 0, stdout: '[]', stderr: '' };
},
});
return { gh, calls, waits, notices };
}

it('recognises primary and secondary refusals, and nothing else', () => {
expect(isRateLimited(LIMITED)).toBe(true);
expect(isRateLimited('HTTP 403: API rate limit exceeded for user ID 1.')).toBe(true);
expect(isRateLimited('You have exceeded a secondary rate limit.')).toBe(true);
expect(isRateLimited('Could not resolve to an Organization')).toBe(false);
});

it('takes the reset of the bucket that ran dry, not one with quota left', () => {
const now = 1_000_000_000_000;
const reset = parseRateLimitReset(
{
resources: {
core: { remaining: 10, reset: now / 1000 + 3000 },
graphql: { remaining: 0, reset: now / 1000 + 120 },
},
},
now,
);
expect(reset.getTime()).toBe(now + 120_000);
expect(parseRateLimitReset(undefined, now).getTime()).toBe(now + 60_000);
});

it('waits out a reset inside the budget, then sweeps', async () => {
const { gh, waits, notices } = limitedGh(1, 60, 15 * 60_000);
const { summary, text } = await runSweep(gh, baseOptions());

expect(waits).toHaveLength(1);
expect(notices[0]).toContain('waiting until it resets');
expect(text).not.toContain('FAILED');
expect(summary.failed).toBe(0);
});

it('fails the run, not just warns, when the reset is past the budget', async () => {
const { gh, waits } = limitedGh(99, 45 * 60, 15 * 60_000);
const { summary, text } = await runSweep(
gh,
baseOptions({ orgs: ['acme', 'other'], users: ['someone'] }),
);

expect(waits).toHaveLength(0);
expect(text).toContain('FAILED org:acme — GitHub API rate limit exhausted');
expect(text).not.toContain('skipped inaccessible');
expect(summary.failed).toBe(3);
});
});

describe('defaults', () => {
it('waits ten minutes and polls every twenty seconds', () => {
expect(defaults.fixWaitMs).toBe(600_000);
Expand Down
Loading