From 7171dfbdf08b9575f60d1d7a2b93a614668e3c51 Mon Sep 17 00:00:00 2001 From: claude_code Date: Sun, 21 Jun 2026 19:55:45 +0300 Subject: [PATCH] fix(ai): classify AI provider error status in logs and UI Provider auth failures were logged with the provider's opaque message only (e.g. OpenRouter returns "401: User not found." for a bad/missing API key), which reads like a missing wiki user rather than a credentials problem. describeProviderError now prepends a clear, human-readable English label for a small set of well-known HTTP statuses while keeping the original detail (status + provider message + truncated response-body snippet): - 401/403 -> authentication failed (invalid or missing API key) - 402 -> insufficient credits or quota - 429 -> rate limit exceeded Other statuses and status-less errors are formatted exactly as before. The label is a static string and never contains the API key. Benefits every caller (embedding processor, indexer, AI "Test endpoint" UI) at once. Tests: switch the plain status+message case to a non-classified status (500); add 401/403/402/429 cases; keep 502/503 as regression guards for the unchanged path. --- .../src/integrations/ai/ai-error.util.spec.ts | 54 +++++++++++++++++-- .../src/integrations/ai/ai-error.util.ts | 30 ++++++++++- 2 files changed, 79 insertions(+), 5 deletions(-) diff --git a/apps/server/src/integrations/ai/ai-error.util.spec.ts b/apps/server/src/integrations/ai/ai-error.util.spec.ts index 414701d4..5828e6bd 100644 --- a/apps/server/src/integrations/ai/ai-error.util.spec.ts +++ b/apps/server/src/integrations/ai/ai-error.util.spec.ts @@ -22,10 +22,58 @@ describe('describeProviderError', () => { expect(describeProviderError('boom')).toBe('boom'); }); - it('formats statusCode + message', () => { + it('formats statusCode + message (non-classified status)', () => { + // 500 is not in the well-known status map, so no label is prepended and the + // plain ": " path is exercised. expect( - describeProviderError({ statusCode: 401, message: 'Unauthorized' }), - ).toBe('401: Unauthorized'); + describeProviderError({ statusCode: 500, message: 'Server error' }), + ).toBe('500: Server error'); + }); + + it('prepends an auth label for 401 (the real cause behind "User not found.")', () => { + const out = describeProviderError({ + statusCode: 401, + message: 'User not found.', + }); + expect(out).toBe( + 'AI provider authentication failed (invalid or missing API key) — 401: User not found.', + ); + // The provider status is still present after the label. + expect(out).toContain('401:'); + // With a response body, the snippet is appended AFTER the label/detail. + const withBody = describeProviderError({ + statusCode: 401, + message: 'User not found.', + responseBody: '{"error":{"message":"User not found.","code":401}}', + }); + expect( + withBody.startsWith( + 'AI provider authentication failed (invalid or missing API key) — 401: User not found. | response body: ', + ), + ).toBe(true); + expect(withBody).toContain('| response body:'); + }); + + it('prepends the same auth label for 403', () => { + expect( + describeProviderError({ statusCode: 403, message: 'Forbidden' }), + ).toBe( + 'AI provider authentication failed (invalid or missing API key) — 403: Forbidden', + ); + }); + + it('prepends a billing label for 402', () => { + expect( + describeProviderError({ statusCode: 402, message: 'Payment Required' }), + ).toBe( + 'AI provider rejected the request: insufficient credits or quota — 402: Payment Required', + ); + }); + + it('prepends a rate-limit label for 429', () => { + expect( + describeProviderError({ statusCode: 429, message: 'Too Many Requests' }), + ).toBe('AI provider rate limit exceeded — 429: Too Many Requests'); }); it('falls back to message when there is no statusCode', () => { diff --git a/apps/server/src/integrations/ai/ai-error.util.ts b/apps/server/src/integrations/ai/ai-error.util.ts index 0a0f949b..2d24ab47 100644 --- a/apps/server/src/integrations/ai/ai-error.util.ts +++ b/apps/server/src/integrations/ai/ai-error.util.ts @@ -10,6 +10,12 @@ * None of these fields contain the API key (it is sent as an Authorization * header and never echoed in the response body), so this is safe to log/return. * + * A small set of well-known HTTP statuses (auth / billing / rate limit) are + * classified and a clear, human-readable English label is prepended, so the + * log/UI states the real cause instead of only the provider's opaque message + * (e.g. a 401 "User not found." is really a bad/missing API key). The label is + * a static string and never contains the API key. + * * `fallback` is used when the error carries no usable message (e.g. a bare * object); defaults to 'Unknown error'. */ @@ -31,9 +37,29 @@ export function describeProviderError( ? `${e.statusCode}: ${e.message ?? ''}`.trim() : (e.message ?? fallback); const body = (e.responseBody ?? e.text ?? '').trim(); - if (!body) return base; // Collapse whitespace so a multi-line HTML body stays on one log line. const oneLine = body.replace(/\s+/g, ' '); const snippet = oneLine.length > 300 ? `${oneLine.slice(0, 300)}…` : oneLine; - return `${base} | response body: ${snippet}`; + const detail = body ? `${base} | response body: ${snippet}` : base; + // Classify well-known HTTP statuses so the log/UI states the real problem + // (auth / billing / rate limit) instead of only the provider's opaque message. + const label = classifyStatus(e.statusCode); + return label ? `${label} — ${detail}` : detail; +} + +// Map a small set of well-known provider HTTP statuses to a clear, +// human-readable cause. Returns null for anything else so the existing +// ": | response body: …" output is preserved unchanged. +function classifyStatus(statusCode?: number): string | null { + switch (statusCode) { + case 401: + case 403: + return 'AI provider authentication failed (invalid or missing API key)'; + case 402: + return 'AI provider rejected the request: insufficient credits or quota'; + case 429: + return 'AI provider rate limit exceeded'; + default: + return null; + } }