diff --git a/canvas/src/components/__tests__/TestConnectionButton.test.tsx b/canvas/src/components/__tests__/TestConnectionButton.test.tsx index 15f1dd9cf..b4dd0cda0 100644 --- a/canvas/src/components/__tests__/TestConnectionButton.test.tsx +++ b/canvas/src/components/__tests__/TestConnectionButton.test.tsx @@ -11,13 +11,21 @@ import { render, screen, fireEvent, cleanup, act } from "@testing-library/react" import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { TestConnectionButton } from "../ui/TestConnectionButton"; import type { SecretGroup } from "@/types/secrets"; -import { validateSecret } from "@/lib/api/secrets"; +import { validateSecret, ApiError } from "@/lib/api/secrets"; // ─── Mock validateSecret ────────────────────────────────────────────────────── // vi.mock is hoisted, so validateSecret (imported above) refers to the mocked // namespace value once vi.mock runs. Use vi.mocked() to access it in tests. vi.mock("@/lib/api/secrets", () => ({ validateSecret: vi.fn(), + ApiError: class ApiError extends Error { + status: number; + constructor(status: number, message: string) { + super(message); + this.name = "ApiError"; + this.status = status; + } + }, })); // SecretGroup is a string literal type: 'github' | 'anthropic' | 'openrouter' | 'custom' @@ -102,7 +110,7 @@ describe("TestConnectionButton — state machine", () => { expect(screen.getByText("Permission denied")).toBeTruthy(); }); - it("shows generic error message on unexpected exception", async () => { + it("shows a connectivity message on a genuine network exception", async () => { vi.mocked(validateSecret).mockRejectedValue(new Error("timeout")); render(); @@ -110,8 +118,23 @@ describe("TestConnectionButton — state machine", () => { await act(async () => { /* flush */ }); expect(screen.getByRole("alert")).toBeTruthy(); - // The error detail is hardcoded to "Connection timed out. Service may be down." - expect(document.body.querySelector('[role="alert"]')?.textContent).toMatch(/timed out/i); + // A real thrown network error → honest connectivity message (not a + // fabricated "service down"); see internal#492. + expect(document.body.querySelector('[role="alert"]')?.textContent).toMatch( + /could not reach the validation service/i, + ); + }); + + it("does not claim a timeout when the validate endpoint 404s (internal#492)", async () => { + vi.mocked(validateSecret).mockRejectedValue(new ApiError(404, "Not Found")); + render(); + + fireEvent.click(screen.getByRole("button")); + await act(async () => { /* flush */ }); + + const alert = document.body.querySelector('[role="alert"]')?.textContent ?? ""; + expect(alert).not.toMatch(/timed out/i); + expect(alert).toMatch(/not available/i); }); }); diff --git a/canvas/src/components/ui/TestConnectionButton.tsx b/canvas/src/components/ui/TestConnectionButton.tsx index 940c06e49..a85714ef8 100644 --- a/canvas/src/components/ui/TestConnectionButton.tsx +++ b/canvas/src/components/ui/TestConnectionButton.tsx @@ -2,7 +2,7 @@ import { useState, useCallback, useRef, useEffect } from 'react'; import type { TestConnectionState, SecretGroup } from '@/types/secrets'; -import { validateSecret } from '@/lib/api/secrets'; +import { validateSecret, ApiError } from '@/lib/api/secrets'; interface TestConnectionButtonProps { provider: SecretGroup; @@ -55,9 +55,23 @@ export function TestConnectionButton({ } onResult?.(result.valid); resetTimerRef.current = setTimeout(() => setState('idle'), RESET_DELAYS[nextState]!); - } catch { + } catch (err) { + // Distinguish a real failure shape rather than always claiming a + // timeout. A reachable server that answered with an HTTP status + // (ApiError) did NOT time out — most commonly the validation route + // is not available (404/501), which must not masquerade as + // "service down". Only an actual thrown network/abort error is a + // connectivity failure. setState('failure'); - setErrorDetail('Connection timed out. Service may be down.'); + if (err instanceof ApiError) { + setErrorDetail( + err.status === 404 || err.status === 501 + ? 'Key validation is not available for this service yet. The key was not tested.' + : `Could not verify key (server returned ${err.status}). Saving is unaffected.`, + ); + } else { + setErrorDetail('Could not reach the validation service. Check your connection and try again.'); + } onResult?.(false); resetTimerRef.current = setTimeout(() => setState('idle'), RESET_DELAYS.failure); } diff --git a/canvas/src/components/ui/__tests__/TestConnectionButton.test.tsx b/canvas/src/components/ui/__tests__/TestConnectionButton.test.tsx index 62d61ff11..21114ef1d 100644 --- a/canvas/src/components/ui/__tests__/TestConnectionButton.test.tsx +++ b/canvas/src/components/ui/__tests__/TestConnectionButton.test.tsx @@ -28,8 +28,20 @@ const mockValidateSecret = vi.fn(); vi.mock("@/lib/api/secrets", () => ({ validateSecret: (...args: unknown[]) => mockValidateSecret(...args), + ApiError: class ApiError extends Error { + status: number; + constructor(status: number, message: string) { + super(message); + this.name = "ApiError"; + this.status = status; + } + }, })); +// Re-import the mocked ApiError so test cases construct the same class the +// component's `instanceof` check sees. +import { ApiError } from "@/lib/api/secrets"; + beforeEach(() => { vi.useFakeTimers(); vi.clearAllMocks(); @@ -201,8 +213,27 @@ describe("TestConnectionButton — failure path", () => { }); describe("TestConnectionButton — catch path", () => { - it("shows 'Connection timed out' on network error", async () => { - mockValidateSecret.mockRejectedValue(new Error("timeout")); + it("does NOT claim a timeout when the validate endpoint 404s (regression: internal#492)", async () => { + // The validate route is unimplemented on the server and returns a fast + // 404. Before the fix this rendered the misleading hardcoded string + // "Connection timed out. Service may be down." It must instead state + // honestly that validation isn't available and the key was not tested. + mockValidateSecret.mockRejectedValue(new ApiError(404, "Not Found")); + render( + , + ); + fireEvent.click(document.querySelector('button[type="button"]')!); + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + expect(document.body.textContent).not.toContain("Connection timed out"); + expect(document.body.textContent).not.toContain("Service may be down"); + expect(document.body.textContent).toContain("not available"); + expect(document.body.textContent).toContain("not tested"); + }); + + it("reports a non-404 server error with its status, not a timeout", async () => { + mockValidateSecret.mockRejectedValue(new ApiError(500, "Internal Server Error")); render( , ); @@ -210,7 +241,20 @@ describe("TestConnectionButton — catch path", () => { await act(async () => { await vi.advanceTimersByTimeAsync(0); }); - expect(document.body.textContent).toContain("Connection timed out"); + expect(document.body.textContent).toContain("500"); + expect(document.body.textContent).not.toContain("Connection timed out"); + }); + + it("shows a connectivity message on a genuine network error", async () => { + mockValidateSecret.mockRejectedValue(new Error("network down")); + render( + , + ); + fireEvent.click(document.querySelector('button[type="button"]')!); + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + expect(document.body.textContent).toContain("Could not reach the validation service"); }); it("calls onResult(false) on network error", async () => {