diff --git a/bun.lock b/bun.lock index f372366..4b18b26 100644 --- a/bun.lock +++ b/bun.lock @@ -147,6 +147,8 @@ "version": "1.0.0", "dependencies": { "@octokit/auth-app": "^8.2.0", + "@octokit/plugin-retry": "^8.1.0", + "@octokit/plugin-throttling": "^11.0.3", "@octokit/rest": "^22.0.1", "@oslojs/crypto": "^1.0.1", "@react-email/components": "1.0.10", @@ -976,6 +978,10 @@ "@octokit/plugin-rest-endpoint-methods": ["@octokit/plugin-rest-endpoint-methods@17.0.0", "", { "dependencies": { "@octokit/types": "^16.0.0" }, "peerDependencies": { "@octokit/core": ">=6" } }, "sha512-B5yCyIlOJFPqUUeiD0cnBJwWJO8lkJs5d8+ze9QDP6SvfiXSz1BF+91+0MeI1d2yxgOhU/O+CvtiZ9jSkHhFAw=="], + "@octokit/plugin-retry": ["@octokit/plugin-retry@8.1.0", "", { "dependencies": { "@octokit/request-error": "^7.0.2", "@octokit/types": "^16.0.0", "bottleneck": "^2.15.3" }, "peerDependencies": { "@octokit/core": ">=7" } }, "sha512-O1FZgXeiGb2sowEr/hYTr6YunGdSAFWnr2fyW39Ah85H8O33ELASQxcvOFF5LE6Tjekcyu2ms4qAzJVhSaJxTw=="], + + "@octokit/plugin-throttling": ["@octokit/plugin-throttling@11.0.3", "", { "dependencies": { "@octokit/types": "^16.0.0", "bottleneck": "^2.15.3" }, "peerDependencies": { "@octokit/core": "^7.0.0" } }, "sha512-34eE0RkFCKycLl2D2kq7W+LovheM/ex3AwZCYN8udpi6bxsyjZidb2McXs69hZhLmJlDqTSP8cH+jSRpiaijBg=="], + "@octokit/request": ["@octokit/request@10.0.10", "", { "dependencies": { "@octokit/endpoint": "^11.0.3", "@octokit/request-error": "^7.0.2", "@octokit/types": "^16.0.0", "content-type": "^2.0.0", "json-with-bigint": "^3.5.3", "universal-user-agent": "^7.0.2" } }, "sha512-KxNC2pTqqhszMNrf12ZRd4PonRgyJdsM4F/jySiddQK+DsRcfBtUvqn8t7UsyZhnRJHvX46OohDt5N3VqIWC2w=="], "@octokit/request-error": ["@octokit/request-error@7.1.0", "", { "dependencies": { "@octokit/types": "^16.0.0" } }, "sha512-KMQIfq5sOPpkQYajXHwnhjCC0slzCNScLHs9JafXc4RAJI+9f+jNDlBNaIMTvazOPLgb4BnlhGJOTbnN0wIjPw=="], @@ -984,7 +990,7 @@ "@octokit/types": ["@octokit/types@16.0.0", "", { "dependencies": { "@octokit/openapi-types": "^27.0.0" } }, "sha512-sKq+9r1Mm4efXW1FCk7hFSeJo4QKreL/tTbR0rz/qx/r1Oa2VV83LTA/H/MuCOX7uCIJmQVRKBcbmWoySjAnSg=="], - "@opencode-ai/sdk": ["@opencode-ai/sdk@1.17.10", "", { "dependencies": { "cross-spawn": "7.0.6" } }, "sha512-s9OcS7pubNCimS98B9ERJ/59veOj1SSGHD0qGBxGIx+164wSspUlHsAWhQIihvF8eZe16F5VY1XUQIEXGBTm2Q=="], + "@opencode-ai/sdk": ["@opencode-ai/sdk@1.17.18", "", { "dependencies": { "cross-spawn": "7.0.6" } }, "sha512-c/C9PhY8PrbcxDY+JIYtOZsrmMD0KzoVvxq+RGUrZ6LQp57SuVBbT4lfwA2G8Se5RNC1N5JtYjiuaXeECnF2SQ=="], "@opentelemetry/api": ["@opentelemetry/api@1.9.0", "", {}, "sha512-3giAOQvZiH5F9bMlMiv8+GSPMeqg0dbaeo58/0SlA9sxSqZhnUtxzX9/2FzyhS9sWQf5S0GJE0AKBrFqjpeYcg=="], @@ -1828,6 +1834,8 @@ "bl": ["bl@4.1.0", "", { "dependencies": { "buffer": "^5.5.0", "inherits": "^2.0.4", "readable-stream": "^3.4.0" } }, "sha512-1W07cM9gS6DcLperZfFSj+bWLtaPGSOHWhPiGzXmvVJbRLdG82sH/Kn8EtW1VqWVA54AKf2h5k5BbnIbwF3h6w=="], + "bottleneck": ["bottleneck@2.19.5", "", {}, "sha512-VHiNCbI1lKdl44tGrhNfU3lup0Tj/ZBMJB5/2ZbNXRCPuRCO7ed2mgcK4r17y+KB2EfuYuRaVlwNbAeaWGSpbw=="], + "bplist-creator": ["bplist-creator@0.1.0", "", { "dependencies": { "stream-buffers": "2.2.x" } }, "sha512-sXaHZicyEEmY86WyueLTQesbeoH/mquvarJaQNbjuOQO+7gbFcDEWqKmcWA4cOTLzFlfgvkiVxolk1k5bBIpmg=="], "bplist-parser": ["bplist-parser@0.3.2", "", { "dependencies": { "big-integer": "1.6.x" } }, "sha512-apC2+fspHGI3mMKj+dGevkGo/tCqVB8jMb6i+OX+E29p0Iposz07fABkRIfVUPNd5A5VbuOz1bZbnmkKLYF+wQ=="], diff --git a/packages/backend/convex/githubClient.ts b/packages/backend/convex/githubClient.ts index e82f0d2..48b242b 100644 --- a/packages/backend/convex/githubClient.ts +++ b/packages/backend/convex/githubClient.ts @@ -1,4 +1,6 @@ import { createAppAuth } from '@octokit/auth-app'; +import { retry } from '@octokit/plugin-retry'; +import { throttling } from '@octokit/plugin-throttling'; import { Octokit } from '@octokit/rest'; import { ConvexError } from 'convex/values'; @@ -51,8 +53,12 @@ const getEnv = (name: string) => { const normalizePrivateKey = (value: string) => value.replaceAll('\\n', '\n'); +const SpoonOctokit = Octokit.plugin(retry, throttling); + +const MAX_THROTTLE_RETRIES = 2; + export const getInstallationOctokit = (installationId: string) => - new Octokit({ + new SpoonOctokit({ authStrategy: createAppAuth, auth: { appId: getEnv('GITHUB_APP_ID'), @@ -65,8 +71,44 @@ export const getInstallationOctokit = (installationId: string) => 'X-GitHub-Api-Version': '2022-11-28', }, }, + throttle: { + onRateLimit: (retryAfter, options, _octokit, retryCount) => { + console.warn( + `[github] rate limit hit for ${options.method} ${options.url}; retry ${retryCount} after ${retryAfter}s`, + ); + return retryCount < MAX_THROTTLE_RETRIES; + }, + onSecondaryRateLimit: (retryAfter, options, _octokit, retryCount) => { + console.warn( + `[github] secondary rate limit hit for ${options.method} ${options.url}; retry ${retryCount} after ${retryAfter}s`, + ); + return retryCount < MAX_THROTTLE_RETRIES; + }, + }, }); +/** + * Pure predicate: does this thrown value look like a GitHub rate-limit / + * secondary-limit error? Detects the Octokit `RequestError` shape without + * importing node-only modules so it can be unit-tested directly. + */ +export const isRateLimitError = (error: unknown): boolean => { + if (typeof error !== 'object' || error === null) return false; + const err = error as { + status?: number; + message?: string; + response?: { headers?: Record }; + }; + if (err.status !== 403 && err.status !== 429) return false; + if (err.status === 429) return true; + const message = typeof err.message === 'string' ? err.message.toLowerCase() : ''; + if (message.includes('rate limit')) return true; + const headers = err.response?.headers ?? {}; + if (headers['x-ratelimit-remaining'] === '0') return true; + if (headers['x-ratelimit-reset'] !== undefined) return true; + return false; +}; + export const getSpoonInstallationId = ( spoon: Doc<'spoons'>, connection?: Doc<'gitConnections'> | null, diff --git a/packages/backend/convex/githubSync.ts b/packages/backend/convex/githubSync.ts index 78692be..a5cc6ce 100644 --- a/packages/backend/convex/githubSync.ts +++ b/packages/backend/convex/githubSync.ts @@ -13,6 +13,7 @@ import { getInstallationOctokit, getRepository, getSpoonInstallationId, + isRateLimitError, listPullRequests, syncForkBranch, } from './githubClient'; @@ -360,13 +361,16 @@ const refreshOwnedSpoon = async ( }; } catch (error) { const message = error instanceof Error ? error.message : String(error); + const rateLimited = isRateLimitError(error); await Promise.all([ ctx.runMutation(internal.spoons.patchSyncFields, { spoonId, - syncStatus: 'error', + syncStatus: rateLimited ? 'rate_limited' : 'error', lastGithubRefreshAt: Date.now(), lastCheckedAt: Date.now(), - lastError: message, + lastError: rateLimited + ? `Rate limited by GitHub; will retry. ${message}` + : message, }), ctx.runMutation(internal.syncRuns.patchInternal, { syncRunId, diff --git a/packages/backend/convex/schema.ts b/packages/backend/convex/schema.ts index 1b70718..1e23bbc 100644 --- a/packages/backend/convex/schema.ts +++ b/packages/backend/convex/schema.ts @@ -119,6 +119,7 @@ const applicationTables = { v.literal('checking'), v.literal('conflict'), v.literal('error'), + v.literal('rate_limited'), ), ), upstreamAheadBy: v.optional(v.number()), diff --git a/packages/backend/convex/spoons.ts b/packages/backend/convex/spoons.ts index b215281..567023d 100644 --- a/packages/backend/convex/spoons.ts +++ b/packages/backend/convex/spoons.ts @@ -63,6 +63,7 @@ const spoonSyncStatus = v.union( v.literal('checking'), v.literal('conflict'), v.literal('error'), + v.literal('rate_limited'), ); const hasForkMetadata = (args: { diff --git a/packages/backend/package.json b/packages/backend/package.json index 1307f59..6a6b985 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -30,6 +30,8 @@ }, "dependencies": { "@octokit/auth-app": "^8.2.0", + "@octokit/plugin-retry": "^8.1.0", + "@octokit/plugin-throttling": "^11.0.3", "@octokit/rest": "^22.0.1", "@oslojs/crypto": "^1.0.1", "@react-email/components": "1.0.10", diff --git a/packages/backend/tests/unit/rate-limit.test.ts b/packages/backend/tests/unit/rate-limit.test.ts new file mode 100644 index 0000000..d349da7 --- /dev/null +++ b/packages/backend/tests/unit/rate-limit.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, test } from 'vitest'; + +import { isRateLimitError } from '../../convex/githubClient'; + +describe('isRateLimitError', () => { + test('detects a 403 primary rate limit via ratelimit-remaining header', () => { + expect( + isRateLimitError({ + status: 403, + response: { headers: { 'x-ratelimit-remaining': '0' } }, + }), + ).toBe(true); + }); + + test('detects a 429 status', () => { + expect(isRateLimitError({ status: 429 })).toBe(true); + }); + + test('detects a secondary rate limit via message', () => { + expect( + isRateLimitError({ status: 403, message: 'secondary rate limit' }), + ).toBe(true); + }); + + test('returns false for a 404', () => { + expect(isRateLimitError({ status: 404 })).toBe(false); + }); + + test('returns false for a 500', () => { + expect(isRateLimitError({ status: 500 })).toBe(false); + }); + + test('returns false for a generic Error', () => { + expect(isRateLimitError(new Error('boom'))).toBe(false); + }); + + test('returns false for non-object values', () => { + expect(isRateLimitError(null)).toBe(false); + expect(isRateLimitError(undefined)).toBe(false); + expect(isRateLimitError('rate limit')).toBe(false); + }); +});