mirror of
https://github.com/stablyai/orca.git
synced 2026-10-31 17:17:38 +00:00
fix(github): count gh pr and issue commands against the GraphQL rate limit (#28778)
* fix(github): count gh pr and issue commands against the GraphQL rate limit gh serves pr/issue/repo view-style subcommands over GraphQL, but the breaker filed every non-api command under core and only matched REST's "API rate limit exceeded" wording, so GraphQL exhaustion never tripped it. Map those subcommands to the graphql bucket, match GraphQL's "API rate limit already exceeded", and guard project-view issue close/reopen against the GraphQL quota. Fixes #28767 * test(github): drop redundant Date.now() args in graphql breaker test Keeps the flaky-test-pattern ratchet at its baseline. --------- Co-authored-by: Neil <neil@stably.ai>
This commit is contained in:
@@ -33,10 +33,14 @@ describe('classifyGhRateLimitBucket', () => {
|
||||
expect(classifyGhRateLimitBucket(['search', 'prs', '--author', 'x'])).toBe('search')
|
||||
})
|
||||
|
||||
it('classifies graphql and defaults everything else to core', () => {
|
||||
it('classifies graphql endpoints and GraphQL-backed subcommands, defaulting the rest to core', () => {
|
||||
expect(classifyGhRateLimitBucket(['api', 'graphql', '-f', 'query=…'])).toBe('graphql')
|
||||
expect(classifyGhRateLimitBucket(['pr', 'list', '--limit', '36'])).toBe('graphql')
|
||||
expect(classifyGhRateLimitBucket(['issue', 'close', '5', '--repo', 'a/b'])).toBe('graphql')
|
||||
expect(classifyGhRateLimitBucket(['repo', 'view', 'a/b', '--json', 'isFork'])).toBe('graphql')
|
||||
expect(classifyGhRateLimitBucket(['api', 'repos/a/b/pulls?per_page=36'])).toBe('core')
|
||||
expect(classifyGhRateLimitBucket(['pr', 'list', '--limit', '36'])).toBe('core')
|
||||
expect(classifyGhRateLimitBucket(['run', 'list', '--json', 'status'])).toBe('core')
|
||||
expect(classifyGhRateLimitBucket(['pr'])).toBe('core')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -69,6 +73,9 @@ describe('isGhPrimaryRateLimitStderr', () => {
|
||||
'gh: API rate limit exceeded for user ID 1775218. If you reach out to GitHub Support… (HTTP 403)'
|
||||
)
|
||||
).toBe(true)
|
||||
expect(
|
||||
isGhPrimaryRateLimitStderr('GraphQL: API rate limit already exceeded for user ID 1775218.')
|
||||
).toBe(true)
|
||||
expect(isGhPrimaryRateLimitStderr('You have exceeded a secondary rate limit.')).toBe(false)
|
||||
expect(isGhPrimaryRateLimitStderr('gh: Not Found (HTTP 404)')).toBe(false)
|
||||
})
|
||||
|
||||
@@ -78,13 +78,23 @@ function findGhApiEndpoint(args: readonly string[]): string | null {
|
||||
return null
|
||||
}
|
||||
|
||||
// gh serves these over GraphQL; a mixed REST trip (e.g. GHES `pr edit` reviewers) is corrected by the reset probe.
|
||||
const GH_GRAPHQL_SUBCOMMANDS = new Map<string, ReadonlySet<string>>([
|
||||
['issue', new Set(['close', 'edit', 'list', 'reopen', 'view'])],
|
||||
[
|
||||
'pr',
|
||||
new Set(['checks', 'close', 'create', 'edit', 'list', 'merge', 'ready', 'reopen', 'view'])
|
||||
],
|
||||
['repo', new Set(['view'])]
|
||||
])
|
||||
|
||||
export function classifyGhRateLimitBucket(args: readonly string[]): GhRateLimitBucket {
|
||||
const command = args[0]
|
||||
if (command === 'search') {
|
||||
return 'search'
|
||||
}
|
||||
if (command !== 'api') {
|
||||
return 'core'
|
||||
return GH_GRAPHQL_SUBCOMMANDS.get(command)?.has(args[1]) ? 'graphql' : 'core'
|
||||
}
|
||||
const endpoint = findGhApiEndpoint(args)
|
||||
if (!endpoint) {
|
||||
@@ -114,10 +124,10 @@ export function isGhLocalOnlyCommand(args: readonly string[]): boolean {
|
||||
return args[0] === 'auth' && args[1] === 'token'
|
||||
}
|
||||
|
||||
/** Recognize primary limits; secondary limits use Retry-After handling. */
|
||||
/** Recognize primary limits (GraphQL says "already exceeded"); secondary limits use Retry-After handling. */
|
||||
export function isGhPrimaryRateLimitStderr(stderr: string): boolean {
|
||||
const s = stderr.toLowerCase()
|
||||
return s.includes('api rate limit exceeded') && !s.includes('secondary rate limit')
|
||||
return /api rate limit (?:already )?exceeded/.test(s) && !s.includes('secondary rate limit')
|
||||
}
|
||||
|
||||
export function recordGhPrimaryRateLimit(
|
||||
|
||||
@@ -139,9 +139,29 @@ describe('ghExecFileAsync rate-limit breaker', () => {
|
||||
).rejects.toThrow('rate limit')
|
||||
|
||||
expect(
|
||||
getGhRateLimitBlockedUntilMs('core', Date.now(), 'native:github.acme-corp.com')
|
||||
getGhRateLimitBlockedUntilMs('graphql', Date.now(), 'native:github.acme-corp.com')
|
||||
).not.toBeNull()
|
||||
expect(getGhRateLimitBlockedUntilMs('core', Date.now(), 'native:github.com')).toBeNull()
|
||||
expect(getGhRateLimitBlockedUntilMs('graphql', Date.now(), 'native:github.com')).toBeNull()
|
||||
})
|
||||
|
||||
it('trips the graphql bucket on a GraphQL rate limit and leaves REST calls running', async () => {
|
||||
mockGhFailure('GraphQL: API rate limit already exceeded for user ID 1775218.')
|
||||
await expect(
|
||||
ghExecFileAsync(['pr', 'view', '12', '--repo', 'a/b', '--json', 'number'])
|
||||
).rejects.toThrow('rate limit')
|
||||
expect(getGhRateLimitBlockedUntilMs('graphql')).not.toBeNull()
|
||||
expect(getGhRateLimitBlockedUntilMs('core')).toBeNull()
|
||||
|
||||
await expect(
|
||||
ghExecFileAsync(['issue', 'close', '5', '--repo', 'a/b'], { idempotent: false })
|
||||
).rejects.toMatchObject({ ghRateLimitBlocked: true })
|
||||
expect(spawnMock).toHaveBeenCalledTimes(1)
|
||||
|
||||
mockGhSuccess('[]')
|
||||
await expect(ghExecFileAsync(['api', 'repos/a/b/pulls'])).resolves.toMatchObject({
|
||||
stdout: '[]'
|
||||
})
|
||||
expect(spawnMock).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('scopes a WSL UNC-cwd 403 to the distro runtime and probes that scope', async () => {
|
||||
|
||||
Reference in New Issue
Block a user