From 63c843afac5c243252af75e3ecc5a3ad22e8f4e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=A2=E3=83=AC=E3=82=AF=E3=82=B5=E3=83=B3=E3=83=80?= =?UTF-8?q?=E3=83=BC=2Eeth?= Date: Tue, 4 Jun 2024 19:00:54 +0900 Subject: [PATCH 01/16] feat: add error icon in modal header --- src/home/fetch-github/fetch-issues-preview.ts | 2 +- src/home/rendering/display-popup-modal.ts | 21 +++++++++++++++---- src/home/rendering/render-github-issues.ts | 1 + src/home/rendering/render-preview-modal.ts | 7 +++++++ static/style/inverted-style.css | 8 ++++++- static/style/style.css | 8 ++++++- 6 files changed, 40 insertions(+), 7 deletions(-) diff --git a/src/home/fetch-github/fetch-issues-preview.ts b/src/home/fetch-github/fetch-issues-preview.ts index fe75e99b..70052a6e 100644 --- a/src/home/fetch-github/fetch-issues-preview.ts +++ b/src/home/fetch-github/fetch-issues-preview.ts @@ -95,7 +95,7 @@ export async function fetchIssuePreviews(): Promise { } function rateLimitModal(message: string) { - displayPopupMessage(`GitHub API rate limit exceeded.`, message); + displayPopupMessage(`GitHub API rate limit exceeded.`, message, false); } type RateLimit = { diff --git a/src/home/rendering/display-popup-modal.ts b/src/home/rendering/display-popup-modal.ts index 2306fab6..b8d6f247 100644 --- a/src/home/rendering/display-popup-modal.ts +++ b/src/home/rendering/display-popup-modal.ts @@ -1,12 +1,12 @@ import { toolbar } from "../ready-toolbar"; import { gitHubLoginButton } from "./render-github-login-button"; import { preview, previewBodyInner, titleAnchor, titleHeader } from "./render-preview-modal"; -export function displayPopupMessage(header: string, message: string, url?: string) { - titleHeader.textContent = header; +export function displayPopupMessage(modalHeader: string, modalBody: string, isError: boolean, url?: string) { + titleHeader.textContent = modalHeader; if (url) { titleAnchor.href = url; } - previewBodyInner.innerHTML = message; + previewBodyInner.innerHTML = modalBody; preview.classList.add("active"); document.body.classList.add("preview-active"); @@ -17,6 +17,19 @@ export function displayPopupMessage(header: string, message: string, url?: strin behavior: "smooth", }); - gitHubLoginButton?.classList.add("highlight"); + if (isError) { + preview.classList.add("error"); + } else { + preview.classList.remove("error"); + gitHubLoginButton?.classList.add("highlight"); + } + } +} + +export function showError(error: string, showToast = false, description?: string) { + console.error(error, description); + + if (showToast) { + displayPopupMessage("Something went wrong", error, true); } } diff --git a/src/home/rendering/render-github-issues.ts b/src/home/rendering/render-github-issues.ts index 2d585a59..3d419537 100644 --- a/src/home/rendering/render-github-issues.ts +++ b/src/home/rendering/render-github-issues.ts @@ -174,6 +174,7 @@ export function viewIssueDetails(full: GitHubIssue) { // Show the preview preview.classList.add("active"); + preview.classList.remove("error"); document.body.classList.add("preview-active"); } diff --git a/src/home/rendering/render-preview-modal.ts b/src/home/rendering/render-preview-modal.ts index af5d46dc..c5c1607f 100644 --- a/src/home/rendering/render-preview-modal.ts +++ b/src/home/rendering/render-preview-modal.ts @@ -22,6 +22,13 @@ const openNewLinkIcon = ``; +const error = document.createElement("span"); +error.classList.add("error"); +error.innerHTML = errorIcon; + +titleAnchor.appendChild(error); titleAnchor.appendChild(openNewLink); previewHeader.appendChild(titleAnchor); previewBody.appendChild(previewBodyInner); diff --git a/static/style/inverted-style.css b/static/style/inverted-style.css index 7990a2e7..ec4cbe55 100644 --- a/static/style/inverted-style.css +++ b/static/style/inverted-style.css @@ -466,12 +466,18 @@ .preview a { word-break: break-all; } - .preview a[href*="//"] .open-new-link svg + .preview .preview-header a[href*="//"] svg { fill: #00000080; vertical-align: middle; height: 20px; } + .preview.active.error > div > div.preview-header > a > span.error { + display: unset; + } + .preview.active > div > div.preview-header > a > span.error { + display: none; + } .preview-body-inner { line-height: 1.25; } diff --git a/static/style/style.css b/static/style/style.css index 942f24f8..eaf83ffe 100644 --- a/static/style/style.css +++ b/static/style/style.css @@ -466,12 +466,18 @@ .preview a { word-break: break-all; } - .preview a[href*="//"] .open-new-link svg + .preview .preview-header a[href*="//"] svg { fill: #ffffff80; vertical-align: middle; height: 20px; } + .preview.active.error > div > div.preview-header > a > span.error { + display: unset; + } + .preview.active > div > div.preview-header > a > span.error { + display: none; + } .preview-body-inner { line-height: 1.25; } From fcc3c32a3d7b6e7a4e1459df7c1a7df62e30fbc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=A2=E3=83=AC=E3=82=AF=E3=82=B5=E3=83=B3=E3=83=80?= =?UTF-8?q?=E3=83=BC=2Eeth?= Date: Tue, 4 Jun 2024 18:50:51 +0900 Subject: [PATCH 02/16] chore: rounded error icon fits better --- src/home/rendering/render-preview-modal.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/home/rendering/render-preview-modal.ts b/src/home/rendering/render-preview-modal.ts index c5c1607f..7ffad85e 100644 --- a/src/home/rendering/render-preview-modal.ts +++ b/src/home/rendering/render-preview-modal.ts @@ -23,7 +23,7 @@ const openNewLink = document.createElement("span"); openNewLink.classList.add("open-new-link"); openNewLink.innerHTML = openNewLinkIcon; -const errorIcon = ``; +const errorIcon = ``; const error = document.createElement("span"); error.classList.add("error"); error.innerHTML = errorIcon; From 7459f8363c20175d3bd3b241f317915ca993061c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=A2=E3=83=AC=E3=82=AF=E3=82=B5=E3=83=B3=E3=83=80?= =?UTF-8?q?=E3=83=BC=2Eeth?= Date: Tue, 4 Jun 2024 18:56:35 +0900 Subject: [PATCH 03/16] feat: render uncaught errors in modal --- src/home/home.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/home/home.ts b/src/home/home.ts index 2a5c627b..1c0c979f 100644 --- a/src/home/home.ts +++ b/src/home/home.ts @@ -20,6 +20,14 @@ if (!container) { export const taskManager = new TaskManager(container); // window["taskManager"] = taskManager; +window.onerror = function renderErrorsInModal(event: Event | string, url?: string, line?: number, col?: number, errorObj?: Error) { + if (typeof event === "string") { + showError(`${event}`, true, `Error: ${errorObj?.name}`); + } else { + showError(`Error: ${event.type}`, true); + } +}; + void (async function home() { try { void authentication(); From a4a36892080e1dcbf475ef12e52b8b900f7b25c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=A2=E3=83=AC=E3=82=AF=E3=82=B5=E3=83=B3=E3=83=80?= =?UTF-8?q?=E3=83=BC=2Eeth?= Date: Tue, 4 Jun 2024 19:03:14 +0900 Subject: [PATCH 04/16] fix: import showError --- src/home/home.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/home/home.ts b/src/home/home.ts index 1c0c979f..3e9614e8 100644 --- a/src/home/home.ts +++ b/src/home/home.ts @@ -6,6 +6,7 @@ import { fetchIssuesFull } from "./fetch-github/fetch-issues-full"; import { readyToolbar } from "./ready-toolbar"; import { generateSortingToolbar } from "./sorting/generate-sorting-buttons"; import { TaskManager } from "./task-manager"; +import { showError } from './rendering/display-popup-modal'; initiateDevRelTracking(); generateSortingToolbar(); @@ -67,3 +68,8 @@ function renderServiceMessage() { } } } + +// @ts-expect-error this is for testing purposes only +window["injectErrorTester"] = function injectErrorTester(error: string = "Test error", description: string = "Test description") { + showError(`${error}`, true, description); +}; From c05c44977174a83f649274112be63a6eb18366a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=A2=E3=83=AC=E3=82=AF=E3=82=B5=E3=83=B3=E3=83=80?= =?UTF-8?q?=E3=83=BC=2Eeth?= Date: Tue, 4 Jun 2024 19:05:38 +0900 Subject: [PATCH 05/16] fix: detail around only showing error or link to issue modal icon --- src/home/home.ts | 2 +- static/style/inverted-style.css | 8 ++++++++ static/style/style.css | 8 ++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/home/home.ts b/src/home/home.ts index 3e9614e8..b873e981 100644 --- a/src/home/home.ts +++ b/src/home/home.ts @@ -6,7 +6,7 @@ import { fetchIssuesFull } from "./fetch-github/fetch-issues-full"; import { readyToolbar } from "./ready-toolbar"; import { generateSortingToolbar } from "./sorting/generate-sorting-buttons"; import { TaskManager } from "./task-manager"; -import { showError } from './rendering/display-popup-modal'; +import { showError } from "./rendering/display-popup-modal"; initiateDevRelTracking(); generateSortingToolbar(); diff --git a/static/style/inverted-style.css b/static/style/inverted-style.css index ec4cbe55..068879ad 100644 --- a/static/style/inverted-style.css +++ b/static/style/inverted-style.css @@ -472,6 +472,14 @@ vertical-align: middle; height: 20px; } + + .preview.active.error .preview-header > a { + pointer-events: none; + } + .preview.active.error .preview-header > a .open-new-link { + display: none; + } + .preview.active.error > div > div.preview-header > a > span.error { display: unset; } diff --git a/static/style/style.css b/static/style/style.css index eaf83ffe..69dc45a1 100644 --- a/static/style/style.css +++ b/static/style/style.css @@ -472,6 +472,14 @@ vertical-align: middle; height: 20px; } + + .preview.active.error .preview-header > a { + pointer-events: none; + } + .preview.active.error .preview-header > a .open-new-link { + display: none; + } + .preview.active.error > div > div.preview-header > a > span.error { display: unset; } From 1fa3d3ff11065757576bfd8014ffb60da36cf208 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=A2=E3=83=AC=E3=82=AF=E3=82=B5=E3=83=B3=E3=83=80?= =?UTF-8?q?=E3=83=BC=2Eeth?= Date: Tue, 4 Jun 2024 19:17:48 +0900 Subject: [PATCH 06/16] chore: currently testing to catch uncaught errors --- src/home/fetch-github/fetch-issues-preview.ts | 6 ++--- src/home/home.ts | 26 +++++++++++-------- src/home/rendering/display-popup-modal.ts | 9 +++---- 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/home/fetch-github/fetch-issues-preview.ts b/src/home/fetch-github/fetch-issues-preview.ts index 70052a6e..e1877637 100644 --- a/src/home/fetch-github/fetch-issues-preview.ts +++ b/src/home/fetch-github/fetch-issues-preview.ts @@ -1,10 +1,10 @@ +import { RequestError } from "@octokit/request-error"; import { Octokit } from "@octokit/rest"; import { getGitHubAccessToken, getGitHubUserName } from "../getters/get-github-access-token"; +import { getGitHubUser } from "../getters/get-github-user"; import { GitHubIssue } from "../github-types"; 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 { const octokit = new Octokit({ auth: await getGitHubAccessToken() }); @@ -95,7 +95,7 @@ export async function fetchIssuePreviews(): Promise { } function rateLimitModal(message: string) { - displayPopupMessage(`GitHub API rate limit exceeded.`, message, false); + displayPopupMessage({ modalHeader: `GitHub API rate limit exceeded.`, modalBody: message, isError: false }); } type RateLimit = { diff --git a/src/home/home.ts b/src/home/home.ts index b873e981..59567c6b 100644 --- a/src/home/home.ts +++ b/src/home/home.ts @@ -4,9 +4,9 @@ import { initiateDevRelTracking } from "./devrel-tracker"; import { fetchAndDisplayPreviewsFromCache } from "./fetch-github/fetch-and-display-previews"; import { fetchIssuesFull } from "./fetch-github/fetch-issues-full"; import { readyToolbar } from "./ready-toolbar"; +import { showError } from "./rendering/display-popup-modal"; import { generateSortingToolbar } from "./sorting/generate-sorting-buttons"; import { TaskManager } from "./task-manager"; -import { showError } from "./rendering/display-popup-modal"; initiateDevRelTracking(); generateSortingToolbar(); @@ -21,14 +21,6 @@ if (!container) { export const taskManager = new TaskManager(container); // window["taskManager"] = taskManager; -window.onerror = function renderErrorsInModal(event: Event | string, url?: string, line?: number, col?: number, errorObj?: Error) { - if (typeof event === "string") { - showError(`${event}`, true, `Error: ${errorObj?.name}`); - } else { - showError(`Error: ${event.type}`, true); - } -}; - void (async function home() { try { void authentication(); @@ -70,6 +62,18 @@ function renderServiceMessage() { } // @ts-expect-error this is for testing purposes only -window["injectErrorTester"] = function injectErrorTester(error: string = "Test error", description: string = "Test description") { - showError(`${error}`, true, description); +window["showErrorTester"] = function showErrorTester(error: string = "Test error", description: string = "Test description") { + showError(error, description); +}; + +// @ts-expect-error this is for testing purposes only +window["throwUncaughtError"] = function throwUncaughtError() { + throw new Error("Test error"); +}; + +window.onerror = function renderErrorsInModal(event: Event | string, url?: string, line?: number, col?: number, errorObj?: Error) { + const error = errorObj || new Error(event as string); + const description = `URL: ${url}, Line: ${line}, Col: ${col}, Error: ${error.message}`; + showError("Uncaught Error", description); + return false; }; diff --git a/src/home/rendering/display-popup-modal.ts b/src/home/rendering/display-popup-modal.ts index b8d6f247..a5a26e67 100644 --- a/src/home/rendering/display-popup-modal.ts +++ b/src/home/rendering/display-popup-modal.ts @@ -1,7 +1,7 @@ import { toolbar } from "../ready-toolbar"; import { gitHubLoginButton } from "./render-github-login-button"; import { preview, previewBodyInner, titleAnchor, titleHeader } from "./render-preview-modal"; -export function displayPopupMessage(modalHeader: string, modalBody: string, isError: boolean, url?: string) { +export function displayPopupMessage({ modalHeader, modalBody, isError, url }: { modalHeader: string; modalBody: string; isError: boolean; url?: string }) { titleHeader.textContent = modalHeader; if (url) { titleAnchor.href = url; @@ -26,10 +26,7 @@ export function displayPopupMessage(modalHeader: string, modalBody: string, isEr } } -export function showError(error: string, showToast = false, description?: string) { +export function showError(error: string, description: string) { console.error(error, description); - - if (showToast) { - displayPopupMessage("Something went wrong", error, true); - } + displayPopupMessage({ modalHeader: error, modalBody: description, isError: true }); } From 59b8cc949382ce96aeaae3bd6814bf1e822f4805 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=A2=E3=83=AC=E3=82=AF=E3=82=B5=E3=83=B3=E3=83=80?= =?UTF-8?q?=E3=83=BC=2Eeth?= Date: Tue, 4 Jun 2024 19:39:15 +0900 Subject: [PATCH 07/16] refactor: clean up error logic, and tested by throwing error in program --- src/home/home.ts | 51 +++++++++-------------- src/home/rendering/display-popup-modal.ts | 11 +++-- 2 files changed, 28 insertions(+), 34 deletions(-) diff --git a/src/home/home.ts b/src/home/home.ts index 59567c6b..464216ae 100644 --- a/src/home/home.ts +++ b/src/home/home.ts @@ -4,13 +4,23 @@ import { initiateDevRelTracking } from "./devrel-tracker"; import { fetchAndDisplayPreviewsFromCache } from "./fetch-github/fetch-and-display-previews"; import { fetchIssuesFull } from "./fetch-github/fetch-issues-full"; import { readyToolbar } from "./ready-toolbar"; -import { showError } from "./rendering/display-popup-modal"; +import { renderErrorInModal } from "./rendering/display-popup-modal"; import { generateSortingToolbar } from "./sorting/generate-sorting-buttons"; import { TaskManager } from "./task-manager"; +// All unhandled errors are caught and displayed in a modal +window.addEventListener("error", (event: ErrorEvent) => renderErrorInModal(event.error)); + +// All unhandled promise rejections are caught and displayed in a modal +window.addEventListener("unhandledrejection", (event: PromiseRejectionEvent) => { + renderErrorInModal(event.reason as Error); + event.preventDefault(); +}); + initiateDevRelTracking(); generateSortingToolbar(); renderServiceMessage(); + grid(document.getElementById("grid") as HTMLElement, () => document.body.classList.add("grid-loaded")); // @DEV: display grid background const container = document.getElementById("issues-container") as HTMLDivElement; @@ -19,21 +29,15 @@ if (!container) { } export const taskManager = new TaskManager(container); -// window["taskManager"] = taskManager; void (async function home() { - try { - void authentication(); - void readyToolbar(); - const previews = await fetchAndDisplayPreviewsFromCache(); - const fullTasks = await fetchIssuesFull(previews); - taskManager.syncTasks(fullTasks); - console.trace({ fullTasks }); - await taskManager.writeToStorage(); - return fullTasks; - } catch (error) { - console.error(error); - } + void authentication(); + void readyToolbar(); + const previews = await fetchAndDisplayPreviewsFromCache(); + const fullTasks = await fetchIssuesFull(previews); + taskManager.syncTasks(fullTasks); + console.trace({ fullTasks }); + await taskManager.writeToStorage(); if ("serviceWorker" in navigator) { window.addEventListener("load", () => { @@ -47,6 +51,8 @@ void (async function home() { ); }); } + + return fullTasks; })(); function renderServiceMessage() { @@ -60,20 +66,3 @@ function renderServiceMessage() { } } } - -// @ts-expect-error this is for testing purposes only -window["showErrorTester"] = function showErrorTester(error: string = "Test error", description: string = "Test description") { - showError(error, description); -}; - -// @ts-expect-error this is for testing purposes only -window["throwUncaughtError"] = function throwUncaughtError() { - throw new Error("Test error"); -}; - -window.onerror = function renderErrorsInModal(event: Event | string, url?: string, line?: number, col?: number, errorObj?: Error) { - const error = errorObj || new Error(event as string); - const description = `URL: ${url}, Line: ${line}, Col: ${col}, Error: ${error.message}`; - showError("Uncaught Error", description); - return false; -}; diff --git a/src/home/rendering/display-popup-modal.ts b/src/home/rendering/display-popup-modal.ts index a5a26e67..3c97e345 100644 --- a/src/home/rendering/display-popup-modal.ts +++ b/src/home/rendering/display-popup-modal.ts @@ -26,7 +26,12 @@ export function displayPopupMessage({ modalHeader, modalBody, isError, url }: { } } -export function showError(error: string, description: string) { - console.error(error, description); - displayPopupMessage({ modalHeader: error, modalBody: description, isError: true }); +export function renderErrorInModal(error: Error) { + console.error(error); + displayPopupMessage({ + modalHeader: error.name, + modalBody: error.message, + isError: true, + }); + return false; } From 26b3121c5133516d40b799fdca1f44a44a1ef654 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=A2=E3=83=AC=E3=82=AF=E3=82=B5=E3=83=B3=E3=83=80?= =?UTF-8?q?=E3=83=BC=2Eeth?= Date: Tue, 4 Jun 2024 19:45:06 +0900 Subject: [PATCH 08/16] chore: more manual testing of error displaying --- src/home/rendering/display-popup-modal.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/home/rendering/display-popup-modal.ts b/src/home/rendering/display-popup-modal.ts index 3c97e345..5910d0ac 100644 --- a/src/home/rendering/display-popup-modal.ts +++ b/src/home/rendering/display-popup-modal.ts @@ -16,13 +16,13 @@ export function displayPopupMessage({ modalHeader, modalBody, isError, url }: { left: toolbar.scrollWidth, behavior: "smooth", }); + } - if (isError) { - preview.classList.add("error"); - } else { - preview.classList.remove("error"); - gitHubLoginButton?.classList.add("highlight"); - } + if (isError) { + preview.classList.add("error"); + } else { + preview.classList.remove("error"); + gitHubLoginButton?.classList.add("highlight"); } } From 45cc92271a4ee220f5f13a26e4052c06aad719a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=A2=E3=83=AC=E3=82=AF=E3=82=B5=E3=83=B3=E3=83=80?= =?UTF-8?q?=E3=83=BC=2Eeth?= Date: Tue, 4 Jun 2024 20:33:35 +0900 Subject: [PATCH 09/16] test: catch missing env vars --- cypress/e2e/devpool.cy.ts | 28 ++++++++++++++++++++++++++-- src/home/home.ts | 1 - 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/cypress/e2e/devpool.cy.ts b/cypress/e2e/devpool.cy.ts index 1c9a1d3f..eba1fda6 100644 --- a/cypress/e2e/devpool.cy.ts +++ b/cypress/e2e/devpool.cy.ts @@ -1,5 +1,12 @@ import { RestEndpointMethodTypes } from "@octokit/rest"; +const GITHUB_USERNAME = Cypress.env("GITHUB_USERNAME"); +const GITHUB_PASSWORD = Cypress.env("GITHUB_PASSWORD"); + +if (!GITHUB_USERNAME || !GITHUB_PASSWORD) { + throw new Error("Please provide GITHUB_USERNAME and GITHUB_PASSWORD environment variables"); +} + describe("DevPool", () => { let issue1: RestEndpointMethodTypes["issues"]["get"]["response"]["data"]; let issue2: RestEndpointMethodTypes["issues"]["get"]["response"]["data"]; @@ -186,8 +193,8 @@ describe("DevPool", () => { cy.get("#filter").should("not.be.visible"); cy.get("#github-login-button").click(); cy.origin("https://github.com/login", () => { - cy.get("#login_field").type(Cypress.env("GITHUB_USERNAME")); - cy.get("#password").type(Cypress.env("GITHUB_PASSWORD")); + cy.get("#login_field").type(GITHUB_USERNAME); + cy.get("#password").type(GITHUB_PASSWORD); cy.get(".position-relative > .btn").click(); // This part of the test can sometimes fail if the endpoint for OAuth is hit too many times, asking the user to // authorize the app again. It should not happen in a normal testing scenario since it's only hit once, but more @@ -205,4 +212,21 @@ describe("DevPool", () => { cy.get("#authenticated").should("exist"); cy.get("#filter").should("be.visible"); }); + + describe("Display error modal", () => { + it("should display an error modal when fetching issue previews fails on page load", () => { + cy.intercept("GET", "https://api.github.com/repos/ubiquity/devpool-directory/issues*", { + statusCode: 500, + body: "Internal Server Error", + }).as("getPublicIssues"); + + cy.visit("/"); + + cy.wait("@getPublicIssues"); + + cy.get(".preview-header").should("be.visible"); + cy.get(".preview-header").should("contain", "Something went wrong"); + cy.get(".preview-body-inner").should("contain", "HttpError: Internal Server Error"); + }); + }); }); diff --git a/src/home/home.ts b/src/home/home.ts index 464216ae..d51bc7e2 100644 --- a/src/home/home.ts +++ b/src/home/home.ts @@ -51,7 +51,6 @@ void (async function home() { ); }); } - return fullTasks; })(); From e5e3511f7fd2a8b5a553732276ec6785010d7c64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=A2=E3=83=AC=E3=82=AF=E3=82=B5=E3=83=B3=E3=83=80?= =?UTF-8?q?=E3=83=BC=2Eeth?= Date: Tue, 4 Jun 2024 20:57:29 +0900 Subject: [PATCH 10/16] test: fix env var loader --- .cspell.json | 6 +++++- .env.example | 4 +++- cypress.config.ts | 19 +++++++++++++++---- cypress/e2e/devpool.cy.ts | 14 +++++--------- package.json | 3 ++- 5 files changed, 30 insertions(+), 16 deletions(-) diff --git a/.cspell.json b/.cspell.json index c477c7f7..dccf6a5c 100644 --- a/.cspell.json +++ b/.cspell.json @@ -1,4 +1,8 @@ { "version": "0.2", - "words": ["devpool", "supabase"] + "words": [ + "devpool", + "supabase", + "UBIQUIBOT" + ] } diff --git a/.env.example b/.env.example index f7321ea2..00f6613d 100644 --- a/.env.example +++ b/.env.example @@ -1,2 +1,4 @@ SUPABASE_URL= -SUPABASE_ANON_KEY= \ No newline at end of file +SUPABASE_ANON_KEY= +UBIQUIBOT_GITHUB_USERNAME= +UBIQUIBOT_GITHUB_PASSWORD= \ No newline at end of file diff --git a/cypress.config.ts b/cypress.config.ts index 3481949e..799dfe65 100644 --- a/cypress.config.ts +++ b/cypress.config.ts @@ -11,10 +11,21 @@ export default defineConfig({ }, viewportHeight: 900, viewportWidth: 1440, - env: { - GITHUB_USERNAME: process.env.UBIQUIBOT_GITHUB_USERNAME, - GITHUB_PASSWORD: process.env.UBIQUIBOT_GITHUB_PASSWORD, - }, + env: readEnvironmentVariables(), watchForFileChanges: false, video: true, }); + +function readEnvironmentVariables() { + const UBIQUIBOT_GITHUB_USERNAME = process.env["UBIQUIBOT_GITHUB_USERNAME"]; + const UBIQUIBOT_GITHUB_PASSWORD = process.env["UBIQUIBOT_GITHUB_PASSWORD"]; + + if (!UBIQUIBOT_GITHUB_USERNAME) { + throw new Error("Please provide `UBIQUIBOT_GITHUB_USERNAME` environment variable"); + } + + if (!UBIQUIBOT_GITHUB_PASSWORD) { + throw new Error("Please provide `UBIQUIBOT_GITHUB_PASSWORD` environment variable"); + } + return { UBIQUIBOT_GITHUB_USERNAME, UBIQUIBOT_GITHUB_PASSWORD }; +} diff --git a/cypress/e2e/devpool.cy.ts b/cypress/e2e/devpool.cy.ts index eba1fda6..ca41f575 100644 --- a/cypress/e2e/devpool.cy.ts +++ b/cypress/e2e/devpool.cy.ts @@ -1,12 +1,5 @@ import { RestEndpointMethodTypes } from "@octokit/rest"; -const GITHUB_USERNAME = Cypress.env("GITHUB_USERNAME"); -const GITHUB_PASSWORD = Cypress.env("GITHUB_PASSWORD"); - -if (!GITHUB_USERNAME || !GITHUB_PASSWORD) { - throw new Error("Please provide GITHUB_USERNAME and GITHUB_PASSWORD environment variables"); -} - describe("DevPool", () => { let issue1: RestEndpointMethodTypes["issues"]["get"]["response"]["data"]; let issue2: RestEndpointMethodTypes["issues"]["get"]["response"]["data"]; @@ -193,8 +186,11 @@ describe("DevPool", () => { cy.get("#filter").should("not.be.visible"); cy.get("#github-login-button").click(); cy.origin("https://github.com/login", () => { - cy.get("#login_field").type(GITHUB_USERNAME); - cy.get("#password").type(GITHUB_PASSWORD); + const username = Cypress.env("UBIQUIBOT_GITHUB_USERNAME"); + const password = Cypress.env("UBIQUIBOT_GITHUB_PASSWORD"); + + cy.get("#login_field").type(username); + cy.get("#password").type(password, { parseSpecialCharSequences: false }); cy.get(".position-relative > .btn").click(); // This part of the test can sometimes fail if the endpoint for OAuth is hit too many times, asking the user to // authorize the app again. It should not happen in a normal testing scenario since it's only hit once, but more diff --git a/package.json b/package.json index 8b4eaf4a..8a8b13b0 100644 --- a/package.json +++ b/package.json @@ -16,7 +16,8 @@ "format:prettier": "prettier --write .", "prepare": "husky install", "cypress:open": "cypress open", - "cypress:run": "cypress run" + "cypress:run": "cypress run", + "cypress:headed": "cypress run --headed" }, "keywords": [ "typescript", From 011a412c896d11be8a1ad2b51f561a9cb741c22c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=A2=E3=83=AC=E3=82=AF=E3=82=B5=E3=83=B3=E3=83=80?= =?UTF-8?q?=E3=83=BC=2Eeth?= Date: Wed, 5 Jun 2024 15:05:52 +0900 Subject: [PATCH 11/16] fix: display caught errors in modal as well --- src/home/fetch-github/fetch-avatar.ts | 3 ++- src/home/fetch-github/fetch-issues-preview.ts | 6 +++--- src/home/getters/get-github-user.ts | 6 ++++-- src/home/getters/get-local-store.ts | 3 ++- src/home/rendering/display-popup-modal.ts | 8 ++++++-- 5 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/home/fetch-github/fetch-avatar.ts b/src/home/fetch-github/fetch-avatar.ts index 98b7bb23..7b1c13b7 100644 --- a/src/home/fetch-github/fetch-avatar.ts +++ b/src/home/fetch-github/fetch-avatar.ts @@ -1,6 +1,7 @@ import { Octokit } from "@octokit/rest"; import { getGitHubAccessToken } from "../getters/get-github-access-token"; import { getImageFromCache, saveImageToCache } from "../getters/get-indexed-db"; +import { renderErrorInModal } from '../rendering/display-popup-modal'; import { organizationImageCache } from "./fetch-issues-full"; export async function fetchAvatar(orgName: string) { @@ -38,7 +39,7 @@ export async function fetchAvatar(orgName: string) { organizationImageCache.set(orgName, blob); } } catch (error) { - console.error(`Failed to fetch avatar for organization ${orgName}: ${error}`); + renderErrorInModal(error as Error, `Failed to fetch avatar for organization ${orgName}: ${error}`); const { data: { avatar_url: avatarUrl }, } = await octokit.rest.users.getByUsername({ username: orgName }); diff --git a/src/home/fetch-github/fetch-issues-preview.ts b/src/home/fetch-github/fetch-issues-preview.ts index e1877637..52041923 100644 --- a/src/home/fetch-github/fetch-issues-preview.ts +++ b/src/home/fetch-github/fetch-issues-preview.ts @@ -3,7 +3,7 @@ import { Octokit } from "@octokit/rest"; import { getGitHubAccessToken, getGitHubUserName } from "../getters/get-github-access-token"; import { getGitHubUser } from "../getters/get-github-user"; import { GitHubIssue } from "../github-types"; -import { displayPopupMessage } from "../rendering/display-popup-modal"; +import { displayPopupMessage, renderErrorInModal } from "../rendering/display-popup-modal"; import { TaskNoFull } from "./preview-to-full-mapping"; async function checkPrivateRepoAccess(): Promise { @@ -80,7 +80,7 @@ export async function fetchIssuePreviews(): Promise { if (error instanceof RequestError && error.status === 403) { await handleRateLimit(octokit, error); } else { - console.error("Error fetching issue previews:", error); + renderErrorInModal(error as Error, "Error fetching issue previews"); } } @@ -122,7 +122,7 @@ export async function handleRateLimit(octokit?: Octokit, error?: RequestError) { 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); + renderErrorInModal(err as Error, "Error handling GitHub rate limit"); } } diff --git a/src/home/getters/get-github-user.ts b/src/home/getters/get-github-user.ts index e87e0621..84835441 100644 --- a/src/home/getters/get-github-user.ts +++ b/src/home/getters/get-github-user.ts @@ -1,9 +1,10 @@ +import { RequestError } from "@octokit/request-error"; import { Octokit } from "@octokit/rest"; +import { handleRateLimit } from "../fetch-github/fetch-issues-preview"; import { GitHubUser, GitHubUserResponse } from "../github-types"; +import { renderErrorInModal } from "../rendering/display-popup-modal"; 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 { @@ -44,6 +45,7 @@ async function getNewGitHubUser(providerToken: string | null): Promise Date: Wed, 5 Jun 2024 15:06:10 +0900 Subject: [PATCH 12/16] chore: formatting --- .cspell.json | 6 +----- src/home/fetch-github/fetch-avatar.ts | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/.cspell.json b/.cspell.json index dccf6a5c..253c437c 100644 --- a/.cspell.json +++ b/.cspell.json @@ -1,8 +1,4 @@ { "version": "0.2", - "words": [ - "devpool", - "supabase", - "UBIQUIBOT" - ] + "words": ["devpool", "supabase", "UBIQUIBOT"] } diff --git a/src/home/fetch-github/fetch-avatar.ts b/src/home/fetch-github/fetch-avatar.ts index 7b1c13b7..fbc2ad61 100644 --- a/src/home/fetch-github/fetch-avatar.ts +++ b/src/home/fetch-github/fetch-avatar.ts @@ -1,7 +1,7 @@ import { Octokit } from "@octokit/rest"; import { getGitHubAccessToken } from "../getters/get-github-access-token"; import { getImageFromCache, saveImageToCache } from "../getters/get-indexed-db"; -import { renderErrorInModal } from '../rendering/display-popup-modal'; +import { renderErrorInModal } from "../rendering/display-popup-modal"; import { organizationImageCache } from "./fetch-issues-full"; export async function fetchAvatar(orgName: string) { From 1d16140a2c0b4a404f8339d5cf126fb5a7abba8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=A2=E3=83=AC=E3=82=AF=E3=82=B5=E3=83=B3=E3=83=80?= =?UTF-8?q?=E3=83=BC=2Eeth?= Date: Wed, 5 Jun 2024 17:04:12 +0900 Subject: [PATCH 13/16] fix: wrap async event handlers to catch and render errors in modal --- .../display-github-user-information.ts | 5 ++- src/home/rendering/render-github-issues.ts | 31 +++++++++------- .../rendering/render-github-login-button.ts | 3 +- src/home/sorting/sorting-manager.ts | 37 ++++++++++++------- 4 files changed, 46 insertions(+), 30 deletions(-) diff --git a/src/home/rendering/display-github-user-information.ts b/src/home/rendering/display-github-user-information.ts index 398d3a5d..85975e7c 100644 --- a/src/home/rendering/display-github-user-information.ts +++ b/src/home/rendering/display-github-user-information.ts @@ -1,5 +1,6 @@ import { isOrgMemberWithoutScope } from "../getters/get-github-access-token"; import { GitHubUser } from "../github-types"; +import { renderErrorInModal } from "./display-popup-modal"; import { getSupabase, renderAugmentAccessButton } from "./render-github-login-button"; export async function displayGitHubUserInformation(gitHubUser: GitHubUser) { @@ -25,8 +26,8 @@ export async function displayGitHubUserInformation(gitHubUser: GitHubUser) { const supabase = getSupabase(); const { error } = await supabase.auth.signOut(); if (error) { - console.error("Error logging out:", error); - alert(error); + renderErrorInModal(error, "Error logging out"); + alert("Error logging out"); } window.location.reload(); }); diff --git a/src/home/rendering/render-github-issues.ts b/src/home/rendering/render-github-issues.ts index 3d419537..e98c6a38 100644 --- a/src/home/rendering/render-github-issues.ts +++ b/src/home/rendering/render-github-issues.ts @@ -3,6 +3,7 @@ import { organizationImageCache } from "../fetch-github/fetch-issues-full"; import { TaskMaybeFull } from "../fetch-github/preview-to-full-mapping"; import { GitHubIssue } from "../github-types"; import { taskManager } from "../home"; +import { renderErrorInModal } from "./display-popup-modal"; import { preview, previewBodyInner, titleAnchor, titleHeader } from "./render-preview-modal"; import { setupKeyboardNavigation } from "./setup-keyboard-navigation"; @@ -81,23 +82,27 @@ function setUpIssueElement( )}${image}`; issueElement.addEventListener("click", () => { - const issueWrapper = issueElement.parentElement; + try { + const issueWrapper = issueElement.parentElement; - if (!issueWrapper) { - throw new Error("No issue container found"); - } + if (!issueWrapper) { + throw new Error("No issue container found"); + } - Array.from(issueWrapper.parentElement?.children || []).forEach((sibling) => { - sibling.classList.remove("selected"); - }); + Array.from(issueWrapper.parentElement?.children || []).forEach((sibling) => { + sibling.classList.remove("selected"); + }); - issueWrapper.classList.add("selected"); + issueWrapper.classList.add("selected"); - const full = task.full; - if (!full) { - window.open(match?.input, "_blank"); - } else { - previewIssue(task); + const full = task.full; + if (!full) { + window.open(match?.input, "_blank"); + } else { + previewIssue(task); + } + } catch (error) { + return renderErrorInModal(error as Error); } }); } diff --git a/src/home/rendering/render-github-login-button.ts b/src/home/rendering/render-github-login-button.ts index 2cec107d..f894509c 100644 --- a/src/home/rendering/render-github-login-button.ts +++ b/src/home/rendering/render-github-login-button.ts @@ -1,5 +1,6 @@ import { createClient } from "@supabase/supabase-js"; import { toolbar } from "../ready-toolbar"; +import { renderErrorInModal } from "./display-popup-modal"; declare const SUPABASE_URL: string; // @DEV: passed in at build time check build/esbuild-build.ts declare const SUPABASE_ANON_KEY: string; // @DEV: passed in at build time check build/esbuild-build.ts @@ -26,7 +27,7 @@ async function gitHubLoginButtonHandler(scopes = "public_repo read:org") { }, }); if (error) { - console.error("Error logging in:", error); + renderErrorInModal(error, "Error logging in"); } } diff --git a/src/home/sorting/sorting-manager.ts b/src/home/sorting/sorting-manager.ts index 0e0b5105..3417a910 100644 --- a/src/home/sorting/sorting-manager.ts +++ b/src/home/sorting/sorting-manager.ts @@ -1,6 +1,7 @@ import { fetchAndDisplayPreviewsFromCache } from "../fetch-github/fetch-and-display-previews"; import { getGitHubAccessToken } from "../getters/get-github-access-token"; import { taskManager } from "../home"; +import { renderErrorInModal } from "../rendering/display-popup-modal"; import { Sorting } from "./generate-sorting-buttons"; export class SortingManager { @@ -39,18 +40,22 @@ export class SortingManager { const issuesContainer = document.getElementById("issues-container") as HTMLDivElement; textBox.addEventListener("input", () => { - const filterText = textBox.value.toLowerCase(); - const issues = Array.from(issuesContainer.children) as HTMLDivElement[]; - issues.forEach((issue) => { - const issuePreviewId = issue.children[0].getAttribute("data-preview-id"); - if (!issuePreviewId) throw new Error(`No preview id found for issue ${issue}`); - const fullIssue = taskManager.getTaskByPreviewId(Number(issuePreviewId)).full; - if (!fullIssue) throw new Error(`No full issue found for preview id ${issuePreviewId}`); - const searchableProperties = ["title", "body", "number", "html_url"] as const; - const searchableStrings = searchableProperties.map((prop) => fullIssue[prop]?.toString().toLowerCase()); - const isVisible = searchableStrings.some((str) => str?.includes(filterText)); - issue.style.display = isVisible ? "block" : "none"; - }); + try { + const filterText = textBox.value.toLowerCase(); + const issues = Array.from(issuesContainer.children) as HTMLDivElement[]; + issues.forEach((issue) => { + const issuePreviewId = issue.children[0].getAttribute("data-preview-id"); + if (!issuePreviewId) throw new Error(`No preview id found for issue ${issue}`); + const fullIssue = taskManager.getTaskByPreviewId(Number(issuePreviewId)).full; + if (!fullIssue) throw new Error(`No full issue found for preview id ${issuePreviewId}`); + const searchableProperties = ["title", "body", "number", "html_url"] as const; + const searchableStrings = searchableProperties.map((prop) => fullIssue[prop]?.toString().toLowerCase()); + const isVisible = searchableStrings.some((str) => str?.includes(filterText)); + issue.style.display = isVisible ? "block" : "none"; + }); + } catch (error) { + return renderErrorInModal(error as Error); + } }); return textBox; @@ -67,7 +72,7 @@ export class SortingManager { buttons.appendChild(input); buttons.appendChild(label); - input.addEventListener("click", () => this._handleSortingClick(input, option)); + input.addEventListener("click", () => this._handleSortingClick(input, option).catch(renderErrorCatch)); // @DEV: need to test if this async catch works as expected }); return buttons; @@ -103,7 +108,7 @@ export class SortingManager { input.setAttribute("data-ordering", ordering); // instantly load from cache - fetchAndDisplayPreviewsFromCache(option as Sorting, { ordering }).catch((error) => console.error(error)); + fetchAndDisplayPreviewsFromCache(option as Sorting, { ordering }).catch(renderErrorCatch); // @DEV: need to test if this async catch works as expected // load from network in the background // const fetchedPreviews = await fetchIssuePreviews(); @@ -114,3 +119,7 @@ export class SortingManager { // return fetchAvatars(); } } + +function renderErrorCatch(event: ErrorEvent) { + return renderErrorInModal(event.error); +} From 9243a6ebc3031510930cfd874f474797b7b6a673 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=A2=E3=83=AC=E3=82=AF=E3=82=B5=E3=83=B3=E3=83=80?= =?UTF-8?q?=E3=83=BC=2Eeth?= Date: Wed, 5 Jun 2024 17:35:43 +0900 Subject: [PATCH 14/16] feat: handle special errors differently for rate limiting and automatic log out --- .cspell.json | 7 +- src/home/fetch-github/fetch-issues-preview.ts | 78 ++++++------------- src/home/fetch-github/handle-rate-limit.ts | 42 ++++++++++ src/home/getters/get-github-user.ts | 8 +- src/home/home.ts | 1 - src/home/rendering/display-popup-modal.ts | 14 ++-- src/home/sorting/sorting-manager.ts | 15 +++- 7 files changed, 99 insertions(+), 66 deletions(-) create mode 100644 src/home/fetch-github/handle-rate-limit.ts diff --git a/.cspell.json b/.cspell.json index 253c437c..41acb5fb 100644 --- a/.cspell.json +++ b/.cspell.json @@ -1,4 +1,9 @@ { "version": "0.2", - "words": ["devpool", "supabase", "UBIQUIBOT"] + "words": [ + "devpool", + "ratelimit", + "supabase", + "UBIQUIBOT" + ] } diff --git a/src/home/fetch-github/fetch-issues-preview.ts b/src/home/fetch-github/fetch-issues-preview.ts index 52041923..917548af 100644 --- a/src/home/fetch-github/fetch-issues-preview.ts +++ b/src/home/fetch-github/fetch-issues-preview.ts @@ -1,9 +1,10 @@ import { RequestError } from "@octokit/request-error"; import { Octokit } from "@octokit/rest"; import { getGitHubAccessToken, getGitHubUserName } from "../getters/get-github-access-token"; -import { getGitHubUser } from "../getters/get-github-user"; import { GitHubIssue } from "../github-types"; -import { displayPopupMessage, renderErrorInModal } from "../rendering/display-popup-modal"; +import { displayPopupMessage } from "../rendering/display-popup-modal"; +import { gitHubLoginButton } from "../rendering/render-github-login-button"; +import { handleRateLimit } from "./handle-rate-limit"; import { TaskNoFull } from "./preview-to-full-mapping"; async function checkPrivateRepoAccess(): Promise { @@ -58,20 +59,7 @@ export async function fetchIssuePreviews(): Promise { // Fetch issues from the private repository only if the user has access if (hasPrivateRepoAccess) { - const { data: privateResponse } = await octokit.issues.listForRepo({ - owner: "ubiquity", - repo: "devpool-directory-private", - state: "open", - }); - const privateIssues = privateResponse.filter((issue: GitHubIssue) => !issue.pull_request); - - // Mark private issues - const privateIssuesWithFlag = privateIssues.map((issue) => { - return issue; - }); - - // Combine public and private issues - freshIssues = [...privateIssuesWithFlag, ...publicIssues]; + await fetchPrivateIssues(publicIssues); } else { // If user doesn't have access, only load issues from the public repository freshIssues = publicIssues; @@ -80,7 +68,11 @@ export async function fetchIssuePreviews(): Promise { if (error instanceof RequestError && error.status === 403) { await handleRateLimit(octokit, error); } else { - renderErrorInModal(error as Error, "Error fetching issue previews"); + // renderErrorInModal(error as Error, "You have been rate limited. Please login to increase your limits."); // @DEV: user another method to render the modal not as an error + displayPopupMessage({ modalHeader: "GitHub API rate limit exceeded.", modalBody: "You have been rate limited. Please login to increase your limits.", isError: false }); + gitHubLoginButton?.classList.add("highlight"); + // throw error; + // console.error("You have been rate limited. Please login to increase your limits. ", error); } } @@ -92,45 +84,25 @@ export async function fetchIssuePreviews(): Promise { })) as TaskNoFull[]; return tasks; -} - -function rateLimitModal(message: string) { - displayPopupMessage({ modalHeader: `GitHub API rate limit exceeded.`, modalBody: message, isError: false }); -} - -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"]); - } + async function fetchPrivateIssues(publicIssues: GitHubIssue[]) { + const { data: privateResponse } = await octokit.issues.listForRepo({ + owner: "ubiquity", + repo: "devpool-directory-private", + state: "open", + }); + const privateIssues = privateResponse.filter((issue: GitHubIssue) => !issue.pull_request); - if (octokit) { - try { - const core = await octokit.rest.rateLimit.get(); - const remaining = core.data.resources.core.remaining; - const reset = core.data.resources.core.reset; + // Mark private issues + const privateIssuesWithFlag = privateIssues.map((issue) => { + return issue; + }); - rate.reset = !rate.reset && remaining === 0 ? reset : rate.reset; - rate.user = (await getGitHubUser()) ? true : false; - } catch (err) { - renderErrorInModal(err as Error, "Error handling GitHub rate limit"); - } + // Combine public and private issues + freshIssues = [...privateIssuesWithFlag, ...publicIssues]; } +} - 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}.`); - } +export function rateLimitModal(message: string) { + displayPopupMessage({ modalHeader: `GitHub API rate limit exceeded.`, modalBody: message, isError: false }); } diff --git a/src/home/fetch-github/handle-rate-limit.ts b/src/home/fetch-github/handle-rate-limit.ts new file mode 100644 index 00000000..62d2e104 --- /dev/null +++ b/src/home/fetch-github/handle-rate-limit.ts @@ -0,0 +1,42 @@ +import { RequestError } from "@octokit/request-error"; +import { Octokit } from "@octokit/rest"; +import { getGitHubUser } from "../getters/get-github-user"; +import { renderErrorInModal } from "../rendering/display-popup-modal"; +import { rateLimitModal } from "./fetch-issues-preview"; + +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) { + renderErrorInModal(err as Error, "Error handling GitHub rate limit"); + } + } + + 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 84835441..b37776d3 100644 --- a/src/home/getters/get-github-user.ts +++ b/src/home/getters/get-github-user.ts @@ -1,8 +1,7 @@ import { RequestError } from "@octokit/request-error"; import { Octokit } from "@octokit/rest"; -import { handleRateLimit } from "../fetch-github/fetch-issues-preview"; +import { handleRateLimit } from "../fetch-github/handle-rate-limit"; import { GitHubUser, GitHubUserResponse } from "../github-types"; -import { renderErrorInModal } from "../rendering/display-popup-modal"; import { OAuthToken } from "./get-github-access-token"; import { getLocalStore } from "./get-local-store"; declare const SUPABASE_STORAGE_KEY: string; // @DEV: passed in at build time check build/esbuild-build.ts @@ -45,7 +44,10 @@ async function getNewGitHubUser(providerToken: string | null): Promise this._handleSortingClick(input, option).catch(renderErrorCatch)); // @DEV: need to test if this async catch works as expected + input.addEventListener("click", () => { + try { + void this._handleSortingClick(input, option); + } catch (error) { + renderErrorCatch(error as ErrorEvent); + } + }); }); return buttons; @@ -108,8 +114,11 @@ export class SortingManager { input.setAttribute("data-ordering", ordering); // instantly load from cache - fetchAndDisplayPreviewsFromCache(option as Sorting, { ordering }).catch(renderErrorCatch); // @DEV: need to test if this async catch works as expected - + try { + void fetchAndDisplayPreviewsFromCache(option as Sorting, { ordering }); + } catch (error) { + renderErrorCatch(error as ErrorEvent); + } // load from network in the background // const fetchedPreviews = await fetchIssuePreviews(); // const cachedTasks = taskManager.getTasks(); From a53c213e183b0bbc7a5e1ed49c3c71b2306bfa38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=A2=E3=83=AC=E3=82=AF=E3=82=B5=E3=83=B3=E3=83=80?= =?UTF-8?q?=E3=83=BC=2Eeth?= Date: Wed, 5 Jun 2024 17:36:10 +0900 Subject: [PATCH 15/16] style: formatter --- .cspell.json | 7 +------ src/home/fetch-github/fetch-issues-preview.ts | 6 +++++- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/.cspell.json b/.cspell.json index 41acb5fb..6863ea30 100644 --- a/.cspell.json +++ b/.cspell.json @@ -1,9 +1,4 @@ { "version": "0.2", - "words": [ - "devpool", - "ratelimit", - "supabase", - "UBIQUIBOT" - ] + "words": ["devpool", "ratelimit", "supabase", "UBIQUIBOT"] } diff --git a/src/home/fetch-github/fetch-issues-preview.ts b/src/home/fetch-github/fetch-issues-preview.ts index 917548af..0d210d08 100644 --- a/src/home/fetch-github/fetch-issues-preview.ts +++ b/src/home/fetch-github/fetch-issues-preview.ts @@ -69,7 +69,11 @@ export async function fetchIssuePreviews(): Promise { await handleRateLimit(octokit, error); } else { // renderErrorInModal(error as Error, "You have been rate limited. Please login to increase your limits."); // @DEV: user another method to render the modal not as an error - displayPopupMessage({ modalHeader: "GitHub API rate limit exceeded.", modalBody: "You have been rate limited. Please login to increase your limits.", isError: false }); + displayPopupMessage({ + modalHeader: "GitHub API rate limit exceeded.", + modalBody: "You have been rate limited. Please login to increase your limits.", + isError: false, + }); gitHubLoginButton?.classList.add("highlight"); // throw error; // console.error("You have been rate limited. Please login to increase your limits. ", error); From 88eafce528748e4dab9a06df0efdc3678576eedf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=A2=E3=83=AC=E3=82=AF=E3=82=B5=E3=83=B3=E3=83=80?= =?UTF-8?q?=E3=83=BC=2Eeth?= Date: Thu, 6 Jun 2024 18:24:37 +0900 Subject: [PATCH 16/16] chore: remove commented out code --- src/home/fetch-github/fetch-issues-preview.ts | 12 ++++++------ src/home/getters/get-github-user.ts | 3 --- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/home/fetch-github/fetch-issues-preview.ts b/src/home/fetch-github/fetch-issues-preview.ts index 0d210d08..bc22653e 100644 --- a/src/home/fetch-github/fetch-issues-preview.ts +++ b/src/home/fetch-github/fetch-issues-preview.ts @@ -75,8 +75,6 @@ export async function fetchIssuePreviews(): Promise { isError: false, }); gitHubLoginButton?.classList.add("highlight"); - // throw error; - // console.error("You have been rate limited. Please login to increase your limits. ", error); } } @@ -98,12 +96,14 @@ export async function fetchIssuePreviews(): Promise { const privateIssues = privateResponse.filter((issue: GitHubIssue) => !issue.pull_request); // Mark private issues - const privateIssuesWithFlag = privateIssues.map((issue) => { - return issue; - }); + // TODO: indicate private issues in the UI + + // const privateIssuesWithFlag = privateIssues.map((issue) => { + // return issue; + // }); // Combine public and private issues - freshIssues = [...privateIssuesWithFlag, ...publicIssues]; + freshIssues = [...privateIssues, ...publicIssues]; } } diff --git a/src/home/getters/get-github-user.ts b/src/home/getters/get-github-user.ts index b37776d3..b79a6968 100644 --- a/src/home/getters/get-github-user.ts +++ b/src/home/getters/get-github-user.ts @@ -44,9 +44,6 @@ async function getNewGitHubUser(providerToken: string | null): Promise