From 7a442a0c62bfe8171684f6ad4fcf9887cbd35096 Mon Sep 17 00:00:00 2001 From: Mao Nakamoto <41178744+maonakamoto@users.noreply.github.com> Date: Fri, 28 Aug 2026 13:54:44 +0200 Subject: [PATCH] fix(ai): a total LLM outage was reporting itself as success Found by driving the Lex demo in production: the chat answered "I'm sorry, I encountered an error processing your request." Behind it, every AI feature on the site is down and has been invisible. The box has an invalid Groq key, no other provider key, and no Ollama: Groq API error: 401 (model openai/gpt-oss-120b) {"code":"invalid_api_key"} Groq API error: 401 (model openai/gpt-oss-20b) {"code":"invalid_api_key"} Every endpoint answered HTTP 200 anyway: /api/demo/chat {"success":true,"data":{"response":""}} /api/quick-chat {"success":true,"data":{"response":"I'm having a moment..."}} /api/professional-chat same /api/health {"success":true,"data":{"status":"healthy"}} Green everywhere, product dead. demo/chat was the worst of the three: its catch returns `content: context`, and a bot demo has no context, so a failed generation became a successful empty string. - demo/chat throws LLMUnavailableError when there is nothing to fall back on; the knowledge-base demo still serves its retrieved passage - quick-chat and professional-chat return 503 LLM_UNAVAILABLE instead of apologising with 200 - lib/llm-health.ts records what generations actually did; /api/health reports it and returns 503 once the chain is consistently failing, so "healthy" stops meaning "the database answered" tests/__tests__/api/professional-chat.test.ts asserted the bug -- 200, the word "moment", provider 'fallback'. It now asserts 503 LLM_UNAVAILABLE. A test that pins broken behaviour is worse than no test. Never-twice: tests/__tests__/lib/llm-health.test.ts covers the tracker's state machine and asserts at source level that no chat route answers an LLM failure with success, and that health consults the LLM chain. This makes the outage visible. It does NOT fix it -- that needs a valid GROQ_API_KEY (or OPENROUTER_API_KEY) in /opt/botsmann/shared/.env on the box. Until then these endpoints will correctly return 503. verify: format, lint, typecheck, 268 tests, build -- all green. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HVwg8DKHQktxJuHeLM3xpG --- app/api/demo/chat/route.ts | 33 +++++++- app/api/health/route.ts | 29 ++++++- app/api/professional-chat/route.ts | 13 ++- app/api/quick-chat/route.ts | 14 ++-- lib/api/index.ts | 1 + lib/api/responses.ts | 15 ++++ lib/llm-health.ts | 71 ++++++++++++++++ tests/__tests__/api/professional-chat.test.ts | 11 ++- tests/__tests__/lib/llm-health.test.ts | 84 +++++++++++++++++++ 9 files changed, 249 insertions(+), 22 deletions(-) create mode 100644 lib/llm-health.ts create mode 100644 tests/__tests__/lib/llm-health.test.ts diff --git a/app/api/demo/chat/route.ts b/app/api/demo/chat/route.ts index b9dc02c4..db06f42b 100644 --- a/app/api/demo/chat/route.ts +++ b/app/api/demo/chat/route.ts @@ -1,8 +1,15 @@ import { type NextRequest, NextResponse } from 'next/server'; import { z } from 'zod'; -import { jsonError, jsonValidationError, formatZodErrors, HTTP_STATUS } from '@/lib/api'; +import { + jsonError, + jsonValidationError, + jsonLLMUnavailable, + formatZodErrors, + HTTP_STATUS, +} from '@/lib/api'; import { generateWithBestProvider, type ModelProvider } from '@/lib/llm-client'; import { logger } from '@/lib/logger'; +import { recordLLMSuccess, recordLLMFailure } from '@/lib/llm-health'; import { enforceRateLimit } from '@/lib/rate-limit'; import { sanitizeSystemPrompt, @@ -15,6 +22,14 @@ import { // Types // ============================================================================ +/** Raised when no provider could produce an answer and there is no fallback. */ +class LLMUnavailableError extends Error { + constructor(message: string, options?: { cause?: unknown }) { + super(message, options); + this.name = 'LLMUnavailableError'; + } +} + interface KnowledgeChunk { id: string; topic: string; @@ -104,6 +119,7 @@ async function generateResponse( { role: 'user', content: userContent }, ]); + recordLLMSuccess(); return { content: result.content, provider: result.provider, @@ -111,7 +127,16 @@ async function generateResponse( }; } catch (error) { logger.error('LLM generation failed:', error); - // Fallback to context-only response + recordLLMFailure(error); + + // The knowledge-base demo can still serve the retrieved passage. A + // bot-specific demo has no context to fall back on, so returning it would + // mean answering with an empty string -- which is what made a total LLM + // outage look like a successful request. + if (!context) { + throw new LLMUnavailableError('No LLM provider could answer', { cause: error }); + } + return { content: context, provider: 'ollama', // placeholder @@ -490,6 +515,10 @@ export async function POST(request: NextRequest) { if (error instanceof z.ZodError) { return jsonValidationError('Validation failed', formatZodErrors(error)); } + if (error instanceof LLMUnavailableError) { + logger.error('Chat API: no LLM provider available', error); + return jsonLLMUnavailable(); + } logger.error('Chat API error:', error); return jsonError('Internal server error', 'INTERNAL_ERROR', HTTP_STATUS.INTERNAL_ERROR); } diff --git a/app/api/health/route.ts b/app/api/health/route.ts index 254c0d4c..d394126d 100644 --- a/app/api/health/route.ts +++ b/app/api/health/route.ts @@ -1,8 +1,22 @@ import { supabase, isSupabaseConfigured } from '@/lib/supabase'; import { jsonSuccess, jsonServiceUnavailable } from '@/lib/api'; import { logger } from '@/lib/logger'; +import { getLLMHealth } from '@/lib/llm-health'; +/** + * Health check. + * + * Reports the database AND the LLM chain. It used to report only the database, + * so on 2026-08-28 it answered "healthy" while every AI feature on the site was + * failing on an invalid Groq key -- the product's core capability was dead and + * nothing that watches this endpoint could tell. + * + * LLM state comes from what the chat routes actually observed, so it costs + * nothing here and reflects real traffic rather than a synthetic probe. + */ export async function GET() { + const llm = getLLMHealth(); + try { if (!isSupabaseConfigured()) { return jsonServiceUnavailable('Database not configured'); @@ -14,7 +28,20 @@ export async function GET() { throw error; } - return jsonSuccess({ status: 'healthy', database: 'connected' }, { cache: 'PUBLIC_SHORT' }); + // The database being up is not the same as the product working. + if (llm.status === 'down') { + logger.error('Health check: LLM chain is down', { lastError: llm.lastError }); + return jsonServiceUnavailable('AI provider unavailable'); + } + + return jsonSuccess( + { + status: llm.status === 'degraded' ? 'degraded' : 'healthy', + database: 'connected', + llm, + }, + { cache: 'PUBLIC_SHORT' }, + ); } catch (error) { logger.error('Health check failed:', error); return jsonServiceUnavailable('Database connection failed'); diff --git a/app/api/professional-chat/route.ts b/app/api/professional-chat/route.ts index fdd91fc7..441627e9 100644 --- a/app/api/professional-chat/route.ts +++ b/app/api/professional-chat/route.ts @@ -11,9 +11,10 @@ import { type NextRequest } from 'next/server'; import { generateEmbedding } from '@/lib/embeddings'; import { generateLLMResponse } from '@/lib/llm-client'; import { logger } from '@/lib/logger'; +import { recordLLMSuccess, recordLLMFailure } from '@/lib/llm-health'; import { getServiceClient } from '@/lib/supabase'; import { verifyUser } from '@/lib/api-utils'; -import { jsonSuccess, jsonError, HTTP_STATUS } from '@/lib/api'; +import { jsonSuccess, jsonError, HTTP_STATUS, jsonLLMUnavailable } from '@/lib/api'; import { enforceRateLimit } from '@/lib/rate-limit'; import { PROFESSIONAL_DOCUMENT_ACCESS, type DocumentCategory } from '@/types/document'; import { @@ -216,6 +217,8 @@ export async function POST(request: NextRequest) { ).catch((err) => logger.error('[Professional Chat API] Context extraction error:', err)); } + recordLLMSuccess(); + return jsonSuccess({ response: llmResponse.content, provider: llmResponse.provider, @@ -225,12 +228,8 @@ export async function POST(request: NextRequest) { }); } catch (llmError) { logger.error('[Professional Chat API] LLM error:', llmError); - - return jsonSuccess({ - response: "I'm having a moment... could you try again?", - provider: 'fallback', - model: 'none', - }); + recordLLMFailure(llmError); + return jsonLLMUnavailable(); } } catch (error) { logger.error('[Professional Chat API] Unhandled error:', error); diff --git a/app/api/quick-chat/route.ts b/app/api/quick-chat/route.ts index 041d9bbf..dba1e3fb 100644 --- a/app/api/quick-chat/route.ts +++ b/app/api/quick-chat/route.ts @@ -10,7 +10,8 @@ import { type NextRequest } from 'next/server'; import { generateLLMResponse } from '@/lib/llm-client'; import { logger } from '@/lib/logger'; -import { jsonSuccess, jsonError, HTTP_STATUS } from '@/lib/api'; +import { recordLLMSuccess, recordLLMFailure } from '@/lib/llm-health'; +import { jsonSuccess, jsonError, HTTP_STATUS, jsonLLMUnavailable } from '@/lib/api'; import { enforceRateLimit } from '@/lib/rate-limit'; import { sanitizeSystemPrompt, @@ -91,6 +92,8 @@ export async function POST(request: NextRequest) { logger.log(`[Quick Chat API] LLM response in ${Date.now() - llmStartTime} ms`); logger.log(`[Quick Chat API] Total time: ${Date.now() - startTime} ms`); + recordLLMSuccess(); + return jsonSuccess({ response: llmResponse.content, provider: llmResponse.provider, @@ -98,13 +101,8 @@ export async function POST(request: NextRequest) { }); } catch (llmError) { logger.error('[Quick Chat API] LLM error:', llmError); - - // Return a friendly fallback response - return jsonSuccess({ - response: "I'm having a moment... could you try again?", - provider: 'fallback', - model: 'none', - }); + recordLLMFailure(llmError); + return jsonLLMUnavailable(); } } catch (error) { logger.error('[Quick Chat API] Unhandled error:', error); diff --git a/lib/api/index.ts b/lib/api/index.ts index 13bdd258..551ba598 100644 --- a/lib/api/index.ts +++ b/lib/api/index.ts @@ -15,6 +15,7 @@ export { jsonUnauthorized, jsonNotFound, jsonServiceUnavailable, + jsonLLMUnavailable, // Validation helpers validateBody, hasValidationError, diff --git a/lib/api/responses.ts b/lib/api/responses.ts index 26f1e9a1..7ab0a431 100644 --- a/lib/api/responses.ts +++ b/lib/api/responses.ts @@ -97,6 +97,7 @@ export type ErrorCode = | 'RATE_LIMIT' | 'DATABASE_ERROR' | 'SERVICE_UNAVAILABLE' + | 'LLM_UNAVAILABLE' | 'INTERNAL_ERROR'; /** @@ -233,6 +234,20 @@ export function jsonServiceUnavailable( return jsonError(message, 'SERVICE_UNAVAILABLE', HTTP_STATUS.SERVICE_UNAVAILABLE); } +/** + * No LLM provider could answer. + * + * This is a 503, not a 200 with an apology in the message body. A chat route + * that answers "I'm having a moment... could you try again?" with HTTP 200 + * looks healthy to every uptime check while the product's core feature is + * dead -- which is exactly how a total outage went unnoticed. + */ +export function jsonLLMUnavailable( + message = 'The AI service is temporarily unavailable. Please try again shortly.', +): NextResponse { + return jsonError(message, 'LLM_UNAVAILABLE', HTTP_STATUS.SERVICE_UNAVAILABLE); +} + /** * Convert ZodError to ValidationError array */ diff --git a/lib/llm-health.ts b/lib/llm-health.ts new file mode 100644 index 00000000..eed3678e --- /dev/null +++ b/lib/llm-health.ts @@ -0,0 +1,71 @@ +/** + * Observed health of the LLM provider chain. + * + * On 2026-08-28 every AI feature on the site was dead -- the Groq key was + * returning 401 and no other provider was configured -- and nothing noticed. + * The chat routes caught the error and answered HTTP 200 with + * "I'm having a moment... could you try again?", and /api/health reported + * "healthy" because it only ever checked the database. A total outage of the + * product's core feature was invisible to every automated check. + * + * So the routes now record what actually happened, and health reports it. + * This is deliberately in-process: botsmann runs as a single systemd service + * on one box, so module state is shared by every request. If it is ever + * scaled horizontally this becomes per-instance and wants a shared store. + */ + +/** Consecutive failures before we call the chain down rather than flaky. */ +const DOWN_AFTER_CONSECUTIVE_FAILURES = 3; + +export type LLMHealthStatus = 'ok' | 'degraded' | 'down' | 'unknown'; + +export interface LLMHealth { + status: LLMHealthStatus; + consecutiveFailures: number; + lastError: string | null; + lastSuccessAt: string | null; + lastFailureAt: string | null; +} + +let consecutiveFailures = 0; +let lastError: string | null = null; +let lastSuccessAt: number | null = null; +let lastFailureAt: number | null = null; + +/** Call after a generation that produced usable content. */ +export function recordLLMSuccess(): void { + consecutiveFailures = 0; + lastError = null; + lastSuccessAt = Date.now(); +} + +/** Call when generation threw, or returned nothing usable. */ +export function recordLLMFailure(error: unknown): void { + consecutiveFailures += 1; + lastFailureAt = Date.now(); + lastError = error instanceof Error ? error.message : String(error ?? 'unknown error'); +} + +export function getLLMHealth(): LLMHealth { + let status: LLMHealthStatus; + if (consecutiveFailures >= DOWN_AFTER_CONSECUTIVE_FAILURES) status = 'down'; + else if (consecutiveFailures > 0) status = 'degraded'; + else if (lastSuccessAt !== null) status = 'ok'; + else status = 'unknown'; + + return { + status, + consecutiveFailures, + lastError, + lastSuccessAt: lastSuccessAt ? new Date(lastSuccessAt).toISOString() : null, + lastFailureAt: lastFailureAt ? new Date(lastFailureAt).toISOString() : null, + }; +} + +/** Test seam. */ +export function resetLLMHealth(): void { + consecutiveFailures = 0; + lastError = null; + lastSuccessAt = null; + lastFailureAt = null; +} diff --git a/tests/__tests__/api/professional-chat.test.ts b/tests/__tests__/api/professional-chat.test.ts index d8d9a047..b6041148 100644 --- a/tests/__tests__/api/professional-chat.test.ts +++ b/tests/__tests__/api/professional-chat.test.ts @@ -130,7 +130,10 @@ describe('POST /api/professional-chat', () => { expect(data.code).toBe('RATE_LIMIT'); }); - it('handles LLM errors gracefully', async () => { + // This used to assert 200 + "I'm having a moment...", which is how a total + // LLM outage stayed invisible: the endpoint looked healthy to every check + // while answering nobody. A failure is a failure. + it('reports an LLM failure as 503, not a cheerful 200', async () => { mockGenerateLLM.mockRejectedValue(new Error('LLM unavailable')); const req = makeRequest({ @@ -142,9 +145,9 @@ describe('POST /api/professional-chat', () => { const res = await POST(req); const data = await res.json(); - expect(res.status).toBe(200); - expect(data.data.response).toContain('moment'); - expect(data.data.provider).toBe('fallback'); + expect(res.status).toBe(503); + expect(data.success).toBe(false); + expect(data.code).toBe('LLM_UNAVAILABLE'); }); it('includes conversation history in LLM call', async () => { diff --git a/tests/__tests__/lib/llm-health.test.ts b/tests/__tests__/lib/llm-health.test.ts new file mode 100644 index 00000000..ad22eab1 --- /dev/null +++ b/tests/__tests__/lib/llm-health.test.ts @@ -0,0 +1,84 @@ +/** + * Guard: an LLM outage must not look like success. + * + * On 2026-08-28 the Groq key was returning 401, no other provider was + * configured, and every AI endpoint answered HTTP 200: + * + * /api/demo/chat -> {"success":true,"data":{"response":""}} + * /api/quick-chat -> {"success":true,"data":{"response":"I'm having a moment..."}} + * /api/professional-chat -> same + * /api/health -> {"success":true,"data":{"status":"healthy"}} + * + * Every automated check was green while the product's core feature was dead. + * These tests cover the two halves of that: the tracker's state machine, and + * the source-level promise that no chat route answers a failure with success. + */ +import { readFileSync } from 'fs'; +import { join } from 'path'; +import { recordLLMSuccess, recordLLMFailure, getLLMHealth, resetLLMHealth } from '@/lib/llm-health'; + +describe('llm health tracker', () => { + beforeEach(() => resetLLMHealth()); + + it('starts unknown, before anything has been observed', () => { + expect(getLLMHealth().status).toBe('unknown'); + }); + + it('is ok after a success', () => { + recordLLMSuccess(); + expect(getLLMHealth().status).toBe('ok'); + }); + + it('is degraded on the first failures, not down', () => { + recordLLMFailure(new Error('401 Invalid API Key')); + expect(getLLMHealth().status).toBe('degraded'); + recordLLMFailure(new Error('401 Invalid API Key')); + expect(getLLMHealth().status).toBe('degraded'); + }); + + it('is down once failures are consistent', () => { + for (let i = 0; i < 3; i += 1) recordLLMFailure(new Error('401 Invalid API Key')); + const health = getLLMHealth(); + expect(health.status).toBe('down'); + expect(health.consecutiveFailures).toBe(3); + expect(health.lastError).toContain('401'); + }); + + it('recovers to ok on the next success', () => { + for (let i = 0; i < 5; i += 1) recordLLMFailure(new Error('boom')); + expect(getLLMHealth().status).toBe('down'); + recordLLMSuccess(); + const health = getLLMHealth(); + expect(health.status).toBe('ok'); + expect(health.consecutiveFailures).toBe(0); + expect(health.lastError).toBeNull(); + }); +}); + +describe('chat routes do not dress a failure as success', () => { + const ROUTES = [ + 'app/api/demo/chat/route.ts', + 'app/api/quick-chat/route.ts', + 'app/api/professional-chat/route.ts', + ]; + + it.each(ROUTES)('%s reports LLM failure with a real status', (route) => { + const source = readFileSync(join(process.cwd(), route), 'utf-8'); + + // it must have a way to say "unavailable"... + expect(source).toMatch(/jsonLLMUnavailable|LLMUnavailableError/); + // ...and it must record the outcome so /api/health can see it + expect(source).toContain('recordLLMFailure'); + }); + + it.each(ROUTES)('%s no longer apologises with HTTP 200', (route) => { + const source = readFileSync(join(process.cwd(), route), 'utf-8'); + expect(source).not.toContain("I'm having a moment"); + }); + + it('health reports the LLM chain, not just the database', () => { + const source = readFileSync(join(process.cwd(), 'app/api/health/route.ts'), 'utf-8'); + expect(source).toContain('getLLMHealth'); + expect(source).toMatch(/llm/); + }); +});