fix(sync): don't misclassify plain 403 auth errors as rate limited
This commit is contained in:
@@ -102,10 +102,20 @@ export const isRateLimitError = (error: unknown): boolean => {
|
|||||||
if (err.status !== 403 && err.status !== 429) return false;
|
if (err.status !== 403 && err.status !== 429) return false;
|
||||||
if (err.status === 429) return true;
|
if (err.status === 429) return true;
|
||||||
const message = typeof err.message === 'string' ? err.message.toLowerCase() : '';
|
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 ?? {};
|
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-remaining'] === '0') return true;
|
||||||
if (headers['x-ratelimit-reset'] !== undefined) return true;
|
|
||||||
return false;
|
return false;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|||||||
@@ -22,6 +22,24 @@ describe('isRateLimitError', () => {
|
|||||||
).toBe(true);
|
).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', () => {
|
test('returns false for a 404', () => {
|
||||||
expect(isRateLimitError({ status: 404 })).toBe(false);
|
expect(isRateLimitError({ status: 404 })).toBe(false);
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user