From 7441fdabf953bcc4fd86447ad6442f7d2beabd38 Mon Sep 17 00:00:00 2001 From: Rick Staa Date: Sun, 17 Sep 2023 15:45:17 +0200 Subject: [PATCH] feat: rate limit error chaching (#2448) * feat: rate limit error chaching Rate limit error caching to alleviate PATs. * refactor: improve code comments --- api/gist.js | 7 ++++++- 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 ++-- 9 files changed, 54 insertions(+), 18 deletions(-) diff --git a/api/gist.js b/api/gist.js index 3134bb5c4c65eb..3e7dca040ff02e 100644 --- a/api/gist.js +++ b/api/gist.js @@ -77,7 +77,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}`, + ); // Use lower cache period for errors. return res.send(renderError(err.message, err.secondaryMessage)); } }; diff --git a/api/index.js b/api/index.js index d171c80f907b0c..c513f7cea89a94 100644 --- a/api/index.js +++ b/api/index.js @@ -57,7 +57,7 @@ export default async (req, res) => { ); let cacheSeconds = clampValue( - parseInt(cache_seconds || CONSTANTS.FOUR_HOURS, 10), + parseInt(cache_seconds || CONSTANTS.CARD_CACHE_SECONDS, 10), CONSTANTS.FOUR_HOURS, CONSTANTS.ONE_DAY, ); @@ -100,7 +100,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}`, + ); // Use lower cache period for errors. return res.send(renderError(err.message, err.secondaryMessage)); } }; diff --git a/api/pin.js b/api/pin.js index 3383b00a33ac02..d505746a4a0844 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, ); @@ -83,7 +83,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}`, + ); // Use lower cache period for errors. return res.send(renderError(err.message, err.secondaryMessage)); } }; diff --git a/api/top-langs.js b/api/top-langs.js index fdb6702a32d960..55967519aa6442 100644 --- a/api/top-langs.js +++ b/api/top-langs.js @@ -63,7 +63,7 @@ export default async (req, res) => { ); let cacheSeconds = clampValue( - parseInt(cache_seconds || CONSTANTS.FOUR_HOURS, 10), + parseInt(cache_seconds || CONSTANTS.CARD_CACHE_SECONDS, 10), CONSTANTS.FOUR_HOURS, CONSTANTS.ONE_DAY, ); @@ -99,7 +99,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}`, + ); // Use lower cache period for errors. return res.send(renderError(err.message, err.secondaryMessage)); } }; diff --git a/api/wakatime.js b/api/wakatime.js index ec0c813c081f80..65f026e3747286 100644 --- a/api/wakatime.js +++ b/api/wakatime.js @@ -42,7 +42,7 @@ export default async (req, res) => { const stats = await fetchWakatimeStats({ username, api_domain }); let cacheSeconds = clampValue( - parseInt(cache_seconds || CONSTANTS.FOUR_HOURS, 10), + parseInt(cache_seconds || CONSTANTS.CARD_CACHE_SECONDS, 10), CONSTANTS.FOUR_HOURS, CONSTANTS.ONE_DAY, ); @@ -50,10 +50,6 @@ export default async (req, res) => { ? parseInt(process.env.CACHE_SECONDS, 10) || cacheSeconds : cacheSeconds; - if (!cache_seconds) { - cacheSeconds = CONSTANTS.FOUR_HOURS; - } - res.setHeader( "Cache-Control", `max-age=${ @@ -82,7 +78,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}`, + ); // Use lower cache period for errors. return res.send(renderError(err.message, err.secondaryMessage)); } }; diff --git a/src/common/retryer.js b/src/common/retryer.js index 74f9a0fd3de2c0..3f294d3751327b 100644 --- a/src/common/retryer.js +++ b/src/common/retryer.js @@ -72,5 +72,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 871c49377df9de..3cfd0e19b0737f 100644 --- a/src/common/utils.js +++ b/src/common/utils.js @@ -373,11 +373,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, + ERROR_CACHE_SECONDS, }; const SECONDARY_ERROR_MESSAGES = { diff --git a/tests/api.test.js b/tests/api.test.js index d29db222b9516f..1353f1ffdcce90 100644 --- a/tests/api.test.js +++ b/tests/api.test.js @@ -184,13 +184,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 257a70d1ba40ed..b0b4bd79df857d 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"; import { expect, it, describe } from "@jest/globals"; @@ -44,7 +44,7 @@ describe("Test Retryer", () => { try { await retryer(fetcherFail, {}); } catch (err) { - expect(fetcherFail).toBeCalledTimes(8); + expect(fetcherFail).toBeCalledTimes(RETRIES + 1); expect(err.message).toBe("Downtime due to GitHub API rate limiting"); } });