From 3d8dea90e2072a5ce25aa148cf6ecb06aa7d123f Mon Sep 17 00:00:00 2001 From: anuraghazra Date: Wed, 15 Jul 2020 14:59:25 +0530 Subject: [PATCH 1/2] fix: github rate limiter with multiple PATs --- api/index.js | 2 ++ api/pin.js | 2 ++ src/fetchRepo.js | 17 ++++++---- src/fetchStats.js | 68 +++++++++++++++++++++++++++++++--------- src/utils.js | 12 +++++-- tests/fetchStats.test.js | 2 +- 6 files changed, 78 insertions(+), 25 deletions(-) diff --git a/api/index.js b/api/index.js index 392b6943cfdc3..803680ee3021b 100644 --- a/api/index.js +++ b/api/index.js @@ -18,7 +18,9 @@ module.exports = async (req, res) => { } = req.query; let stats; + res.setHeader("Cache-Control", "public, max-age=300"); res.setHeader("Content-Type", "image/svg+xml"); + try { stats = await fetchStats(username); } catch (err) { diff --git a/api/pin.js b/api/pin.js index 2d69c0e75daee..a28733c370cf2 100644 --- a/api/pin.js +++ b/api/pin.js @@ -14,6 +14,8 @@ module.exports = async (req, res) => { } = req.query; let repoData; + + res.setHeader("Cache-Control", "public, max-age=300"); res.setHeader("Content-Type", "image/svg+xml"); try { diff --git a/src/fetchRepo.js b/src/fetchRepo.js index c7b7c3e3f5fd9..a248a7e9939dc 100644 --- a/src/fetchRepo.js +++ b/src/fetchRepo.js @@ -5,8 +5,9 @@ async function fetchRepo(username, reponame) { throw new Error("Invalid username or reponame"); } - const res = await request({ - query: ` + const res = await request( + { + query: ` fragment RepoInfo on Repository { name stargazers { @@ -33,11 +34,15 @@ async function fetchRepo(username, reponame) { } } `, - variables: { - login: username, - repo: reponame, + variables: { + login: username, + repo: reponame, + }, }, - }); + { + Authorization: `bearer ${process.env.PAT_1}`, + } + ); const data = res.data.data; diff --git a/src/fetchStats.js b/src/fetchStats.js index dc2859022dd4a..a3cd3cdbc6797 100644 --- a/src/fetchStats.js +++ b/src/fetchStats.js @@ -2,25 +2,25 @@ const { request } = require("./utils"); const calculateRank = require("./calculateRank"); require("dotenv").config(); -async function fetchStats(username) { - if (!username) throw Error("Invalid username"); - - const res = await request({ - query: ` +// creating a fetcher function to reduce duplication +const fetcher = (username, token) => { + return request( + { + query: ` query userInfo($login: String!) { user(login: $login) { name login - repositoriesContributedTo(first: 100, contributionTypes: [COMMIT, ISSUE, PULL_REQUEST, REPOSITORY]) { - totalCount - } contributionsCollection { totalCommitContributions } - pullRequests(first: 100) { + repositoriesContributedTo(first: 1, contributionTypes: [COMMIT, ISSUE, PULL_REQUEST, REPOSITORY]) { totalCount } - issues(first: 100) { + pullRequests(first: 1) { + totalCount + } + issues(first: 1) { totalCount } followers { @@ -36,9 +36,45 @@ async function fetchStats(username) { } } } - `, - variables: { login: username }, - }); + `, + variables: { login: username }, + }, + { + // set the token + Authorization: `bearer ${token}`, + } + ); +}; + +async function retryer(username, RETRIES) { + try { + console.log(`Trying PAT_${RETRIES + 1}`); + + // try to fetch with the first token since RETRIES is 0 index i'm adding +1 + let response = await fetcher(username, process.env[`PAT_${RETRIES + 1}`]); + + // if rate limit is hit increase the RETRIES and recursively call the retryer + // with username, and current RETRIES + if ( + response.data.errors && + response.data.errors[0].type === "RATE_LIMITED" + ) { + console.log(`PAT_${RETRIES} Failed`); + RETRIES++; + // directly return from the function + return await retryer(username, RETRIES); + } + + // finally return the response + return response; + } catch (err) { + console.log(err); + } +} + +async function fetchStats(username) { + let RETRIES = 0; + if (!username) throw Error("Invalid username"); const stats = { name: "", @@ -47,12 +83,14 @@ async function fetchStats(username) { totalIssues: 0, totalStars: 0, contributedTo: 0, - rank: "C", + rank: { level: "C", score: 0 }, }; + let res = await retryer(username, RETRIES); + if (res.data.errors) { console.log(res.data.errors); - throw Error("Could not fetch user"); + throw Error(res.data.errors[0].message || "Could not fetch user"); } const user = res.data.data.user; diff --git a/src/utils.js b/src/utils.js index 470a9e83c9f27..b6b74f5ac2a05 100644 --- a/src/utils.js +++ b/src/utils.js @@ -33,13 +33,13 @@ function isValidHexColor(hexColor) { ).test(hexColor); } -function request(data) { +function request(data, headers) { return new Promise((resolve, reject) => { axios({ url: "https://api.github.com/graphql", method: "post", headers: { - Authorization: `bearer ${process.env.GITHUB_TOKEN}`, + ...headers, }, data, }) @@ -48,4 +48,10 @@ function request(data) { }); } -module.exports = { renderError, kFormatter, encodeHTML, isValidHexColor, request }; +module.exports = { + renderError, + kFormatter, + encodeHTML, + isValidHexColor, + request, +}; diff --git a/tests/fetchStats.test.js b/tests/fetchStats.test.js index 8ae45fa554ffd..ebcfde464f160 100644 --- a/tests/fetchStats.test.js +++ b/tests/fetchStats.test.js @@ -74,7 +74,7 @@ describe("Test fetchStats", () => { mock.onPost("https://api.github.com/graphql").reply(200, error); await expect(fetchStats("anuraghazra")).rejects.toThrow( - "Could not fetch user" + "Could not resolve to a User with the login of 'noname'." ); }); }); From 429d65a52bdd1618ca022b2577e1e080d1fe239a Mon Sep 17 00:00:00 2001 From: anuraghazra Date: Wed, 15 Jul 2020 17:46:31 +0530 Subject: [PATCH 2/2] refactor: refactored retryer logic & handled invalid tokens --- api/index.js | 2 +- api/pin.js | 2 +- src/fetchRepo.js | 24 +++++++++++---------- src/fetchStats.js | 36 ++++--------------------------- src/retryer.js | 43 +++++++++++++++++++++++++++++++++++++ tests/retryer.test.js | 50 +++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 112 insertions(+), 45 deletions(-) create mode 100644 src/retryer.js create mode 100644 tests/retryer.test.js diff --git a/api/index.js b/api/index.js index 803680ee3021b..94f734cf755a1 100644 --- a/api/index.js +++ b/api/index.js @@ -18,7 +18,7 @@ module.exports = async (req, res) => { } = req.query; let stats; - res.setHeader("Cache-Control", "public, max-age=300"); + res.setHeader("Cache-Control", "public, max-age=1800"); res.setHeader("Content-Type", "image/svg+xml"); try { diff --git a/api/pin.js b/api/pin.js index a28733c370cf2..8659c5048e915 100644 --- a/api/pin.js +++ b/api/pin.js @@ -15,7 +15,7 @@ module.exports = async (req, res) => { let repoData; - res.setHeader("Cache-Control", "public, max-age=300"); + res.setHeader("Cache-Control", "public, max-age=1800"); res.setHeader("Content-Type", "image/svg+xml"); try { diff --git a/src/fetchRepo.js b/src/fetchRepo.js index a248a7e9939dc..710061ee371eb 100644 --- a/src/fetchRepo.js +++ b/src/fetchRepo.js @@ -1,11 +1,8 @@ const { request } = require("./utils"); +const retryer = require("./retryer"); -async function fetchRepo(username, reponame) { - if (!username || !reponame) { - throw new Error("Invalid username or reponame"); - } - - const res = await request( +const fetcher = (variables, token) => { + return request( { query: ` fragment RepoInfo on Repository { @@ -34,15 +31,20 @@ async function fetchRepo(username, reponame) { } } `, - variables: { - login: username, - repo: reponame, - }, + variables, }, { - Authorization: `bearer ${process.env.PAT_1}`, + Authorization: `bearer ${token}`, } ); +}; + +async function fetchRepo(username, reponame) { + if (!username || !reponame) { + throw new Error("Invalid username or reponame"); + } + + let res = await retryer(fetcher, { login: username, repo: reponame }); const data = res.data.data; diff --git a/src/fetchStats.js b/src/fetchStats.js index a3cd3cdbc6797..f8bb715855492 100644 --- a/src/fetchStats.js +++ b/src/fetchStats.js @@ -1,9 +1,9 @@ const { request } = require("./utils"); +const retryer = require("./retryer"); const calculateRank = require("./calculateRank"); require("dotenv").config(); -// creating a fetcher function to reduce duplication -const fetcher = (username, token) => { +const fetcher = (variables, token) => { return request( { query: ` @@ -37,43 +37,15 @@ const fetcher = (username, token) => { } } `, - variables: { login: username }, + variables, }, { - // set the token Authorization: `bearer ${token}`, } ); }; -async function retryer(username, RETRIES) { - try { - console.log(`Trying PAT_${RETRIES + 1}`); - - // try to fetch with the first token since RETRIES is 0 index i'm adding +1 - let response = await fetcher(username, process.env[`PAT_${RETRIES + 1}`]); - - // if rate limit is hit increase the RETRIES and recursively call the retryer - // with username, and current RETRIES - if ( - response.data.errors && - response.data.errors[0].type === "RATE_LIMITED" - ) { - console.log(`PAT_${RETRIES} Failed`); - RETRIES++; - // directly return from the function - return await retryer(username, RETRIES); - } - - // finally return the response - return response; - } catch (err) { - console.log(err); - } -} - async function fetchStats(username) { - let RETRIES = 0; if (!username) throw Error("Invalid username"); const stats = { @@ -86,7 +58,7 @@ async function fetchStats(username) { rank: { level: "C", score: 0 }, }; - let res = await retryer(username, RETRIES); + let res = await retryer(fetcher, { login: username }); if (res.data.errors) { console.log(res.data.errors); diff --git a/src/retryer.js b/src/retryer.js new file mode 100644 index 0000000000000..b62bd8abb7a4d --- /dev/null +++ b/src/retryer.js @@ -0,0 +1,43 @@ +const retryer = async (fetcher, variables, retries = 0) => { + if (retries > 7) { + throw new Error("Maximum retries exceeded"); + } + try { + console.log(`Trying PAT_${retries + 1}`); + + // try to fetch with the first token since RETRIES is 0 index i'm adding +1 + let response = await fetcher( + variables, + process.env[`PAT_${retries + 1}`], + retries + ); + + // prettier-ignore + const isRateExceeded = response.data.errors && response.data.errors[0].type === "RATE_LIMITED"; + + // if rate limit is hit increase the RETRIES and recursively call the retryer + // with username, and current RETRIES + if (isRateExceeded) { + console.log(`PAT_${retries + 1} Failed`); + retries++; + // directly return from the function + return retryer(fetcher, variables, retries); + } + + // finally return the response + return response; + } catch (err) { + // prettier-ignore + // also checking for bad credentials if any tokens gets invalidated + const isBadCredential = err.response.data && err.response.data.message === "Bad credentials"; + + if (isBadCredential) { + console.log(`PAT_${retries + 1} Failed`); + retries++; + // directly return from the function + return retryer(fetcher, variables, retries); + } + } +}; + +module.exports = retryer; diff --git a/tests/retryer.test.js b/tests/retryer.test.js new file mode 100644 index 0000000000000..8b8a9289215e9 --- /dev/null +++ b/tests/retryer.test.js @@ -0,0 +1,50 @@ +require("@testing-library/jest-dom"); +const retryer = require("../src/retryer"); + +const fetcher = jest.fn((variables, token) => { + console.log(variables, token); + return new Promise((res, rej) => res({ data: "ok" })); +}); + +const fetcherFail = jest.fn(() => { + return new Promise((res, rej) => + res({ data: { errors: [{ type: "RATE_LIMITED" }] } }) + ); +}); + +const fetcherFailOnSecondTry = jest.fn((_vars, _token, retries) => { + return new Promise((res, rej) => { + // faking rate limit + if (retries < 1) { + return res({ data: { errors: [{ type: "RATE_LIMITED" }] } }); + } + return res({ data: "ok" }); + }); +}); + +describe("Test Retryer", () => { + it("retryer should return value and have zero retries on first try", async () => { + let res = await retryer(fetcher, {}); + + expect(fetcher).toBeCalledTimes(1); + expect(res).toStrictEqual({ data: "ok" }); + }); + + it("retryer should return value and have 2 retries", async () => { + let res = await retryer(fetcherFailOnSecondTry, {}); + + expect(fetcherFailOnSecondTry).toBeCalledTimes(2); + expect(res).toStrictEqual({ data: "ok" }); + }); + + it("retryer should throw error if maximum retries reached", async () => { + let res; + + try { + res = await retryer(fetcherFail, {}); + } catch (err) { + expect(fetcherFail).toBeCalledTimes(8); + expect(err.message).toBe("Maximum retries exceeded"); + } + }); +});