diff --git a/packages/backend/convex/githubClient.ts b/packages/backend/convex/githubClient.ts index 48b242b..1b2b5f1 100644 --- a/packages/backend/convex/githubClient.ts +++ b/packages/backend/convex/githubClient.ts @@ -102,10 +102,20 @@ export const isRateLimitError = (error: unknown): boolean => { 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; + if ( + message.includes('rate limit') || + message.includes('secondary rate limit') || + message.includes('abuse') + ) { + return true; + } const headers = err.response?.headers ?? {}; + // Genuinely exhausted primary rate limit. Note: GitHub sends X-RateLimit-* + // headers (including x-ratelimit-reset) on essentially every REST response, + // so a bare reset header must NOT be treated as a rate limit — a plain 403 + // auth failure ("Resource not accessible by integration") carries a positive + // x-ratelimit-remaining and must stay classified as an error, not rate limit. if (headers['x-ratelimit-remaining'] === '0') return true; - if (headers['x-ratelimit-reset'] !== undefined) return true; return false; }; diff --git a/packages/backend/tests/unit/rate-limit.test.ts b/packages/backend/tests/unit/rate-limit.test.ts index d349da7..cdd6454 100644 --- a/packages/backend/tests/unit/rate-limit.test.ts +++ b/packages/backend/tests/unit/rate-limit.test.ts @@ -22,6 +22,24 @@ describe('isRateLimitError', () => { ).toBe(true); }); + test('does not misclassify a plain 403 auth error as rate limited', () => { + // GitHub sends X-RateLimit-* headers on essentially every REST response, + // including a permission-denied 403 where remaining is still positive. + // A reset header being present must NOT count as a rate limit. + expect( + isRateLimitError({ + status: 403, + message: 'Resource not accessible by integration', + response: { + headers: { + 'x-ratelimit-remaining': '4998', + 'x-ratelimit-reset': '1700000000', + }, + }, + }), + ).toBe(false); + }); + test('returns false for a 404', () => { expect(isRateLimitError({ status: 404 })).toBe(false); });