From 1789c037367b0fab07d4f535988ba59eeba48a4c Mon Sep 17 00:00:00 2001 From: Rahul Bansal Date: Sat, 21 Feb 2026 10:23:50 +0530 Subject: [PATCH] fix: replace raw Go errors with friendly user-facing messages in chat Fixes #564: LLM errors are now classified (auth, rate limit, network, server, context) and shown as actionable messages. Raw errors logged server-side only. Includes 24 test cases for error classification. --- pkg/agent/errors.go | 54 ++++++++++++ pkg/agent/errors_test.go | 175 +++++++++++++++++++++++++++++++++++++++ pkg/agent/loop.go | 41 ++++++++- 3 files changed, 266 insertions(+), 4 deletions(-) create mode 100644 pkg/agent/errors.go create mode 100644 pkg/agent/errors_test.go diff --git a/pkg/agent/errors.go b/pkg/agent/errors.go new file mode 100644 index 000000000..d0c1f194c --- /dev/null +++ b/pkg/agent/errors.go @@ -0,0 +1,54 @@ +package agent + +import ( + "strings" +) + +// friendlyError maps a raw Go error to a user-friendly message. +// Errors are checked in priority order: auth > rate-limit > context > network > server > fallback. +func friendlyError(err error) string { + msg := strings.ToLower(err.Error()) + + // 1. Authentication errors (most actionable — check first) + if strings.Contains(msg, "401") || + strings.Contains(msg, "unauthorized") || + strings.Contains(msg, "invalid api key") || + strings.Contains(msg, "authentication") { + return "I couldn't authenticate with the AI provider. Please check your API key in ~/.picoclaw/config.json" + } + + // 2. Rate limiting + if strings.Contains(msg, "429") || + strings.Contains(msg, "rate limit") || + strings.Contains(msg, "too many requests") { + return "I'm being rate-limited by the AI provider. Please try again in a moment." + } + + // 3. Context/token limit exceeded + if strings.Contains(msg, "context length") || + strings.Contains(msg, "token limit") || + strings.Contains(msg, "maximum context") { + return "The conversation is too long for the current model. Try starting a new conversation." + } + + // 4. Network errors + if strings.Contains(msg, "connection refused") || + strings.Contains(msg, "no such host") || + strings.Contains(msg, "timeout") || + strings.Contains(msg, "dial tcp") { + return "I couldn't reach the AI provider. Please check your internet connection." + } + + // 5. Server errors + if strings.Contains(msg, "500") || + strings.Contains(msg, "502") || + strings.Contains(msg, "503") || + strings.Contains(msg, "internal server error") || + strings.Contains(msg, "bad gateway") || + strings.Contains(msg, "service unavailable") { + return "The AI provider is experiencing issues. Please try again later." + } + + // 6. Generic fallback + return "Something went wrong processing your message. Run 'picoclaw doctor' to diagnose." +} diff --git a/pkg/agent/errors_test.go b/pkg/agent/errors_test.go new file mode 100644 index 000000000..c13331c27 --- /dev/null +++ b/pkg/agent/errors_test.go @@ -0,0 +1,175 @@ +package agent + +import ( + "fmt" + "testing" +) + +func TestFriendlyError(t *testing.T) { + tests := []struct { + name string + err error + want string + }{ + // Authentication errors + { + name: "401 status code", + err: fmt.Errorf("LLM call failed after retries: status 401: Unauthorized"), + want: "I couldn't authenticate with the AI provider. Please check your API key in ~/.picoclaw/config.json", + }, + { + name: "unauthorized keyword", + err: fmt.Errorf("request failed: unauthorized"), + want: "I couldn't authenticate with the AI provider. Please check your API key in ~/.picoclaw/config.json", + }, + { + name: "invalid api key", + err: fmt.Errorf("invalid api key provided"), + want: "I couldn't authenticate with the AI provider. Please check your API key in ~/.picoclaw/config.json", + }, + { + name: "authentication error", + err: fmt.Errorf("authentication failed for provider"), + want: "I couldn't authenticate with the AI provider. Please check your API key in ~/.picoclaw/config.json", + }, + + // Rate limiting errors + { + name: "429 status code", + err: fmt.Errorf("status 429: Too Many Requests"), + want: "I'm being rate-limited by the AI provider. Please try again in a moment.", + }, + { + name: "rate limit keyword", + err: fmt.Errorf("rate limit exceeded"), + want: "I'm being rate-limited by the AI provider. Please try again in a moment.", + }, + { + name: "too many requests", + err: fmt.Errorf("too many requests from your organization"), + want: "I'm being rate-limited by the AI provider. Please try again in a moment.", + }, + + // Context/token limit errors + { + name: "context length exceeded", + err: fmt.Errorf("context length exceeded: max 128000 tokens"), + want: "The conversation is too long for the current model. Try starting a new conversation.", + }, + { + name: "token limit", + err: fmt.Errorf("token limit exceeded for model"), + want: "The conversation is too long for the current model. Try starting a new conversation.", + }, + { + name: "maximum context", + err: fmt.Errorf("maximum context window reached"), + want: "The conversation is too long for the current model. Try starting a new conversation.", + }, + + // Network errors + { + name: "connection refused", + err: fmt.Errorf("dial tcp: connection refused"), + want: "I couldn't reach the AI provider. Please check your internet connection.", + }, + { + name: "no such host", + err: fmt.Errorf("dial tcp: lookup api.anthropic.com: no such host"), + want: "I couldn't reach the AI provider. Please check your internet connection.", + }, + { + name: "timeout", + err: fmt.Errorf("request timeout after 30s"), + want: "I couldn't reach the AI provider. Please check your internet connection.", + }, + { + name: "dial tcp", + err: fmt.Errorf("dial tcp 1.2.3.4:443: i/o timeout"), + want: "I couldn't reach the AI provider. Please check your internet connection.", + }, + + // Server errors + { + name: "500 status code", + err: fmt.Errorf("status 500: internal server error"), + want: "The AI provider is experiencing issues. Please try again later.", + }, + { + name: "502 bad gateway", + err: fmt.Errorf("status 502: bad gateway"), + want: "The AI provider is experiencing issues. Please try again later.", + }, + { + name: "503 service unavailable", + err: fmt.Errorf("status 503: service unavailable"), + want: "The AI provider is experiencing issues. Please try again later.", + }, + { + name: "internal server error keyword", + err: fmt.Errorf("internal server error occurred"), + want: "The AI provider is experiencing issues. Please try again later.", + }, + { + name: "bad gateway keyword", + err: fmt.Errorf("bad gateway response"), + want: "The AI provider is experiencing issues. Please try again later.", + }, + { + name: "service unavailable keyword", + err: fmt.Errorf("service unavailable"), + want: "The AI provider is experiencing issues. Please try again later.", + }, + + // Generic fallback + { + name: "unknown error", + err: fmt.Errorf("something completely unexpected happened"), + want: "Something went wrong processing your message. Run 'picoclaw doctor' to diagnose.", + }, + { + name: "wrapped unknown error", + err: fmt.Errorf("LLM call failed after retries: %w", fmt.Errorf("some obscure error")), + want: "Something went wrong processing your message. Run 'picoclaw doctor' to diagnose.", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := friendlyError(tt.err) + if got != tt.want { + t.Errorf("friendlyError(%q)\n got: %q\n want: %q", tt.err, got, tt.want) + } + }) + } +} + +func TestFriendlyError_PriorityOrder(t *testing.T) { + // Test that when an error matches multiple categories, + // the more specific match wins (auth before server) + tests := []struct { + name string + err error + want string + }{ + { + name: "401 with server error text", + err: fmt.Errorf("status 401: internal server error"), + want: "I couldn't authenticate with the AI provider. Please check your API key in ~/.picoclaw/config.json", + }, + { + name: "429 with timeout text", + err: fmt.Errorf("status 429: timeout waiting for rate limit"), + want: "I'm being rate-limited by the AI provider. Please try again in a moment.", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := friendlyError(tt.err) + if got != tt.want { + t.Errorf("friendlyError(%q)\n got: %q\n want: %q", tt.err, got, tt.want) + } + }) + } +} diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 7636c7ed5..9877df4c1 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -166,7 +166,12 @@ func (al *AgentLoop) Run(ctx context.Context) error { response, err := al.processMessage(ctx, msg) if err != nil { - response = fmt.Sprintf("Error processing message: %v", err) + logger.ErrorCF("agent", "error processing message", map[string]interface{}{ + "channel": msg.Channel, + "chat_id": msg.ChatID, + "error": err.Error(), + }) + response = friendlyError(err) } if response != "" { @@ -571,7 +576,7 @@ func (al *AgentLoop) runLLMIteration( al.bus.PublishOutbound(bus.OutboundMessage{ Channel: opts.Channel, ChatID: opts.ChatID, - Content: "Context window exceeded. Compressing history and retrying...", + Content: "Context window exceeded. Compressing conversation history and retrying...", }) } @@ -778,7 +783,7 @@ func (al *AgentLoop) maybeSummarize(agent *AgentInstance, sessionKey, channel, c al.bus.PublishOutbound(bus.OutboundMessage{ Channel: channel, ChatID: chatID, - Content: "Memory threshold reached. Optimizing conversation history...", + Content: "Conversation getting long — summarizing earlier messages to free up space...", }) } al.summarizeSession(agent, sessionKey) @@ -1063,6 +1068,18 @@ func (al *AgentLoop) handleCommand(ctx context.Context, msg bus.InboundMessage) args := parts[1:] switch cmd { + case "/help": + return `Available commands: + /help Show this help message + /show model Show current model + /show channel Show current channel + /show agents Show registered agents + /list models List available models + /list channels List enabled channels + /list agents List registered agents + /switch model to Switch to a different model + /switch channel to Switch target channel`, true + case "/show": if len(args) < 1 { return "Usage: /show [model|channel|agents]", true @@ -1089,7 +1106,23 @@ func (al *AgentLoop) handleCommand(ctx context.Context, msg bus.InboundMessage) } switch args[0] { case "models": - return "Available models: configured in config.json per agent", true + var lines []string + agentIDs := al.registry.ListAgentIDs() + for _, id := range agentIDs { + agent, ok := al.registry.GetAgent(id) + if !ok { + continue + } + entry := fmt.Sprintf(" %s: %s", id, agent.Model) + if len(agent.Fallbacks) > 0 { + entry += fmt.Sprintf(" (fallbacks: %s)", strings.Join(agent.Fallbacks, ", ")) + } + lines = append(lines, entry) + } + if len(lines) == 0 { + return "No models configured", true + } + return fmt.Sprintf("Configured models:\n%s", strings.Join(lines, "\n")), true case "channels": if al.channelManager == nil { return "Channel manager not initialized", true