From aa2e8ff3c527f705144357df112a191e135fbfc9 Mon Sep 17 00:00:00 2001 From: Rick Staa Date: Sat, 21 Jan 2023 10:08:40 +0100 Subject: [PATCH 1/2] feat: rate limit error chaching Rate limit error caching to alleviate PATs. --- api/index.js | 9 +++++++-- api/pin.js | 9 +++++++-- api/top-langs.js | 9 +++++++-- api/wakatime.js | 13 +++++++------ src/common/retryer.js | 2 +- src/common/utils.js | 10 ++++++++++ tests/api.test.js | 9 +++++++-- tests/retryer.test.js | 4 ++-- 8 files changed, 48 insertions(+), 17 deletions(-) diff --git a/api/index.js b/api/index.js index b449d43b49080..251c44ed67c62 100644 --- a/api/index.js +++ b/api/index.js @@ -56,7 +56,7 @@ export default async (req, res) => { ); const cacheSeconds = clampValue( - parseInt(cache_seconds || CONSTANTS.FOUR_HOURS, 10), + parseInt(cache_seconds || CONSTANTS.CARD_CACHE_SECONDS, 10), CONSTANTS.FOUR_HOURS, CONSTANTS.ONE_DAY, ); @@ -93,7 +93,12 @@ export default async (req, res) => { }), ); } catch (err) { - res.setHeader("Cache-Control", `no-cache, no-store, must-revalidate`); // Don't cache error responses. + res.setHeader( + "Cache-Control", + `max-age=${CONSTANTS.ERROR_CACHE_SECONDS / 2}, s-maxage=${ + CONSTANTS.ERROR_CACHE_SECONDS + }, stale-while-revalidate=${CONSTANTS.ONE_DAY}`, + ); // Cache the error response less frequently. return res.send(renderError(err.message, err.secondaryMessage)); } }; diff --git a/api/pin.js b/api/pin.js index 4838b0f02fece..5e02971389d38 100644 --- a/api/pin.js +++ b/api/pin.js @@ -40,7 +40,7 @@ export default async (req, res) => { const repoData = await fetchRepo(username, repo); let cacheSeconds = clampValue( - parseInt(cache_seconds || CONSTANTS.FOUR_HOURS, 10), + parseInt(cache_seconds || CONSTANTS.CARD_CACHE_SECONDS, 10), CONSTANTS.FOUR_HOURS, CONSTANTS.ONE_DAY, ); @@ -80,7 +80,12 @@ export default async (req, res) => { }), ); } catch (err) { - res.setHeader("Cache-Control", `no-cache, no-store, must-revalidate`); // Don't cache error responses. + res.setHeader( + "Cache-Control", + `max-age=${CONSTANTS.ERROR_CACHE_SECONDS / 2}, s-maxage=${ + CONSTANTS.ERROR_CACHE_SECONDS + }, stale-while-revalidate=${CONSTANTS.ONE_DAY}`, + ); // Cache the error response less frequently. return res.send(renderError(err.message, err.secondaryMessage)); } }; diff --git a/api/top-langs.js b/api/top-langs.js index 19cccb894e33a..436172addfdc3 100644 --- a/api/top-langs.js +++ b/api/top-langs.js @@ -48,7 +48,7 @@ export default async (req, res) => { ); const cacheSeconds = clampValue( - parseInt(cache_seconds || CONSTANTS.FOUR_HOURS, 10), + parseInt(cache_seconds || CONSTANTS.CARD_CACHE_SECONDS, 10), CONSTANTS.FOUR_HOURS, CONSTANTS.ONE_DAY, ); @@ -80,7 +80,12 @@ export default async (req, res) => { }), ); } catch (err) { - res.setHeader("Cache-Control", `no-cache, no-store, must-revalidate`); // Don't cache error responses. + res.setHeader( + "Cache-Control", + `max-age=${CONSTANTS.ERROR_CACHE_SECONDS / 2}, s-maxage=${ + CONSTANTS.ERROR_CACHE_SECONDS + }, stale-while-revalidate=${CONSTANTS.ONE_DAY}`, + ); // Cache the error response less frequently. return res.send(renderError(err.message, err.secondaryMessage)); } }; diff --git a/api/wakatime.js b/api/wakatime.js index d439c5b7ac8c6..41964e371a86b 100644 --- a/api/wakatime.js +++ b/api/wakatime.js @@ -43,15 +43,11 @@ export default async (req, res) => { const stats = await fetchWakatimeStats({ username, api_domain, range }); let cacheSeconds = clampValue( - parseInt(cache_seconds || CONSTANTS.FOUR_HOURS, 10), + parseInt(cache_seconds || CONSTANTS.CARD_CACHE_SECONDS, 10), CONSTANTS.FOUR_HOURS, CONSTANTS.ONE_DAY, ); - if (!cache_seconds) { - cacheSeconds = CONSTANTS.FOUR_HOURS; - } - res.setHeader( "Cache-Control", `max-age=${ @@ -80,7 +76,12 @@ export default async (req, res) => { }), ); } catch (err) { - res.setHeader("Cache-Control", `no-cache, no-store, must-revalidate`); // Don't cache error responses. + res.setHeader( + "Cache-Control", + `max-age=${CONSTANTS.ERROR_CACHE_SECONDS / 2}, s-maxage=${ + CONSTANTS.ERROR_CACHE_SECONDS + }, stale-while-revalidate=${CONSTANTS.ONE_DAY}`, + ); // Cache the error response less frequently. return res.send(renderError(err.message, err.secondaryMessage)); } }; diff --git a/src/common/retryer.js b/src/common/retryer.js index 5351cbe8cf99a..902d13f6d976b 100644 --- a/src/common/retryer.js +++ b/src/common/retryer.js @@ -60,5 +60,5 @@ const retryer = async (fetcher, variables, retries = 0) => { } }; -export { retryer }; +export { retryer, RETRIES }; export default retryer; diff --git a/src/common/utils.js b/src/common/utils.js index 1215fc9ac8cc2..fcaa57dbfbebb 100644 --- a/src/common/utils.js +++ b/src/common/utils.js @@ -293,11 +293,21 @@ const noop = () => {}; const logger = process.env.NODE_ENV !== "test" ? console : { log: noop, error: noop }; +// Cache settings. +const CARD_CACHE_SECONDS = 14400; +const ERROR_CACHE_SECONDS = 600; + const CONSTANTS = { + ONE_MINUTE: 60, + FIVE_MINUTES: 300, + TEN_MINUTES: 600, + FIFTEEN_MINUTES: 900, THIRTY_MINUTES: 1800, TWO_HOURS: 7200, FOUR_HOURS: 14400, ONE_DAY: 86400, + CARD_CACHE_SECONDS: CARD_CACHE_SECONDS, + ERROR_CACHE_SECONDS: ERROR_CACHE_SECONDS, }; const SECONDARY_ERROR_MESSAGES = { diff --git a/tests/api.test.js b/tests/api.test.js index 461f3e18abb6d..09ca98a2925f7 100644 --- a/tests/api.test.js +++ b/tests/api.test.js @@ -171,13 +171,18 @@ describe("Test /api/", () => { ]); }); - it("should not store cache when error", async () => { + it("should set shorter cache when error", async () => { const { req, res } = faker({}, error); await api(req, res); expect(res.setHeader.mock.calls).toEqual([ ["Content-Type", "image/svg+xml"], - ["Cache-Control", `no-cache, no-store, must-revalidate`], + [ + "Cache-Control", + `max-age=${CONSTANTS.ERROR_CACHE_SECONDS / 2}, s-maxage=${ + CONSTANTS.ERROR_CACHE_SECONDS + }, stale-while-revalidate=${CONSTANTS.ONE_DAY}`, + ], ]); }); diff --git a/tests/retryer.test.js b/tests/retryer.test.js index 1fcf6658387b9..6777386d926a3 100644 --- a/tests/retryer.test.js +++ b/tests/retryer.test.js @@ -1,6 +1,6 @@ import { jest } from "@jest/globals"; import "@testing-library/jest-dom"; -import { retryer } from "../src/common/retryer.js"; +import { retryer, RETRIES } from "../src/common/retryer.js"; import { logger } from "../src/common/utils.js"; const fetcher = jest.fn((variables, token) => { @@ -45,7 +45,7 @@ describe("Test Retryer", () => { try { res = await retryer(fetcherFail, {}); } catch (err) { - expect(fetcherFail).toBeCalledTimes(8); + expect(fetcherFail).toBeCalledTimes(RETRIES + 1); expect(err.message).toBe("Maximum retries exceeded"); } }); From c504c9338ed830ad619e008ba9ff76422398687f Mon Sep 17 00:00:00 2001 From: Rick Staa Date: Sun, 22 Jan 2023 10:50:12 +0100 Subject: [PATCH 2/2] my quick tests for implementing stale-if-error --- api/index.js | 23 +++++++- src/common/exceptions.js | 82 +++++++++++++++++++++++++++ src/common/index.js | 3 +- src/common/retryer.js | 15 ++++- src/common/utils.js | 51 +---------------- src/fetchers/repo-fetcher.js | 3 +- src/fetchers/stats-fetcher.js | 3 +- src/fetchers/top-languages-fetcher.js | 3 +- src/fetchers/wakatime-fetcher.js | 2 +- 9 files changed, 122 insertions(+), 63 deletions(-) create mode 100644 src/common/exceptions.js diff --git a/api/index.js b/api/index.js index 251c44ed67c62..2e3fa9c0915d4 100644 --- a/api/index.js +++ b/api/index.js @@ -7,6 +7,7 @@ import { parseBoolean, renderError, } from "../src/common/utils.js"; +import { HttpException } from "../src/common/exceptions.js"; import { fetchStats } from "../src/fetchers/stats-fetcher.js"; import { isLocaleAvailable } from "../src/translations.js"; @@ -65,7 +66,8 @@ export default async (req, res) => { "Cache-Control", `max-age=${ cacheSeconds / 2 - }, s-maxage=${cacheSeconds}, stale-while-revalidate=${CONSTANTS.ONE_DAY}`, + }, s-maxage=${cacheSeconds}, stale-while-revalidate=${CONSTANTS.ONE_DAY}, + stale-if-error=${CONSTANTS.ONE_HOUR}`, ); return res.send( @@ -93,12 +95,27 @@ export default async (req, res) => { }), ); } catch (err) { + // Throw error if REST and GraphQL API calls fail. This way we can return a cached + if (err instanceof HttpException) { + // throw err; + // throw new Error(err.message); + // return res.status(404).; + // return {statusCode: 404, body: err.message} + return res + .status(500) + .send( + renderError(err.errors[0].message, err.errors[0].secondaryMessage), + ); + } + + // Cache the error response less frequently. res.setHeader( "Cache-Control", `max-age=${CONSTANTS.ERROR_CACHE_SECONDS / 2}, s-maxage=${ CONSTANTS.ERROR_CACHE_SECONDS - }, stale-while-revalidate=${CONSTANTS.ONE_DAY}`, - ); // Cache the error response less frequently. + }, stale-while-revalidate=${CONSTANTS.ONE_DAY} + stale-if-error=${CONSTANTS.ONE_HOUR}`, + ); return res.send(renderError(err.message, err.secondaryMessage)); } }; diff --git a/src/common/exceptions.js b/src/common/exceptions.js new file mode 100644 index 0000000000000..b7d752371dbcf --- /dev/null +++ b/src/common/exceptions.js @@ -0,0 +1,82 @@ +/** + * @file GRS Exception/Errors. + */ + +const SECONDARY_ERROR_MESSAGES = { + MAX_RETRY: + "Please add an env variable called PAT_1 with your github token in vercel", + USER_NOT_FOUND: "Make sure the provided username is not an organization", + GRAPHQL_ERROR: "Please try again later", +}; + +/** + * Custom error class to handle custom GRS errors. + * + * @extends Error + */ +class CustomError extends Error { + /** + * @param {string} message Error message. + * @param {string} type Error type. + */ + constructor(message, type) { + super(message); + this.type = type; + this.secondaryMessage = SECONDARY_ERROR_MESSAGES[type] || type; + } + + static MAX_RETRY = "MAX_RETRY"; + static USER_NOT_FOUND = "USER_NOT_FOUND"; + static GRAPHQL_ERROR = "GRAPHQL_ERROR"; +} + +/** + * Missing query parameter class. + * + * @extends Error + */ +class MissingParamError extends Error { + /** + * @param {string[]} missedParams + * @param {string?=} secondaryMessage + */ + constructor(missedParams, secondaryMessage) { + const msg = `Missing params ${missedParams + .map((p) => `"${p}"`) + .join(", ")} make sure you pass the parameters in URL`; + super(msg); + this.missedParams = missedParams; + this.secondaryMessage = secondaryMessage; + } +} + +/** + * HttpException class. + * + * @extends Error + */ +class HttpException extends Error { + /** + * Create a HttpException. + * + * @param {number} statusCode - The status code. + * @param {string} message - The error message. + * @param {string[]} errors - The errors. + */ + constructor( + statusCode, + message = "Exception occurred during the processing of HTTP requests.", + errors = [], + ) { + super(message); + this.statusCode = statusCode; + this.errors = errors; + } +} + +export { + SECONDARY_ERROR_MESSAGES, + CustomError, + MissingParamError, + HttpException, +}; diff --git a/src/common/index.js b/src/common/index.js index 2e7e9cb20fe0b..81e77bb15c906 100644 --- a/src/common/index.js +++ b/src/common/index.js @@ -21,10 +21,9 @@ export { wrapTextMultiline, logger, CONSTANTS, - CustomError, - MissingParamError, measureText, lowercaseTrim, chunkArray, parseEmojis, } from "./utils.js"; +export { CustomError, MissingParamError, HttpException } from "./exceptions.js"; diff --git a/src/common/retryer.js b/src/common/retryer.js index 902d13f6d976b..ef34d74b31eb0 100644 --- a/src/common/retryer.js +++ b/src/common/retryer.js @@ -1,4 +1,8 @@ -import { CustomError, logger } from "./utils.js"; +import { logger } from "./utils.js"; +import { + CustomError, + HttpException, +} from "./exceptions.js"; // Script variables. const PATs = Object.keys(process.env).filter((key) => @@ -16,8 +20,13 @@ const RETRIES = PATs ? PATs : 7; * @returns Promise */ const retryer = async (fetcher, variables, retries = 0) => { - if (retries > RETRIES) { - throw new CustomError("Maximum retries exceeded", CustomError.MAX_RETRY); + // if (retries > RETRIES) { + if (true) { // FIXME: Test out error + throw new HttpException( + statusCode=500, + message=`Max GraphQL retries exceeded. Please add an env variable called PAT_1 with your github token in vercel.`, + errors=[new CustomError("Maximum retries exceeded", CustomError.MAX_RETRY)] + ); } try { // try to fetch with the first token since RETRIES is 0 index i'm adding +1 diff --git a/src/common/utils.js b/src/common/utils.js index fcaa57dbfbebb..952f69120277a 100644 --- a/src/common/utils.js +++ b/src/common/utils.js @@ -22,8 +22,7 @@ const renderError = (message, secondaryMessage = "") => { .small { font: 600 12px 'Segoe UI', Ubuntu, Sans-Serif; fill: #252525 } .gray { fill: #858585 } - Something went wrong! file an issue at https://tiny.one/readme-stats @@ -288,7 +287,7 @@ const wrapTextMultiline = (text, width = 59, maxLines = 3) => { return multiLineText; }; -const noop = () => {}; +const noop = () => { }; // return console instance based on the environment const logger = process.env.NODE_ENV !== "test" ? console : { log: noop, error: noop }; @@ -310,50 +309,6 @@ const CONSTANTS = { ERROR_CACHE_SECONDS: ERROR_CACHE_SECONDS, }; -const SECONDARY_ERROR_MESSAGES = { - MAX_RETRY: - "Please add an env variable called PAT_1 with your github token in vercel", - USER_NOT_FOUND: "Make sure the provided username is not an organization", - GRAPHQL_ERROR: "Please try again later", -}; - -/** - * Custom error class to handle custom GRS errors. - */ -class CustomError extends Error { - /** - * @param {string} message Error message. - * @param {string} type Error type. - */ - constructor(message, type) { - super(message); - this.type = type; - this.secondaryMessage = SECONDARY_ERROR_MESSAGES[type] || type; - } - - static MAX_RETRY = "MAX_RETRY"; - static USER_NOT_FOUND = "USER_NOT_FOUND"; - static GRAPHQL_ERROR = "GRAPHQL_ERROR"; -} - -/** - * Missing query parameter class. - */ -class MissingParamError extends Error { - /** - * @param {string[]} missedParams - * @param {string?=} secondaryMessage - */ - constructor(missedParams, secondaryMessage) { - const msg = `Missing params ${missedParams - .map((p) => `"${p}"`) - .join(", ")} make sure you pass the parameters in URL`; - super(msg); - this.missedParams = missedParams; - this.secondaryMessage = secondaryMessage; - } -} - /** * Retrieve text length. * @@ -451,8 +406,6 @@ export { wrapTextMultiline, logger, CONSTANTS, - CustomError, - MissingParamError, measureText, lowercaseTrim, chunkArray, diff --git a/src/fetchers/repo-fetcher.js b/src/fetchers/repo-fetcher.js index ff7a2be8164cc..59991db874062 100644 --- a/src/fetchers/repo-fetcher.js +++ b/src/fetchers/repo-fetcher.js @@ -1,6 +1,7 @@ // @ts-check import { retryer } from "../common/retryer.js"; -import { MissingParamError, request } from "../common/utils.js"; +import { request } from "../common/utils.js"; +import { MissingParamError } from "../common/exceptions.js"; /** * Repo data fetcher. diff --git a/src/fetchers/stats-fetcher.js b/src/fetchers/stats-fetcher.js index a7df1e504db2f..41ba8579b620b 100644 --- a/src/fetchers/stats-fetcher.js +++ b/src/fetchers/stats-fetcher.js @@ -5,12 +5,11 @@ import githubUsernameRegex from "github-username-regex"; import { calculateRank } from "../calculateRank.js"; import { retryer } from "../common/retryer.js"; import { - CustomError, logger, - MissingParamError, request, wrapTextMultiline, } from "../common/utils.js"; +import { CustomError, MissingParamError } from "../common/exceptions.js"; dotenv.config(); diff --git a/src/fetchers/top-languages-fetcher.js b/src/fetchers/top-languages-fetcher.js index 86d794435be08..cf7c612797e63 100644 --- a/src/fetchers/top-languages-fetcher.js +++ b/src/fetchers/top-languages-fetcher.js @@ -1,12 +1,11 @@ // @ts-check import { retryer } from "../common/retryer.js"; import { - CustomError, logger, - MissingParamError, request, wrapTextMultiline, } from "../common/utils.js"; +import { CustomError, MissingParamError } from "../common/exceptions.js"; /** * Top languages fetcher object. diff --git a/src/fetchers/wakatime-fetcher.js b/src/fetchers/wakatime-fetcher.js index fa1f3d890920f..07d1b7647ce23 100644 --- a/src/fetchers/wakatime-fetcher.js +++ b/src/fetchers/wakatime-fetcher.js @@ -1,5 +1,5 @@ import axios from "axios"; -import { MissingParamError } from "../common/utils.js"; +import { MissingParamError } from "../common/exceptions.js"; /** * WakaTime data fetcher.