fix: prevent totalCommitsFetch error result from being cached

This commit makes sure that when the
`https://api.github.com/search/commits?q=author:anuraghazra` API fails
the result is not cached.

Author: rickstaa

api/index.js

@@ -50,20 +50,25 @@ export default async (req, res) => {
   }
 
   try {
-    const stats = await fetchStats(
+    const statsResp = await fetchStats(
       username,
       parseBoolean(count_private),
       parseBoolean(include_all_commits),
       parseArray(exclude_repo),
     );
+    const stats = statsResp.stats;
 
     const cacheSeconds = clampValue(
       parseInt(cache_seconds || CONSTANTS.FOUR_HOURS, 10),
       CONSTANTS.FOUR_HOURS,
       CONSTANTS.ONE_DAY,
     );
 
-    res.setHeader("Cache-Control", `public, max-age=${cacheSeconds}`);
+    if (statsResp.success) {
+      res.setHeader("Cache-Control", `public, max-age=${cacheSeconds}`);
+    } else {
+      res.setHeader("Cache-Control", `no-cache, no-store, must-revalidate`); // Don't cache unsuccessful responses.
+    }
 
     return res.send(
       renderStatsCard(stats, {

src/fetchers/stats-fetcher.js

@@ -67,7 +67,7 @@ const fetcher = (variables, token) => {
  *
  * @param {import('axios').AxiosRequestHeaders} variables Fetcher variables.
  * @param {string} token Github token.
- * @returns {Promise<import('../common/types').StatsFetcherResponse>} Repositories fetcher response.
+ * @returns {Promise<import('../common/types').RepositoriesFetcherResponse>} Repositories fetcher response.
  */
 const repositoriesFetcher = (variables, token) => {
   return request(
@@ -98,46 +98,54 @@ const repositoriesFetcher = (variables, token) => {
   );
 };
 
+/**
+ * Fetch all commits for a given username.
+ *
+ * @param {import('axios').AxiosRequestHeaders} variables Fetcher variables.
+ * @param {string} token Github token.
+ * @returns {Promise<import('../common/types').TotalCommitsFetcherResponse>} Total commits fetcher response.
+ *
+ * @see https://developer.github.com/v3/search/#search-commits.
+ */
+const totalCommitsFetcher = (variables, token) => {
+  return axios({
+    method: "get",
+    url: `https://api.github.com/search/commits?q=author:${variables.login}`,
+    headers: {
+      "Content-Type": "application/json",
+      Accept: "application/vnd.github.cloak-preview",
+      Authorization: `token ${token}`,
+    },
+  });
+};
+
 /**
  * Fetch all the commits for all the repositories of a given username.
  *
  * @param {*} username Github username.
- * @returns {Promise<number>} Total commits.
+ * @returns {Promise<Object>} Object containing the total number of commits and a success boolean.
  *
  * @description Done like this because the Github API does not provide a way to fetch all the commits. See
  * #92#issuecomment-661026467 and #211 for more information.
  */
-const totalCommitsFetcher = async (username) => {
+const fetchTotalCommits = async (username) => {
   if (!githubUsernameRegex.test(username)) {
     logger.log("Invalid username");
-    return 0;
+    return { totalCommits: 0, success: false };
   }
 
-  // https://developer.github.com/v3/search/#search-commits
-  const fetchTotalCommits = (variables, token) => {
-    return axios({
-      method: "get",
-      url: `https://api.github.com/search/commits?q=author:${variables.login}`,
-      headers: {
-        "Content-Type": "application/json",
-        Accept: "application/vnd.github.cloak-preview",
-        Authorization: `token ${token}`,
-      },
-    });
-  };
-
   try {
-    let res = await retryer(fetchTotalCommits, { login: username });
+    let res = await retryer(totalCommitsFetcher, { login: username });
     let total_count = res.data.total_count;
     if (!!total_count && !isNaN(total_count)) {
-      return res.data.total_count;
+      return { totalCommits: res.data.total_count, success: true };
     }
   } catch (err) {
     logger.log(err);
   }
   // just return 0 if there is something wrong so that
   // we don't break the whole app
-  return 0;
+  return { totalCommits: 0, success: false };
 };
 
 /**
@@ -186,7 +194,7 @@ const totalStarsFetcher = async (username, repoToHide) => {
  * @param {string} username Github username.
  * @param {boolean} count_private Include private contributions.
  * @param {boolean} include_all_commits Include all commits.
- * @returns {Promise<import("./types").StatsData>} Stats data.
+ * @returns {Promise<Object>}  Object containing the stats data a success boolean that can be used to handle the caching behavior.
  */
 async function fetchStats(
   username,
@@ -247,9 +255,13 @@ async function fetchStats(
   stats.totalCommits = user.contributionsCollection.totalCommitContributions;
 
   // if include_all_commits then just get that,
-  // since totalCommitsFetcher already sends totalCommits no need to +=
+  // NOTE: Since fetchTotalCommits already sends totalCommits no need to +=.
+  let totalCommitsResp;
+  let success = true;
   if (include_all_commits) {
-    stats.totalCommits = await totalCommitsFetcher(username);
+    totalCommitsResp = await fetchTotalCommits(username);
+    stats.totalCommits = totalCommitsResp.totalCommits;
+    success = totalCommitsResp.success;
   }
 
   // if count_private then add private commits to totalCommits so far.
@@ -274,7 +286,7 @@ async function fetchStats(
     issues: stats.totalIssues,
   });
 
-  return stats;
+  return { stats, success };
 }
 
 export { fetchStats };

tests/fetchStats.test.js

@@ -108,7 +108,7 @@ afterEach(() => {
 
 describe("Test fetchStats", () => {
   it("should fetch correct stats", async () => {
-    let stats = await fetchStats("anuraghazra");
+    let { stats } = await fetchStats("anuraghazra");
     const rank = calculateRank({
       totalCommits: 100,
       totalRepos: 5,
@@ -140,7 +140,7 @@ describe("Test fetchStats", () => {
       .onPost("https://api.github.com/graphql")
       .replyOnce(200, repositoriesWithZeroStarsData);
 
-    let stats = await fetchStats("anuraghazra");
+    let { stats } = await fetchStats("anuraghazra");
     const rank = calculateRank({
       totalCommits: 100,
       totalRepos: 5,
@@ -172,7 +172,7 @@ describe("Test fetchStats", () => {
   });
 
   it("should fetch and add private contributions", async () => {
-    let stats = await fetchStats("anuraghazra", true);
+    let { stats } = await fetchStats("anuraghazra", true);
     const rank = calculateRank({
       totalCommits: 150,
       totalRepos: 5,
@@ -201,7 +201,7 @@ describe("Test fetchStats", () => {
       .onGet("https://api.github.com/search/commits?q=author:anuraghazra")
       .reply(200, { total_count: 1000 });
 
-    let stats = await fetchStats("anuraghazra", true, true);
+    let { stats } = await fetchStats("anuraghazra", true, true);
     const rank = calculateRank({
       totalCommits: 1050,
       totalRepos: 5,
@@ -225,12 +225,37 @@ describe("Test fetchStats", () => {
     });
   });
 
+  it("should return `true` success boolean when 'include_all_commits' is `false`", async () => {
+    let { success } = await fetchStats("anuraghazra", true, false);
+    expect(success).toStrictEqual(true);
+  });
+
+  it("should return `true` success boolean when 'include_all_commits' is `true` and total commits were fetched successfully", async () => {
+    mock
+      .onGet("https://api.github.com/search/commits?q=author:anuraghazra")
+      .reply(200, { total_count: 1000 });
+
+    let { success } = await fetchStats("anuraghazra", true, true);
+    expect(success).toStrictEqual(true);
+  });
+
+  it("should return `false` success boolean when 'include_all_commits' is `true` and total commits could not be fetched", async () => {
+    mock
+      .onGet("https://api.github.com/search/commits?q=author:anuraghazra")
+      .reply(404, "Could not fetch total commits");
+
+    let { success } = await fetchStats("anuraghazra", true, true);
+    expect(success).toStrictEqual(false);
+  });
+
   it("should exclude stars of the `test-repo-1` repository", async () => {
     mock
       .onGet("https://api.github.com/search/commits?q=author:anuraghazra")
       .reply(200, { total_count: 1000 });
 
-    let stats = await fetchStats("anuraghazra", true, true, ["test-repo-1"]);
+    let { stats } = await fetchStats("anuraghazra", true, true, [
+      "test-repo-1",
+    ]);
     const rank = calculateRank({
       totalCommits: 1050,
       totalRepos: 5,