diff --git a/src/main/updater-release-builds.test.ts b/src/main/updater-release-builds.test.ts index ad82b840c9b..1f65a4d00d4 100644 --- a/src/main/updater-release-builds.test.ts +++ b/src/main/updater-release-builds.test.ts @@ -313,6 +313,33 @@ describe('listReleaseBuilds', () => { expect(requestHeaders(1)).toEqual({ Accept: 'application/vnd.github+json' }) }) + // Why: GitHub attaches `x-ratelimit-remaining: 0` to some secondary limits too, and + // those carry Retry-After. Tripping the primary breaker on one would block every + // unrelated core gh command until the hourly reset over a short abuse-throttle. + it('does not trip the gh breaker for a secondary limit carrying retry-after', async () => { + tokenMock.mockResolvedValue('gho_abc') + fetchMock + .mockResolvedValueOnce( + jsonResponse(null, { + ok: false, + status: 403, + headers: { + 'x-ratelimit-remaining': '0', + 'x-ratelimit-reset': '1800000600', + 'retry-after': '60' + } + }) + ) + .mockResolvedValueOnce(jsonResponse([release('v1.4.159')])) + + await expect( + listReleaseBuilds('stable', 'darwin').then((builds) => builds.map((build) => build.version)) + ).resolves.toEqual(['1.4.159']) + + expect(recordRateLimitMock).not.toHaveBeenCalled() + expect(requestHeaders(1)).toEqual({ Accept: 'application/vnd.github+json' }) + }) + it('skips the token while the gh breaker has the core bucket blocked', async () => { blockedUntilMock.mockReturnValue(Date.now() + 60_000) tokenMock.mockResolvedValue('gho_abc') diff --git a/src/main/updater-release-builds.ts b/src/main/updater-release-builds.ts index af93a369806..7e07287b2c1 100644 --- a/src/main/updater-release-builds.ts +++ b/src/main/updater-release-builds.ts @@ -42,6 +42,16 @@ function isRateLimited(res: Response): boolean { ) } +/** + * Only a spent primary bucket may trip the shared gh breaker. GitHub also sends + * `x-ratelimit-remaining: 0` on some secondary 403/429s, and those carry Retry-After — + * blocking every core gh command until the primary reset would be far wider than the + * limit GitHub actually applied. + */ +function isPrimaryRateLimited(res: Response): boolean { + return res.headers.get('x-ratelimit-remaining') === '0' && !res.headers.has('retry-after') +} + /** Primary limits carry the reset epoch; secondary limits carry Retry-After as seconds or an HTTP date. */ export function rateLimitResetAtMs(headers: Headers, nowMs: number): number | null { const resetEpochSeconds = Number(headers.get('x-ratelimit-reset')) @@ -176,8 +186,9 @@ export async function listReleaseBuilds( res = await fetchReleases(repo, null) } else if (token && isRateLimited(res)) { // Why: the token's bucket and the per-IP bucket are separate, so the other one - // may still have quota. Tell the breaker first so gh calls fail fast until the reset. - const resetAtMs = rateLimitResetAtMs(res.headers, Date.now()) + // may still have quota. Tell the breaker first — only for a primary limit — so gh + // calls fail fast until the reset. + const resetAtMs = isPrimaryRateLimited(res) ? rateLimitResetAtMs(res.headers, Date.now()) : null if (resetAtMs !== null) { recordGhPrimaryRateLimit('core', resetAtMs) }