diff --git a/.env.example b/.env.example index a487fe0200..ffb1085612 100644 --- a/.env.example +++ b/.env.example @@ -56,10 +56,12 @@ STORAGE_ENCRYPTION_KEY_VERSION=v1 DISABLE_SQLITE_AUTO_BACKUP=false # ── Redis (Rate Limiting) ── -# Redis connection URL for the rate limiter backend. +# Redis connection URL for the rate limiter backend. OPT-IN: leave this +# commented out to use the built-in in-memory rate limiter. Setting it to a +# non-running localhost (#4878) makes ioredis flood "[REDIS] Error:" logs. # Used by: src/shared/utils/rateLimiter.ts -# Default: redis://localhost:6379 (or redis://redis:6379 in Docker) -REDIS_URL=redis://localhost:6379 +# Example: redis://localhost:6379 (or redis://redis:6379 in Docker) +# REDIS_URL=redis://localhost:6379 # ═══════════════════════════════════════════════════════════════════════════════ # 3. NETWORK & PORTS diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e5fc041c7..0466e695ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ _In development — bullets added per PR; finalized at release._ ### 🔧 Bug Fixes +- **fix(dashboard):** free proxy pool no longer mis-reports state — "Add to pool" stops optimistically showing "In Pool" when the connectivity probe fails (route returns 422 and the UI now gates on the parsed `success` flag), "Sync All" persists and surfaces a real `lastSyncAt` even when a sync returns zero new proxies, and `REDIS_URL` is now opt-in in `.env.example` (with a state-change-gated `[REDIS] Error:` log throttle) so a non-running localhost no longer floods the logs (#4878). - **fix(dashboard):** show custom provider given-name instead of internal id across dashboard pages — cache, combo health, compression analytics, cost overview, health/autopilot, provider stats, route explainability, provider utilization, runtime. Adds shared `resolveProviderName` resolver and `useProviderNodeMap` hook. (#4603) --- diff --git a/src/app/(dashboard)/dashboard/settings/components/proxy/FreePoolTab.tsx b/src/app/(dashboard)/dashboard/settings/components/proxy/FreePoolTab.tsx index 533006f64d..f3f52d590b 100644 --- a/src/app/(dashboard)/dashboard/settings/components/proxy/FreePoolTab.tsx +++ b/src/app/(dashboard)/dashboard/settings/components/proxy/FreePoolTab.tsx @@ -102,7 +102,11 @@ export default function FreePoolTab() { const res = await fetch(`/api/settings/free-proxies/${id}/add-to-pool`, { method: "POST", }); - if (res.ok) { + // #4878: gate on the parsed body, not just res.ok. The route used to return + // a default 200 with { success:false } on a failed connectivity probe, which + // flipped the row to "In Pool" optimistically even though nothing was added. + const data = await res.json().catch(() => null); + if (res.ok && data?.success) { setProxies((prev) => prev.map((p) => (p.id === id ? { ...p, inPool: true } : p))); } } catch {} diff --git a/src/app/api/settings/free-proxies/[id]/add-to-pool/route.ts b/src/app/api/settings/free-proxies/[id]/add-to-pool/route.ts index 9861d8c5e7..f1590b1da6 100644 --- a/src/app/api/settings/free-proxies/[id]/add-to-pool/route.ts +++ b/src/app/api/settings/free-proxies/[id]/add-to-pool/route.ts @@ -79,11 +79,17 @@ export async function POST(request: Request, { params }: { params: Promise<{ id: try { const testResult = await _connectivityTester(freeProxy.host, freeProxy.port, freeProxy.type); if (!testResult.success) { - return Response.json({ - success: false, - error: "Proxy test failed", - latencyMs: testResult.latencyMs, - }); + // #4878: a failed connectivity probe must surface a non-2xx status so the + // frontend (which gates on res.ok) does NOT optimistically mark the proxy + // as "In Pool". 422 = the request was well-formed but the proxy is unusable. + return Response.json( + { + success: false, + error: "Proxy test failed", + latencyMs: testResult.latencyMs, + }, + { status: 422 } + ); } const newPoolProxyId = await promoteFreeProxyToPool(id, { diff --git a/src/app/api/settings/free-proxies/sync/route.ts b/src/app/api/settings/free-proxies/sync/route.ts index 4fdcaefa3e..d7b9e8bc1b 100644 --- a/src/app/api/settings/free-proxies/sync/route.ts +++ b/src/app/api/settings/free-proxies/sync/route.ts @@ -3,6 +3,7 @@ import { createErrorResponse, createErrorResponseFromUnknown } from "@/lib/api/e import { isValidationFailure, validateBody } from "@/shared/validation/helpers"; import { freeProxySyncSchema } from "@/shared/validation/freeProxySchemas"; import { getEnabledProviders, getProvider } from "@/lib/freeProxyProviders"; +import { recordFreeProxySync } from "@/lib/localDb"; import type { FreeProxySourceId } from "@/lib/freeProxyProviders/types"; export async function POST(request: Request) { @@ -45,7 +46,12 @@ export async function POST(request: Request) { results[provider.id] = await provider.sync(); } - return Response.json({ success: true, results }); + // #4878: persist the sync timestamp so the UI's "last sync" advances even + // when a sync returns zero new/updated proxies (otherwise it stayed frozen + // at MAX(last_validated)). + const lastSyncAt = await recordFreeProxySync(); + + return Response.json({ success: true, results, lastSyncAt }); } catch (error) { return createErrorResponseFromUnknown(error, "Failed to sync free proxies"); } diff --git a/src/lib/db/freeProxies.ts b/src/lib/db/freeProxies.ts index 3ea7fd46db..57593c98ef 100644 --- a/src/lib/db/freeProxies.ts +++ b/src/lib/db/freeProxies.ts @@ -270,6 +270,35 @@ export async function clearFreeProxiesBySource(source: FreeProxySourceId): Promi return result.changes; } +// #4878: the displayed "last sync" used to be derived from MAX(last_validated), +// which only advances when a provider returns at least one new/updated proxy. A +// sync that returns zero rows (or whose providers all fail) left the timestamp +// frozen, so "Sync All" appeared to do nothing. We persist an explicit sync +// timestamp in the generic key_value store and prefer it in the stats. +const FREE_PROXY_SYNC_NAMESPACE = "free_proxies"; +const FREE_PROXY_SYNC_KEY = "last_sync_at"; + +/** + * Persist the moment a free-proxy sync completed. Returns the stored ISO string + * so the route can echo it back. `at` is overridable for deterministic tests. + */ +export async function recordFreeProxySync(at?: string): Promise { + const db = getDbInstance(); + const ts = at ?? new Date().toISOString(); + db.prepare( + "INSERT OR REPLACE INTO key_value (namespace, key, value) VALUES (?, ?, ?)" + ).run(FREE_PROXY_SYNC_NAMESPACE, FREE_PROXY_SYNC_KEY, ts); + backupDbFile("pre-write"); + return ts; +} + +function getRecordedFreeProxySync(db: ReturnType): string | null { + const row = db + .prepare("SELECT value FROM key_value WHERE namespace = ? AND key = ?") + .get(FREE_PROXY_SYNC_NAMESPACE, FREE_PROXY_SYNC_KEY) as { value?: string } | undefined; + return row?.value != null ? String(row.value) : null; +} + export async function getFreeProxyStats(): Promise { const db = getDbInstance(); const totals = db @@ -288,11 +317,16 @@ export async function getFreeProxyStats(): Promise { ) .all() as DbRow[]; + // Prefer the explicitly recorded sync timestamp (#4878); fall back to the + // newest last_validated only when no sync has ever been recorded. + const recordedSyncAt = getRecordedFreeProxySync(db); + const derivedSyncAt = totals.last_sync_at != null ? String(totals.last_sync_at) : null; + return { total: Number(totals.total) || 0, inPool: Number(totals.in_pool_count) || 0, avgQuality: totals.avg_quality != null ? Math.round(Number(totals.avg_quality)) : null, bySource: bySource.map((r) => ({ source: String(r.source), count: Number(r.count) })), - lastSyncAt: totals.last_sync_at != null ? String(totals.last_sync_at) : null, + lastSyncAt: recordedSyncAt ?? derivedSyncAt, }; } diff --git a/src/lib/localDb.ts b/src/lib/localDb.ts index 2ff7601a6e..68b1245590 100755 --- a/src/lib/localDb.ts +++ b/src/lib/localDb.ts @@ -533,6 +533,7 @@ export { deleteFreeProxy, clearFreeProxiesBySource, getFreeProxyStats, + recordFreeProxySync, } from "./db/freeProxies"; export type { FreeProxyRecord, FreeProxyStats } from "./db/freeProxies"; diff --git a/src/shared/utils/rateLimiter.ts b/src/shared/utils/rateLimiter.ts index 1ff9431f22..a4da29cf88 100644 --- a/src/shared/utils/rateLimiter.ts +++ b/src/shared/utils/rateLimiter.ts @@ -13,6 +13,36 @@ export function isRedisConfigured(): boolean { return REDIS_URL.length > 0; } +/** + * State-change-gated log throttle for the Redis error handler. + * + * #4878: when REDIS_URL points at a non-running server, ioredis retries on a + * backoff and fires the "error" event on every attempt, flooding the logs with + * identical "[REDIS] Error:" lines. We only want to log when the error STATE + * actually changes (first occurrence, or a different error message), not on + * every retry of the same failure. + */ +export function createRedisLogThrottle() { + let lastLogged: string | null = null; + return { + shouldLog(message: string): boolean { + if (message === lastLogged) return false; + lastLogged = message; + return true; + }, + reset(): void { + lastLogged = null; + }, + }; +} + +const redisLogThrottle = createRedisLogThrottle(); + +// Exposed for unit tests — returns a fresh, isolated throttle instance. +export function _createRedisLogThrottleForTests() { + return createRedisLogThrottle(); +} + export function getRedisClient() { if (!isRedisConfigured()) { throw new Error("Redis is not configured"); @@ -26,7 +56,14 @@ export function getRedisClient() { return Math.min(times * 50, 2000); // Exponential backoff }, }); - redisClient.on("error", (err) => console.error("[REDIS] Error:", err.message)); + redisClient.on("error", (err) => { + // Throttle: log once per error-state change instead of on every retry (#4878). + if (redisLogThrottle.shouldLog(err.message)) { + console.error("[REDIS] Error:", err.message); + } + }); + // A successful connection resets the throttle so the next failure logs again. + redisClient.on("ready", () => redisLogThrottle.reset()); } return redisClient; } diff --git a/tests/unit/proxy-pool-sync-4878.test.ts b/tests/unit/proxy-pool-sync-4878.test.ts new file mode 100644 index 0000000000..557cd3799a --- /dev/null +++ b/tests/unit/proxy-pool-sync-4878.test.ts @@ -0,0 +1,140 @@ +import "../../open-sse/utils/setupPolyfill.ts"; +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-pool-sync-4878-")); +const ORIGINAL_DATA_DIR = process.env.DATA_DIR; + +process.env.DATA_DIR = TEST_DATA_DIR; +delete process.env.OMNIROUTE_API_KEY; + +const core = await import("../../src/lib/db/core.ts"); +const freeProxiesDb = await import("../../src/lib/db/freeProxies.ts"); +const addToPoolRoute = await import( + "../../src/app/api/settings/free-proxies/[id]/add-to-pool/route.ts" +); +const rateLimiter = await import("../../src/shared/utils/rateLimiter.ts"); + +function reset() { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); +} + +function makeReq(): Request { + return new Request("http://localhost/test", { method: "POST" }); +} + +test.beforeEach(() => { + reset(); + addToPoolRoute._resetConnectivityTesterForTests(); +}); + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + if (ORIGINAL_DATA_DIR === undefined) { + delete process.env.DATA_DIR; + } else { + process.env.DATA_DIR = ORIGINAL_DATA_DIR; + } +}); + +// ── SUB-FIX 1: non-2xx status when the add fails ───────────────────────────── + +test("#4878 add-to-pool returns a non-2xx status when connectivity test fails", async () => { + const { id } = await freeProxiesDb.upsertFreeProxy({ + source: "1proxy", + host: "10.9.0.1", + port: 8080, + type: "http", + countryCode: null, + qualityScore: null, + latencyMs: null, + anonymity: null, + lastValidated: null, + }); + + addToPoolRoute._setConnectivityTesterForTests(async () => ({ success: false, latencyMs: 7 })); + + const res = await addToPoolRoute.POST(makeReq(), { params: Promise.resolve({ id }) }); + assert.ok( + !res.ok && res.status >= 400, + `expected a non-2xx status on failure, got ${res.status}` + ); + const body = await res.json(); + assert.equal(body.success, false); + assert.ok(body.error); + + // Proxy must NOT have been added to the pool. + const fp = await freeProxiesDb.getFreeProxyById(id); + assert.ok(!fp?.inPool); +}); + +test("#4878 add-to-pool still returns 2xx + success:true on the happy path", async () => { + const { id } = await freeProxiesDb.upsertFreeProxy({ + source: "1proxy", + host: "10.9.0.2", + port: 8080, + type: "http", + countryCode: null, + qualityScore: null, + latencyMs: null, + anonymity: null, + lastValidated: null, + }); + + addToPoolRoute._setConnectivityTesterForTests(async () => ({ + success: true, + latencyMs: 5, + publicIp: "1.2.3.4", + })); + + const res = await addToPoolRoute.POST(makeReq(), { params: Promise.resolve({ id }) }); + assert.ok(res.ok, `expected a 2xx status on success, got ${res.status}`); + const body = await res.json(); + assert.equal(body.success, true); +}); + +// ── SUB-FIX 2: sync timestamp is persisted and surfaced in stats ───────────── + +test("#4878 recordFreeProxySync persists a lastSyncAt surfaced by getFreeProxyStats", async () => { + // No proxies upserted at all → MAX(last_validated) is NULL. The sync + // timestamp must still be reported once a sync has run. + const before = await freeProxiesDb.getFreeProxyStats(); + assert.equal(before.lastSyncAt, null); + + const ts = await freeProxiesDb.recordFreeProxySync(); + assert.ok(typeof ts === "string" && ts.length > 0); + + const after = await freeProxiesDb.getFreeProxyStats(); + assert.equal(after.lastSyncAt, ts); +}); + +test("#4878 recordFreeProxySync advances lastSyncAt on a subsequent sync", async () => { + const first = await freeProxiesDb.recordFreeProxySync("2020-01-01T00:00:00.000Z"); + assert.equal(first, "2020-01-01T00:00:00.000Z"); + const second = await freeProxiesDb.recordFreeProxySync("2030-06-25T12:00:00.000Z"); + assert.equal(second, "2030-06-25T12:00:00.000Z"); + + const stats = await freeProxiesDb.getFreeProxyStats(); + assert.equal(stats.lastSyncAt, "2030-06-25T12:00:00.000Z"); +}); + +// ── SUB-FIX 3: Redis error-log throttle is state-change-gated ───────────────── + +test("#4878 shouldLogRedisError only logs once per error-state change", () => { + const tracker = rateLimiter._createRedisLogThrottleForTests(); + + // First error in a run → log. + assert.equal(tracker.shouldLog("ECONNREFUSED"), true); + // Same error repeated (retry flood) → suppressed. + assert.equal(tracker.shouldLog("ECONNREFUSED"), false); + assert.equal(tracker.shouldLog("ECONNREFUSED"), false); + // A different error message → state changed → log once more. + assert.equal(tracker.shouldLog("ETIMEDOUT"), true); + assert.equal(tracker.shouldLog("ETIMEDOUT"), false); +});