From b58bc5c7ea4ef3cfe008245cfcab4e48c1251a34 Mon Sep 17 00:00:00 2001 From: Alan Daniel Date: Mon, 13 Apr 2026 14:47:44 -0400 Subject: [PATCH] Fix GitHub API timeouts caused by Octokit throttle plugin and surface timeout warnings (#93) --- .../layouts/dashboard-bottombar.tsx | 122 ++++++++++-------- .../components/layouts/dashboard-layout.tsx | 10 +- .../src/lib/github-request-policy.ts | 8 -- apps/dashboard/src/lib/github.functions.ts | 41 ++++++ apps/dashboard/src/lib/github.server.ts | 81 ++---------- apps/dashboard/src/lib/github.types.ts | 2 + apps/dashboard/src/lib/warning-store.ts | 24 ++++ 7 files changed, 157 insertions(+), 131 deletions(-) diff --git a/apps/dashboard/src/components/layouts/dashboard-bottombar.tsx b/apps/dashboard/src/components/layouts/dashboard-bottombar.tsx index af9435d..1c81ac9 100644 --- a/apps/dashboard/src/components/layouts/dashboard-bottombar.tsx +++ b/apps/dashboard/src/components/layouts/dashboard-bottombar.tsx @@ -12,58 +12,78 @@ export function DashboardBottomBar() { return (
- {warnings.map((warning) => ( -
- - {warning.message} - {warning.action - ? (() => { - const action = warning.action; + {warnings.map((warning) => { + const isError = warning.severity === "error"; - return action.kind === "link" ? ( - - {action.label} - - ) : ( - - ); - })() - : null} - {warning.dismissible && ( - - )} -
- ))} + return ( +
+ + {warning.message} + {warning.action + ? (() => { + const action = warning.action; + + return action.kind === "link" ? ( + + {action.label} + + ) : ( + + ); + })() + : null} + {warning.dismissible && ( + + )} +
+ ); + })}
); } diff --git a/apps/dashboard/src/components/layouts/dashboard-layout.tsx b/apps/dashboard/src/components/layouts/dashboard-layout.tsx index 33e9a80..92682e8 100644 --- a/apps/dashboard/src/components/layouts/dashboard-layout.tsx +++ b/apps/dashboard/src/components/layouts/dashboard-layout.tsx @@ -8,7 +8,10 @@ import { } from "#/lib/github.query"; import { useHasMounted } from "#/lib/use-has-mounted"; import { useMediaQuery } from "#/lib/use-media-query"; -import { surfaceForbiddenOrgWarnings } from "#/lib/warning-store"; +import { + surfaceForbiddenOrgWarnings, + surfaceTimeoutWarning, +} from "#/lib/warning-store"; import { DashboardBottomBar } from "./dashboard-bottombar"; import { DashboardMobileNav } from "./dashboard-mobile-nav"; import { @@ -57,6 +60,11 @@ export function DashboardLayout() { useEffect(() => { surfaceForbiddenOrgWarnings(issuesQuery.data?.forbiddenOrgs); }, [issuesQuery.data?.forbiddenOrgs]); + useEffect(() => { + surfaceTimeoutWarning( + pullsQuery.data?.timedOut || issuesQuery.data?.timedOut, + ); + }, [pullsQuery.data?.timedOut, issuesQuery.data?.timedOut]); const pullCount = hasMounted && pullsQuery.data diff --git a/apps/dashboard/src/lib/github-request-policy.ts b/apps/dashboard/src/lib/github-request-policy.ts index f70bf4f..674e29d 100644 --- a/apps/dashboard/src/lib/github-request-policy.ts +++ b/apps/dashboard/src/lib/github-request-policy.ts @@ -2,7 +2,6 @@ import { getRequest } from "@tanstack/react-start/server"; import type { Octokit as OctokitType } from "octokit"; import { debug } from "./debug"; -const GITHUB_READ_RETRY_COUNT = 1; export const GITHUB_REQUEST_TIMEOUT_MS = 12_000; type GitHubRequestOptions = Parameters< @@ -29,10 +28,6 @@ type GitHubRequestPolicyOptions = { tokenLabel?: string; }; -function isSafeGitHubRetryMethod(method: string | undefined) { - return method === "GET" || method === "HEAD" || method === "OPTIONS"; -} - function createGitHubRequestTimeoutSignal( requestSignal: AbortSignal | undefined, ) { @@ -116,9 +111,6 @@ export function configureGitHubRequestPolicies( octokit.hook.before("request", (options: GitHubRequestOptions) => { const requestOptions = options.request ?? {}; options.request = requestOptions; - requestOptions.retries = isSafeGitHubRetryMethod(options.method) - ? GITHUB_READ_RETRY_COUNT - : 0; requestOptions.signal ??= createGitHubRequestTimeoutSignal( getIncomingRequestSignal(), ); diff --git a/apps/dashboard/src/lib/github.functions.ts b/apps/dashboard/src/lib/github.functions.ts index 067f4df..27976d2 100644 --- a/apps/dashboard/src/lib/github.functions.ts +++ b/apps/dashboard/src/lib/github.functions.ts @@ -1286,6 +1286,31 @@ class GitHubOperationTimeoutError extends Error { } } +/** + * Module-level tracker for recent GitHub API timeouts. + * Automatically recorded by `withGitHubOperationTimeout` so that + * callers that swallow the error (fallback-to-REST, return []) still + * contribute to the global "GitHub is timing out" signal. + */ +const TIMEOUT_TRACKER_WINDOW_MS = 60_000; +let recentTimeoutTimestamps: number[] = []; + +function recordGitHubTimeout() { + const now = Date.now(); + recentTimeoutTimestamps.push(now); + recentTimeoutTimestamps = recentTimeoutTimestamps.filter( + (t) => now - t < TIMEOUT_TRACKER_WINDOW_MS, + ); +} + +function hasRecentGitHubTimeouts(): boolean { + const now = Date.now(); + recentTimeoutTimestamps = recentTimeoutTimestamps.filter( + (t) => now - t < TIMEOUT_TRACKER_WINDOW_MS, + ); + return recentTimeoutTimestamps.length > 0; +} + function getRemainingSearchTimeoutMs(deadlineAt: number, maxTimeoutMs: number) { return Math.max(0, Math.min(maxTimeoutMs, deadlineAt - Date.now())); } @@ -1296,6 +1321,7 @@ async function withGitHubOperationTimeout( task: (signal: AbortSignal) => Promise, ) { if (timeoutMs <= 0) { + recordGitHubTimeout(); throw new GitHubOperationTimeoutError(label, timeoutMs); } @@ -1305,6 +1331,7 @@ async function withGitHubOperationTimeout( const timeoutPromise = new Promise((_, reject) => { timeoutId = setTimeout(() => { controller.abort(); + recordGitHubTimeout(); reject(new GitHubOperationTimeoutError(label, timeoutMs)); }, timeoutMs); }); @@ -3968,6 +3995,7 @@ async function getMyPullsResult({ const results: MyPullsResult[] = []; const rateLimits: GitHubGraphQLRateLimit[] = []; const forbiddenOrgs: string[] = []; + let timedOut = false; for (const source of sources) { const sourceTimeoutMs = getRemainingSearchTimeoutMs( @@ -4105,6 +4133,9 @@ async function getMyPullsResult({ source.label, error, ); + if (error instanceof GitHubOperationTimeoutError) { + timedOut = true; + } const org = extractForbiddenOrg(error); if (org) forbiddenOrgs.push(org); } @@ -4114,6 +4145,9 @@ async function getMyPullsResult({ if (forbiddenOrgs.length > 0) { data.forbiddenOrgs = [...new Set(forbiddenOrgs)]; } + if (timedOut || hasRecentGitHubTimeouts()) { + data.timedOut = true; + } return { kind: "success", @@ -4148,6 +4182,7 @@ async function getMyIssuesResult({ const results: MyIssuesResult[] = []; const rateLimits: GitHubGraphQLRateLimit[] = []; const forbiddenOrgs: string[] = []; + let timedOut = false; for (const source of sources) { const sourceTimeoutMs = getRemainingSearchTimeoutMs( @@ -4260,6 +4295,9 @@ async function getMyIssuesResult({ source.label, error, ); + if (error instanceof GitHubOperationTimeoutError) { + timedOut = true; + } const org = extractForbiddenOrg(error); if (org) forbiddenOrgs.push(org); } @@ -4269,6 +4307,9 @@ async function getMyIssuesResult({ if (forbiddenOrgs.length > 0) { data.forbiddenOrgs = [...new Set(forbiddenOrgs)]; } + if (timedOut || hasRecentGitHubTimeouts()) { + data.timedOut = true; + } return { kind: "success", diff --git a/apps/dashboard/src/lib/github.server.ts b/apps/dashboard/src/lib/github.server.ts index 9f17fee..1f7984d 100644 --- a/apps/dashboard/src/lib/github.server.ts +++ b/apps/dashboard/src/lib/github.server.ts @@ -8,18 +8,10 @@ import { import { configureGitHubRequestPolicies } from "./github-request-policy"; const GITHUB_CLIENT_USER_AGENT = "diffkit-dashboard"; -const GITHUB_SECONDARY_RATE_LIMIT_FALLBACK_SECONDS = 60; const GITHUB_INSTALLATION_TOKEN_CACHE_VERSION = "v1"; const GITHUB_INSTALLATION_TOKEN_REFRESH_BUFFER_MS = 5 * 60 * 1000; const GITHUB_INSTALLATION_TOKEN_MIN_KV_TTL_SECONDS = 60; -type GitHubThrottleRequestOptions = { - method?: string; - url: string; -}; - -type GitHubThrottleClient = Pick; - type GitHubInstallationTokenCacheEntry = { installationId: number; token: string; @@ -62,37 +54,8 @@ export async function getGitHubClient(userId: string): Promise { const octokit = new Octokit({ auth: await getGitHubAccessTokenByUserId(userId), userAgent: GITHUB_CLIENT_USER_AGENT, - retry: { enabled: true }, - throttle: { - enabled: true, - id: `github-user:${userId}`, - fallbackSecondaryRateRetryAfter: - GITHUB_SECONDARY_RATE_LIMIT_FALLBACK_SECONDS, - onRateLimit: ( - retryAfter: number, - options: GitHubThrottleRequestOptions, - throttledOctokit: GitHubThrottleClient, - retryCount: number, - ) => { - throttledOctokit.log.warn( - `GitHub rate limit for ${options.method} ${options.url}; retryAfter=${retryAfter}s retryCount=${retryCount}`, - ); - - return false; - }, - onSecondaryRateLimit: ( - retryAfter: number, - options: GitHubThrottleRequestOptions, - throttledOctokit: GitHubThrottleClient, - retryCount: number, - ) => { - throttledOctokit.log.warn( - `GitHub secondary rate limit for ${options.method} ${options.url}; retryAfter=${retryAfter}s retryCount=${retryCount}`, - ); - - return false; - }, - }, + retry: { enabled: false }, + throttle: { enabled: false }, }); configureGitHubRequestPolicies(octokit, { @@ -109,37 +72,8 @@ export async function getGitHubInstallationClient( const octokit = new Octokit({ auth: tokenEntry.token, userAgent: GITHUB_CLIENT_USER_AGENT, - retry: { enabled: true }, - throttle: { - enabled: true, - id: `github-installation:${installationId}`, - fallbackSecondaryRateRetryAfter: - GITHUB_SECONDARY_RATE_LIMIT_FALLBACK_SECONDS, - onRateLimit: ( - retryAfter: number, - options: GitHubThrottleRequestOptions, - throttledOctokit: GitHubThrottleClient, - retryCount: number, - ) => { - throttledOctokit.log.warn( - `GitHub rate limit for ${options.method} ${options.url}; retryAfter=${retryAfter}s retryCount=${retryCount}`, - ); - - return false; - }, - onSecondaryRateLimit: ( - retryAfter: number, - options: GitHubThrottleRequestOptions, - throttledOctokit: GitHubThrottleClient, - retryCount: number, - ) => { - throttledOctokit.log.warn( - `GitHub secondary rate limit for ${options.method} ${options.url}; retryAfter=${retryAfter}s retryCount=${retryCount}`, - ); - - return false; - }, - }, + retry: { enabled: false }, + throttle: { enabled: false }, }); configureGitHubRequestPolicies(octokit, { @@ -244,10 +178,15 @@ async function mintGitHubInstallationToken( ); } + const AppOctokit = Octokit.defaults({ + retry: { enabled: false }, + throttle: { enabled: false }, + }); + const app = new App({ appId, privateKey, - Octokit, + Octokit: AppOctokit, }); configureGitHubRequestPolicies(app.octokit, { diff --git a/apps/dashboard/src/lib/github.types.ts b/apps/dashboard/src/lib/github.types.ts index e31fe5c..c5a821f 100644 --- a/apps/dashboard/src/lib/github.types.ts +++ b/apps/dashboard/src/lib/github.types.ts @@ -114,6 +114,7 @@ export type MyPullsResult = { mentioned: PullSummary[]; involved: PullSummary[]; forbiddenOrgs?: string[]; + timedOut?: boolean; }; export type MyIssuesResult = { @@ -121,6 +122,7 @@ export type MyIssuesResult = { authored: IssueSummary[]; mentioned: IssueSummary[]; forbiddenOrgs?: string[]; + timedOut?: boolean; }; export type CommandPaletteSearchResult = { diff --git a/apps/dashboard/src/lib/warning-store.ts b/apps/dashboard/src/lib/warning-store.ts index 69e13e6..116eed0 100644 --- a/apps/dashboard/src/lib/warning-store.ts +++ b/apps/dashboard/src/lib/warning-store.ts @@ -14,9 +14,12 @@ export type WarningAction = repo?: string; }; +export type WarningSeverity = "warning" | "error"; + export interface Warning { id: string; message: string; + severity?: WarningSeverity; dismissible?: boolean; action?: WarningAction; } @@ -75,6 +78,27 @@ export function surfaceForbiddenOrgWarnings(orgs: string[] | undefined) { } } +const GITHUB_API_TIMEOUT_WARNING_ID = "github-api-timeout"; + +/** + * Surface a warning when GitHub API requests are timing out, + * indicating GitHub may be experiencing issues. + */ +export function surfaceTimeoutWarning(timedOut: boolean | undefined) { + if (!timedOut) { + removeWarning(GITHUB_API_TIMEOUT_WARNING_ID); + return; + } + + addWarning({ + id: GITHUB_API_TIMEOUT_WARNING_ID, + message: + "Some requests are taking too long and timing out. Data may be incomplete.", + severity: "error", + dismissible: true, + }); +} + /** * Check a MutationResult for permission errors and surface a warning. * Call this client-side after a mutation returns.