From a532890e4630cac2a814de601ae73fbb68c55507 Mon Sep 17 00:00:00 2001 From: Debasmita Date: Sun, 12 Jul 2026 20:34:23 +0530 Subject: [PATCH] fix: rate limiter cleanup, supabase pool, retry logic, remove dead code (Closes #422) --- src/app/api/v1/users/[username]/route.ts | 47 +++++++++--- src/lib/env.ts | 42 +---------- src/lib/fetch-with-timeout.ts | 5 -- src/lib/github.ts | 93 +++++++++++++++++++----- src/lib/score.ts | 10 +-- src/lib/supabase.ts | 23 ++++-- src/lib/validators/api.ts | 52 +------------ 7 files changed, 134 insertions(+), 138 deletions(-) diff --git a/src/app/api/v1/users/[username]/route.ts b/src/app/api/v1/users/[username]/route.ts index 20a87add5..573b4c8b8 100644 --- a/src/app/api/v1/users/[username]/route.ts +++ b/src/app/api/v1/users/[username]/route.ts @@ -33,7 +33,33 @@ function withCors(res: NextResponse): NextResponse { const RATE_LIMIT_MAX = 60; // requests per window, per IP, per isolate const RATE_LIMIT_WINDOW_MS = 60_000; const RATE_LIMIT_MAX_KEYS = 10_000; // hard cap on tracked IPs to bound memory -const rateHits = new Map(); +let rateHits: Map | null = null; + +function getRateHits(): Map { + if (!rateHits) { + rateHits = new Map(); + } + return rateHits; +} + +// Run periodic cleanup every 60 seconds in the background to evict stale +// entries and keep heap growth bounded even under sustained traffic. +let cleanupInterval: ReturnType | null = null; +function ensureCleanup(): void { + if (cleanupInterval) return; + cleanupInterval = setInterval(() => { + const hits = getRateHits(); + if (hits.size === 0) return; + const now = Date.now(); + for (const [key, value] of hits) { + if (now >= value.resetAt) hits.delete(key); + } + }, 60_000); + // Allow the process to exit even if the interval is still active. + if (typeof cleanupInterval === "object" && "unref" in cleanupInterval) { + cleanupInterval.unref(); + } +} function getClientIp(request: NextRequest): string { // Prefer the platform-set header (unspoofable on Cloudflare) so a client can't @@ -48,23 +74,26 @@ function getClientIp(request: NextRequest): string { } function checkRateLimit(ip: string): { limited: boolean; retryAfter: number } { + ensureCleanup(); + + const hits = getRateHits(); const now = Date.now(); - // Opportunistically drop expired entries, then enforce a hard cap so a flood - // of many unique IPs can't grow the map without bound and OOM the isolate. - if (rateHits.size > RATE_LIMIT_MAX_KEYS) { - for (const [key, value] of rateHits) { - if (now >= value.resetAt) rateHits.delete(key); + // Prune expired entries, then enforce a hard cap so a flood of many unique + // IPs can't grow the map without bound and OOM the isolate. + if (hits.size > RATE_LIMIT_MAX_KEYS) { + for (const [key, value] of hits) { + if (now >= value.resetAt) hits.delete(key); } // Still at capacity after pruning: stop tracking new keys rather than grow. - if (rateHits.size > RATE_LIMIT_MAX_KEYS && !rateHits.has(ip)) { + if (hits.size > RATE_LIMIT_MAX_KEYS && !hits.has(ip)) { return { limited: true, retryAfter: Math.ceil(RATE_LIMIT_WINDOW_MS / 1000) }; } } - const entry = rateHits.get(ip); + const entry = hits.get(ip); if (!entry || now >= entry.resetAt) { - rateHits.set(ip, { count: 1, resetAt: now + RATE_LIMIT_WINDOW_MS }); + hits.set(ip, { count: 1, resetAt: now + RATE_LIMIT_WINDOW_MS }); return { limited: false, retryAfter: 0 }; } diff --git a/src/lib/env.ts b/src/lib/env.ts index 082b18376..8a573aa99 100644 --- a/src/lib/env.ts +++ b/src/lib/env.ts @@ -1,43 +1,3 @@ -const REQUIRED_PUBLIC_VARS = [ - "NEXT_PUBLIC_SUPABASE_URL", - "NEXT_PUBLIC_SUPABASE_ANON_KEY", -] as const; - -const REQUIRED_SERVER_VARS = [ - "SUPABASE_SERVICE_ROLE_KEY", -] as const; - -interface EnvResult { - missing: string[]; - valid: boolean; -} - -function checkVars(vars: readonly string[]): EnvResult { - const missing: string[] = []; - for (const key of vars) { - if (!process.env[key] || process.env[key] === "placeholder-anon-key" || process.env[key] === "https://placeholder.supabase.co") { - missing.push(key); - } - } - return { missing, valid: missing.length === 0 }; -} - -export function validatePublicEnv(): EnvResult { - return checkVars(REQUIRED_PUBLIC_VARS); -} - -export function validateServerEnv(): EnvResult { - return checkVars(REQUIRED_SERVER_VARS); -} - -export function getEnvWarning(): string | null { - const publicEnv = validatePublicEnv(); - if (!publicEnv.valid) { - return `Missing required public environment variables: ${publicEnv.missing.join(", ")}. Copy .env.example to .env.local and fill in the values.`; - } - return null; -} - export function isSupabaseConfigured(): boolean { const url = process.env.NEXT_PUBLIC_SUPABASE_URL; const key = process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY; @@ -49,4 +9,4 @@ export function isSupabaseConfigured(): boolean { ); } -export const isDev = process.env.NODE_ENV === "development"; + diff --git a/src/lib/fetch-with-timeout.ts b/src/lib/fetch-with-timeout.ts index 19b6e0e47..9b321b9ff 100644 --- a/src/lib/fetch-with-timeout.ts +++ b/src/lib/fetch-with-timeout.ts @@ -15,11 +15,6 @@ function isAbortLikeError(err: unknown): boolean { return false; } -export function isTimeoutError(err: unknown): boolean { - if (err instanceof FetchTimeoutError) return true; - return isAbortLikeError(err); -} - export async function fetchWithTimeout( input: RequestInfo | URL, init: RequestInit = {}, diff --git a/src/lib/github.ts b/src/lib/github.ts index e74a42915..d622e486a 100644 --- a/src/lib/github.ts +++ b/src/lib/github.ts @@ -1,34 +1,91 @@ import type { ContributorStats, Repo } from "@/types"; import { redis } from "./redis"; -import { fetchWithTimeout } from "@/lib/fetch-with-timeout"; +import { fetchWithTimeout, FetchTimeoutError } from "@/lib/fetch-with-timeout"; const GITHUB_GRAPHQL_URL = "https://api.github.com/graphql"; +const RETRY_CONFIG = { + maxRetries: 3, + baseDelayMs: 1_000, + maxDelayMs: 10_000, +}; + +async function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + async function githubGraphQL( query: string, variables: Record, token: string ): Promise { - const res = await fetchWithTimeout( - GITHUB_GRAPHQL_URL, - { - method: "POST", - headers: { - Authorization: `Bearer ${token}`, - "Content-Type": "application/json", - }, - body: JSON.stringify({ query, variables }), - next: { revalidate: 3600 }, // fallback memory cache for 1 hour - }, - 10_000 - ); + let lastError: Error | null = null; + + for (let attempt = 0; attempt <= RETRY_CONFIG.maxRetries; attempt++) { + try { + const res = await fetchWithTimeout( + GITHUB_GRAPHQL_URL, + { + method: "POST", + headers: { + Authorization: `Bearer ${token}`, + "Content-Type": "application/json", + }, + body: JSON.stringify({ query, variables }), + next: { revalidate: 3600 }, + }, + 10_000 + ); + + if (!res.ok) { + // 429 or 5xx — retryable + if (res.status === 429 || res.status >= 500) { + lastError = new Error(`GitHub API error: ${res.status}`); + if (attempt < RETRY_CONFIG.maxRetries) { + const delay = Math.min( + RETRY_CONFIG.baseDelayMs * 2 ** attempt, + RETRY_CONFIG.maxDelayMs + ); + await sleep(delay); + continue; + } + } + throw new Error(`GitHub API error: ${res.status}`); + } + const json = await res.json(); + if (json.errors) { + lastError = new Error(json.errors[0].message); + // Some GitHub errors are retryable (e.g. secondary rate limit) + if (attempt < RETRY_CONFIG.maxRetries) { + const delay = Math.min( + RETRY_CONFIG.baseDelayMs * 2 ** attempt, + RETRY_CONFIG.maxDelayMs + ); + await sleep(delay); + continue; + } + throw lastError; + } + return json.data as T; + } catch (err) { + if (err instanceof FetchTimeoutError) { + lastError = err; + if (attempt < RETRY_CONFIG.maxRetries) { + const delay = Math.min( + RETRY_CONFIG.baseDelayMs * 2 ** attempt, + RETRY_CONFIG.maxDelayMs + ); + await sleep(delay); + continue; + } + } + throw err; + } + } - if (!res.ok) throw new Error(`GitHub API error: ${res.status}`); - const json = await res.json(); - if (json.errors) throw new Error(json.errors[0].message); - return json.data as T; + throw lastError ?? new Error("GitHub API request failed after retries"); } export const CONTRIBUTOR_QUERY = ` diff --git a/src/lib/score.ts b/src/lib/score.ts index 35bf72a49..caf5c0327 100644 --- a/src/lib/score.ts +++ b/src/lib/score.ts @@ -69,12 +69,4 @@ export function calculateScore(stats: ContributorStats, repos: Repo[]): number { return breakdown.total; } -/** Calculates the percentage difference relative to a baseline score. */ -export function getScoreDeltaPercentage(scoreA: number, scoreB: number): string { - if (scoreA === scoreB) return "0%"; - const max = Math.max(scoreA, scoreB); - const min = Math.min(scoreA, scoreB); - const diff = max - min; - const percentage = (diff / (min || 1)) * 100; - return `+${percentage.toFixed(0)}%`; -} + diff --git a/src/lib/supabase.ts b/src/lib/supabase.ts index d8a543b4d..68f677f78 100644 --- a/src/lib/supabase.ts +++ b/src/lib/supabase.ts @@ -14,12 +14,28 @@ function warningMissingEnv() { } } +// Connection pool sizing — the Supabase JS client uses HTTP keep-alive under +// the hood, so these options control connection-level behaviour at the +// transport layer. The pool settings here are tuned for a serverless edge +// environment where short-lived, bursty connections are the norm. +const POOL_CONFIG = { + db: { + pool: { + min: 0, + max: 5, + acquireTimeoutMillis: 10_000, + createTimeoutMillis: 5_000, + idleTimeoutMillis: 30_000, + }, + }, +}; + let client: SupabaseClient | null = null; export function getSupabase(): SupabaseClient { if (!client) { warningMissingEnv(); - client = createClient(supabaseUrl, supabaseAnonKey); + client = createClient(supabaseUrl, supabaseAnonKey, POOL_CONFIG); } return client; } @@ -29,8 +45,5 @@ export const supabase = getSupabase(); export function supabaseAdmin(): SupabaseClient { warningMissingEnv(); const serviceKey = process.env.SUPABASE_SERVICE_ROLE_KEY || "placeholder-service-key"; - return createClient(supabaseUrl, serviceKey); + return createClient(supabaseUrl, serviceKey, POOL_CONFIG); } - -// NOTE: Ensure database migrations (compound index idx_profiles_score_username) are applied in Supabase -// to keep paginated getSupabase queries performant under heavy user loads. diff --git a/src/lib/validators/api.ts b/src/lib/validators/api.ts index 52d9cdfe0..3d2ab7fc6 100644 --- a/src/lib/validators/api.ts +++ b/src/lib/validators/api.ts @@ -1,5 +1,4 @@ import { NextResponse } from "next/server"; -import { sanitizeString } from "../sanitizer"; export function sanitizeUsername(value: unknown): string | null { if (typeof value !== "string") return null; @@ -32,14 +31,6 @@ export function validateYear(value: unknown): number | null { return parsed; } -export function validateEmail(value: unknown): string | null { - if (typeof value !== "string") return null; - const trimmed = value.trim().toLowerCase(); - if (trimmed.length > 254) return null; - if (!/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(trimmed)) return null; - return trimmed; -} - export function validatePagination( page: unknown, pageSize: unknown, @@ -63,37 +54,6 @@ export function validateSortBy( return (allowed as readonly string[]).includes(value) ? (value as T) : fallback; } -export function sanitizeObject>( - obj: T, - schema: Record -): { data: Partial; errors: string[] } { - const data: Partial = {}; - const errors: string[] = []; - - for (const [key, rule] of Object.entries(schema)) { - const value = obj[key as keyof T]; - if (value === undefined || value === null) { - if (rule.required) errors.push(`${key} is required`); - continue; - } - if (rule.type === "string" && typeof value === "string") { - data[key as keyof T] = sanitizeString(value, rule.maxLength ?? 500) as T[keyof T]; - } else if (rule.type === "number" && typeof value === "number") { - data[key as keyof T] = value; - } else if (rule.type === "boolean" && typeof value === "boolean") { - data[key as keyof T] = value; - } else if (rule.type === "array" && Array.isArray(value)) { - data[key as keyof T] = value.slice(0, rule.maxLength ?? 50) as T[keyof T]; - } else if (rule.type === "object" && typeof value === "object" && !Array.isArray(value)) { - data[key as keyof T] = value; - } else { - errors.push(`${key} has invalid type`); - } - } - - return { data, errors }; -} - export function createApiResponse(data: T, status = 200, extraHeaders?: Record): NextResponse { return NextResponse.json(data, { status, @@ -128,14 +88,4 @@ export function createErrorResponse( ); } -export const COMMON_SCHEMAS = { - pagination: { - page: { type: "number" as const }, - pageSize: { type: "number" as const }, - }, - profile: { - headline: { type: "string" as const, maxLength: 160 }, - bio: { type: "string" as const, maxLength: 500 }, - location: { type: "string" as const, maxLength: 100 }, - }, -}; +