From 64f56e88b406ff8c7b7fd267f356e64963b9c2fc Mon Sep 17 00:00:00 2001 From: Alexandr Garbuzov Date: Sun, 17 Sep 2023 12:14:40 +0300 Subject: [PATCH] refactor: change confusing behavior of showing 0 commints when upstream API fails (#3238) --- src/fetchers/stats-fetcher.js | 21 +++++++------ tests/fetchStats.test.js | 58 +++++------------------------------ 2 files changed, 19 insertions(+), 60 deletions(-) diff --git a/src/fetchers/stats-fetcher.js b/src/fetchers/stats-fetcher.js index dd62b46..f021a72 100644 --- a/src/fetchers/stats-fetcher.js +++ b/src/fetchers/stats-fetcher.js @@ -153,8 +153,8 @@ const statsFetcher = async (username) => { */ const totalCommitsFetcher = async (username) => { if (!githubUsernameRegex.test(username)) { - logger.log("Invalid username"); - return 0; + logger.log("Invalid username provided."); + throw new Error("Invalid username provided."); } // https://developer.github.com/v3/search/#search-commits @@ -170,18 +170,19 @@ const totalCommitsFetcher = async (username) => { }); }; + let res; try { - let res = await retryer(fetchTotalCommits, { login: username }); - let total_count = res.data.total_count; - if (!!total_count && !isNaN(total_count)) { - return res.data.total_count; - } + res = await retryer(fetchTotalCommits, { login: username }); } catch (err) { logger.log(err); + throw new Error(err); } - // just return 0 if there is something wrong so that - // we don't break the whole app - return 0; + + const totalCount = res.data.total_count; + if (!totalCount || isNaN(totalCount)) { + throw new Error("Could not fetch total commits."); + } + return totalCount; }; /** diff --git a/tests/fetchStats.test.js b/tests/fetchStats.test.js index ac4d0a3..56125d8 100644 --- a/tests/fetchStats.test.js +++ b/tests/fetchStats.test.js @@ -210,62 +210,20 @@ describe("Test fetchStats", () => { }); }); - it("should return 0 commits when all_commits true and invalid username", async () => { - let stats = await fetchStats("asdf///---", true); - expect(stats).toStrictEqual({ - contributedTo: 61, - name: "Anurag Hazra", - totalCommits: 0, - totalIssues: 200, - totalPRs: 300, - totalPRsMerged: 240, - mergedPRsPercentage: 80, - totalReviews: 50, - totalStars: 300, - totalDiscussionsStarted: 10, - totalDiscussionsAnswered: 40, - rank: calculateRank({ - all_commits: true, - commits: 0, - prs: 300, - reviews: 50, - issues: 200, - repos: 5, - stars: 300, - followers: 100, - }), - }); + it("should throw specific error when include_all_commits true and invalid username", async () => { + expect(fetchStats("asdf///---", true)).rejects.toThrow( + new Error("Invalid username provided."), + ); }); - it("should return 0 commits when all_commits true and API returns error", async () => { + it("should throw specific error when include_all_commits true and API returns error", async () => { mock .onGet("https://api.github.com/search/commits?q=author:anuraghazra") .reply(200, { error: "Some test error message" }); - let stats = await fetchStats("anuraghazra", true); - expect(stats).toStrictEqual({ - contributedTo: 61, - name: "Anurag Hazra", - totalCommits: 0, - totalIssues: 200, - totalPRs: 300, - totalPRsMerged: 240, - mergedPRsPercentage: 80, - totalReviews: 50, - totalStars: 300, - totalDiscussionsStarted: 10, - totalDiscussionsAnswered: 40, - rank: calculateRank({ - all_commits: true, - commits: 0, - prs: 300, - reviews: 50, - issues: 200, - repos: 5, - stars: 300, - followers: 100, - }), - }); + expect(fetchStats("anuraghazra", true)).rejects.toThrow( + new Error("Could not fetch total commits."), + ); }); it("should exclude stars of the `test-repo-1` repository", async () => {