Fix error handling in planning tab: display sanitized errors and return control to user - #255
Conversation
…d return control to user
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
This PR improves the Planning tab’s error UX by sanitizing error strings before displaying them to the user and by returning the tab to an idle/retryable state after failures, aligning the UI behavior with Issue #250’s acceptance criteria.
Changes:
- Introduces
sanitizeErrorMessage()to transform technical/raw error payloads into user-facing messages. - Adjusts streaming and non-streaming error handlers to display an error message, scroll the viewport to it, and return to
PlanningStateIdle. - Refines viewport error styling detection to be case-insensitive without repeated lowercasing.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| internal/tui/planning_tab.go | Adds error sanitization and updates planning tab error handling/state transitions and viewport behavior. |
| .kiro-krew/specs/issue-250-fix-error-handling-in-planning-tab.md | Adds a design spec documenting the problem, approach, and validation steps for Issue #250. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Try to parse as JSON-RPC error structure | ||
| var jsonError map[string]interface{} | ||
| if err := json.Unmarshal([]byte(rawError), &jsonError); err == nil { | ||
| // Check for nested error structures: data > message > code | ||
| if data, ok := jsonError["data"].(map[string]interface{}); ok { | ||
| if message, ok := data["message"].(map[string]interface{}); ok { | ||
| if code, ok := message["code"].(string); ok && code != "" { | ||
| return code | ||
| } | ||
| } | ||
| // Check for direct message in data | ||
| if message, ok := data["message"].(string); ok && message != "" { | ||
| return message | ||
| } | ||
| } | ||
| // Check for direct error message field | ||
| if message, ok := jsonError["message"].(string); ok && message != "" { | ||
| return message | ||
| } | ||
| if errorMsg, ok := jsonError["error"].(string); ok && errorMsg != "" { | ||
| return errorMsg | ||
| } | ||
| } |
| // Sanitize error message for user display | ||
| sanitizedError := sanitizeErrorMessage(response.Error) | ||
| errorMessage := fmt.Sprintf("Error: %s", sanitizedError) | ||
|
|
||
| // Add error message | ||
| if response.Error != "" { | ||
| pt.currentResponse.WriteString(fmt.Sprintf("\n[Error: %s]", response.Error)) | ||
| // Preserve accumulated response before displaying error | ||
| accumulatedResponse := pt.currentResponse.String() | ||
| if accumulatedResponse != "" { | ||
| pt.AddMessage("assistant", accumulatedResponse) | ||
| } |
| if msg.isError { | ||
| oldState := pt.state | ||
| pt.state = session.PlanningStateFailed | ||
| logging.Warn("state transition on error response", "tab_id", pt.id, "from", oldState, "to", pt.state) | ||
| pt.AddMessage("assistant", fmt.Sprintf("[Error: %s]", msg.content)) | ||
|
|
||
| // Sanitize error message for user display | ||
| sanitizedError := sanitizeErrorMessage(msg.content) | ||
| errorMessage := fmt.Sprintf("Error: %s", sanitizedError) | ||
|
|
||
| // Add sanitized error message | ||
| pt.AddMessage("assistant", errorMessage) | ||
|
|
||
| // Transition to Idle state to allow immediate retry | ||
| pt.state = session.PlanningStateIdle | ||
| logging.Info("state transition on error response", "tab_id", pt.id, "from", oldState, "to", pt.state) | ||
|
|
| // sanitizeErrorMessage parses and formats error messages into user-friendly text. | ||
| // It handles JSON-RPC error structures and plain text errors, truncating very long messages. | ||
| func sanitizeErrorMessage(rawError string) string { | ||
| if rawError == "" { | ||
| return "An unknown error occurred" | ||
| } | ||
|
|
- Handle prefixed JSON errors (e.g., 'failed to send prompt: {...}')
- Support data field as string (most common JSON-RPC case)
- Set state to Idle BEFORE AddMessage in both error paths
Addresses Copilot review feedback on PR #255:
- Fixes sanitization failures on wrapped/prefixed JSON
- Prevents incorrect state persistence via saveSessionState()
- Ensures session state is correct after error handling
Signed-off-by: Joseph Brinkman <joe.brinkman@improving.com>
Summary
This PR fixes error handling in the planning tab to provide a better user experience when prompts fail. Previously, when errors occurred (such as "Improperly formed request" from the ACP stream), users would see raw JSON error messages and the UI would get stuck in a failed state without clear recovery options.
What Changed
Error Message Sanitization
sanitizeErrorMessage()function that parses JSON-RPC error structuresState Management Improvements
PlanningStateIdleafter displaying errorsPlanningStateFailedstate that confused users[planner] >style after errorsViewport and UX Enhancements
GotoBottom())Key Files Modified
internal/tui/planning_tab.go- Complete error handling overhaul with sanitization and state management fixes.kiro-krew/specs/issue-250-*.md- Design specification documentTesting
User Experience Impact
Before:
{"code":-32603,"message":"Internal error",...}After:
Closes #250