From 54038216e43211d70811dbaee4a6d40a166a7ec5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 9 May 2026 16:04:15 +0200 Subject: [PATCH] fix(insights): map AI provider errors to 422/503 instead of 500 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v1.4.6 T5 fixed only the parse-error branch (502 → 422 to keep Cloudflare's HTML-error rewrite from breaking `await res.json()` on the dashboard). Provider errors — `OpenAI request failed (401)` for an invalid admin key, or transient `OpenAI request failed (5xx)` — still propagated to the apiHandler's generic 500. Cloudflare rewrites the 5xx to its own HTML page, so the React Query mutation crashed with "Unexpected token '<'" instead of surfacing a readable message. Wraps `provider.generateCompletion(...)` in try/catch and mirrors the v1.4.5 ai/test categorisation: - 401 / 403 from the provider → 422 with "AI provider rejected the request — check your API key in Settings > AI" - 5xx from the provider → 503 with "AI provider temporarily unavailable, try again in a moment" - 429 from the provider → 429 (rate-limit passthrough) - any other status → 422 generic provider-connection failure Full error details (message, httpStatus, bodyExcerpt, providerType) land in the Wide Event via `annotate()` so the operator can debug without leaking provider URLs / partial keys to the client. Adds unit tests covering 401/403/429/500/503/unknown mapping. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../insights/generate/__tests__/route.test.ts | 179 ++++++++++++++++++ src/app/api/insights/generate/route.ts | 52 ++++- 2 files changed, 225 insertions(+), 6 deletions(-) create mode 100644 src/app/api/insights/generate/__tests__/route.test.ts diff --git a/src/app/api/insights/generate/__tests__/route.test.ts b/src/app/api/insights/generate/__tests__/route.test.ts new file mode 100644 index 000000000..12d7c4a70 --- /dev/null +++ b/src/app/api/insights/generate/__tests__/route.test.ts @@ -0,0 +1,179 @@ +/** + * Provider-error mapping for /api/insights/generate. + * + * v1.4.6 T5 fixed the parse-error path (502 → 422 to keep Cloudflare's + * HTML-error rewrite from breaking `await res.json()`). The provider- + * error path still propagated upstream errors (e.g. `OpenAI request + * failed (401)` for an invalid admin key) to the apiHandler's generic + * 500 handler. v1.5 now mirrors the v1.4.5 ai/test categorisation: + * + * - 401/403 from the provider → 422 with a readable message + * - 5xx from the provider → 503 (transient) + * - 429 from the provider → 429 (passthrough, not 5xx) + * - any other status → 422 (generic provider-connection failure) + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +// Mocks must be hoisted before importing the route. +vi.mock("@/lib/api-handler", () => ({ + apiHandler: unknown>(fn: T) => fn, + requireAuth: vi.fn(async () => ({ + user: { id: "u-1", locale: "en" }, + session: { id: "s-1" }, + })), +})); + +vi.mock("@/lib/db", () => ({ + prisma: { + user: { + findUnique: vi.fn(async () => ({ + insightsPrivacyMode: "aggregated", + insightsCachedAt: null, + insightsCachedText: null, + locale: "en", + })), + update: vi.fn(async () => ({})), + }, + }, +})); + +vi.mock("@/lib/auth/audit", () => ({ + auditLog: vi.fn(async () => undefined), +})); + +vi.mock("@/lib/ai/provider", async () => { + const actual = + await vi.importActual( + "@/lib/ai/provider", + ); + return { + ...actual, + resolveProvider: vi.fn(), + }; +}); + +vi.mock("@/lib/rate-limit", () => ({ + checkRateLimit: vi.fn(async () => ({ allowed: true })), +})); + +vi.mock("@/lib/logging/context", () => ({ + annotate: vi.fn(), +})); + +vi.mock("@/lib/insights/features", () => ({ + extractFeatures: vi.fn(async () => ({ stub: true })), +})); + +vi.mock("@/lib/insights/prompt", () => ({ + getInsightsSystemPrompt: vi.fn(() => "system"), + buildUserPrompt: vi.fn(() => "user"), +})); + +vi.mock("@/lib/i18n/server-locale", () => ({ + resolveServerLocale: vi.fn(async () => "en"), +})); + +import { POST } from "../route"; +import { resolveProvider } from "@/lib/ai/provider"; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +interface ApiErrorEnvelope { + data: null; + error: string; +} + +function jsonRequest(body: unknown = {}): Request { + const text = JSON.stringify(body); + return new Request("http://localhost/api/insights/generate", { + method: "POST", + headers: { + "content-type": "application/json", + "content-length": String(text.length), + }, + body: text, + }); +} + +function makeProviderThatThrows( + err: Error & { httpStatus?: number; bodyExcerpt?: string }, +) { + vi.mocked(resolveProvider).mockResolvedValue({ + type: "openai", + generateCompletion: vi.fn(async () => { + throw err; + }), + // The route only calls generateCompletion; pad the type for TS. + } as unknown as Awaited>); +} + +describe("POST /api/insights/generate — provider error mapping", () => { + it("maps a 401 from the provider to 422 with a readable message", async () => { + const err = Object.assign(new Error("OpenAI request failed (401)"), { + httpStatus: 401, + bodyExcerpt: "{\"error\":{\"code\":\"invalid_api_key\"}}", + }); + makeProviderThatThrows(err); + + const res = await POST(jsonRequest() as never); + expect(res.status).toBe(422); + const body = (await res.json()) as ApiErrorEnvelope; + expect(body.error).toMatch(/AI provider rejected/i); + expect(body.error).toMatch(/API key/i); + }); + + it("maps a 403 from the provider to 422 with the same readable message", async () => { + const err = Object.assign(new Error("OpenAI request failed (403)"), { + httpStatus: 403, + }); + makeProviderThatThrows(err); + + const res = await POST(jsonRequest() as never); + expect(res.status).toBe(422); + const body = (await res.json()) as ApiErrorEnvelope; + expect(body.error).toMatch(/AI provider rejected/i); + }); + + it("maps a 500 from the provider to 503 with a transient message", async () => { + const err = Object.assign(new Error("OpenAI request failed (500)"), { + httpStatus: 500, + }); + makeProviderThatThrows(err); + + const res = await POST(jsonRequest() as never); + expect(res.status).toBe(503); + const body = (await res.json()) as ApiErrorEnvelope; + expect(body.error).toMatch(/temporarily unavailable/i); + }); + + it("maps a 503 from the provider to 503 with a transient message", async () => { + const err = Object.assign(new Error("OpenAI request failed (503)"), { + httpStatus: 503, + }); + makeProviderThatThrows(err); + + const res = await POST(jsonRequest() as never); + expect(res.status).toBe(503); + }); + + it("maps a 429 from the provider to 429 (rate-limit passthrough)", async () => { + const err = Object.assign(new Error("OpenAI request failed (429)"), { + httpStatus: 429, + }); + makeProviderThatThrows(err); + + const res = await POST(jsonRequest() as never); + expect(res.status).toBe(429); + }); + + it("maps an unknown error (no httpStatus) to 422 with a generic message", async () => { + makeProviderThatThrows(new Error("network unreachable")); + + const res = await POST(jsonRequest() as never); + expect(res.status).toBe(422); + const body = (await res.json()) as ApiErrorEnvelope; + expect(body.error).toMatch(/provider/i); + }); +}); diff --git a/src/app/api/insights/generate/route.ts b/src/app/api/insights/generate/route.ts index 107bb0156..f571e9143 100644 --- a/src/app/api/insights/generate/route.ts +++ b/src/app/api/insights/generate/route.ts @@ -85,12 +85,52 @@ export const POST = apiHandler(async (request: NextRequest) => { locale, ); - const result = await provider.generateCompletion({ - systemPrompt: getInsightsSystemPrompt(locale), - userPrompt, - temperature: 0.3, - maxTokens: 1500, - }); + let result; + try { + result = await provider.generateCompletion({ + systemPrompt: getInsightsSystemPrompt(locale), + userPrompt, + temperature: 0.3, + maxTokens: 1500, + }); + } catch (e) { + // v1.4.6 T5 mapped only the parse-error branch from 502→422. Provider + // errors (e.g. invalid OpenAI key surfacing as + // `OpenAI request failed (401)`) still propagated to the apiHandler + // generic 500. Cloudflare rewrites 5xx to its own HTML error page, + // which breaks `await res.json()` on the dashboard. Mirror the v1.4.5 + // ai/test categorisation so the React Query mutation can read the + // body and surface a readable message. + const err = e as Error & { httpStatus?: number; bodyExcerpt?: string }; + annotate({ + meta: { + insights_provider_error: err.message.slice(0, 500), + insights_provider_status: err.httpStatus ?? null, + insights_provider_body_excerpt: err.bodyExcerpt?.slice(0, 500) ?? null, + insights_provider_type: provider.type, + }, + }); + const status = err.httpStatus ?? 0; + if (status === 401 || status === 403) { + return apiError( + "AI provider rejected the request — check your API key in Settings > AI", + 422, + ); + } + if (status === 429) { + return apiError("AI provider rate-limited the request", 429); + } + if (status >= 500) { + return apiError( + "AI provider temporarily unavailable, try again in a moment", + 503, + ); + } + return apiError( + "AI provider connection failed — check your AI settings", + 422, + ); + } let insights: InsightResult | Record; try {