From 47544c67c3458cd4d6185eb831dc509be975323e Mon Sep 17 00:00:00 2001 From: jcardonne <50085165+jcardonne@users.noreply.github.com> Date: Sun, 12 Jul 2026 23:43:26 +0200 Subject: [PATCH] feat(fetch): rate-limit backoff + enable auto-fetch by default (5min) Make background auto-fetch safe to have on by default. The active+visible tab is the only one that fetches, so at 5min that's ~12 API-bucket calls/hr (PR list + avatars) - a rounding error against GitHub's 5000/hr and GitLab's per-minute limits. git fetch itself rides the git protocol, a separate, effectively-unlimited bucket. The realistic failure mode is a secondary/abuse limit (GitHub 403 + Retry-After, GitLab 429), where retrying on schedule can prolong the block. So backgroundFetch and refreshPrs now recognise a rate-limit error (isRateLimited) and pause the loop until it clears (rateLimitBackoffMs honours an explicit retry-after hint, else 15min), toasting once on entry. Foreground/user-initiated ops are never gated by the backoff. Default interval flips 0 -> 5min; users who explicitly chose "Off" keep it (0 is stored), so only never-configured users get the new default. --- src/components/RepoView.tsx | 41 +++++++++++++++++++++++++++++-------- src/storage.ts | 5 ++++- src/util.test.ts | 25 +++++++++++++++++++++- src/util.ts | 17 +++++++++++++++ 4 files changed, 78 insertions(+), 10 deletions(-) diff --git a/src/components/RepoView.tsx b/src/components/RepoView.tsx index 273a186..28e774c 100644 --- a/src/components/RepoView.tsx +++ b/src/components/RepoView.tsx @@ -22,7 +22,7 @@ import type { SubmoduleInfo, StashInfo, } from "../types"; -import { affectedPaths, avatarUrl, type AvatarContext, hasUncommittedChange, relativeTime } from "../util"; +import { affectedPaths, avatarUrl, type AvatarContext, hasUncommittedChange, isRateLimited, rateLimitBackoffMs, relativeTime } from "../util"; import Toolbar from "./Toolbar"; import Sidebar from "./Sidebar"; import GraphView from "./GraphView"; @@ -166,6 +166,9 @@ export default function RepoView({ path, isActive, onLoaded, onOpenPath }: Props // Epoch ms our last own git op settled - the watcher listener ignores its own // debounced echo within a short grace window after this (see run()). const lastOpAt = useRef(0); + // Epoch ms until which background network is paused after a provider rate-limit + // (see noteBackoff). Foreground/user-initiated ops are never gated by this. + const backoffUntil = useRef(0); // Play the exit animation, then unmount after it (kept in sync with the // .toast.closing CSS below). @@ -191,6 +194,22 @@ export default function RepoView({ path, isActive, onLoaded, onOpenPath }: Props ); useEffect(() => () => window.clearTimeout(toastTimer.current), []); + // A background network error that looks like a provider rate-limit pauses the + // auto-fetch loop until the window passes, instead of retrying on schedule + // (which can prolong a secondary limit). Only extends the pause forward, and + // toasts once - on entering backoff, not on every subsequent throttled hit. + const noteBackoff = useCallback( + (msg: string) => { + if (!isRateLimited(msg)) return; // transient/offline: the normal throttle covers it + const wasActive = Date.now() < backoffUntil.current; + backoffUntil.current = Math.max(backoffUntil.current, Date.now() + rateLimitBackoffMs(msg)); + if (!wasActive) { + notify(`Auto-fetch paused: rate limited. Resuming around ${new Date(backoffUntil.current).toLocaleTimeString()}.`); + } + }, + [notify] + ); + // Provider account avatars (GitHub/GitLab profile pictures) for the committers // in view, resolved by the backend (cached on disk) and merged in as they // arrive - upgrading the no-reply/Gravatar fallbacks. Skipped for repos whose @@ -254,8 +273,8 @@ export default function RepoView({ path, isActive, onLoaded, onOpenPath }: Props setPrs([]); return; } - api.listPrs(path).then(setPrs).catch(() => {}); - }, [path, repo?.provider]); + api.listPrs(path).then(setPrs).catch((e) => noteBackoff(String(e))); + }, [path, repo?.provider, noteBackoff]); useEffect(() => refreshPrs(), [refreshPrs]); // Branch name -> its open PR, for the graph badge icon + "Open pull request" menu. @@ -376,16 +395,22 @@ export default function RepoView({ path, isActive, onLoaded, onOpenPath }: Props // background fetch offline shouldn't nag. const backgroundFetch = useCallback(() => { // Self-gating so every caller (interval + focus) honours ONE throttle: skip - // if auto-fetch is off, offline, or a fetch ran within the last interval. - // Without this the fixed interval tick could fire seconds after a focus fetch. + // if auto-fetch is off, offline, still inside a rate-limit backoff, or a fetch + // ran within the last interval. Without this the fixed interval tick could + // fire seconds after a focus fetch. const minutes = getFetchIntervalMinutes(); - if (minutes <= 0 || !navigator.onLine || Date.now() - lastFetchRef.current < minutes * 60_000) { + if ( + minutes <= 0 || + !navigator.onLine || + Date.now() < backoffUntil.current || + Date.now() - lastFetchRef.current < minutes * 60_000 + ) { return; } lastFetchRef.current = Date.now(); - api.fetchRemotes(path).then(() => refresh({ stats: false })).catch(() => {}); + api.fetchRemotes(path).then(() => refresh({ stats: false })).catch((e) => noteBackoff(String(e))); refreshPrs(); - }, [path, refresh, refreshPrs]); + }, [path, refresh, refreshPrs, noteBackoff]); // Per-worktree dirty ("WIP") indicators are an opt-in scan: each worktree is // opened and status-walked, so this runs on demand (first load + the sidebar diff --git a/src/storage.ts b/src/storage.ts index aca3381..f19cc1e 100644 --- a/src/storage.ts +++ b/src/storage.ts @@ -137,8 +137,11 @@ export function setRightPanelWidth(width: number): void { const FETCH_INTERVAL_KEY = "gitchef.fetchIntervalMinutes"; /// Minutes between background auto-fetches for the active repo; 0 = disabled. +/// Defaults to 5 (safe on GitHub/GitLab: only the active+visible tab fetches, and +/// backgroundFetch backs off on a rate-limit). Users who explicitly picked "Off" +/// have 0 stored, so this default only affects those who never touched it. export function getFetchIntervalMinutes(): number { - return read(FETCH_INTERVAL_KEY, 0); + return read(FETCH_INTERVAL_KEY, 5); } export function setFetchIntervalMinutes(minutes: number): void { localStorage.setItem(FETCH_INTERVAL_KEY, JSON.stringify(minutes)); diff --git a/src/util.test.ts b/src/util.test.ts index f6b6298..0c42de1 100644 --- a/src/util.test.ts +++ b/src/util.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from "vitest"; -import { affectedPaths, avatarUrl, edgePath, noreplyAvatarUrl } from "./util"; +import { affectedPaths, avatarUrl, edgePath, isRateLimited, noreplyAvatarUrl, rateLimitBackoffMs } from "./util"; import type { FileStatus } from "./types"; describe("noreplyAvatarUrl", () => { @@ -115,3 +115,26 @@ describe("affectedPaths", () => { expect(affectedPaths([fs("a.ts"), fs("a.ts")])).toEqual(["a.ts"]); }); }); + +describe("isRateLimited", () => { + it("matches GitHub primary + secondary and GitLab 429 wording", () => { + expect(isRateLimited("API rate limit exceeded for user")).toBe(true); + expect(isRateLimited("You have exceeded a secondary rate limit")).toBe(true); + expect(isRateLimited("429 Too Many Requests")).toBe(true); + }); + it("does not treat plain auth 403 / network errors as rate limits", () => { + expect(isRateLimited("HTTP 403: Bad credentials")).toBe(false); + expect(isRateLimited("could not resolve host github.com")).toBe(false); + }); +}); + +describe("rateLimitBackoffMs", () => { + it("honours an explicit retry-after hint, clamped", () => { + expect(rateLimitBackoffMs("secondary rate limit; retry after 90")).toBe(90_000); + expect(rateLimitBackoffMs("retry-after: 5")).toBe(30_000); // clamped up to 30s min + expect(rateLimitBackoffMs("retry after 99999")).toBe(60 * 60_000); // clamped to 1h max + }); + it("falls back to the default when no hint is present", () => { + expect(rateLimitBackoffMs("API rate limit exceeded")).toBe(15 * 60_000); + }); +}); diff --git a/src/util.ts b/src/util.ts index 0e3d01a..e17e771 100644 --- a/src/util.ts +++ b/src/util.ts @@ -124,6 +124,23 @@ export function hasUncommittedChange(status: StatusResult, path: string): boolea ); } +/// Does this git / gh / glab error look like a provider rate-limit (primary, +/// secondary/abuse, or a 429 throttle)? We match the text GitHub and GitLab +/// surface because the CLI path gives us no structured headers - only stderr. +/// Deliberately NOT matching a bare 403 (that's usually auth, not throttling). +export function isRateLimited(msg: string): boolean { + return /rate limit|secondary rate|abuse detection|too many requests|\b429\b/i.test(msg); +} + +/// How long to pause background network after a rate-limit error. Honours an +/// explicit "retry after s" / "try again in seconds" hint when the +/// provider gives one (clamped to 30s..1h), else a conservative default. +export function rateLimitBackoffMs(msg: string, fallbackMs = 15 * 60_000): number { + const m = msg.match(/retry[\s-]?after[\s:]+(\d+)/i) || msg.match(/try again in (\d+)\s*second/i); + if (m) return Math.min(60 * 60_000, Math.max(30_000, Number(m[1]) * 1000)); + return fallbackMs; +} + /// The git paths a stage/unstage/discard must touch for these files. A renamed /// file needs BOTH its new path and its rename source (`old_path`) so the old /// name's deletion and the new name's addition always move together.