Merge pull request #76 from LordDashMe/fix/node-24-compat-and-504-timeout

Fix node 24 compatibility and 504 timeout

Author: LordDashMe

.gitignore

@@ -2,3 +2,4 @@
 node_modules/
 jest-cache/
 jest-coverage/
+package-lock.json

api/index.js

@@ -9,16 +9,40 @@
 
 const { StargazersController } = require('../src/StargazersController');
 const { ContributionController } = require('../src/ContributionController');
+const { ErrorTemplate } = require('../src/ErrorTemplate');
 
 module.exports = async (req, res) => {
 
   const { username } = req.query;
-  
+
   res.setHeader('Cache-Control', 'public, max-age=1800');
   res.setHeader('Content-Type', 'image/svg+xml');
 
-  const isStargazer = await StargazersController(username);
-  const template = await ContributionController(username, isStargazer);
+  try {
+
+    if (!username) {
+      throw new Error('The "username" query parameter is required.');
+    }
+
+    // The stargazer flag is only needed at the point the card is rendered, so
+    // the lookup is started here without awaiting. That lets it run alongside
+    // the contribution stats request instead of running before it, which
+    // halves the round trips sitting in front of the response.
+    const isStargazer = StargazersController(username);
+    const template = await ContributionController(username, isStargazer);
+
+    res.send(template);
+
+  } catch (error) {
+
+    console.error(error);
 
-  res.send(template);
+    // A response has to be sent on every path. An unhandled rejection here
+    // means the request is never answered and the platform eventually kills
+    // it with a timeout, which is reported as a 504 rather than the real
+    // error. The failure is not cacheable, so the header set above is
+    // replaced before the fallback card is returned.
+    res.setHeader('Cache-Control', 'no-cache, no-store, must-revalidate');
+    res.send(ErrorTemplate(error.message));
+  }
 };

docker-compose.yml

@@ -4,7 +4,7 @@ services:
     container_name: 'github-contribution-stats-app'
     tty: true
     stdin_open: true
-    image: 'node:20-alpine'
+    image: 'node:24-alpine'
     volumes:
       - '.:/app/'
     networks:

package.json

@@ -1,6 +1,6 @@
 {
   "name": "github-contribution-stats",
-  "version": "1.1.1",
+  "version": "1.2.0",
   "description": "Dynamically generated Github Contribution Stats.",
   "main": "index.js",
   "scripts": {
@@ -17,13 +17,16 @@
     "url": "https://github.com/LordDashMe/github-contribution-stats/issues"
   },
   "homepage": "https://github.com/LordDashMe/github-contribution-stats#readme",
+  "engines": {
+    "node": "24.x"
+  },
   "devDependencies": {
-    "axios-mock-adapter": "^1.22.0",
-    "jest": "^29.7.0",
-    "jest-environment-jsdom": "^29.7.0"
+    "axios-mock-adapter": "^2.1.0",
+    "jest": "^30.4.2",
+    "jest-environment-jsdom": "^30.4.1"
   },
   "dependencies": {
-    "axios": "^1.6.8",
-    "dotenv": "^16.4.5"
+    "axios": "^1.19.0",
+    "dotenv": "^17.4.2"
   }
 }

src/ContributionController.js

@@ -16,8 +16,16 @@ const ContributionController = async (username, isStargazer) => {
 
   const stats = await FetchStats(username);
 
+  // A failed request resolves to an empty object, reaching into it blindly
+  // throws a TypeError that reads nothing like the actual cause. The common
+  // reasons are an expired access token, an exhausted rate limit or a
+  // username that does not exist.
+  if (!stats || !stats.data || !stats.data.user) {
+    throw new Error(`Unable to fetch the Github stats for the username "${username}".`);
+  }
+
   const computedContribution = ComputeContributions(stats.data.user.contributionsCollection);
-  
+
   ContributionRatings.newInstance();
   ContributionRatings.setThisYearCommits(computedContribution.thisYear);
   ContributionRatings.setThisMonthCommits(computedContribution.thisMonth);
@@ -27,8 +35,12 @@ const ContributionController = async (username, isStargazer) => {
   ContributionRatings.setCodeReviews(computedContribution.codeReviews);
   ContributionRatings.calculate();
 
+  // Accepts either a plain boolean or the still pending lookup started by the
+  // caller, awaiting a non promise value resolves to the value itself.
+  const stargazer = await isStargazer;
+
   return CardTemplates(
-    isStargazer,
+    stargazer,
     ContributionRatings.getLetterSign(),
     ContributionRatings.getTranslation(),
     ContributionRatings.getColor(), 

src/ErrorTemplate.js

@@ -0,0 +1,59 @@
+/*
+ * This file is part of the Github Contribution Stats.
+ *
+ * (c) Joshua Clifford Reyes <reyesjoshuaclifford@gmail.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+/**
+ * The Error Template Function.
+ *
+ * The endpoint is consumed as an image, so a failure still needs to render as
+ * a valid SVG. Returning nothing leaves the request hanging until the platform
+ * timeout is reached, which surfaces to the user as a broken image.
+ *
+ * @author Joshua Clifford Reyes <reyesjoshuaclifford@gmail.com>
+ *
+ * @param {String} message The reason why the card cannot be generated.
+ *
+ * @return {String}
+ */
+const ErrorTemplate = (message) => {
+
+  // The message is injected into markup, the five XML predefined
+  // entities are escaped so a malformed value cannot break the document.
+  const safeMessage = String(message)
+    .replace(/&/g, '&amp;')
+    .replace(/</g, '&lt;')
+    .replace(/>/g, '&gt;')
+    .replace(/"/g, '&quot;')
+    .replace(/'/g, '&apos;');
+
+  return `
+    <svg width="328" height="120" viewBox="0 0 328 120" xmlns="http://www.w3.org/2000/svg">
+      <style>
+        .error-title {
+          font-family: "Segoe UI", Roboto, Ubuntu, "Helvetica Neue", sans-serif;
+          font-size: 16px;
+          font-weight: 700;
+          fill: #bf3838;
+        }
+        .error-message {
+          font-family: "Segoe UI", Roboto, Ubuntu, "Helvetica Neue", sans-serif;
+          font-size: 11px;
+          fill: #666;
+        }
+      </style>
+      <rect x="0.5" y="0.5" rx="6" height="119" stroke="#e4e2e2" width="327" fill="#fffefe" stroke-opacity="1" />
+      <text x="25" y="40" class="error-title">Github Contribution Stats</text>
+      <text x="25" y="65" class="error-message">Unable to generate the card right now.</text>
+      <text x="25" y="85" class="error-message">${safeMessage}</text>
+    </svg>
+  `;
+};
+
+module.exports = {
+  ErrorTemplate: ErrorTemplate
+};

src/StargazersController.js

@@ -10,57 +10,89 @@
 const { StargazersChecker } = require('./StargazersChecker');
 const { FetchRepositoryStargazers } = require('./FetchRepositoryStargazers');
 
+// The stargazers list is walked one page at a time and a username that never
+// starred the repository only settles on the very last page. Both limits below
+// stop that walk from growing with the repository and taking the whole request
+// down with it, the flag only controls a footer on the card so falling back to
+// "not a stargazer" is far better than failing to answer at all.
+const MAX_PAGES = 10;
+const DEADLINE_IN_MILLISECONDS = 4000;
+
 const StargazersController = async (username, items) => {
 
   // The default value or the max allowed items from the github
   // to be fetch for each page.
   if (typeof items === 'undefined') {
-    items = '100'; 
+    items = '100';
+  }
+
+  if (!username) {
+    return false;
   }
 
+  const deadline = Date.now() + DEADLINE_IN_MILLISECONDS;
+
   let isStargazer = false;
   let nextPage = true;
   let stargazerCheckingEnd = false;
   let cursor = '';
+  let pages = 0;
 
-  do {
+  try {
 
-    let repositoryStargazers = {};
+    do {
 
-    if (cursor) {
-      repositoryStargazers = await FetchRepositoryStargazers(items, cursor); 
-    } else {
-      repositoryStargazers = await FetchRepositoryStargazers(items); 
-    }
+      let repositoryStargazers = {};
 
-    if (typeof repositoryStargazers.data !== 'undefined' && typeof repositoryStargazers.data.repository.stargazers !== 'undefined') {
-      
-      isStargazer = StargazersChecker(username, repositoryStargazers.data.repository.stargazers);
-
-      // Is a legit user who starred the repository :-)
-      if (isStargazer) {
-        stargazerCheckingEnd = true;
-        break;
+      if (cursor) {
+        repositoryStargazers = await FetchRepositoryStargazers(items, cursor);
+      } else {
+        repositoryStargazers = await FetchRepositoryStargazers(items);
       }
-      
-      nextPage = repositoryStargazers.data.repository.stargazers.pageInfo.hasNextPage;
 
-      // At this point we are now sure that the user 
-      // is not a stargazer of the repository.
-      if (!nextPage) {
-        break;
+      pages++;
+
+      if (typeof repositoryStargazers.data !== 'undefined' && typeof repositoryStargazers.data.repository.stargazers !== 'undefined') {
+
+        isStargazer = StargazersChecker(username, repositoryStargazers.data.repository.stargazers);
+
+        // Is a legit user who starred the repository :-)
+        if (isStargazer) {
+          stargazerCheckingEnd = true;
+          break;
+        }
+
+        nextPage = repositoryStargazers.data.repository.stargazers.pageInfo.hasNextPage;
+
+        // At this point we are now sure that the user
+        // is not a stargazer of the repository.
+        if (!nextPage) {
+          break;
+        }
+
+        // Give up the search once either budget is spent, the remaining pages
+        // are not worth the risk of timing out the request.
+        if (pages >= MAX_PAGES || Date.now() >= deadline) {
+          break;
+        }
+
+        // Continue the searching if the user is a stargazer for the repository.
+        // Maybe we can find it on the next page.
+        const edges = repositoryStargazers.data.repository.stargazers.edges;
+        cursor = edges[edges.length - 1].cursor;
+        continue;
       }
 
-      // Continue the searching if the user is a stargazer for the repository.
-      // Maybe we can find it on the next page.
-      const edges = repositoryStargazers.data.repository.stargazers.edges;
-      cursor = edges[edges.length - 1].cursor;
-      continue;
-    }
+      stargazerCheckingEnd = true;
+
+    } while (!stargazerCheckingEnd && nextPage);
 
-    stargazerCheckingEnd = true;
+  } catch (error) {
 
-  } while (!stargazerCheckingEnd && nextPage);
+    // Never let a cosmetic lookup take down the card.
+    console.error(error);
+    return false;
+  }
 
   return isStargazer;
 

tests/src/ContributionController.test.js

@@ -131,4 +131,27 @@ describe('Tests for Contribution Controller.', () => {
 
   });
 
+  it('should throw a readable error when the stats request fails.', async () => {
+
+    // An expired token or an exhausted rate limit resolves to an empty object,
+    // the controller has to name the cause instead of dereferencing into it.
+    mockAxios.onPost('https://api.github.com/graphql').reply(401, {});
+
+    await expect(ContributionController('LordDashMe')).rejects.toThrow(
+      'Unable to fetch the Github stats for the username "LordDashMe".'
+    );
+
+  });
+
+  it('should accept a pending stargazer lookup as well as a plain boolean.', async () => {
+
+    mockAxios.onPost('https://api.github.com/graphql').reply(200, mockGithubGrapQLResponse);
+
+    const fromPromise = await ContributionController('LordDashMe', Promise.resolve(true));
+    const fromBoolean = await ContributionController('LordDashMe', true);
+
+    expect(fromPromise).toBe(fromBoolean);
+
+  });
+
 });

tests/src/ErrorTemplate.test.js

@@ -0,0 +1,24 @@
+const { ErrorTemplate } = require('../../src/ErrorTemplate');
+
+describe('Tests for Error Template.', () => {
+
+  it('should render a valid svg carrying the given message.', () => {
+
+    const template = ErrorTemplate('Something went wrong.');
+
+    expect(template).toContain('<svg');
+    expect(template).toContain('</svg>');
+    expect(template).toContain('Something went wrong.');
+
+  });
+
+  it('should escape the xml predefined entities in the message.', () => {
+
+    const template = ErrorTemplate(`<script>&"'`);
+
+    expect(template).toContain('&lt;script&gt;&amp;&quot;&apos;');
+    expect(template).not.toContain('<script>');
+
+  });
+
+});