Skip to content

Commit adc0b52

Browse files
opheliagoldsteinqwerty541
authored andcommitted
fix: react on both type and message-based rate-limit signals (anuraghazra#4440)
Co-authored-by: Alexandr <[email protected]>
1 parent ef47181 commit adc0b52

File tree

2 files changed

+37
-3
lines changed

2 files changed

+37
-3
lines changed

src/common/retryer.js

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,14 @@ const retryer = async (fetcher, variables, env, retries = 0) => {
2828
if (!RETRIES) {
2929
throw new CustomError("No GitHub API tokens found", CustomError.NO_TOKENS);
3030
}
31+
3132
if (retries > RETRIES) {
3233
throw new CustomError(
3334
"Downtime due to GitHub API rate limiting",
3435
CustomError.MAX_RETRY,
3536
);
3637
}
38+
3739
try {
3840
// try to fetch with the first token since RETRIES is 0 index i'm adding +1
3941
let response = await fetcher(
@@ -43,12 +45,16 @@ const retryer = async (fetcher, variables, env, retries = 0) => {
4345
retries,
4446
);
4547

46-
const isRateExceeded =
47-
response.data.errors && response.data.errors[0].type === "RATE_LIMITED";
48+
// react on both type and message-based rate-limit signals.
49+
const errors = response?.data?.errors;
50+
const errorType = errors?.[0]?.type;
51+
const errorMsg = errors?.[0]?.message || "";
52+
const isRateLimited =
53+
(errors && errorType === "RATE_LIMITED") || /rate limit/i.test(errorMsg);
4854

4955
// if rate limit is hit increase the RETRIES and recursively call the retryer
5056
// with username, and current RETRIES
51-
if (isRateExceeded) {
57+
if (isRateLimited) {
5258
logger.log(`PAT_${retries + 1} Failed`);
5359
retries++;
5460
// directly return from the function

tests/retryer.test.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,27 @@ const fetcherFailOnSecondTry = jest.fn((_vars, _token, _useFetch, retries) => {
2525
});
2626
});
2727

28+
const fetcherFailWithMessageBasedRateLimitErr = jest.fn(
29+
(_vars, _token, retries) => {
30+
return new Promise((res) => {
31+
// faking rate limit
32+
if (retries < 1) {
33+
return res({
34+
data: {
35+
errors: [
36+
{
37+
type: "ASDF",
38+
message: "API rate limit already exceeded for user ID 11111111",
39+
},
40+
],
41+
},
42+
});
43+
}
44+
return res({ data: "ok" });
45+
});
46+
},
47+
);
48+
2849
describe("Test Retryer", () => {
2950
it("retryer should return value and have zero retries on first try", async () => {
3051
let res = await retryer(fetcher, {}, process.env);
@@ -40,6 +61,13 @@ describe("Test Retryer", () => {
4061
expect(res).toStrictEqual({ data: "ok" });
4162
});
4263

64+
it("retryer should return value and have 2 retries with message based rate limit error", async () => {
65+
let res = await retryer(fetcherFailWithMessageBasedRateLimitErr, {});
66+
67+
expect(fetcherFailWithMessageBasedRateLimitErr).toBeCalledTimes(2);
68+
expect(res).toStrictEqual({ data: "ok" });
69+
});
70+
4371
it("retryer should throw specific error if maximum retries reached", async () => {
4472
try {
4573
await retryer(fetcherFail, {}, process.env);

0 commit comments

Comments
 (0)