From aadcbc67cfcfba41fc14ed5e33979a4c6c93444d Mon Sep 17 00:00:00 2001 From: SuYao Date: Tue, 27 May 2025 21:45:04 +0800 Subject: [PATCH] fix: Optimize error message formatting (#5988) * fix: Optimize error message formatting * fix: improve error unit test * refactor: simplify error handling in ErrorBlock component - Replaced custom StyledAlert with a more streamlined Alert component for error messages. - Reduced complexity by removing unnecessary JSX wrappers and improving readability. - Adjusted styling for the Alert component to maintain visual consistency. * fix: update error handling in ErrorBlock component - Removed unnecessary message prop from Alert component to simplify error display. - Maintained existing error handling logic while improving code clarity. --- .../providers/AiProvider/OpenAIProvider.ts | 39 ++++++++++++------- .../src/services/StreamProcessingService.ts | 3 ++ src/renderer/src/store/thunk/messageThunk.ts | 4 +- .../src/utils/__tests__/error.test.ts | 30 ++++++++------ src/renderer/src/utils/error.ts | 9 ++++- 5 files changed, 55 insertions(+), 30 deletions(-) diff --git a/src/renderer/src/providers/AiProvider/OpenAIProvider.ts b/src/renderer/src/providers/AiProvider/OpenAIProvider.ts index 31e8082f8c..c9d2383c8f 100644 --- a/src/renderer/src/providers/AiProvider/OpenAIProvider.ts +++ b/src/renderer/src/providers/AiProvider/OpenAIProvider.ts @@ -84,6 +84,7 @@ export type OpenAIStreamChunk = | { type: 'reasoning' | 'text-delta'; textDelta: string } | { type: 'tool-calls'; delta: any } | { type: 'finish'; finishReason: any; usage: any; delta: any; chunk: any } + | { type: 'unknown'; chunk: any } export default class OpenAIProvider extends BaseOpenAIProvider { constructor(provider: Provider) { @@ -631,21 +632,25 @@ export default class OpenAIProvider extends BaseOpenAIProvider { break } - const delta = chunk.choices[0]?.delta - if (delta?.reasoning_content || delta?.reasoning) { - yield { type: 'reasoning', textDelta: delta.reasoning_content || delta.reasoning } - } - if (delta?.content) { - yield { type: 'text-delta', textDelta: delta.content } - } - if (delta?.tool_calls) { - yield { type: 'tool-calls', delta: delta } - } + if (chunk.choices && chunk.choices.length > 0) { + const delta = chunk.choices[0]?.delta + if (delta?.reasoning_content || delta?.reasoning) { + yield { type: 'reasoning', textDelta: delta.reasoning_content || delta.reasoning } + } + if (delta?.content) { + yield { type: 'text-delta', textDelta: delta.content } + } + if (delta?.tool_calls) { + yield { type: 'tool-calls', delta: delta } + } - const finishReason = chunk.choices[0]?.finish_reason - if (!isEmpty(finishReason)) { - yield { type: 'finish', finishReason, usage: chunk.usage, delta, chunk } - break + const finishReason = chunk?.choices[0]?.finish_reason + if (!isEmpty(finishReason)) { + yield { type: 'finish', finishReason, usage: chunk.usage, delta, chunk } + break + } + } else { + yield { type: 'unknown', chunk } } } } @@ -832,6 +837,12 @@ export default class OpenAIProvider extends BaseOpenAIProvider { } break } + case 'unknown': { + onChunk({ + type: ChunkType.ERROR, + error: chunk.chunk + }) + } } } diff --git a/src/renderer/src/services/StreamProcessingService.ts b/src/renderer/src/services/StreamProcessingService.ts index 1d1682e7b1..0209ad1194 100644 --- a/src/renderer/src/services/StreamProcessingService.ts +++ b/src/renderer/src/services/StreamProcessingService.ts @@ -88,6 +88,9 @@ export function createStreamProcessor(callbacks: StreamProcessorCallbacks = {}) if (data.type === ChunkType.IMAGE_COMPLETE && callbacks.onImageGenerated) { callbacks.onImageGenerated(data.image) } + if (data.type === ChunkType.ERROR && callbacks.onError) { + callbacks.onError(data.error) + } // Note: Usage and Metrics are usually handled at the end or accumulated differently, // so direct callbacks might not be the best fit here. They are often part of the final message state. } catch (error) { diff --git a/src/renderer/src/store/thunk/messageThunk.ts b/src/renderer/src/store/thunk/messageThunk.ts index 2c5f872fb1..4ad208c9e1 100644 --- a/src/renderer/src/store/thunk/messageThunk.ts +++ b/src/renderer/src/store/thunk/messageThunk.ts @@ -20,7 +20,7 @@ import type { import { AssistantMessageStatus, MessageBlockStatus, MessageBlockType } from '@renderer/types/newMessage' import { Response } from '@renderer/types/newMessage' import { uuid } from '@renderer/utils' -import { isAbortError } from '@renderer/utils/error' +import { formatErrorMessage, isAbortError } from '@renderer/utils/error' import { extractUrlsFromMarkdown } from '@renderer/utils/linkConverter' import { createAssistantMessage, @@ -587,7 +587,7 @@ const fetchAndProcessAssistantResponseImpl = async ( const serializableError = { name: error.name, - message: pauseErrorLanguagePlaceholder || error.message || 'Stream processing error', + message: pauseErrorLanguagePlaceholder || error.message || formatErrorMessage(error), originalMessage: error.message, stack: error.stack, status: error.status || error.code, diff --git a/src/renderer/src/utils/__tests__/error.test.ts b/src/renderer/src/utils/__tests__/error.test.ts index 078b6724c2..fa130d82bb 100644 --- a/src/renderer/src/utils/__tests__/error.test.ts +++ b/src/renderer/src/utils/__tests__/error.test.ts @@ -50,19 +50,21 @@ describe('error', () => { }) describe('formatErrorMessage', () => { - it('should format error as JSON string', () => { - console.error = vi.fn() // Mock console.error + it('should format error with indentation and header', () => { + console.error = vi.fn() const error = new Error('Test error') const result = formatErrorMessage(error) expect(console.error).toHaveBeenCalled() - expect(result).toContain('```json') - expect(result).toContain('"message": "Test error"') + expect(result).toContain('Error Details:') + expect(result).toContain(' {') + expect(result).toContain(' "message": "Test error"') + expect(result).toContain(' }') expect(result).not.toContain('"stack":') }) - it('should remove sensitive information', () => { + it('should remove sensitive information and format with proper indentation', () => { console.error = vi.fn() const error = { @@ -74,27 +76,30 @@ describe('error', () => { const result = formatErrorMessage(error) - expect(result).toContain('"message": "API error"') + expect(result).toContain('Error Details:') + expect(result).toContain(' {') + expect(result).toContain(' "message": "API error"') + expect(result).toContain(' }') expect(result).not.toContain('Authorization') expect(result).not.toContain('stack') expect(result).not.toContain('request_id') }) - it('should handle errors during formatting', () => { + it('should handle errors during formatting with simple error message', () => { console.error = vi.fn() const problematicError = { get message() { - throw new Error('Cannot access message') + throw new Error('Cannot access') } } const result = formatErrorMessage(problematicError) - expect(result).toContain('```') - expect(result).toContain('Unable') + expect(result).toContain('Error Details:') + expect(result).toContain('"message": ""') }) - it('should handle non-serializable errors', () => { + it('should handle non-serializable errors with simple error message', () => { console.error = vi.fn() const nonSerializableError = { @@ -114,7 +119,8 @@ describe('error', () => { } const result = formatErrorMessage(nonSerializableError) - expect(result).toBeTruthy() + expect(result).toContain('Error Details:') + expect(result).toContain('"toString": ""') }) }) diff --git a/src/renderer/src/utils/error.ts b/src/renderer/src/utils/error.ts index 0ee522f08b..4150b200f2 100644 --- a/src/renderer/src/utils/error.ts +++ b/src/renderer/src/utils/error.ts @@ -35,10 +35,15 @@ export function formatErrorMessage(error: any): string { delete detailedError?.headers delete detailedError?.stack delete detailedError?.request_id - return '```json\n' + JSON.stringify(detailedError, null, 2) + '\n```' + + const formattedJson = JSON.stringify(detailedError, null, 2) + .split('\n') + .map((line) => ` ${line}`) + .join('\n') + return `Error Details:\n${formattedJson}` } catch (e) { try { - return '```\n' + String(error) + '\n```' + return `Error: ${String(error)}` } catch { return 'Error: Unable to format error message' }