Merge pull request #22 from Keyrxng/development

chore: rate limit modal and auto login

.env.example

@@ -1,2 +1,2 @@
 SUPABASE_URL=
-SUPABASE_ANON_KEY=
+SUPABASE_ANON_KEY=

.eslintrc

@@ -27,4 +27,4 @@
       { "selector": "variable", "types": ["boolean"], "format": ["PascalCase"], "prefix": ["is", "has", "should", "can", "did", "will"] }
     ]
   }
-}
+}

cypress/e2e/devpool.cy.ts

@@ -71,14 +71,80 @@ describe("DevPool", () => {
     cy.get('div[id="issues-container"]').children().should("have.length", 2);
   });
 
-  it("Display a message on rate limited", () => {
-    cy.intercept("https://api.github.com/repos/*/*/issues**", (req) => {
-      req.reply({
-        statusCode: 403,
+  describe("Display message on rate limited", () => {
+    const HHMMSS_REGEX = /([01]?[0-9]|2[0-3]):([0-5]?[0-9]):([0-5]?[0-9])/;
+    const PLEASE_LOG_IN = "Please log in to GitHub to increase your GitHub API limits, otherwise you can try again at";
+    const RATE_LIMITED = "You have been rate limited. Please try again at";
+
+    beforeEach(() => {
+      cy.intercept("https://api.github.com/rate_limit", {
+        statusCode: 200,
+        body: {
+          resources: {
+            core: {
+              limit: 5000,
+              used: 5000,
+              remaining: 0,
+              reset: 1617700000,
+            },
+          },
+        },
       });
-    }).as("getIssues");
-    cy.visit("/");
-    cy.get(".preview-header").should("exist");
+      cy.intercept("https://api.github.com/user", (req) => {
+        req.reply({
+          statusCode: 403,
+          body: {},
+          headers: { "x-ratelimit-reset": "1617700000" },
+        });
+      }).as("getUser");
+      cy.intercept("https://api.github.com/repos/*/*/issues**", (req) => {
+        req.reply({
+          statusCode: 403,
+          headers: { "x-ratelimit-reset": "1617700000" },
+        });
+      }).as("getIssues");
+    });
+
+    it("Should display retry timeframe and login request with no tasks and no user", () => {
+      cy.visit("/");
+      cy.get(".preview-header").should("exist");
+      cy.get(".preview-body-inner").should(($body) => {
+        const text = $body.text();
+        expect(text).to.include(PLEASE_LOG_IN);
+        expect(HHMMSS_REGEX.test(text)).to.be.true;
+      });
+    });
+
+    it("Should display retry timeframe with no tasks loaded and a logged in user", () => {
+      cy.intercept("https://api.github.com/user", {
+        statusCode: 200,
+        body: { login: "mockUser" },
+      }).as("getUser");
+
+      cy.visit("/");
+      cy.get(".preview-header").should("exist");
+      cy.get(".preview-body-inner").should(($body) => {
+        const text = $body.text();
+        expect(text).to.include(RATE_LIMITED);
+        expect(HHMMSS_REGEX.test(text)).to.be.true;
+      });
+    });
+
+    it("Should log an error if the auth provider fails", () => {
+      cy.on("window:before:load", (win) => {
+        cy.stub(win.console, "error").as("consoleError");
+      });
+
+      const urlParams = `#error=server_error&error_code=500&error_description=Error getting user profile from external provider`;
+      cy.visit(`/${urlParams}`);
+      cy.get(".preview-header").should("exist");
+      cy.get(".preview-body-inner").should(($body) => {
+        const text = $body.text();
+        expect(text).to.include(PLEASE_LOG_IN);
+        expect(HHMMSS_REGEX.test(text)).to.be.true;
+      });
+      cy.get("@consoleError").should("be.calledWith", "GitHub login provider: Error getting user profile from external provider");
+    });
   });
 
   it("Items can be sorted", () => {

package.json

@@ -26,6 +26,7 @@
     "open-source"
   ],
   "dependencies": {
+    "@octokit/request-error": "^6.1.0",
     "@octokit/rest": "^20.0.2",
     "@supabase/supabase-js": "^2.39.0",
     "dotenv": "^16.3.1",

src/home/fetch-github/fetch-and-display-previews.ts

@@ -66,7 +66,7 @@ export async function fetchAvatars() {
   const urlPattern = /https:\/\/github\.com\/(?<org>[^/]+)\/(?<repo>[^/]+)\/issues\/(?<issue_number>\d+)/;
 
   const avatarPromises = cachedTasks.map(async (task) => {
-    const match = task.preview.body.match(urlPattern);
+    const match = task.preview.body?.match(urlPattern);
     const orgName = match?.groups?.org;
     if (orgName) {
       return fetchAvatar(orgName);

src/home/fetch-github/fetch-issues-preview.ts

@@ -1,9 +1,10 @@
 import { Octokit } from "@octokit/rest";
 import { getGitHubAccessToken, getGitHubUserName } from "../getters/get-github-access-token";
 import { GitHubIssue } from "../github-types";
-import { taskManager } from "../home";
 import { displayPopupMessage } from "../rendering/display-popup-modal";
 import { TaskNoFull } from "./preview-to-full-mapping";
+import { getGitHubUser } from "../getters/get-github-user";
+import { RequestError } from "@octokit/request-error";
 
 async function checkPrivateRepoAccess(): Promise<boolean> {
   const octokit = new Octokit({ auth: await getGitHubAccessToken() });
@@ -23,7 +24,7 @@ async function checkPrivateRepoAccess(): Promise<boolean> {
       }
       return false;
     } catch (error) {
-      if (error.status === 404) {
+      if (error instanceof RequestError && error.status === 404) {
         // If the status is 404, it means the user is not a collaborator, hence no access
         return false;
       } else {
@@ -39,7 +40,6 @@ async function checkPrivateRepoAccess(): Promise<boolean> {
 
 export async function fetchIssuePreviews(): Promise<TaskNoFull[]> {
   const octokit = new Octokit({ auth: await getGitHubAccessToken() });
-
   let freshIssues: GitHubIssue[] = [];
   let hasPrivateRepoAccess = false; // Flag to track access to the private repository
 
@@ -77,14 +77,10 @@ export async function fetchIssuePreviews(): Promise<TaskNoFull[]> {
       freshIssues = publicIssues;
     }
   } catch (error) {
-    if (403 === error.status) {
-      console.error(`GitHub API rate limit exceeded.`);
-      if (taskManager.getTasks().length == 0) {
-        // automatically login if there are no issues loaded
-        automaticLogin(error);
-      }
+    if (error instanceof RequestError && error.status === 403) {
+      await handleRateLimit(octokit, error);
     } else {
-      console.error(`Failed to fetch issue previews: ${error}`);
+      console.error("Error fetching issue previews:", error);
     }
   }
 
@@ -97,12 +93,44 @@ export async function fetchIssuePreviews(): Promise<TaskNoFull[]> {
 
   return tasks;
 }
-function automaticLogin(error: unknown) {
-  const resetTime = error.response.headers["x-ratelimit-reset"];
-  const resetParsed = new Date(resetTime * 1000).toLocaleTimeString();
-
-  displayPopupMessage(
-    `GitHub API rate limit exceeded.`,
-    `You have been rate limited. Please log in to GitHub to increase your GitHub API limits, otherwise you can try again at ${resetParsed}.`
-  );
+
+function rateLimitModal(message: string) {
+  displayPopupMessage(`GitHub API rate limit exceeded.`, message);
+}
+
+type RateLimit = {
+  reset: number | null;
+  user: boolean;
+};
+
+export async function handleRateLimit(octokit?: Octokit, error?: RequestError) {
+  const rate: RateLimit = {
+    reset: null,
+    user: false,
+  };
+
+  if (error?.response?.headers["x-ratelimit-reset"]) {
+    rate.reset = parseInt(error.response.headers["x-ratelimit-reset"]);
+  }
+
+  if (octokit) {
+    try {
+      const core = await octokit.rest.rateLimit.get();
+      const remaining = core.data.resources.core.remaining;
+      const reset = core.data.resources.core.reset;
+
+      rate.reset = !rate.reset && remaining === 0 ? reset : rate.reset;
+      rate.user = (await getGitHubUser()) ? true : false;
+    } catch (err) {
+      console.error("Error handling GitHub rate limit", err);
+    }
+  }
+
+  const resetParsed = rate.reset && new Date(rate.reset * 1000).toLocaleTimeString();
+
+  if (!rate.user) {
+    rateLimitModal(`You have been rate limited. Please log in to GitHub to increase your GitHub API limits, otherwise you can try again at ${resetParsed}.`);
+  } else {
+    rateLimitModal(`You have been rate limited. Please try again at ${resetParsed}.`);
+  }
 }

src/home/getters/get-github-user.ts

@@ -2,15 +2,13 @@ import { Octokit } from "@octokit/rest";
 import { GitHubUser, GitHubUserResponse } from "../github-types";
 import { OAuthToken } from "./get-github-access-token";
 import { getLocalStore } from "./get-local-store";
+import { handleRateLimit } from "../fetch-github/fetch-issues-preview";
+import { RequestError } from "@octokit/request-error";
 declare const SUPABASE_STORAGE_KEY: string; // @DEV: passed in at build time check build/esbuild-build.ts
 
 export async function getGitHubUser(): Promise<GitHubUser | null> {
   const activeSessionToken = await getSessionToken();
-  if (activeSessionToken) {
-    return getNewGitHubUser(activeSessionToken);
-  } else {
-    return null;
-  }
+  return getNewGitHubUser(activeSessionToken);
 }
 
 async function getSessionToken(): Promise<string | null> {
@@ -30,13 +28,22 @@ async function getNewSessionToken(): Promise<string | null> {
   const params = new URLSearchParams(hash.substr(1)); // remove the '#' and parse
   const providerToken = params.get("provider_token");
   if (!providerToken) {
-    return null;
+    const error = params.get("error_description");
+    // supabase auth provider has failed for some reason
+    console.error(`GitHub login provider: ${error}`);
   }
-  return providerToken;
+  return providerToken || null;
 }
 
-async function getNewGitHubUser(providerToken: string): Promise<GitHubUser> {
+async function getNewGitHubUser(providerToken: string | null): Promise<GitHubUser | null> {
   const octokit = new Octokit({ auth: providerToken });
-  const response = (await octokit.request("GET /user")) as GitHubUserResponse;
-  return response.data;
+  try {
+    const response = (await octokit.request("GET /user")) as GitHubUserResponse;
+    return response.data;
+  } catch (error) {
+    if (error instanceof RequestError && error.status === 403) {
+      await handleRateLimit(providerToken ? octokit : undefined, error);
+    }
+  }
+  return null;
 }

tsconfig.json

@@ -1,15 +1,13 @@
 {
   "compilerOptions": {
     /* Visit https://aka.ms/tsconfig to read more about this file */
-
     /* Projects */
     // "incremental": true,                              /* Save .tsbuildinfo files to allow for incremental compilation of projects. */
     // "composite": true,                                /* Enable constraints that allow a TypeScript project to be used with project references. */
     // "tsBuildInfoFile": "./.tsbuildinfo",              /* Specify the path to .tsbuildinfo incremental compilation file. */
     // "disableSourceOfProjectReferenceRedirect": true,  /* Disable preferring source files instead of declaration files when referencing composite projects. */
     // "disableSolutionSearching": true,                 /* Opt a project out of multi-project reference checking when editing. */
     // "disableReferencedProjectLoad": true,             /* Reduce the number of projects loaded automatically by TypeScript. */
-
     /* Language and Environment */
     "target": "es2016" /* Set the JavaScript language version for emitted JavaScript and include compatible library declarations. */,
     "lib": ["DOM", "ESNext"] /* Specify a set of bundled library declaration files that describe the target runtime environment. */,
@@ -23,7 +21,6 @@
     // "noLib": true,                                    /* Disable including any library files, including the default lib.d.ts. */
     // "useDefineForClassFields": true,                  /* Emit ECMAScript-standard-compliant class fields. */
     // "moduleDetection": "auto",                        /* Control what method is used to detect module-format JS files. */
-
     /* Modules */
     "module": "commonjs" /* Specify what module code is generated. */,
     // "rootDir": "./",                                  /* Specify the root folder within your source files. */
@@ -42,12 +39,10 @@
     "resolveJsonModule": true /* Enable importing .json files. */,
     // "allowArbitraryExtensions": true,                 /* Enable importing files with any extension, provided a declaration file is present. */
     // "noResolve": true,                                /* Disallow 'import's, 'require's or '<reference>'s from expanding the number of files TypeScript should add to a project. */
-
     /* JavaScript Support */
     // "allowJs": true,                                  /* Allow JavaScript files to be a part of your program. Use the 'checkJS' option to get errors from these files. */
     // "checkJs": true,                                  /* Enable error reporting in type-checked JavaScript files. */
     // "maxNodeModuleJsDepth": 1,                        /* Specify the maximum folder depth used for checking JavaScript files from 'node_modules'. Only applicable with 'allowJs'. */
-
     /* Emit */
     // "declaration": true,                              /* Generate .d.ts files from TypeScript and JavaScript files in your project. */
     // "declarationMap": true,                           /* Create sourcemaps for d.ts files. */
@@ -72,15 +67,13 @@
     // "preserveConstEnums": true,                       /* Disable erasing 'const enum' declarations in generated code. */
     // "declarationDir": "./",                           /* Specify the output directory for generated declaration files. */
     // "preserveValueImports": true,                     /* Preserve unused imported values in the JavaScript output that would otherwise be removed. */
-
     /* Interop Constraints */
     // "isolatedModules": true,                          /* Ensure that each file can be safely transpiled without relying on other imports. */
     // "verbatimModuleSyntax": true,                     /* Do not transform or elide any imports or exports not marked as type-only, ensuring they are written in the output file's format based on the 'module' setting. */
     // "allowSyntheticDefaultImports": true,             /* Allow 'import x from y' when a module doesn't have a default export. */
     "esModuleInterop": true /* Emit additional JavaScript to ease support for importing CommonJS modules. This enables 'allowSyntheticDefaultImports' for type compatibility. */,
     // "preserveSymlinks": true,                         /* Disable resolving symlinks to their realpath. This correlates to the same flag in node. */
     "forceConsistentCasingInFileNames": true /* Ensure that casing is correct in imports. */,
-
     /* Type Checking */
     "strict": true /* Enable all strict type-checking options. */,
     // "noImplicitAny": true,                            /* Enable error reporting for expressions and declarations with an implied 'any' type. */
@@ -101,7 +94,6 @@
     // "noPropertyAccessFromIndexSignature": true,       /* Enforces using indexed accessors for keys declared using an indexed type. */
     // "allowUnusedLabels": true,                        /* Disable error reporting for unused labels. */
     // "allowUnreachableCode": true,                     /* Disable error reporting for unreachable code. */
-
     /* Completeness */
     // "skipDefaultLibCheck": true,                      /* Skip type checking .d.ts files that are included with TypeScript. */
     "skipLibCheck": true /* Skip type checking all .d.ts files. */