diff --git a/apps/server/src/core/ai-chat/ai-chat.service.ts b/apps/server/src/core/ai-chat/ai-chat.service.ts index 91b8ebd4..8230dc6d 100644 --- a/apps/server/src/core/ai-chat/ai-chat.service.ts +++ b/apps/server/src/core/ai-chat/ai-chat.service.ts @@ -10,6 +10,7 @@ import { } from 'ai'; import { AiService } from '../../integrations/ai/ai.service'; import { AiSettingsService } from '../../integrations/ai/ai-settings.service'; +import { describeProviderError } from '../../integrations/ai/ai-error.util'; import { AiChatRepo } from '@docmost/db/repos/ai-chat/ai-chat.repo'; import { AiChatMessageRepo } from '@docmost/db/repos/ai-chat/ai-chat-message.repo'; import { User, Workspace, AiChatMessage } from '@docmost/db/types/entity.types'; @@ -271,15 +272,10 @@ export class AiChatService { onError: async ({ error }) => { // NestJS Logger.error(message, stack?, context?): pass the real message // (with statusCode when present) + the stack string, not the Error - // object, so the actual provider cause is clearly logged. - const e = error as { - statusCode?: number; - message?: string; - stack?: string; - }; - const errorText = e?.statusCode - ? `${e.statusCode}: ${e.message ?? String(error)}` - : (e?.message ?? String(error)); + // object, so the actual provider cause is clearly logged. Reuse the + // shared formatter so provider error formatting stays unified. + const e = error as { stack?: string }; + const errorText = describeProviderError(error, String(error)); this.logger.error(`AI chat stream error: ${errorText}`, e?.stack); // Persist whatever text we have (likely empty) so the turn is recorded, // and record the error text in metadata so it is visible in history. @@ -340,10 +336,9 @@ export class AiChatService { result.pipeUIMessageStreamToResponse(res.raw, { headers: { 'X-Accel-Buffering': 'no' }, onError: (error: unknown) => { - const e = error as { statusCode?: number; message?: string }; - return e?.statusCode - ? `${e.statusCode}: ${e.message}` - : (e?.message ?? 'AI stream error'); + // Reuse the shared formatter so provider error formatting stays + // unified between the log line and the streamed error message. + return describeProviderError(error, 'AI stream error'); }, }); @@ -538,8 +533,8 @@ function compactValue(value: unknown, depth: number): unknown { * recovers the name. Falls back to a single `text` part built from * `fallbackText` when the steps carry no text. */ -// Exported only so the unit tests can import this pure helper; exporting it -// does not change runtime behavior. +// Exported only so the unit tests can import these pure helpers; exporting +// them does not change runtime behavior. export function assistantParts( steps: ReadonlyArray | undefined, fallbackText: string, diff --git a/apps/server/src/integrations/ai/ai-error.util.spec.ts b/apps/server/src/integrations/ai/ai-error.util.spec.ts new file mode 100644 index 00000000..c9b7fb3e --- /dev/null +++ b/apps/server/src/integrations/ai/ai-error.util.spec.ts @@ -0,0 +1,61 @@ +import { describeProviderError } from './ai-error.util'; + +/** + * Unit tests for describeProviderError: the shared formatter used both for the + * server log line and for the error text streamed back to the client. This + * pins the behaviour, including the one behaviour change introduced when the + * two inline formatters were unified: a truncated, single-line snippet of the + * provider `responseBody`/`text` is appended (so a misconfigured endpoint's + * HTML error page is diagnosable). The util guarantees the API key is never in + * the response body, so this is safe to surface. + */ +describe('describeProviderError', () => { + it('uses the fallback for a null/empty/undefined error', () => { + expect(describeProviderError(null, 'AI stream error')).toBe( + 'AI stream error', + ); + expect(describeProviderError('', 'AI stream error')).toBe('AI stream error'); + expect(describeProviderError(undefined)).toBe('Unknown error'); + }); + + it('returns a non-empty plain string error as-is', () => { + expect(describeProviderError('boom')).toBe('boom'); + }); + + it('formats statusCode + message', () => { + expect( + describeProviderError({ statusCode: 401, message: 'Unauthorized' }), + ).toBe('401: Unauthorized'); + }); + + it('falls back to message when there is no statusCode', () => { + expect(describeProviderError({ message: 'nope' })).toBe('nope'); + }); + + it('appends a whitespace-collapsed response body snippet', () => { + const out = describeProviderError({ + statusCode: 502, + message: 'Bad Gateway', + responseBody: '\n upstream error\n', + }); + expect(out.startsWith('502: Bad Gateway | response body: ')).toBe(true); + // Newlines and runs of spaces are collapsed to single spaces. + expect(out).toContain(' upstream error '); + }); + + it('reads `text` when responseBody is absent', () => { + expect(describeProviderError({ message: 'e', text: 'body-text' })).toBe( + 'e | response body: body-text', + ); + }); + + it('truncates a long body to 300 chars + ellipsis', () => { + const out = describeProviderError({ + message: 'e', + responseBody: 'x'.repeat(500), + }); + expect(out).toContain('…'); + // 'e | response body: ' + 300 chars + '…' + expect(out.length).toBeLessThan('e | response body: '.length + 305); + }); +}); diff --git a/apps/server/src/integrations/ai/ai-error.util.ts b/apps/server/src/integrations/ai/ai-error.util.ts index 68fa328b..0a0f949b 100644 --- a/apps/server/src/integrations/ai/ai-error.util.ts +++ b/apps/server/src/integrations/ai/ai-error.util.ts @@ -9,10 +9,16 @@ * * 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. + * + * `fallback` is used when the error carries no usable message (e.g. a bare + * object); defaults to 'Unknown error'. */ -export function describeProviderError(err: unknown): string { +export function describeProviderError( + err: unknown, + fallback = 'Unknown error', +): string { if (typeof err !== 'object' || err === null) { - return typeof err === 'string' ? err : 'Unknown error'; + return typeof err === 'string' && err ? err : fallback; } const e = err as { statusCode?: number; @@ -23,7 +29,7 @@ export function describeProviderError(err: unknown): string { const base = typeof e.statusCode === 'number' ? `${e.statusCode}: ${e.message ?? ''}`.trim() - : (e.message ?? 'Unknown error'); + : (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.