From 1e3139ab3814218d98687dc69c983e455441b6da Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Mon, 29 Jun 2026 02:01:30 +0300 Subject: [PATCH] feat(github): detect rate limiting and pause PR status calls during cooldown Octokit has no throttling plugin, so under a flood of PR-status calls a primary/secondary rate limit just surfaced as repeated 403s that the cache masked. Add a shared rate-limit gate: PR-status sub-calls note 403/429 responses, and the route short-circuits to cached/stale data during the cooldown instead of issuing more doomed requests. Transient failures (rate limit or the overall timeout) no longer log as hard errors. --- packages/web/server/lib/github/pr-status.js | 7 ++- packages/web/server/lib/github/rate-limit.js | 66 ++++++++++++++++++++ packages/web/server/lib/github/routes.js | 29 +++++++++ 3 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 packages/web/server/lib/github/rate-limit.js diff --git a/packages/web/server/lib/github/pr-status.js b/packages/web/server/lib/github/pr-status.js index 4f8e27f8..f4bfdced 100644 --- a/packages/web/server/lib/github/pr-status.js +++ b/packages/web/server/lib/github/pr-status.js @@ -1,5 +1,6 @@ import { getRemotes, getStatus } from '../git/index.js'; import { resolveGitHubRepoFromDirectory } from './repo/index.js'; +import { noteIfGitHubRateLimit } from './rate-limit.js'; const REPO_DEFAULT_BRANCH_TTL_MS = 5 * 60_000; const defaultBranchCache = new Map(); @@ -182,7 +183,8 @@ const getRepoDefaultBranch = async (octokit, repo) => { fetchedAt: Date.now(), }); return defaultBranch; - } catch { + } catch (error) { + noteIfGitHubRateLimit(error); return null; } }; @@ -210,6 +212,7 @@ const getRepoMetadata = async (octokit, repo) => { }); return data; } catch (error) { + noteIfGitHubRateLimit(error); if (error?.status === 403 || error?.status === 404) { repoMetadataCache.set(repoKey, { data: null, @@ -290,6 +293,7 @@ const safeListPulls = async (octokit, options) => { const response = await octokit.rest.pulls.list(options); return Array.isArray(response?.data) ? response.data : []; } catch (error) { + noteIfGitHubRateLimit(error); if (error?.status === 404 || error?.status === 403) { return []; } @@ -345,6 +349,7 @@ const searchFallbackPr = async ({ octokit, branch, repoNames }) => { // If we get here, search API works for this repo — clear the disabled flag _searchApiDisabledRepos.delete(repoKey); } catch (error) { + noteIfGitHubRateLimit(error); if (error?.status === 403) { _searchApiDisabledRepos.set(repoKey, Date.now()); return null; diff --git a/packages/web/server/lib/github/rate-limit.js b/packages/web/server/lib/github/rate-limit.js new file mode 100644 index 00000000..80a51410 --- /dev/null +++ b/packages/web/server/lib/github/rate-limit.js @@ -0,0 +1,66 @@ +// Lightweight, process-global GitHub rate-limit gate. +// +// Octokit is configured without the throttling plugin, so a primary or +// secondary rate limit surfaces as a thrown 403/429. Resolving PR status for +// many worktrees fans out dozens of calls; once GitHub starts limiting, every +// further call wastes a round-trip and the cache masks the failure. When we +// detect a rate-limit response we record a cooldown and skip GitHub work until +// it passes, so the burst stops and the reason is visible in the logs. + +const MAX_COOLDOWN_MS = 15 * 60 * 1000; +const DEFAULT_COOLDOWN_MS = 60 * 1000; + +let rateLimitedUntil = 0; + +const headerValue = (headers, name) => { + if (!headers) return undefined; + // Octokit/fetch headers can be a plain object or a Headers instance. + if (typeof headers.get === 'function') return headers.get(name); + return headers[name]; +}; + +const parseRetryAfterMs = (error) => { + const headers = error?.response?.headers; + const retryAfter = headerValue(headers, 'retry-after'); + if (retryAfter !== undefined && retryAfter !== null) { + const secs = Number(retryAfter); + if (Number.isFinite(secs) && secs > 0) return secs * 1000; + } + const reset = headerValue(headers, 'x-ratelimit-reset'); + if (reset !== undefined && reset !== null) { + const delta = Number(reset) * 1000 - Date.now(); + if (Number.isFinite(delta) && delta > 0) return delta; + } + return null; +}; + +/** True when an Octokit error represents a primary or secondary rate limit. */ +export const isGitHubRateLimitError = (error) => { + const status = error?.status ?? error?.response?.status; + if (status === 429) return true; + if (status !== 403) return false; + const remaining = headerValue(error?.response?.headers, 'x-ratelimit-remaining'); + if (remaining === '0' || remaining === 0) return true; + if (headerValue(error?.response?.headers, 'retry-after') != null) return true; + const message = String(error?.message ?? '').toLowerCase(); + return message.includes('rate limit'); +}; + +/** Record a cooldown after a detected rate-limit response. */ +export const noteGitHubRateLimit = (error) => { + const retryMs = Math.min(parseRetryAfterMs(error) ?? DEFAULT_COOLDOWN_MS, MAX_COOLDOWN_MS); + const until = Date.now() + retryMs; + if (until > rateLimitedUntil) { + rateLimitedUntil = until; + console.warn(`[github] rate limited — pausing GitHub PR status calls for ~${Math.round(retryMs / 1000)}s`); + } +}; + +/** Convenience: note the error if it is a rate-limit error. Returns whether it was. */ +export const noteIfGitHubRateLimit = (error) => { + if (!isGitHubRateLimitError(error)) return false; + noteGitHubRateLimit(error); + return true; +}; + +export const isGitHubRateLimited = () => Date.now() < rateLimitedUntil; diff --git a/packages/web/server/lib/github/routes.js b/packages/web/server/lib/github/routes.js index 0f20ccc0..765f4a26 100644 --- a/packages/web/server/lib/github/routes.js +++ b/packages/web/server/lib/github/routes.js @@ -419,6 +419,17 @@ export function registerGitHubRoutes(app) { return res.json(cached.data); } + // If GitHub recently rate-limited us, don't pile on more calls that will + // also fail. Serve whatever we last cached (even if stale); otherwise + // report a transient failure so the client keeps its last-known status. + const { isGitHubRateLimited } = await import('./rate-limit.js'); + if (isGitHubRateLimited()) { + if (cached) { + return res.json(cached.data); + } + return res.status(503).json({ error: 'GitHub rate limited' }); + } + // Intercept res.json to cache successful responses before sending // Only caches responses with connected:true — error/edge-case responses are not cached const originalJson = res.json.bind(res); @@ -577,6 +588,24 @@ export function registerGitHubRoutes(app) { clearGitHubAuth(); return res.json({ connected: false }); } + // Transient failures — a rate limit, or the overall resolve timeout + // firing — are expected under heavy load and should not be logged as hard + // errors. Record a rate-limit cooldown when applicable, then serve the + // last cached status (even if stale) or a 503 so the client keeps its + // last-known value instead of clearing the badge. + const { noteIfGitHubRateLimit } = await import('./rate-limit.js'); + const wasRateLimited = noteIfGitHubRateLimit(error); + const wasTimeout = error?.code === 'ETIMEDOUT'; + if (wasRateLimited || wasTimeout) { + const dir = typeof req.query?.directory === 'string' ? req.query.directory.trim() : ''; + const br = typeof req.query?.branch === 'string' ? req.query.branch.trim() : ''; + const rem = typeof req.query?.remote === 'string' ? req.query.remote.trim() : 'origin'; + const cached = prStatusCache.get(`${dir}::${br}::${rem}`); + if (cached) { + return res.json(cached.data); + } + return res.status(503).json({ error: wasRateLimited ? 'GitHub rate limited' : 'GitHub request timed out' }); + } if (isGitHubResourceUnavailable(error)) { return res.json({ connected: true,