From 429d65a52bdd1618ca022b2577e1e080d1fe239a Mon Sep 17 00:00:00 2001 From: anuraghazra Date: Wed, 15 Jul 2020 17:46:31 +0530 Subject: [PATCH] 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 803680ee..94f734cf 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 a28733c3..8659c504 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 a248a7e9..710061ee 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 a3cd3cdb..f8bb7158 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 00000000..b62bd8ab --- /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 00000000..8b8a9289 --- /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"); + } + }); +});