-
-
Notifications
You must be signed in to change notification settings - Fork 47
feat(rate-limit): update quota banner from response headers and persi… #80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,19 +1,63 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { createContext, useContext, useState, useCallback } from 'react' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { fetchOrg, fetchRepos, fetchContributors, fetchIssues, fetchRateLimit } from '../services/github' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { createContext, useContext, useState, useCallback, useEffect } from 'react' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { fetchOrg, fetchRepos, fetchContributors, fetchIssues, } from '../services/github' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { buildAnalyticalModel } from '../services/analytics' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const Ctx = createContext(null) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function getStoredRateLimit() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const stored = localStorage.getItem('oe_rate_limit') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!stored) return null | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const data = JSON.parse(stored) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (Date.now() > data.reset * 1000) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| localStorage.removeItem('oe_rate_limit') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return null | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return data | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| localStorage.removeItem('oe_rate_limit') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return null | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export function AppProvider({ children }) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [pat, setPat] = useState(() => localStorage.getItem('oe_pat') || '') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [orgs, setOrgs] = useState([]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [model, setModel] = useState(null) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [pat, setPat] = useState(() => localStorage.getItem('oe_pat') || '') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [orgs, setOrgs] = useState([]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [model, setModel] = useState(null) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [issuesData, setIssuesData] = useState({}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [rateLimit, setRateLimit] = useState(null) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [loading, setLoading] = useState(false) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [loadMsg, setLoadMsg] = useState('') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [rateLimit, setRateLimit] = useState(getStoredRateLimit) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [loading, setLoading] = useState(false) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [loadMsg, setLoadMsg] = useState('') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [govLoading, setGovLoading] = useState(false) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [error, setError] = useState('') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [error, setError] = useState('') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const handler = e => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setRateLimit(e.detail) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| localStorage.setItem('oe_rate_limit', JSON.stringify(e.detail)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| window.addEventListener('rate-limit-update', handler) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| window.removeEventListener('rate-limit-update', handler) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, []) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+38
to
+49
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate event payload to prevent malformed rate limit data. The event handler (lines 39-42) directly uses 🔒 Proposed fix with validation useEffect(() => {
const handler = e => {
+ const { limit, remaining, used, reset } = e.detail || {}
+
+ if (
+ typeof limit !== 'number' || limit < 0 ||
+ typeof remaining !== 'number' || remaining < 0 ||
+ typeof used !== 'number' || used < 0 ||
+ typeof reset !== 'number' || reset <= 0
+ ) {
+ console.warn('Invalid rate-limit-update event payload:', e.detail)
+ return
+ }
+
setRateLimit(e.detail)
localStorage.setItem('oe_rate_limit', JSON.stringify(e.detail))
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!rateLimit?.reset) return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const timeout = setTimeout(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| localStorage.removeItem('oe_rate_limit') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setRateLimit(null) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, Math.max(0, rateLimit.reset * 1000 - Date.now())) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return () => clearTimeout(timeout) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, [rateLimit]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const savePat = useCallback(token => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setPat(token) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -25,7 +69,7 @@ export function AppProvider({ children }) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setLoading(true); setError(''); setModel(null); setOrgs([]); setIssuesData({}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setLoadMsg('Fetching organization metadata...') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const orgRes = await Promise.allSettled(orgNames.map(n => fetchOrg(n, pat))) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const orgRes = await Promise.allSettled(orgNames.map(n => fetchOrg(n, pat))) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const validOrgs = orgRes.filter(r => r.status === 'fulfilled').map(r => r.value) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!validOrgs.length) throw new Error('No valid organizations found. Check the names and try again.') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setOrgs(validOrgs) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -50,11 +94,9 @@ export function AppProvider({ children }) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setLoadMsg('Building analytical data model...') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setModel(buildAnalyticalModel(validOrgs, reposPerOrg, contribsPerRepo)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const rl = await fetchRateLimit(pat) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (rl) setRateLimit(rl) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Save to recent searches | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const prev = JSON.parse(localStorage.getItem('oe_recent') || '[]') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const prev = JSON.parse(localStorage.getItem('oe_recent') || '[]') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const entry = orgNames.join(', ') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| localStorage.setItem('oe_recent', JSON.stringify([...new Set([entry, ...prev])].slice(0, 6))) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -72,7 +114,7 @@ export function AppProvider({ children }) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const runAudit = useCallback(async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!model || govLoading) return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setGovLoading(true) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const map = {} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const map = {} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const repos = model.allRepos.slice(0, 15) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Batches of 5 using Promise.allSettled | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,14 @@ | ||
| // IndexedDB Cache (L2) | ||
| const DB_NAME = 'orgexplorer_cache' | ||
| const STORE = 'cache' | ||
| const TTL_MS = 3_600_000 // 1 hour | ||
| const STORE = 'cache' | ||
| const TTL_MS = 3_600_000 // 1 hour | ||
|
|
||
| function openDB() { | ||
| return new Promise((resolve, reject) => { | ||
| const req = indexedDB.open(DB_NAME, 1) | ||
| req.onupgradeneeded = e => e.target.result.createObjectStore(STORE, { keyPath: 'k' }) | ||
| req.onsuccess = e => resolve(e.target.result) | ||
| req.onerror = () => reject(req.error) | ||
| req.onsuccess = e => resolve(e.target.result) | ||
| req.onerror = () => reject(req.error) | ||
| }) | ||
| } | ||
|
|
||
|
|
@@ -34,7 +34,7 @@ export async function cacheSet(key, value) { | |
| const tx = db.transaction(STORE, 'readwrite') | ||
| tx.objectStore(STORE).put({ k: key, v: value, ts: Date.now() }) | ||
| tx.oncomplete = () => res(true) | ||
| tx.onerror = () => res(false) | ||
| tx.onerror = () => res(false) | ||
| }) | ||
| } catch { return false } | ||
| } | ||
|
|
@@ -46,7 +46,7 @@ export async function cacheClear() { | |
| const tx = db.transaction(STORE, 'readwrite') | ||
| tx.objectStore(STORE).clear() | ||
| tx.oncomplete = () => res(true) | ||
| tx.onerror = () => res(false) | ||
| tx.onerror = () => res(false) | ||
| }) | ||
| } catch { return false } | ||
| } | ||
|
|
@@ -61,6 +61,18 @@ async function fetchWithCache(url, pat) { | |
| if (pat) headers.Authorization = `token ${pat}` | ||
|
|
||
| const res = await fetch(url, { headers }) | ||
|
|
||
| window.dispatchEvent( | ||
| new CustomEvent('rate-limit-update', { | ||
| detail: { | ||
| limit: Number(res.headers.get('x-ratelimit-limit')), | ||
| remaining: Number(res.headers.get('x-ratelimit-remaining')), | ||
| used: Number(res.headers.get('x-ratelimit-used')), | ||
| reset: Number(res.headers.get('x-ratelimit-reset')) | ||
| } | ||
| }) | ||
| ) | ||
|
Comment on lines
+65
to
+74
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add defensive null checks for rate limit headers. The code converts header values using 🛡️ Proposed fix with validation- window.dispatchEvent(
- new CustomEvent('rate-limit-update', {
- detail: {
- limit: Number(res.headers.get('x-ratelimit-limit')),
- remaining: Number(res.headers.get('x-ratelimit-remaining')),
- used: Number(res.headers.get('x-ratelimit-used')),
- reset: Number(res.headers.get('x-ratelimit-reset'))
- }
- })
- )
+ const limit = res.headers.get('x-ratelimit-limit')
+ const remaining = res.headers.get('x-ratelimit-remaining')
+ const used = res.headers.get('x-ratelimit-used')
+ const reset = res.headers.get('x-ratelimit-reset')
+
+ if (limit && remaining && used && reset) {
+ window.dispatchEvent(
+ new CustomEvent('rate-limit-update', {
+ detail: {
+ limit: Number(limit),
+ remaining: Number(remaining),
+ used: Number(used),
+ reset: Number(reset)
+ }
+ })
+ )
+ }🤖 Prompt for AI Agents |
||
|
|
||
| if (res.status === 403) throw new Error('RATE_LIMIT') | ||
| if (res.status === 404) throw new Error('NOT_FOUND') | ||
| if (!res.ok) throw new Error(`HTTP_${res.status}`) | ||
|
|
@@ -77,7 +89,7 @@ export const fetchOrg = (org, pat) => | |
| export async function fetchRepos(org, pat) { | ||
| const all = [] | ||
| for (let page = 1; page <= 5; page++) { | ||
| const url = `https://api.github.com/orgs/${org}/repos?per_page=100&page=${page}&sort=updated` | ||
| const url = `https://api.github.com/orgs/${org}/repos?per_page=100&page=${page}&sort=updated` | ||
| const data = await fetchWithCache(url, pat) | ||
| all.push(...data) | ||
| if (data.length < 100) break | ||
|
|
@@ -105,7 +117,7 @@ export async function fetchRateLimit(pat) { | |
| try { | ||
| const headers = { Accept: 'application/vnd.github.v3+json' } | ||
| if (pat) headers.Authorization = `token ${pat}` | ||
| const res = await fetch('https://api.github.com/rate_limit', { headers }) | ||
| const res = await fetch('https://api.github.com/rate_limit', { headers }) | ||
| const data = await res.json() | ||
| return data.rate | ||
| } catch { return null } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add fallback for potentially undefined rate limit properties.
If the
rate-limit-updateevent payload is incomplete (due to the missing validation inAppContext),rateLimit.usedorrateLimit.resetcould beundefined, displaying "Used: undefined" or an invalid date.🛡️ Proposed fix with fallbacks
<span style={{ fontSize: 11, color: 'var(--text2)' }}> - Used: {rateLimit.used} • Reset at{' '} - {new Date(rateLimit.reset * 1000).toLocaleTimeString()} + Used: {rateLimit.used ?? 0} • Reset at{' '} + {rateLimit.reset ? new Date(rateLimit.reset * 1000).toLocaleTimeString() : 'Unknown'} </span>🤖 Prompt for AI Agents
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider adding timezone and locale control for better internationalization.
The
toLocaleTimeString()call uses default browser locale and no timezone information is displayed. Users in different timezones may be confused about when the rate limit actually resets, especially since GitHub's rate limits reset in UTC.🌍 Optional enhancement for i18n
<span style={{ fontSize: 11, color: 'var(--text2)' }}> Used: {rateLimit.used} • Reset at{' '} - {new Date(rateLimit.reset * 1000).toLocaleTimeString()} + {new Date(rateLimit.reset * 1000).toLocaleTimeString(undefined, { + hour: '2-digit', + minute: '2-digit', + second: '2-digit', + timeZoneName: 'short' + })} </span>This adds timezone information (e.g., "4:07:57 PM PST") to help users understand when the reset occurs in their local time.
📝 Committable suggestion
🤖 Prompt for AI Agents
Source: Coding guidelines