From 6465f5c840f22a99a750aea4ec1ceb087d5484d6 Mon Sep 17 00:00:00 2001 From: Keyrxng <106303466+Keyrxng@users.noreply.github.com> Date: Fri, 29 Mar 2024 13:11:29 +0000 Subject: [PATCH] fix: handle rate limit --- cypress/e2e/devpool.cy.ts | 74 ++++++++++++------- src/home/fetch-github/fetch-issues-preview.ts | 55 ++++++++++---- src/home/getters/get-github-user.ts | 24 ++---- .../rendering/render-github-login-button.ts | 2 +- 4 files changed, 96 insertions(+), 59 deletions(-) diff --git a/cypress/e2e/devpool.cy.ts b/cypress/e2e/devpool.cy.ts index cf5d1fda..1c9a1d3f 100644 --- a/cypress/e2e/devpool.cy.ts +++ b/cypress/e2e/devpool.cy.ts @@ -72,60 +72,78 @@ describe("DevPool", () => { }); describe("Display message on rate limited", () => { - it("Should display retry timeframe and login request with no tasks and no user", () => { + 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, + }, + }, + }, + }); 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("include.text", "Please log in to GitHub to increase your GitHub API limits, otherwise you can try again at"); + 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", (req) => { - req.reply({ - statusCode: 200, - body: {}, - }); + cy.intercept("https://api.github.com/user", { + statusCode: 200, + body: { login: "mockUser" }, }).as("getUser"); - cy.intercept("https://api.github.com/repos/*/*/issues**", (req) => { - req.reply({ - statusCode: 403, - }); - }).as("getIssues"); cy.visit("/"); cy.get(".preview-header").should("exist"); - cy.get(".preview-body-inner").should("include.text", "You have been rate limited. Please try again at"); + 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 display a hard one-hour retry timeframe with no auth token available", () => { - const urlParams = `#error=server_error&error_code=500&error_description=Error getting user profile from external provider`; - - cy.intercept("https://api.github.com/user", (req) => { - req.reply({ - statusCode: 403, - body: {}, - }); - }).as("getUser"); - cy.intercept("https://api.github.com/repos/*/*/issues**", (req) => { - req.reply({ - statusCode: 403, - }); - }).as("getIssues"); + 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("include.text", "Your access token may have reached it's rate limit, please try again after one hour."); + 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"); }); }); diff --git a/src/home/fetch-github/fetch-issues-preview.ts b/src/home/fetch-github/fetch-issues-preview.ts index 35759560..0d781544 100644 --- a/src/home/fetch-github/fetch-issues-preview.ts +++ b/src/home/fetch-github/fetch-issues-preview.ts @@ -39,8 +39,6 @@ async function checkPrivateRepoAccess(): Promise { export async function fetchIssuePreviews(): Promise { const octokit = new Octokit({ auth: await getGitHubAccessToken() }); - const user = await getGitHubUser(); - let freshIssues: GitHubIssue[] = []; let hasPrivateRepoAccess = false; // Flag to track access to the private repository @@ -78,18 +76,10 @@ export async function fetchIssuePreviews(): Promise { freshIssues = publicIssues; } } catch (error) { - if (403 === error.status) { - console.error(`GitHub API rate limit exceeded.`); - const resetTime = error.response.headers["x-ratelimit-reset"]; - const resetParsed = new Date(resetTime * 1000).toLocaleTimeString(); - - if (!user || user === null) { - 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}.`); - } + if (error.status === 403) { + await handleRateLimit(octokit, error); + } else { + console.error("Error fetching issue previews:", error); } } @@ -106,3 +96,40 @@ export async function fetchIssuePreviews(): Promise { 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?: unknown) { + const rate: RateLimit = { + reset: null, + user: false, + }; + + if (error) { + rate.reset = 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}.`); + } +} diff --git a/src/home/getters/get-github-user.ts b/src/home/getters/get-github-user.ts index 6a8aadb1..dee4533e 100644 --- a/src/home/getters/get-github-user.ts +++ b/src/home/getters/get-github-user.ts @@ -2,7 +2,7 @@ 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 { displayPopupMessage } from "../rendering/display-popup-modal"; +import { handleRateLimit } from "../fetch-github/fetch-issues-preview"; declare const SUPABASE_STORAGE_KEY: string; // @DEV: passed in at build time check build/esbuild-build.ts export async function getGitHubUser(): Promise { @@ -27,32 +27,24 @@ async function getNewSessionToken(): Promise { const params = new URLSearchParams(hash.substr(1)); // remove the '#' and parse const providerToken = params.get("provider_token"); if (!providerToken) { - const code = params.get("error_code"); - + const error = params.get("error_description"); // supabase auth provider has failed for some reason - if (code === "500") { - displayPopupMessage(`GitHub Login Provider`, `Your access token may have reached its rate limit, please try again after one hour.`); - console.error("GitHub login provider"); - } + console.error(`GitHub login provider: ${error}`); } - return providerToken || null; } async function getNewGitHubUser(providerToken: string | null): Promise { const octokit = new Octokit({ auth: providerToken }); + try { const response = (await octokit.request("GET /user")) as GitHubUserResponse; return response.data; } catch (error: unknown) { - if (error.status === 403 && error.message.includes("API rate limit exceeded")) { - 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}.` - ); + if (error.status === 403) { + await handleRateLimit(providerToken ? octokit : undefined, error); + } else { + console.error("Error getting new GitHub user:", error); } } return null; diff --git a/src/home/rendering/render-github-login-button.ts b/src/home/rendering/render-github-login-button.ts index da4a7352..a8b5f8f9 100644 --- a/src/home/rendering/render-github-login-button.ts +++ b/src/home/rendering/render-github-login-button.ts @@ -18,7 +18,7 @@ export async function checkSupabaseSession() { return session; } -export async function gitHubLoginButtonHandler() { +async function gitHubLoginButtonHandler() { const { error } = await supabase.auth.signInWithOAuth({ provider: "github", options: {