fix: use i18nKey for error display and add timeout enforcement by EurFelux · Pull Request #12164 · CherryHQ/cherry-studio

EurFelux · 2025-12-27T09:30:39Z

What this PR does

Before this PR:

Abort/pause-related errors overwrote message with raw i18n keys (e.g. 'pause_placeholder'), losing the original error info for debugging.
Timeout errors (DOMException with name === 'TimeoutError') were not distinguished from user-initiated aborts, causing timeouts to display as "Paused" in the UI.
No AbortSignal.timeout() enforcement existed in the modern AI SDK path (parameterBuilder.ts); messageThunk.ts passed timeout: 30000 in requestOptions but it was never used to create an abort signal — effectively dead code.
No localized timeout error message existed.
The timeout constant defaultTimeout used camelCase, inconsistent with other exported constants.
The streaming-only behavior of the modern AI SDK path was not documented.

After this PR:

Use i18nKey field on SerializedError (instead of overwriting message) for translated error display, preserving original error info for the detail modal.
Rename i18n key pause_placeholder → stream_paused and extract as a named constant (ERROR_I18N_KEY_STREAM_PAUSED).
Add isTimeoutError() utility to distinguish DOMException TimeoutError from user-initiated AbortError, checking both error and error.cause.
Update isAbortError() to explicitly exclude timeout errors.
Add AbortSignal.timeout(DEFAULT_TIMEOUT) (10 min) in parameterBuilder.ts, combined with the external signal via AbortSignal.any().
Remove the unused timeout: 30000 from messageThunk.ts, establishing parameterBuilder.ts as the single timeout control point.
Rename constant defaultTimeout → DEFAULT_TIMEOUT (SCREAMING_SNAKE_CASE) across all import sites.
Add localized request_timeout i18n key (ERROR_I18N_KEY_REQUEST_TIMEOUT) for all supported languages.
Add JSDoc to SerializedError documenting known dynamic properties (i18nKey, providerId).
Add documentation notes clarifying the streamText-only path in index_new.ts and ApiService.ts.

Fixes # (not applicable; relates to #12016)

Why we need it and why it was done in this way

This PR attempts to address #12016, but per the upstream limitation described in vercel/ai#9655, the root cause is not solvable purely from the client side. The changes here document the streaming-only behavior, improve error handling, and add proper timeout enforcement via AbortSignal.timeout().

The following tradeoffs were made:

Keep behavior changes minimal to avoid impacting existing request flows.
Use i18nKey field (already supported by ErrorBlock.tsx and ErrorHandlerMiddleware.ts) instead of overwriting message — this preserves original error info for the detail modal while enabling proper i18n display.
Establish parameterBuilder.ts as the single timeout control point rather than allowing callers to pass their own timeout values, since no caller currently provides a custom timeout.

The following alternatives were considered:

Switching to a non-streaming path (generateText), but that is not used in this runtime path and does not address the upstream limitation.
Overwriting message with i18n keys (original approach) — rejected because it loses debugging context and creates an implicit contract between producers and consumers.
Adding i18nKey as an explicit optional property on SerializedError — rejected due to TypeScript's index signature constraint (string | undefined is incompatible with [key: string]: Serializable); documented via JSDoc instead.

Links to places where the discussion took place: vercel/ai#9655

Breaking changes

None.

Special notes for your reviewer

This PR is a partial mitigation for [Bug]: Grok 4 (including 4.1) reasoning 系列在复杂推理问题下会报network errors ，已经排除了网络问题。 #12016; the underlying issue is upstream per the linked Vercel AI SDK issue.
The i18nKey pattern on SerializedError is already used by ErrorHandlerMiddleware.ts for auth/quota errors. This PR extends the same pattern to abort and timeout errors.
isTimeoutError() checks both error and error.cause for DOMException with name === 'TimeoutError' to handle cases where the AI SDK wraps the original timeout error.
The timeout: 30000 removed from messageThunk.ts was effectively dead code — parameterBuilder.ts previously only passed through requestOptions.signal but never used requestOptions.timeout for abort signal creation.

Checklist

PR: The PR description is expressive enough and will help future contributors
Code: Write code that humans can understand and Keep it simple
Refactor: You have left the code cleaner than you found it (Boy Scout Rule)
Upgrade: Impact of this change on upgrade flows was considered and addressed if required
Documentation: A user-guide update was considered and is present (link) or not required.

Release note

Improve streaming error handling: use i18nKey for localized error display while preserving original error info, add isTimeoutError utility with localized timeout messages, enforce timeout via AbortSignal.timeout() in the modern AI SDK path, and rename pause_placeholder to stream_paused.

EurFelux

Code Review Summary

Thanks for addressing the timeout and streaming-only documentation issues related to #12016. The intent is clear and the upstream limitation context is appreciated. However, I've identified several issues that should be addressed before merging.

🔴 Critical Issues

Abort error message change breaks i18n-based rendering — The baseCallbacks.ts change from 'pause_placeholder' to formatErrorMessageWithPrefix(...) breaks the downstream rendering in ErrorBlock.tsx (see inline comment).
Timeout signal triggers isAbortError → user sees "Paused" instead of timeout error — AbortSignal.timeout() produces a TimeoutError (DOMException with name === 'TimeoutError'), but when combined via AbortSignal.any(), the resulting abort reason propagates. The current isAbortError() checks for DOMException with name === 'AbortError' and messages like 'signal is aborted without reason'. A timeout from AbortSignal.timeout() may match some of these checks depending on the runtime, causing timeouts to be silently swallowed as "Paused" rather than surfaced as timeout errors.

🟡 Moderate Issues

_headers variable naming — Destructuring headers as _headers suggests it's unused, but it is used. This violates the naming convention where _ prefix means "intentionally unused".
Double timeout — Timeout is now applied in two places: parameterBuilder.ts creates AbortSignal.timeout(timeout) and combines it with the caller's signal, but the caller in messageThunk.ts also passes timeout: defaultTimeout. It's unclear which layer is responsible for timeout enforcement. This creates a risk of conflicting or redundant timeout behavior.
TODO/FIXME in production code — The comments // TODO: make it configurable and // FIXME: temporary solution. It may be too long. in messageThunk.ts should be tracked as issues rather than left as code comments, especially in a PR meant for merging.

🟢 Minor Issues

JSDoc comments are informational-only — The added JSDoc comments in index_new.ts and ApiService.ts document existing behavior accurately but don't change any logic. Consider whether inline comments (closer to the streamText call) would be more discoverable.
Type change from Record<string, string> to Record<string, string | undefined> — This headers type relaxation is fine but should be noted as it widens the accepted type at the interface level.

Summary

The core ideas (longer timeout, better documentation) are sound, but the abort error message change introduces a regression in how paused messages are rendered, and the timeout signal interaction with isAbortError needs careful testing. Recommend addressing the critical items before merging.

src/renderer/src/services/messageStreaming/callbacks/baseCallbacks.ts

src/renderer/src/aiCore/prepareParams/parameterBuilder.ts

src/renderer/src/store/thunk/messageThunk.ts

src/renderer/src/store/thunk/__tests__/streamCallback.integration.test.ts

Use formatErrorMessageWithPrefix to properly localize the pause placeholder error message with i18next

Use defaultTimeout from shared config instead of hardcoded values Combine multiple abort signals for better request handling

…essageWithPrefix

- Use dedicated `i18nKey` field on SerializedError instead of overwriting `message` with i18n keys, preserving original error info for debugging - Rename i18n key `pause_placeholder` → `stream_paused` and extract as constant `ERROR_I18N_KEY_STREAM_PAUSED` - Add `isTimeoutError()` to distinguish timeout from user-initiated abort - Fix `_headers` naming to `inputHeaders` in parameterBuilder - Update i18n resources and tests Co-Authored-By: Claude Opus 4.5 <[email protected]>

- Add `ERROR_I18N_KEY_REQUEST_TIMEOUT` constant and `request_timeout` i18n key with translations for all supported languages - Handle timeout errors in baseCallbacks and ActionUtils using the same i18nKey pattern as stream_paused - Add isTimeoutError mock to ActionUtils tests Co-Authored-By: Claude Opus 4.5 <[email protected]>

…nt timeout - Rename `defaultTimeout` → `DEFAULT_TIMEOUT` (SCREAMING_SNAKE_CASE) across all import sites for consistency with other exported constants. - Remove redundant `timeout: defaultTimeout` from messageThunk.ts since parameterBuilder.ts already falls back to DEFAULT_TIMEOUT internally. - Add comment in parameterBuilder.ts clarifying the timeout control point. Co-Authored-By: Claude Opus 4.5 <[email protected]>

EurFelux · 2026-02-03T14:07:58Z

Code Review: Error Handling Improvements

Overview

This PR improves error handling in the streaming AI SDK path by:

Using i18nKey field instead of overwriting message for localized error display
Adding isTimeoutError() utility to distinguish timeout errors from user aborts
Implementing proper timeout enforcement via AbortSignal.timeout() in parameterBuilder.ts
Renaming defaultTimeout to DEFAULT_TIMEOUT for naming consistency
Adding localized timeout error messages across all supported languages

Detailed Review

✅ Strengths

Proper error preservation: Using i18nKey instead of overwriting message preserves original error info for debugging while enabling proper i18n display. This is a good pattern.

Clean timeout detection: The isTimeoutError() utility correctly handles both direct DOMException and wrapped errors via error.cause:

export const isTimeoutError = (error: any): boolean => {
  if (error instanceof DOMException && error.name === 'TimeoutError') {
    return true
  }
  const cause = error?.cause
  if (cause instanceof DOMException && cause.name === 'TimeoutError') {
    return true
  }
  return false
}

Proper signal composition: Using AbortSignal.any() to combine timeout and external signals in parameterBuilder.ts:103-111 is the correct approach.
Consistent naming: Renaming to DEFAULT_TIMEOUT follows the project's SCREAMING_SNAKE_CASE convention for constants.
Good documentation: JSDoc comments on SerializedError explain the dynamic properties pattern well.

⚠️ Suggestions

Test coverage for isTimeoutError: Consider adding unit tests for the isTimeoutError function, especially for the error.cause wrapping case.
Edge case in isAbortError: The function now explicitly excludes timeout errors first:
```
if (isTimeoutError(error)) {
  return false
}
```
This is correct, but ensure this ordering is maintained if the function is refactored.
i18n consistency: The French translation file (fr-fr.json) has an unusual value for stream_paused:
```
"stream_paused": "Прервано"
```
This appears to be Cyrillic text in the French file - should be "Interrompu" or similar.
Documentation note: The comment in parameterBuilder.ts:104 says "No caller currently provides a custom timeout" - this could become stale if callers change. Consider making this more durable or adding a type-level constraint.

🔍 Code Quality

Type safety: The headers type change from Record<string, string> to Record<string, string | undefined> in parameterBuilder.ts is a good defensive change.
Test updates: Tests are properly updated to use the new i18nKey pattern instead of pause_placeholder.

✅ Approval

This PR is well-structured and addresses the issue effectively. The changes are minimal, focused, and follow existing patterns in the codebase. The timeout enforcement is particularly important for preventing hung requests.

LGTM with minor notes above.

EurFelux

Review Summary

Two minor issues identified:

Missing test coverage: The isTimeoutError utility should have unit tests for the error.cause wrapping case.
Translation issue: The French translation for stream_paused contains Cyrillic text which appears to be an error.

Otherwise the PR looks good!

src/renderer/src/utils/error.ts

src/renderer/src/i18n/translate/fr-fr.json

Replace Cyrillic text "Прервано" with proper French "Interrompu" Co-Authored-By: Claude Opus 4.5 <[email protected]>

Add comprehensive tests for isTimeoutError including: - Direct DOM TimeoutError detection - Wrapped timeout errors via error.cause - Negative cases for AbortError and generic errors Co-Authored-By: Claude Opus 4.5 <[email protected]>

EurFelux

Review Summary

This PR is well-crafted and addresses important error handling improvements.

Highlights

i18nKey Pattern: Using i18nKey property instead of overwriting message preserves original error info for debugging while enabling proper localization. This follows the existing pattern used in ErrorHandlerMiddleware.ts.
Timeout Error Detection: The isTimeoutError() utility correctly handles both direct DOMException timeout errors and wrapped errors via error.cause. The explicit exclusion in isAbortError() ensures timeouts are not misclassified as user aborts.
Single Timeout Control: Establishing parameterBuilder.ts as the single timeout control point removes dead code from messageThunk.ts and makes timeout behavior predictable.
Test Coverage: Unit tests for isTimeoutError cover the main cases including wrapped errors.

Minor Notes

The French translation was already fixed from Cyrillic to proper French ("Interrompu")
The AbortSignal.any() approach is clean and handles both timeout and user abort correctly
JSDoc documentation is comprehensive

Recommendation: APPROVE - ready to merge.

src/renderer/src/utils/error.ts

src/renderer/src/aiCore/prepareParams/parameterBuilder.ts

src/renderer/src/services/messageStreaming/callbacks/baseCallbacks.ts

EurFelux · 2026-03-05T03:55:59Z

@kangfenmao @0xfullex Please take a look at it. The error message still shows a pause_placeholder key in production version.

EurFelux requested a review from 0xfullex as a code owner December 27, 2025 09:30

GeorgeDong32 requested a review from DeJeune December 27, 2025 12:18

GeorgeDong32 approved these changes Dec 27, 2025

View reviewed changes

DeJeune approved these changes Dec 28, 2025

View reviewed changes

DeJeune added this to the v1.7.9 milestone Jan 2, 2026

kangfenmao modified the milestones: v1.7.9, v1.7.10 Jan 4, 2026

kangfenmao force-pushed the main branch from 403ab91 to d27d750 Compare January 4, 2026 11:36

DeJeune modified the milestones: v1.7.10, v1.7.16 Jan 29, 2026

EurFelux commented Feb 3, 2026

View reviewed changes

EurFelux added 5 commits February 3, 2026 18:16

docs(ai): clarify streaming-only nature of text completions

2af5cd1

fix(messageStreaming): use i18n for pause placeholder error message

5bf1d99

Use formatErrorMessageWithPrefix to properly localize the pause placeholder error message with i18next

refactor(api): replace hardcoded timeout with configurable default

65a8315

Use defaultTimeout from shared config instead of hardcoded values Combine multiple abort signals for better request handling

test(streamCallback): update error utils mock to include formatErrorM…

7e121c1

…essageWithPrefix

test(streamCallback): fix type assertion in error utils mock

3d15a5a

EurFelux marked this pull request as draft February 3, 2026 10:17

EurFelux and others added 2 commits February 3, 2026 18:39

EurFelux force-pushed the fix/grok branch from cc8c8cd to 560497f Compare February 3, 2026 10:47

EurFelux force-pushed the fix/grok branch from 0b88eb6 to 14410c9 Compare February 3, 2026 11:02

EurFelux changed the title ~~fix: clarify streaming-only path and extend timeouts~~ fix: use i18nKey for error display and add timeout enforcement Feb 3, 2026

EurFelux requested review from DeJeune and GeorgeDong32 February 3, 2026 11:08

EurFelux marked this pull request as ready for review February 3, 2026 11:09

DeJeune approved these changes Feb 3, 2026

View reviewed changes

EurFelux commented Feb 3, 2026

View reviewed changes

src/renderer/src/utils/error.ts Show resolved Hide resolved

src/renderer/src/i18n/translate/fr-fr.json Outdated Show resolved Hide resolved

fix(i18n): correct French translation for stream_paused

0896720

Replace Cyrillic text "Прервано" with proper French "Interrompu" Co-Authored-By: Claude Opus 4.5 <[email protected]>

EurFelux commented Feb 3, 2026

View reviewed changes

src/renderer/src/utils/error.ts Show resolved Hide resolved

src/renderer/src/aiCore/prepareParams/parameterBuilder.ts Show resolved Hide resolved

src/renderer/src/services/messageStreaming/callbacks/baseCallbacks.ts Show resolved Hide resolved

GeorgeDong32 approved these changes Feb 3, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into fix/grok

35f9206

DeJeune modified the milestones: v1.7.16, v1.7.25 Mar 10, 2026

DeJeune added the ready-to-merge This PR has sufficient reviewer approvals but is awaiting code owner approval. label Mar 10, 2026

Merge branch 'main' of github.com:CherryHQ/cherry-studio into fix/grok

15ee86c

kangfenmao merged commit 134280a into main Mar 13, 2026
7 checks passed

kangfenmao deleted the fix/grok branch March 13, 2026 02:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: use i18nKey for error display and add timeout enforcement#12164

fix: use i18nKey for error display and add timeout enforcement#12164
kangfenmao merged 12 commits intomainfrom
fix/grok

EurFelux commented Dec 27, 2025 •

edited

Loading

Uh oh!

EurFelux left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

EurFelux commented Feb 3, 2026

Uh oh!

EurFelux left a comment

Uh oh!

Uh oh!

Uh oh!

EurFelux left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

EurFelux commented Mar 5, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

EurFelux commented Dec 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

Why we need it and why it was done in this way

Breaking changes

Special notes for your reviewer

Checklist

Release note

Uh oh!

EurFelux left a comment

Choose a reason for hiding this comment

Code Review Summary

🔴 Critical Issues

🟡 Moderate Issues

🟢 Minor Issues

Summary

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

EurFelux commented Feb 3, 2026

Code Review: Error Handling Improvements

Overview

Detailed Review

✅ Strengths

⚠️ Suggestions

🔍 Code Quality

✅ Approval

Uh oh!

EurFelux left a comment

Choose a reason for hiding this comment

Review Summary

Uh oh!

Uh oh!

Uh oh!

EurFelux left a comment

Choose a reason for hiding this comment

Review Summary

Highlights

Minor Notes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

EurFelux commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

EurFelux commented Dec 27, 2025 •

edited

Loading

EurFelux commented Mar 5, 2026 •

edited

Loading