Files
sure/test/models/chat_test.rb
Guillem Arias Fauste 169fd139b5 fix(chat): surface and recover from undelivered assistant responses (#2436)
* fix(chat): surface and recover from undelivered assistant responses

When the background worker that runs AssistantResponseJob is down or not
polling the high_priority queue, the eager "pending" assistant message had no
safeguard: chat hung on "Thinking…" forever with no error, timeout, or retry,
and the LLM provider was never even called.

Add three layers of resilience:

- Client watchdog (chat_controller.js): a pending "Thinking…" bubble that waits
  past a threshold (default 90s) with no response asks the server to fail it.
  Keyed on the pending marker — which disappears the instant a real response
  streams — so a slow-but-working response is never falsely timed out.

- Server failure capture (Chat#handle_undelivered_response! +
  MessagesController#report_timeout): clears the dead bubble, records a friendly
  "the assistant didn't respond" error with Retry, and writes a DebugLogEntry so
  support can see it in /settings/debug.

- Worker liveness signal (BackgroundJobHealth + warning banner): reads Sidekiq's
  process/queue state directly from Redis in the web process — so a down worker
  is detectable even though the worker is the thing that's broken — and warns
  self-hosted users when no worker polls high_priority or it's badly backed up.
  Fails open so a Sidekiq/Redis blip never blocks chat.

Tests: Chat model (3), MessagesController (2), BackgroundJobHealth (5).

* fix(chat): address review — server-side timeout guard + watchdog retry safety

- Chat#handle_undelivered_response!: gate the state change behind a row lock and
  a server-side minimum age (UNDELIVERED_RESPONSE_TIMEOUT = 60s). The browser
  watchdog is untrusted, so re-read the row under lock and only fail a bubble
  that is still pending AND has genuinely waited past the timeout — never racing
  a worker that is finishing a slow response. (Codex P2 + CodeRabbit)
- chat_controller.js: only mark a report URL as reported on response.ok (fetch
  resolves on HTTP 4xx/5xx, rejecting only on network errors), and guard
  concurrent duplicate POSTs with an in-flight set, so a failed report retries
  instead of stranding the bubble. (CodeRabbit)
- _worker_health_warning: drop the unused `chat:` local + its render arg. (CodeRabbit)
2026-06-25 05:44:44 +02:00

203 lines
7.1 KiB
Ruby

require "test_helper"
class ChatTest < ActiveSupport::TestCase
setup do
@user = users(:family_admin)
@assistant = mock
end
test "user sees all messages in debug mode" do
chat = chats(:one)
with_env_overrides AI_DEBUG_MODE: "true" do
assert_equal chat.messages.count, chat.conversation_messages.count
end
end
test "user sees assistant and user messages in normal mode" do
chat = chats(:one)
assert_equal 3, chat.conversation_messages.count
end
test "uses chat-scoped stream targets" do
first_chat = chats(:one)
second_chat = chats(:two)
assert_not_equal "messages", first_chat.messages_target
assert_not_equal "chat-error", first_chat.error_target
assert_not_equal first_chat.messages_target, second_chat.messages_target
assert_not_equal first_chat.error_target, second_chat.error_target
end
test "creates with initial message" do
prompt = "Test prompt"
assert_difference "@user.chats.count", 1 do
chat = @user.chats.start!(prompt, model: "gpt-4.1")
assert_equal 2, chat.messages.count
assert_equal 1, chat.messages.where(type: "UserMessage").count
assert_equal 1, chat.messages.where(type: "AssistantMessage", status: "pending").count
end
end
test "creates with default model when model is nil" do
prompt = "Test prompt"
assert_difference "@user.chats.count", 1 do
chat = @user.chats.start!(prompt, model: nil)
assert_equal 2, chat.messages.count
assert_equal Chat.default_model, chat.messages.find_by!(type: "UserMessage").ai_model
end
end
test "creates with default model when model is empty string" do
prompt = "Test prompt"
assert_difference "@user.chats.count", 1 do
chat = @user.chats.start!(prompt, model: "")
assert_equal 2, chat.messages.count
assert_equal Chat.default_model, chat.messages.find_by!(type: "UserMessage").ai_model
end
end
# These three tests assert routing (which provider's effective_model wins),
# not the constant value itself — the assertion side reads through
# Provider::*.effective_model so ENV overrides like ANTHROPIC_MODEL /
# OPENAI_MODEL don't make the tests flake.
test "default_model returns Anthropic's effective_model when LLM_PROVIDER=anthropic and Anthropic is configured" do
Provider::Anthropic.stubs(:configured?).returns(true)
Setting.stubs(:llm_provider).returns("anthropic")
assert_equal Provider::Anthropic.effective_model, Chat.default_model
end
test "default_model falls back to OpenAI's effective_model when Anthropic is preferred but unconfigured" do
Provider::Anthropic.stubs(:configured?).returns(false)
Provider::Openai.stubs(:configured?).returns(true)
Setting.stubs(:llm_provider).returns("anthropic")
assert_equal Provider::Openai.effective_model, Chat.default_model
end
test "default_model uses Anthropic's effective_model when OpenAI is unconfigured" do
Provider::Anthropic.stubs(:configured?).returns(true)
Provider::Openai.stubs(:configured?).returns(false)
Setting.stubs(:llm_provider).returns("openai")
assert_equal Provider::Anthropic.effective_model, Chat.default_model
end
test "creates with configured model when OPENAI_MODEL env is set" do
prompt = "Test prompt"
with_env_overrides OPENAI_MODEL: "custom-model" do
chat = @user.chats.start!(prompt, model: "")
assert_equal "custom-model", chat.messages.find_by!(type: "UserMessage").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
test "handle_undelivered_response! clears a blank pending bubble and records an error + debug log" do
BackgroundJobHealth.stubs(:snapshot).returns({ healthy: false, workers: 0 })
BackgroundJobHealth.stubs(:summary).returns("workers=0")
chat = chats(:two)
pending = chat.messages.create!(type: "AssistantMessage", content: "", ai_model: "gpt-4.1", status: :pending, created_at: 5.minutes.ago)
assert_difference -> { DebugLogEntry.count } => 1, -> { Message.count } => -1 do
assert chat.handle_undelivered_response!(pending)
end
assert_not Message.exists?(pending.id)
assert_equal I18n.t("chat.errors.no_response"), chat.reload.presentable_error_message
end
test "handle_undelivered_response! demotes a partially-streamed pending bubble to failed" do
BackgroundJobHealth.stubs(:snapshot).returns({})
BackgroundJobHealth.stubs(:summary).returns("")
chat = chats(:two)
pending = chat.messages.create!(type: "AssistantMessage", content: "partial answer", ai_model: "gpt-4.1", status: :pending, created_at: 5.minutes.ago)
assert_no_difference -> { Message.count } do
assert chat.handle_undelivered_response!(pending)
end
assert_equal "failed", pending.reload.status
end
test "handle_undelivered_response! ignores a pending bubble younger than the server timeout" do
chat = chats(:two)
fresh = chat.messages.create!(type: "AssistantMessage", content: "", ai_model: "gpt-4.1", status: :pending, created_at: 5.seconds.ago)
assert_no_difference [ "DebugLogEntry.count", "Message.count" ] do
assert_not chat.handle_undelivered_response!(fresh)
end
assert fresh.reload.pending?
end
test "handle_undelivered_response! is a no-op for non-pending messages" do
chat = chats(:one)
complete = chat.messages.create!(type: "AssistantMessage", content: "done", ai_model: "gpt-4.1", status: :complete)
assert_no_difference [ "DebugLogEntry.count", "Message.count" ] do
assert_not chat.handle_undelivered_response!(complete)
end
end
end