refactor(ai-chat)!: unify provider error formatting via describeProviderError

Behaviour change (split out of the test commit per review, and now covered).

Both the stream onError log line and the error text streamed to the client were
formatted by separate inline blocks that only emitted "<status>: <message>".
Route both through the shared describeProviderError() so formatting stays in one
place.

BEHAVIOUR CHANGE: describeProviderError additionally appends a single-line,
300-char-truncated snippet of the provider responseBody/text. So the log line
AND the user-facing stream error now include that snippet (e.g. the HTML error
page from a misconfigured endpoint), which previously neither did. This is
intentional — it makes a misconfigured external endpoint diagnosable — and is
safe: the API key travels in the Authorization header and is never echoed in
the response body (see the util's docstring). A `fallback` param is added so
each call site keeps its own default ('AI stream error' for the stream).

Adds ai-error.util.spec.ts covering the formatter, including the appended /
truncated body snippet, so this behaviour is no longer untested.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
claude code agent 227
2026-06-20 17:59:55 +03:00
committed by vvzvlad
parent 1e650262a4
commit cedea4072b
3 changed files with 80 additions and 18 deletions

View File

@@ -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<StepLike> | undefined,
fallbackText: string,

View File

@@ -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: '<html>\n <body>upstream error</body>\n</html>',
});
expect(out.startsWith('502: Bad Gateway | response body: ')).toBe(true);
// Newlines and runs of spaces are collapsed to single spaces.
expect(out).toContain('<html> <body>upstream error</body> </html>');
});
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);
});
});

View File

@@ -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.