fix(dashboard): proxy-pool success gating, sync timestamp, opt-in Redis (#4878) (#4988)

Co-authored-by: Diego Rodrigues de Sa e Souza <souzamiriamrodrigues790@gmail.com>

.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

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)
 
 ---

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 {}

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, {

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");
   }

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<string> {
+  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<typeof getDbInstance>): 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<FreeProxyStats> {
   const db = getDbInstance();
   const totals = db
@@ -288,11 +317,16 @@ export async function getFreeProxyStats(): Promise<FreeProxyStats> {
     )
     .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,
   };
 }

src/lib/localDb.ts

@@ -533,6 +533,7 @@ export {
   deleteFreeProxy,
   clearFreeProxiesBySource,
   getFreeProxyStats,
+  recordFreeProxySync,
 } from "./db/freeProxies";
 
 export type { FreeProxyRecord, FreeProxyStats } from "./db/freeProxies";

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;
 }

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);
+});