Improve error handling with user-friendly messages and classification (#1591)

* Improve chat LLM error messages

* Fix chat visibility regression in tests

* Harden chat error handling for review feedback

* Fix rubocop private method indentation

* Fix nil presentable_error_message, i18n strings, bare rescue

- Guard `presentable_error_message` with `return nil if error.blank?` so
  chats with no error return nil instead of the fallback string; this
  prevents the API serialisers from emitting a spurious error message and
  stops the mobile polling guard from firing on every successful chat
- Move all hardcoded user-facing error strings into
  config/locales/models/chat/en.yml and reference them via I18n.t()
- Replace bare `rescue` in `error_message_for` with `rescue StandardError`
  to avoid swallowing system-level exceptions
- Update tests to reference I18n keys instead of raw strings, and add
  tests for the nil-error case and the unrecognized-error fallback

https://claude.ai/code/session_01YFMjEds5WVyKPL42xBqMCX

---------

Co-authored-by: SureBot <sure-bot@we-promise.com>
Co-authored-by: Claude <noreply@anthropic.com>

app/models/chat.rb

@@ -1,6 +1,32 @@
 class Chat < ApplicationRecord
   include Debuggable
 
+  RATE_LIMIT_PATTERNS = [
+    /\b429\b/i,
+    /rate limit/i,
+    /too many requests/i,
+    /quota exceeded/i
+  ].freeze
+
+  TEMPORARY_PROVIDER_PATTERNS = [
+    /\b5\d\d\b/i,
+    /service unavailable/i,
+    /temporarily unavailable/i,
+    /gateway timeout/i,
+    /bad gateway/i,
+    /overloaded/i,
+    /time(?:out|d?\s*out)/i,
+    /connection reset/i
+  ].freeze
+
+  AUTH_CONFIGURATION_PATTERNS = [
+    /unauthorized/i,
+    /authentication/i,
+    /invalid api key/i,
+    /incorrect api key/i,
+    /access token/i
+  ].freeze
+
   belongs_to :user
 
   has_one :viewer, class_name: "User", foreign_key: :last_viewed_chat_id, dependent: :nullify # "Last chat user has viewed"
@@ -52,17 +78,26 @@ class Chat < ApplicationRecord
   end
 
   def add_error(e)
-    update! error: e.to_json
+    update!(error: build_error_payload(e).to_json)
     broadcast_append target: "messages", partial: "chats/error", locals: { chat: self }
   end
 
+  def presentable_error_message
+    return nil if error.blank?
+    parsed_error_payload["message"].presence || classify_error_message(error)
+  end
+
+  def technical_error_message
+    parsed_error_payload["technical_message"].presence || parsed_legacy_error_message || error
+  end
+
   def clear_error
     update! error: nil
     broadcast_remove target: "chat-error"
   end
 
-  def assistant
-    @assistant ||= Assistant.for_chat(self)
+  def conversation_messages
+    messages.where(type: [ "UserMessage", "AssistantMessage" ])
   end
 
   def ask_assistant_later(message)
@@ -74,7 +109,57 @@ class Chat < ApplicationRecord
     assistant.respond_to(message)
   end
 
-  def conversation_messages
-    messages.where(type: [ "UserMessage", "AssistantMessage" ])
-  end
+  private
+
+    def build_error_payload(error)
+      technical_message = error_message_for(error)
+
+      {
+        message: classify_error_message(technical_message),
+        technical_message: technical_message,
+        type: error.class.name
+      }
+    end
+
+    def classify_error_message(message)
+      normalized_message = message.to_s.strip
+      return I18n.t("chat.errors.default") if normalized_message.blank?
+
+      if RATE_LIMIT_PATTERNS.any? { |pattern| normalized_message.match?(pattern) }
+        I18n.t("chat.errors.rate_limited")
+      elsif TEMPORARY_PROVIDER_PATTERNS.any? { |pattern| normalized_message.match?(pattern) }
+        I18n.t("chat.errors.temporarily_unavailable")
+      elsif AUTH_CONFIGURATION_PATTERNS.any? { |pattern| normalized_message.match?(pattern) }
+        I18n.t("chat.errors.misconfigured")
+      else
+        I18n.t("chat.errors.default")
+      end
+    end
+
+    def parsed_error_payload
+      return {} if error.blank?
+      return error if error.is_a?(Hash)
+
+      parsed = JSON.parse(error)
+      parsed.is_a?(Hash) ? parsed : {}
+    rescue JSON::ParserError, TypeError
+      {}
+    end
+
+    def error_message_for(error)
+      error.respond_to?(:message) ? error.message.to_s : error.to_s
+    rescue StandardError
+      ""
+    end
+
+    def parsed_legacy_error_message
+      parsed = JSON.parse(error)
+      parsed.is_a?(String) ? parsed : nil
+    rescue JSON::ParserError, TypeError
+      nil
+    end
+
+    def assistant
+      @assistant ||= Assistant.for_chat(self)
+    end
 end

app/views/api/v1/chats/_chat.json.jbuilder

@@ -2,6 +2,6 @@
 
 json.id chat.id
 json.title chat.title
-json.error chat.error.present? ? chat.error : nil
+json.error chat.presentable_error_message
 json.created_at chat.created_at.iso8601
 json.updated_at chat.updated_at.iso8601

app/views/api/v1/chats/index.json.jbuilder

@@ -5,7 +5,7 @@ json.chats @chats do |chat|
   json.title chat.title
   json.last_message_at chat.messages.ordered.first&.created_at&.iso8601
   json.message_count chat.messages.count
-  json.error chat.error.present? ? chat.error : nil
+  json.error chat.presentable_error_message
   json.created_at chat.created_at.iso8601
   json.updated_at chat.updated_at.iso8601
 end

app/views/chats/_error.html.erb

@@ -3,12 +3,12 @@
 <div id="chat-error" class="px-3 py-2 bg-red-100 border border-red-500 rounded-lg">
   <% if chat.debug_mode? %>
     <div class="overflow-x-auto text-xs p-4 bg-red-200 rounded-md mb-2">
-      <code><%= chat.error %></code>
+      <code><%= chat.technical_error_message %></code>
     </div>
   <% end %>
 
   <div class="flex items-center justify-between gap-2">
-    <p class="text-xs text-red-500">Failed to generate response.  Please try again.</p>
+    <p class="text-xs text-red-500"><%= chat.presentable_error_message %></p>
 
     <%= render DS::Button.new(
       text: "Retry",

config/locales/models/chat/en.yml

@@ -0,0 +1,8 @@
+---
+en:
+  chat:
+    errors:
+      rate_limited: "The AI provider is rate limited right now. Please try again in a few minutes."
+      temporarily_unavailable: "The AI provider is temporarily unavailable right now. Please try again in a few minutes."
+      misconfigured: "The AI provider is not configured correctly. Please contact your administrator."
+      default: "Failed to generate a response. Please try again."

mobile/lib/providers/chat_provider.dart

@@ -345,6 +345,16 @@ class ChatProvider with ChangeNotifier {
           notifyListeners();
         }
 
+        if (updatedChat.error != null && updatedChat.error!.isNotEmpty) {
+          if (!shouldUpdate) {
+            _currentChat = updatedChat;
+          }
+          _stopPolling();
+          _errorMessage = updatedChat.error;
+          notifyListeners();
+          return;
+        }
+
         final lastMessage = updatedChat.messages.lastOrNull;
         if (lastMessage != null && lastMessage.isAssistant) {
           final newLen = lastMessage.content.length;

test/models/chat_test.rb

@@ -60,4 +60,56 @@ class ChatTest < ActiveSupport::TestCase
       assert_equal "custom-model", chat.messages.first.ai_model
     end
   end
+
+  test "returns nil presentable error message when no error is stored" do
+    chat = chats(:one)
+
+    chat.update!(error: nil)
+
+    assert_nil chat.presentable_error_message
+  end
+
+  test "surfaces a friendly rate limit error" do
+    chat = chats(:one)
+
+    chat.add_error(StandardError.new("OpenAI API error 429: rate limit exceeded"))
+
+    assert_equal I18n.t("chat.errors.rate_limited"), chat.presentable_error_message
+    assert_match "429", chat.technical_error_message
+  end
+
+  test "surfaces a friendly temporary provider error" do
+    chat = chats(:one)
+
+    chat.add_error(StandardError.new("OpenAI API error 503: service unavailable"))
+
+    assert_equal I18n.t("chat.errors.temporarily_unavailable"), chat.presentable_error_message
+    assert_match "503", chat.technical_error_message
+  end
+
+  test "surfaces a friendly auth configuration error" do
+    chat = chats(:one)
+
+    chat.add_error(StandardError.new("OpenAI API error: invalid api key"))
+
+    assert_equal I18n.t("chat.errors.misconfigured"), chat.presentable_error_message
+    assert_match "invalid api key", chat.technical_error_message
+  end
+
+  test "surfaces a friendly default error for unrecognized errors" do
+    chat = chats(:one)
+
+    chat.add_error(StandardError.new("something totally unknown happened"))
+
+    assert_equal I18n.t("chat.errors.default"), chat.presentable_error_message
+  end
+
+  test "falls back to a friendly message for legacy serialized errors" do
+    chat = chats(:one)
+
+    chat.update!(error: "OpenAI API error 429: rate limit exceeded".to_json)
+
+    assert_equal I18n.t("chat.errors.rate_limited"), chat.presentable_error_message
+    assert_equal "OpenAI API error 429: rate limit exceeded", chat.technical_error_message
+  end
 end