feat: rate limit error chaching (anuraghazra#2448)

* feat: rate limit error chaching Rate limit error caching to alleviate PATs. * refactor: improve code comments
devantler · Sep 24, 2023 · 54120dc · 54120dc
1 parent aaa6c14
commit 54120dc
Show file tree

Hide file tree

Showing 9 changed files with 54 additions and 18 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -42,18 +42,14 @@ 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,
     );
     cacheSeconds = process.env.CACHE_SECONDS
       ? 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
@@ -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
@@ -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
@@ -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
@@ -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");
     }
   });