From 2260b31efdf3eee67873170c54bb4ef5a7fde96a Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Wed, 1 Jul 2026 01:11:58 -0300 Subject: [PATCH] fix: surface relay proxy-test errors instead of silent failure (#5716) (#5765) --- CHANGELOG.md | 2 + .../settings/proxy/test/relayTestResult.ts | 35 ++++++++++++++ src/app/api/settings/proxy/test/route.ts | 35 +++++++++----- .../unit/proxy-relay-test-error-5716.test.ts | 46 +++++++++++++++++++ 4 files changed, 107 insertions(+), 11 deletions(-) create mode 100644 src/app/api/settings/proxy/test/relayTestResult.ts create mode 100644 tests/unit/proxy-relay-test-error-5716.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index b39a3b3546..355d6b6775 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,8 @@ ### 🔧 Bug Fixes +- **proxy (relay test diagnostics):** the Proxy Pool "Test" button showed a bare "failed" with **nothing in the server logs** when a **relay** (Vercel / Deno / Cloudflare) *responded* with a non-200 — e.g. a `401` from an auth-token mismatch after a `STORAGE_ENCRYPTION_KEY` rotation. The relay success-path response set `success: false` but carried no `error` field, so the dashboard had no reason to show and the server logged nothing. The test now returns an actionable `error` (the HTTP status, plus an auth/encryption-key hint on `401`/`403`) and logs the failure server-side; the SOCKS5/HTTP proxy path now logs its failures too. Shaping extracted to `buildRelayTestResult` with a regression guard (`tests/unit/proxy-relay-test-error-5716.test.ts`). Note: this surfaces *why* a relay fails — it does not repair a genuinely broken/misconfigured relay. ([#5716](https://github.com/diegosouzapw/OmniRoute/issues/5716)) + - **providers (onboarding wizard — unsupported validation):** adding a provider whose credentials have **no live validator** (LMArena, PiAPI, …) failed silently in the Add-Provider wizard. The `/api/providers/validate` endpoint returns `HTTP 400 + { unsupported: true }` for these (#5565/#5567), but the wizard's `validateOnboardingApiKey` ran it through `expectOk`, which threw on the non-200 — so the flow jumped to the error step and the connection was **never created**. The wizard now treats `unsupported: true` as a non-blocking "can't verify" and proceeds to save, mirroring `AddApiKeyModal`. Regression guard added to `tests/unit/provider-onboarding-wizard.test.ts`. (related to [#5692](https://github.com/diegosouzapw/OmniRoute/issues/5692)) - **dashboard (Quick Start step 1):** the Quick Start "Create API key" step told users to "Go to **Endpoint** → Registered Keys" and linked to `/dashboard/endpoint`, but API keys are created on the **API Manager** page (`/dashboard/api-manager`, sidebar "API Keys") — the Endpoint page has no "Registered Keys" section, so users followed the link and could not find where to create a key. Step 1 now reads "Go to **API Keys**" and links to `/dashboard/api-manager`. Regression guard: `tests/unit/ui/quick-start-api-keys-link-5695.test.ts`. ([#5695](https://github.com/diegosouzapw/OmniRoute/issues/5695)) diff --git a/src/app/api/settings/proxy/test/relayTestResult.ts b/src/app/api/settings/proxy/test/relayTestResult.ts new file mode 100644 index 0000000000..a6e2d4afee --- /dev/null +++ b/src/app/api/settings/proxy/test/relayTestResult.ts @@ -0,0 +1,35 @@ +// #5716 — shape the relay proxy-test response. Pure (no DB / network / Next +// imports) so it is unit-testable in isolation. When the relay *responds* with a +// non-200 (e.g. 401 from an auth-token mismatch), the old inline response set +// `success: false` but carried no `error` field, so the dashboard rendered a bare +// "failed" with no diagnostic and nothing was logged server-side. + +export interface RelayTestResult { + success: boolean; + publicIp: string | null; + latencyMs: number; + proxyUrl: string; + error?: string; +} + +export function buildRelayTestResult(input: { + statusCode: number; + publicIp: string | null; + latencyMs: number; + relayUrl: string; + relayAuthPresent: boolean; +}): RelayTestResult { + const { statusCode, publicIp, latencyMs, relayUrl, relayAuthPresent } = input; + const success = statusCode === 200; + const result: RelayTestResult = { success, publicIp, latencyMs, proxyUrl: relayUrl }; + if (!success) { + let error = `Relay returned HTTP ${statusCode}`; + if (statusCode === 401 || statusCode === 403) { + error += relayAuthPresent + ? " — the relay rejected the auth token; redeploy the relay so its token matches, or check STORAGE_ENCRYPTION_KEY" + : " — no relay auth token was found; redeploy the relay, or check for a STORAGE_ENCRYPTION_KEY rotation"; + } + result.error = error; + } + return result; +} diff --git a/src/app/api/settings/proxy/test/route.ts b/src/app/api/settings/proxy/test/route.ts index d0dd086aaf..dbe44967b4 100644 --- a/src/app/api/settings/proxy/test/route.ts +++ b/src/app/api/settings/proxy/test/route.ts @@ -13,6 +13,7 @@ import { getProxyById } from "@/lib/localDb"; import { extractRelayAuth } from "@/lib/db/proxies"; import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error"; +import { buildRelayTestResult } from "./relayTestResult"; const BASE_SUPPORTED_PROXY_TYPES = new Set(["http", "https"]); @@ -121,19 +122,28 @@ export async function POST(request: Request) { try { parsedIp = JSON.parse(text) as { ip?: string }; } catch {} - return Response.json({ - success: res.statusCode === 200, + const relayResult = buildRelayTestResult({ + statusCode: res.statusCode, publicIp: parsedIp.ip || null, latencyMs: Date.now() - start, - proxyUrl: relayUrl, + relayUrl, + relayAuthPresent: relayAuth.length > 0, }); + // #5716: a relay that *responds* non-200 (e.g. 401 auth mismatch) used to + // return `success:false` with no reason and no log — a silent failure. + if (!relayResult.success) { + console.warn(`[ProxyTest] relay ${relayHost}: ${relayResult.error}`); + } + return Response.json(relayResult); } catch (relayErr) { + const message = + relayErr instanceof Error && relayErr.name === "AbortError" + ? "Connection timeout (10s)" + : getErrorMessage(relayErr, "Relay test failed"); + console.warn(`[ProxyTest] relay ${relayHost} request failed: ${message}`); return Response.json({ success: false, - error: - relayErr instanceof Error && relayErr.name === "AbortError" - ? "Connection timeout (10s)" - : getErrorMessage(relayErr, "Relay test failed"), + error: message, latencyMs: Date.now() - start, proxyUrl: relayUrl, }); @@ -228,12 +238,15 @@ export async function POST(request: Request) { proxyUrl: publicProxyUrl, }); } catch (fetchError) { + const message = + fetchError instanceof Error && fetchError.name === "AbortError" + ? "Connection timeout (10s)" + : getErrorMessage(fetchError, "Connection failed"); + // #5716: surface the reason in server logs — a failing proxy test was silent. + console.warn(`[ProxyTest] ${proxyType} proxy ${publicProxyUrl} failed: ${message}`); return Response.json({ success: false, - error: - fetchError instanceof Error && fetchError.name === "AbortError" - ? "Connection timeout (10s)" - : getErrorMessage(fetchError, "Connection failed"), + error: message, latencyMs: Date.now() - startTime, proxyUrl: publicProxyUrl, }); diff --git a/tests/unit/proxy-relay-test-error-5716.test.ts b/tests/unit/proxy-relay-test-error-5716.test.ts new file mode 100644 index 0000000000..c54ed330f2 --- /dev/null +++ b/tests/unit/proxy-relay-test-error-5716.test.ts @@ -0,0 +1,46 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +// #5716 — a relay proxy "Test" that got a non-200 from the relay showed a bare +// "failed" with no reason. The test-result shaper must carry an actionable +// `error` for every non-200 status, and none for a 200. +const { buildRelayTestResult } = await import( + "../../src/app/api/settings/proxy/test/relayTestResult.ts" +); + +const base = { publicIp: "1.2.3.4", latencyMs: 12, relayUrl: "https://relay.example" }; + +test("#5716 a 200 relay response is a success with no error", () => { + const r = buildRelayTestResult({ ...base, statusCode: 200, relayAuthPresent: true }); + assert.equal(r.success, true); + assert.equal(r.error, undefined); + assert.equal(r.proxyUrl, "https://relay.example"); +}); + +test("#5716 a non-200 relay response fails WITH a diagnostic error", () => { + const r = buildRelayTestResult({ + ...base, + statusCode: 502, + publicIp: null, + relayAuthPresent: true, + }); + assert.equal(r.success, false); + assert.ok( + typeof r.error === "string" && r.error.includes("502"), + `non-200 relay test must surface the HTTP status; got error=${JSON.stringify(r.error)}` + ); +}); + +test("#5716 a 401 with missing relay auth hints at the auth/encryption-key cause", () => { + const r = buildRelayTestResult({ + ...base, + statusCode: 401, + publicIp: null, + relayAuthPresent: false, + }); + assert.equal(r.success, false); + assert.ok( + typeof r.error === "string" && /auth/i.test(r.error), + `401 with no relay auth should hint at the auth token; got error=${JSON.stringify(r.error)}` + ); +});