fix(chat): show specific error messages instead of generic 'Something went wrong'#1566
fix(chat): show specific error messages instead of generic 'Something went wrong'#1566PranavAgarkar07 wants to merge 1 commit into
Conversation
… went wrong' Classify inference errors into actionable categories (rate_limited, timeout, auth_error, provider_error, context_overflow, etc.) and pass the specific message through to the UI instead of the generic fallback. Changes: - src/openhuman/channels/providers/web.rs: Add classify_inference_error() to categorize errors and produce specific user-facing messages; use the classified type in Sentry/observability reporting - app/src/providers/ChatRuntimeProvider.tsx: Use event.message from the server instead of hardcoded generic constant - app/src/services/chatService.ts: Widen ChatErrorEvent error_type union to include new categories Closes tinyhumansai#1506
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR implements categorized error messaging for chat failures. A new classification helper in Rust inspects error strings to map them to specific error types (rate-limited, auth failure, budget exhaustion, etc.) with appropriate user-facing messages. The TypeScript error event type expands to support these categories. The frontend's error deduplication and response dispatch logic now uses the actual categorized message instead of a hardcoded generic string. ChangesChat Error Categorization
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant ChatRuntime as ChatRuntimeProvider
participant ChatService as ChatService<br/>(Events)
participant WebChannel as web.rs<br/>(Provider)
participant Provider as AI Provider
Client->>WebChannel: send message via start_chat
WebChannel->>Provider: request inference
Provider-->>WebChannel: error (e.g., "rate limit exceeded")
WebChannel->>WebChannel: classify_inference_error() → <br/>("rate_limited", "Model is rate-limited...")
WebChannel->>ChatService: publish chat_error with<br/>classified type & message
ChatService->>ChatRuntime: onError(event)
ChatRuntime->>ChatRuntime: resolve errorContent<br/>from event.message
ChatRuntime->>ChatRuntime: check dedupe against<br/>last thread message
ChatRuntime->>ChatRuntime: addInferenceResponse<br/>with errorContent
ChatRuntime->>Client: render specific error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| ) | ||
| } else if lower.contains("500") || lower.contains("internal server") || lower.contains("service unavailable") || lower.contains("503") { | ||
| ( | ||
| "provider_error", |
There was a problem hiding this comment.
[critical] Fragile substring matching on HTTP status codes
Matching "500" and "503" as bare substrings in error text is dangerous. An error message like "context length exceeded: 500 tokens remaining" or "processed 500 items" would falsely classify as provider_error. Same risk with "429".
Fix: Match as status code patterns — e.g. "status 500", "http 500", "status: 500", "error 429" — not bare digit sequences.
| ( | ||
| "auth_error", | ||
| "There's an authentication issue with the AI provider. Please check your API key in settings.", | ||
| ) |
There was a problem hiding this comment.
[major] Budget error classified twice with conflicting messages
There's already a dedicated is_inference_budget_exceeded_error() (line ~579) with regex-based pattern matching that catches budget errors before they reach the Err path — it returns Ok with its own message. This adds a second budget check here with a different user-facing message:
- Existing: "I don't have any budget available right now. Please top up your credits or choose a plan to continue."
- New (here): "Insufficient credits. Please top up to continue."
If the existing handler catches it first (which it should), this branch is dead code. If it doesn't, users get inconsistent messaging. Either remove this budget_exhausted branch or unify the messages.
| } | ||
| } | ||
|
|
||
| fn prompt_guard_user_message(action: PromptEnforcementAction) -> &'static str { |
There was a problem hiding this comment.
[major] No tests — will fail the coverage merge gate
classify_inference_error has 8 branches and 0 test coverage. The repo enforces ≥80% diff coverage via .github/workflows/coverage.yml. This will block merge.
Needs a #[cfg(test)] mod tests with cases for each classification branch (rate_limited, timeout, auth_error, budget_exhausted, provider_error, context_overflow, model_unavailable, and the fallback).
| client_id_task, thread_id_task, request_id_task, err | ||
| ); | ||
| let (classified_type, classified_message) = classify_inference_error(&err); | ||
| crate::core::observability::report_error( |
There was a problem hiding this comment.
[minor] Missing diagnostic logging for classification decision
Per repo convention (CLAUDE.md debug logging rules), the classification result should be logged so misclassifications are traceable in Sentry/logs:
log::debug!("[web-channel] classified inference error type={} from error={}", classified_type, &err[..err.len().min(200)]);
| request_id?: string; | ||
| message: string; | ||
| error_type: 'network' | 'timeout' | 'tool_error' | 'inference' | 'cancelled'; | ||
| error_type: 'network' | 'timeout' | 'tool_error' | 'inference' | 'cancelled' | 'rate_limited' | 'auth_error' | 'provider_error' | 'context_overflow' | 'model_unavailable' | 'budget_exhausted'; |
There was a problem hiding this comment.
[minor] 11 variants on one line is hard to scan. Consider splitting:
error_type:
| 'network' | 'timeout' | 'tool_error' | 'inference' | 'cancelled'
| 'rate_limited' | 'auth_error' | 'provider_error'
| 'context_overflow' | 'model_unavailable' | 'budget_exhausted';2 participants
Summary
When chat inference fails, users currently always see "Something went wrong. Please try again." with no indication of why. This fix classifies errors into actionable categories and shows a specific message for each.
Changes
Rust backend (
src/openhuman/channels/providers/web.rs):classify_inference_error()that maps error strings to categorized (error_type,message) pairsrate_limited,timeout,auth_error,budget_exhausted,provider_error,context_overflow,model_unavailable,inference(fallback)error_typefor better Sentry groupingFrontend (
app/src/providers/ChatRuntimeProvider.tsx):event.messagefrom the server instead of the hardcoded generic constantFrontend types (
app/src/services/chatService.ts):ChatErrorEvent.error_typeunion to include all new categoriesCloses #1506
Summary by CodeRabbit