mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-12 06:06:10 +00:00
fix(ai): never 5xx from the connection test and report a categorised reason
The connection-test route mapped a provider 5xx to a 502, which a reverse
proxy rewrites to its own HTML error page; the browser's res.json() then
threw "Unexpected token '<'". Every provider-call failure now returns
HTTP 200 with a secret-free { ok:false, providerType, reason }. Reasons
are categorised: credential/auth (401/403, or a 5xx whose body signals an
invalidated/expired session), rate-limit (429), provider server error
(other 5xx) and network/timeout.
The empty-override "test my saved config" path now resolves through the
same provider chain generation uses, instead of the legacy codex->admin
probe that could test a different provider than the one generation runs.
The chain runner already advances past an auth-class failure to the next
enabled provider; add explicit coverage for that on the raw generation
path (401 -> next succeeds, fallback count incremented).
This commit is contained in:
1 parent
192fe45469
commit
564d7dec77
5 files changed
+191
-59
No files matched your search
@@ -65,11 +65,22 @@ function jsonRequest(body: unknown): Request {
|
||||
});
|
||||
}
|
||||
|
||||
interface TestFailureEnvelope {
|
||||
data: { ok: false; providerType: string; reason: string };
|
||||
error: null;
|
||||
}
|
||||
|
||||
// V3 audit: /api/ai/test was returning provider err.message + bodyExcerpt
|
||||
// directly to the client, leaking provider URLs / partial keys / internal
|
||||
// headers. Server now logs full details via annotate() and responds with
|
||||
// a categorised, generic message.
|
||||
describe("POST /api/ai/test — provider error leak guard", () => {
|
||||
// a categorised, secret-free reason.
|
||||
//
|
||||
// This route MUST NEVER return a 5xx: a 5xx origin response is rewritten
|
||||
// by Cloudflare to its own HTML error page, so the browser's res.json()
|
||||
// crashes with `Unexpected token '<', "<!DOCTYPE "`. Every provider-call
|
||||
// failure returns HTTP 200 with `{ ok:false, reason }` the client shows
|
||||
// verbatim.
|
||||
describe("POST /api/ai/test — provider error leak guard + non-5xx contract", () => {
|
||||
function makeProviderThatThrows(
|
||||
err: Error & { httpStatus?: number; bodyExcerpt?: string },
|
||||
) {
|
||||
@@ -81,7 +92,7 @@ describe("POST /api/ai/test — provider error leak guard", () => {
|
||||
} as never);
|
||||
}
|
||||
|
||||
it("does not echo provider err.message back to the client (HIGH coverage gap)", async () => {
|
||||
it("never returns a 5xx and does not echo provider err.message back to the client", async () => {
|
||||
const err = Object.assign(
|
||||
new Error("OpenAI 401 from https://api.openai.com/v1 sk-leaked-key"),
|
||||
{
|
||||
@@ -92,49 +103,60 @@ describe("POST /api/ai/test — provider error leak guard", () => {
|
||||
makeProviderThatThrows(err);
|
||||
|
||||
const response = await POST(emptyRequest() as never);
|
||||
const body = (await response.json()) as ApiErrorEnvelope;
|
||||
|
||||
// v1.4.5: credential failures map to 422 (not 502) so Cloudflare
|
||||
// doesn't replace our JSON body with its HTML error page — that
|
||||
// rewrite was the root cause of the "Unexpected token '<'…" client
|
||||
// crash the maintainer hit when typing a bad OpenAI key.
|
||||
expect(response.status).toBe(422);
|
||||
expect(body.error ?? "").not.toMatch(/sk-/);
|
||||
expect(body.error ?? "").not.toMatch(/api\.openai\.com/);
|
||||
expect(body.error ?? "").not.toMatch(/invalid api key/i);
|
||||
expect(body.error).toBe("Provider rejected the credentials");
|
||||
// Always JSON-parseable, never a 5xx (the Cloudflare HTML-rewrite
|
||||
// root cause of "Unexpected token '<'…").
|
||||
expect(response.status).toBe(200);
|
||||
const body = (await response.json()) as TestFailureEnvelope;
|
||||
expect(body.data.ok).toBe(false);
|
||||
expect(body.data.reason).not.toMatch(/sk-/);
|
||||
expect(body.data.reason).not.toMatch(/api\.openai\.com/);
|
||||
expect(body.data.reason).not.toMatch(/invalid api key/i);
|
||||
expect(body.data.reason).toMatch(/re-authenticate/i);
|
||||
});
|
||||
|
||||
it("returns the 429-categorised message when the provider rate-limits", async () => {
|
||||
it("reclassifies a 5xx whose body signals an invalidated session as a credential failure", async () => {
|
||||
// The shape an operator hit: re-auth, gateway answers 500 with
|
||||
// "authentication token has been invalidated" instead of a 401.
|
||||
makeProviderThatThrows(
|
||||
Object.assign(new Error("upstream 500"), {
|
||||
httpStatus: 500 as const,
|
||||
bodyExcerpt:
|
||||
'{"error":{"message":"Your authentication token has been invalidated. Please try signing in again."}}',
|
||||
}),
|
||||
);
|
||||
const response = await POST(emptyRequest() as never);
|
||||
expect(response.status).toBe(200);
|
||||
const body = (await response.json()) as TestFailureEnvelope;
|
||||
expect(body.data.ok).toBe(false);
|
||||
expect(body.data.reason).toMatch(/re-authenticate/i);
|
||||
});
|
||||
|
||||
it("categorises a 429 rate-limit", async () => {
|
||||
makeProviderThatThrows(
|
||||
Object.assign(new Error("429 from openai"), { httpStatus: 429 as const }),
|
||||
);
|
||||
const response = await POST(emptyRequest() as never);
|
||||
// v1.4.5: rate-limit passes through as 429 (was 502 in v1.4.4) so
|
||||
// the React Query mutation can read the JSON body instead of
|
||||
// tripping over Cloudflare's HTML 502.
|
||||
expect(response.status).toBe(429);
|
||||
expect(((await response.json()) as ApiErrorEnvelope).error).toBe(
|
||||
"Provider rate-limited the request",
|
||||
);
|
||||
expect(response.status).toBe(200);
|
||||
const body = (await response.json()) as TestFailureEnvelope;
|
||||
expect(body.data.reason).toMatch(/rate-limited/i);
|
||||
});
|
||||
|
||||
it("returns the 5xx-categorised message when the provider has a server error", async () => {
|
||||
it("categorises a plain 5xx provider server error (no auth signal)", async () => {
|
||||
makeProviderThatThrows(
|
||||
Object.assign(new Error("503 upstream"), { httpStatus: 503 as const }),
|
||||
);
|
||||
const response = await POST(emptyRequest() as never);
|
||||
expect(((await response.json()) as ApiErrorEnvelope).error).toBe(
|
||||
"Provider returned a server error",
|
||||
);
|
||||
expect(response.status).toBe(200);
|
||||
const body = (await response.json()) as TestFailureEnvelope;
|
||||
expect(body.data.reason).toMatch(/server error/i);
|
||||
});
|
||||
|
||||
it("returns the unknown-error fallback otherwise", async () => {
|
||||
it("categorises a network/timeout failure", async () => {
|
||||
makeProviderThatThrows(new Error("ECONNRESET"));
|
||||
const response = await POST(emptyRequest() as never);
|
||||
expect(((await response.json()) as ApiErrorEnvelope).error).toBe(
|
||||
"Provider connection failed",
|
||||
);
|
||||
expect(response.status).toBe(200);
|
||||
const body = (await response.json()) as TestFailureEnvelope;
|
||||
expect(body.data.reason).toMatch(/could not reach/i);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -81,7 +81,7 @@ export const POST = apiHandler(async (request: NextRequest) => {
|
||||
// V3 audit: do not return provider error message + bodyExcerpt to the
|
||||
// client (leaks provider URL / partial keys / internal headers).
|
||||
// Log full details server-side for the operator and respond with a
|
||||
// categorised, generic message.
|
||||
// categorised, generic reason.
|
||||
const err = e as Error & { httpStatus?: number; bodyExcerpt?: string };
|
||||
annotate({
|
||||
meta: {
|
||||
@@ -91,28 +91,47 @@ export const POST = apiHandler(async (request: NextRequest) => {
|
||||
ai_test_provider: provider.type,
|
||||
},
|
||||
});
|
||||
// Map upstream status to a *client-side* status code that won't be
|
||||
// rewritten by Cloudflare. 5xx from origin causes Cloudflare to swap
|
||||
// the body for its own HTML error page — `await res.json()` in the
|
||||
// browser then crashes with `Unexpected token '<', "<!DOCTYPE "`,
|
||||
// which is exactly how the maintainer encountered this when typing a bad
|
||||
// OpenAI key. 4xx codes pass through untouched.
|
||||
const status = err.httpStatus ?? 0;
|
||||
let outboundStatus: number;
|
||||
let safeMessage: string;
|
||||
if (status === 401 || status === 403) {
|
||||
outboundStatus = 422;
|
||||
safeMessage = "Provider rejected the credentials";
|
||||
} else if (status === 429) {
|
||||
outboundStatus = 429;
|
||||
safeMessage = "Provider rate-limited the request";
|
||||
} else if (status >= 500) {
|
||||
outboundStatus = 502;
|
||||
safeMessage = "Provider returned a server error";
|
||||
} else {
|
||||
outboundStatus = 422;
|
||||
safeMessage = "Provider connection failed";
|
||||
}
|
||||
return apiError(safeMessage, outboundStatus);
|
||||
// This route MUST NEVER return a 5xx. A 5xx origin response is
|
||||
// rewritten by Cloudflare / the reverse proxy to its own HTML error
|
||||
// page, so the browser's `res.json()` crashes with
|
||||
// `Unexpected token '<', "<!DOCTYPE "` (Safari: "The string did not
|
||||
// match the expected pattern"). That is exactly the failure an
|
||||
// operator hit re-authenticating a provider whose token came back as
|
||||
// an invalidated-session 500. We always reply 200 with a categorised,
|
||||
// secret-free `{ ok:false, reason }` the client can show verbatim.
|
||||
const reason = classifyTestFailure(err);
|
||||
return apiSuccess({ ok: false, providerType: provider.type, reason });
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* Map an upstream provider failure to a human-readable, secret-free
|
||||
* reason string. Distinguishes credential/auth, rate-limit, provider
|
||||
* server-error and network/timeout classes. A 5xx whose body carries an
|
||||
* auth signal (invalidated session token, expired key, "sign in again")
|
||||
* is reclassified as a credential failure — that is the shape the
|
||||
* operator's re-auth produced, where the gateway answered 500 instead of
|
||||
* the expected 401.
|
||||
*/
|
||||
function classifyTestFailure(
|
||||
err: Error & { httpStatus?: number; bodyExcerpt?: string },
|
||||
): string {
|
||||
const status = err.httpStatus ?? 0;
|
||||
const haystack = `${err.message} ${err.bodyExcerpt ?? ""}`;
|
||||
const looksLikeAuth =
|
||||
/authentication|invalidated|expired|sign in again|api key/i.test(haystack);
|
||||
|
||||
if (status === 401 || status === 403 || (status >= 500 && looksLikeAuth)) {
|
||||
return "Provider rejected the credentials — re-authenticate in AI settings.";
|
||||
}
|
||||
if (status === 429) {
|
||||
return "Provider rate-limited the request — try again shortly.";
|
||||
}
|
||||
if (status >= 500) {
|
||||
return "The AI provider returned a server error.";
|
||||
}
|
||||
if (status === 0) {
|
||||
return "Could not reach the AI provider.";
|
||||
}
|
||||
return "Could not reach the AI provider.";
|
||||
}
|
||||
Reference in new issue
Block a user