Improve CLI streaming, error handling, and add comprehensive tests
**Core Improvements:** - Fix Pydantic v2 deprecation warning in config.py by migrating to ConfigDict - Add graceful Ctrl-C handling for streaming chat responses - Add connection error handling mid-stream with actionable suggestions - Implement --history-file support for conversation context - Enhance all error messages with specific troubleshooting steps **Chat Command Enhancements:** - Improved SSE streaming with KeyboardInterrupt handling - Added --history-file option to load conversation history from JSON - Support both array and object formats for history files - Better error messages for missing prompts and invalid history files - Graceful handling of connection timeouts during streaming **Error Message Improvements:** - 401: Clear authentication instructions with token expiry hints - 403: Detailed permission error with possible causes - 404: Resource not found with helpful suggestions - 5xx: Server error with admin troubleshooting tips - Network errors: Connection, timeout, and request failures with solutions **Testing:** - Add comprehensive test suite for chat commands (7 tests) - Test streaming and non-streaming responses - Test system prompts, stdin input, and JSON output - Test conversation history loading - Test RAG file and collection context - All 14 tests passing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
8530f74687
commit
fbe6832d3e
4 changed files with 429 additions and 40 deletions
|
|
@ -28,6 +28,7 @@ def send(
|
|||
temperature: float | None = typer.Option(None, "--temperature", "-T", help="Temperature (0.0-2.0)"),
|
||||
max_tokens: int | None = typer.Option(None, "--max-tokens", help="Max response tokens"),
|
||||
json_output: bool = typer.Option(False, "--json", help="Output as JSON"),
|
||||
history_file: str | None = typer.Option(None, "--history-file", help="Load conversation history from JSON file"),
|
||||
) -> None:
|
||||
"""Send a chat message."""
|
||||
obj = ctx.obj or {}
|
||||
|
|
@ -41,10 +42,38 @@ def send(
|
|||
console.print("[red]Error: Prompt required. Use -p or pipe input.[/red]")
|
||||
raise typer.Exit(2)
|
||||
|
||||
# Build messages
|
||||
# Load conversation history if provided
|
||||
messages = []
|
||||
if system:
|
||||
messages.append({"role": "system", "content": system})
|
||||
if history_file:
|
||||
try:
|
||||
from pathlib import Path
|
||||
history_path = Path(history_file)
|
||||
if not history_path.exists():
|
||||
console.print(f"[red]Error: History file not found: {history_file}[/red]")
|
||||
raise typer.Exit(2)
|
||||
|
||||
with open(history_path) as f:
|
||||
history_data = json.load(f)
|
||||
# Support both direct array and object with 'messages' key
|
||||
if isinstance(history_data, list):
|
||||
messages = history_data
|
||||
elif isinstance(history_data, dict) and "messages" in history_data:
|
||||
messages = history_data["messages"]
|
||||
else:
|
||||
console.print("[red]Error: History file must contain array of messages or object with 'messages' key[/red]")
|
||||
raise typer.Exit(2)
|
||||
except json.JSONDecodeError as e:
|
||||
console.print(f"[red]Error: Invalid JSON in history file: {e}[/red]")
|
||||
raise typer.Exit(2)
|
||||
except Exception as e:
|
||||
console.print(f"[red]Error loading history file: {e}[/red]")
|
||||
raise typer.Exit(2)
|
||||
|
||||
# Build messages (add system prompt if not in history)
|
||||
if system and not any(msg.get("role") == "system" for msg in messages):
|
||||
messages.insert(0, {"role": "system", "content": system})
|
||||
|
||||
# Add current user prompt
|
||||
messages.append({"role": "user", "content": prompt})
|
||||
|
||||
# Build request body
|
||||
|
|
@ -78,33 +107,49 @@ def send(
|
|||
) as client:
|
||||
if body.get("stream"):
|
||||
# Streaming response
|
||||
with client.stream(
|
||||
"POST",
|
||||
"/api/v1/chat/completions",
|
||||
json=body,
|
||||
) as response:
|
||||
if response.status_code >= 400:
|
||||
handle_response(response)
|
||||
try:
|
||||
with client.stream(
|
||||
"POST",
|
||||
"/api/v1/chat/completions",
|
||||
json=body,
|
||||
) as response:
|
||||
if response.status_code >= 400:
|
||||
handle_response(response)
|
||||
|
||||
full_content = ""
|
||||
for line in response.iter_lines():
|
||||
if line.startswith("data: "):
|
||||
data_str = line[6:]
|
||||
if data_str.strip() == "[DONE]":
|
||||
break
|
||||
try:
|
||||
data = json.loads(data_str)
|
||||
delta = data.get("choices", [{}])[0].get("delta", {})
|
||||
content = delta.get("content", "")
|
||||
if content:
|
||||
print(content, end="", flush=True)
|
||||
full_content += content
|
||||
except json.JSONDecodeError:
|
||||
continue
|
||||
print() # Final newline
|
||||
full_content = ""
|
||||
try:
|
||||
for line in response.iter_lines():
|
||||
if line.startswith("data: "):
|
||||
data_str = line[6:]
|
||||
if data_str.strip() == "[DONE]":
|
||||
break
|
||||
try:
|
||||
data = json.loads(data_str)
|
||||
delta = data.get("choices", [{}])[0].get("delta", {})
|
||||
content = delta.get("content", "")
|
||||
if content:
|
||||
print(content, end="", flush=True)
|
||||
full_content += content
|
||||
except json.JSONDecodeError:
|
||||
# Skip malformed JSON chunks
|
||||
continue
|
||||
except KeyboardInterrupt:
|
||||
# Gracefully handle Ctrl-C during streaming
|
||||
print() # Newline after partial output
|
||||
console.print("\n[yellow]Stream interrupted by user[/yellow]")
|
||||
if full_content and json_output:
|
||||
console.print(json.dumps({"content": full_content, "interrupted": True}, indent=2))
|
||||
raise typer.Exit(0)
|
||||
|
||||
if json_output:
|
||||
console.print(json.dumps({"content": full_content}, indent=2))
|
||||
print() # Final newline
|
||||
|
||||
if json_output:
|
||||
console.print(json.dumps({"content": full_content}, indent=2))
|
||||
|
||||
except (ConnectionError, TimeoutError) as e:
|
||||
console.print(f"\n[red]Connection error during streaming: {e}[/red]")
|
||||
console.print("[yellow]Try reducing timeout or checking network connection[/yellow]")
|
||||
raise typer.Exit(4)
|
||||
else:
|
||||
# Non-streaming response
|
||||
response = client.post("/api/v1/chat/completions", json=body)
|
||||
|
|
@ -116,6 +161,10 @@ def send(
|
|||
content = data.get("choices", [{}])[0].get("message", {}).get("content", "")
|
||||
console.print(content)
|
||||
|
||||
except KeyboardInterrupt:
|
||||
# Handle Ctrl-C at top level
|
||||
console.print("\n[yellow]Operation cancelled[/yellow]")
|
||||
raise typer.Exit(0)
|
||||
except Exception as e:
|
||||
handle_request_error(e)
|
||||
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@ from pathlib import Path
|
|||
from typing import Any
|
||||
|
||||
import yaml
|
||||
from pydantic import BaseModel, Field
|
||||
from pydantic import BaseModel, ConfigDict, Field
|
||||
from pydantic_settings import BaseSettings
|
||||
|
||||
|
||||
|
|
@ -60,14 +60,12 @@ class Config(BaseModel):
|
|||
class Settings(BaseSettings):
|
||||
"""Environment-based settings that override config file."""
|
||||
|
||||
model_config = ConfigDict(env_prefix="", case_sensitive=False)
|
||||
|
||||
openwebui_uri: str | None = None
|
||||
openwebui_token: str | None = None
|
||||
openwebui_profile: str | None = None
|
||||
|
||||
class Config:
|
||||
env_prefix = ""
|
||||
case_sensitive = False
|
||||
|
||||
|
||||
def load_config() -> Config:
|
||||
"""Load configuration from file, with defaults for missing values."""
|
||||
|
|
|
|||
|
|
@ -120,18 +120,44 @@ def handle_response(response: httpx.Response) -> dict[str, Any]:
|
|||
CLIError: For other error responses
|
||||
"""
|
||||
if response.status_code == 401:
|
||||
raise AuthError("Authentication required. Please run 'openwebui auth login'")
|
||||
raise AuthError(
|
||||
"Authentication required. Please run 'openwebui auth login' first.\n"
|
||||
"If you recently logged in, your token may have expired."
|
||||
)
|
||||
elif response.status_code == 403:
|
||||
raise AuthError("Permission denied. Check your access rights.")
|
||||
raise AuthError(
|
||||
"Permission denied. This operation requires higher privileges.\n"
|
||||
"Possible causes:\n"
|
||||
" - Your user role lacks required permissions\n"
|
||||
" - The API key doesn't have sufficient access\n"
|
||||
" - Try logging in again: openwebui auth login"
|
||||
)
|
||||
elif response.status_code == 404:
|
||||
try:
|
||||
error_data = response.json()
|
||||
message = error_data.get("detail", error_data.get("message", "Resource not found"))
|
||||
except Exception:
|
||||
message = "Resource not found"
|
||||
raise ServerError(
|
||||
f"Not found: {message}\n"
|
||||
"Check that the resource ID, model name, or endpoint is correct."
|
||||
)
|
||||
elif response.status_code >= 500:
|
||||
raise ServerError(f"Server error: {response.status_code} - {response.text}")
|
||||
raise ServerError(
|
||||
f"Server error ({response.status_code}): {response.text}\n"
|
||||
"The OpenWebUI server encountered an error.\n"
|
||||
"Try again in a moment, or check server logs if you're the administrator."
|
||||
)
|
||||
elif response.status_code >= 400:
|
||||
try:
|
||||
error_data = response.json()
|
||||
message = error_data.get("detail", error_data.get("message", response.text))
|
||||
except Exception:
|
||||
message = response.text
|
||||
raise ServerError(f"API error: {response.status_code} - {message}")
|
||||
raise ServerError(
|
||||
f"API error ({response.status_code}): {message}\n"
|
||||
"Check your request parameters and try again."
|
||||
)
|
||||
|
||||
try:
|
||||
return response.json()
|
||||
|
|
@ -142,10 +168,25 @@ def handle_response(response: httpx.Response) -> dict[str, Any]:
|
|||
def handle_request_error(error: Exception) -> None:
|
||||
"""Convert httpx errors to CLI errors."""
|
||||
if isinstance(error, httpx.ConnectError):
|
||||
raise NetworkError(f"Could not connect to server: {error}")
|
||||
raise NetworkError(
|
||||
f"Could not connect to server: {error}\n"
|
||||
"Possible solutions:\n"
|
||||
" - Check that OpenWebUI is running\n"
|
||||
" - Verify the URI: openwebui config init\n"
|
||||
" - Try: openwebui --uri http://localhost:8080 auth login"
|
||||
)
|
||||
elif isinstance(error, httpx.TimeoutException):
|
||||
raise NetworkError(f"Request timed out: {error}")
|
||||
raise NetworkError(
|
||||
f"Request timed out: {error}\n"
|
||||
"Possible solutions:\n"
|
||||
" - Increase timeout: openwebui --timeout 60 ...\n"
|
||||
" - Check your network connection\n"
|
||||
" - The server might be overloaded"
|
||||
)
|
||||
elif isinstance(error, httpx.RequestError):
|
||||
raise NetworkError(f"Request failed: {error}")
|
||||
raise NetworkError(
|
||||
f"Request failed: {error}\n"
|
||||
"Check your network connection and server configuration."
|
||||
)
|
||||
else:
|
||||
raise error
|
||||
|
|
|
|||
301
tests/test_chat.py
Normal file
301
tests/test_chat.py
Normal file
|
|
@ -0,0 +1,301 @@
|
|||
"""Tests for chat commands."""
|
||||
|
||||
import json
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, Mock, patch
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
from typer.testing import CliRunner
|
||||
|
||||
from openwebui_cli.main import app
|
||||
|
||||
runner = CliRunner()
|
||||
|
||||
|
||||
class MockStreamResponse:
|
||||
"""Mock streaming response for testing."""
|
||||
|
||||
def __init__(self, lines, status_code=200):
|
||||
self.lines = lines
|
||||
self.status_code = status_code
|
||||
|
||||
def __enter__(self):
|
||||
return self
|
||||
|
||||
def __exit__(self, *args):
|
||||
pass
|
||||
|
||||
def iter_lines(self):
|
||||
"""Yield lines one by one."""
|
||||
for line in self.lines:
|
||||
yield line
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mock_config(tmp_path, monkeypatch):
|
||||
"""Mock configuration for testing."""
|
||||
config_dir = tmp_path / "openwebui"
|
||||
config_path = config_dir / "config.yaml"
|
||||
|
||||
monkeypatch.setattr("openwebui_cli.config.get_config_dir", lambda: config_dir)
|
||||
monkeypatch.setattr("openwebui_cli.config.get_config_path", lambda: config_path)
|
||||
|
||||
# Create default config
|
||||
from openwebui_cli.config import Config, save_config
|
||||
config = Config()
|
||||
save_config(config)
|
||||
|
||||
return config_path
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mock_keyring(monkeypatch):
|
||||
"""Mock keyring for testing."""
|
||||
token_store = {}
|
||||
|
||||
def get_password(service, key):
|
||||
return token_store.get(f"{service}:{key}")
|
||||
|
||||
def set_password(service, key, password):
|
||||
token_store[f"{service}:{key}"] = password
|
||||
|
||||
monkeypatch.setattr("keyring.get_password", get_password)
|
||||
monkeypatch.setattr("keyring.set_password", set_password)
|
||||
|
||||
|
||||
def test_chat_send_streaming(mock_config, mock_keyring):
|
||||
"""Test streaming chat response."""
|
||||
# Mock streaming response
|
||||
streaming_lines = [
|
||||
'data: {"choices": [{"delta": {"content": "Hello"}}]}',
|
||||
'data: {"choices": [{"delta": {"content": " world"}}]}',
|
||||
'data: {"choices": [{"delta": {"content": "!"}}]}',
|
||||
"data: [DONE]",
|
||||
]
|
||||
|
||||
with patch("openwebui_cli.commands.chat.create_client") as mock_client:
|
||||
mock_stream = MockStreamResponse(streaming_lines)
|
||||
mock_http_client = MagicMock()
|
||||
mock_http_client.__enter__.return_value = mock_http_client
|
||||
mock_http_client.__exit__.return_value = None
|
||||
mock_http_client.stream.return_value = mock_stream
|
||||
mock_client.return_value = mock_http_client
|
||||
|
||||
result = runner.invoke(
|
||||
app,
|
||||
["chat", "send", "-m", "test-model", "-p", "Hello"],
|
||||
)
|
||||
|
||||
assert result.exit_code == 0
|
||||
assert "Hello world!" in result.stdout
|
||||
|
||||
|
||||
def test_chat_send_no_stream(mock_config, mock_keyring):
|
||||
"""Test non-streaming chat response."""
|
||||
response_data = {
|
||||
"choices": [
|
||||
{
|
||||
"message": {
|
||||
"content": "This is a test response"
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
with patch("openwebui_cli.commands.chat.create_client") as mock_client:
|
||||
mock_http_client = MagicMock()
|
||||
mock_http_client.__enter__.return_value = mock_http_client
|
||||
mock_http_client.__exit__.return_value = None
|
||||
mock_response = Mock()
|
||||
mock_response.status_code = 200
|
||||
mock_response.json.return_value = response_data
|
||||
mock_http_client.post.return_value = mock_response
|
||||
mock_client.return_value = mock_http_client
|
||||
|
||||
result = runner.invoke(
|
||||
app,
|
||||
["chat", "send", "-m", "test-model", "-p", "Hello", "--no-stream"],
|
||||
)
|
||||
|
||||
assert result.exit_code == 0
|
||||
assert "This is a test response" in result.stdout
|
||||
|
||||
|
||||
def test_chat_send_with_system_prompt(mock_config, mock_keyring):
|
||||
"""Test chat with system prompt."""
|
||||
response_data = {
|
||||
"choices": [
|
||||
{
|
||||
"message": {
|
||||
"content": "Response with system prompt"
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
with patch("openwebui_cli.commands.chat.create_client") as mock_client:
|
||||
mock_http_client = MagicMock()
|
||||
mock_http_client.__enter__.return_value = mock_http_client
|
||||
mock_http_client.__exit__.return_value = None
|
||||
mock_response = Mock()
|
||||
mock_response.status_code = 200
|
||||
mock_response.json.return_value = response_data
|
||||
mock_http_client.post.return_value = mock_response
|
||||
mock_client.return_value = mock_http_client
|
||||
|
||||
result = runner.invoke(
|
||||
app,
|
||||
[
|
||||
"chat", "send",
|
||||
"-m", "test-model",
|
||||
"-p", "Hello",
|
||||
"-s", "You are a helpful assistant",
|
||||
"--no-stream"
|
||||
],
|
||||
)
|
||||
|
||||
assert result.exit_code == 0
|
||||
|
||||
|
||||
def test_chat_send_with_history_file(tmp_path, mock_config, mock_keyring):
|
||||
"""Test chat with history file."""
|
||||
# Create history file
|
||||
history_file = tmp_path / "history.json"
|
||||
history = [
|
||||
{"role": "user", "content": "What is 2+2?"},
|
||||
{"role": "assistant", "content": "4"},
|
||||
]
|
||||
with open(history_file, "w") as f:
|
||||
json.dump(history, f)
|
||||
|
||||
response_data = {
|
||||
"choices": [
|
||||
{
|
||||
"message": {
|
||||
"content": "Continuing conversation"
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
with patch("openwebui_cli.commands.chat.create_client") as mock_client:
|
||||
mock_http_client = MagicMock()
|
||||
mock_http_client.__enter__.return_value = mock_http_client
|
||||
mock_http_client.__exit__.return_value = None
|
||||
mock_response = Mock()
|
||||
mock_response.status_code = 200
|
||||
mock_response.json.return_value = response_data
|
||||
mock_http_client.post.return_value = mock_response
|
||||
mock_client.return_value = mock_http_client
|
||||
|
||||
result = runner.invoke(
|
||||
app,
|
||||
[
|
||||
"chat", "send",
|
||||
"-m", "test-model",
|
||||
"-p", "What about 3+3?",
|
||||
"--history-file", str(history_file),
|
||||
"--no-stream"
|
||||
],
|
||||
)
|
||||
|
||||
assert result.exit_code == 0
|
||||
|
||||
|
||||
def test_chat_send_stdin(mock_config, mock_keyring):
|
||||
"""Test chat with stdin input."""
|
||||
response_data = {
|
||||
"choices": [
|
||||
{
|
||||
"message": {
|
||||
"content": "Response from stdin"
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
with patch("openwebui_cli.commands.chat.create_client") as mock_client:
|
||||
mock_http_client = MagicMock()
|
||||
mock_http_client.__enter__.return_value = mock_http_client
|
||||
mock_http_client.__exit__.return_value = None
|
||||
mock_response = Mock()
|
||||
mock_response.status_code = 200
|
||||
mock_response.json.return_value = response_data
|
||||
mock_http_client.post.return_value = mock_response
|
||||
mock_client.return_value = mock_http_client
|
||||
|
||||
result = runner.invoke(
|
||||
app,
|
||||
["chat", "send", "-m", "test-model", "--no-stream"],
|
||||
input="Hello from stdin\n",
|
||||
)
|
||||
|
||||
assert result.exit_code == 0
|
||||
|
||||
|
||||
def test_chat_send_json_output(mock_config, mock_keyring):
|
||||
"""Test chat with JSON output format."""
|
||||
streaming_lines = [
|
||||
'data: {"choices": [{"delta": {"content": "Test"}}]}',
|
||||
"data: [DONE]",
|
||||
]
|
||||
|
||||
with patch("openwebui_cli.commands.chat.create_client") as mock_client:
|
||||
mock_stream = MockStreamResponse(streaming_lines)
|
||||
mock_http_client = MagicMock()
|
||||
mock_http_client.__enter__.return_value = mock_http_client
|
||||
mock_http_client.__exit__.return_value = None
|
||||
mock_http_client.stream.return_value = mock_stream
|
||||
mock_client.return_value = mock_http_client
|
||||
|
||||
result = runner.invoke(
|
||||
app,
|
||||
["chat", "send", "-m", "test-model", "-p", "Hello", "--json"],
|
||||
)
|
||||
|
||||
assert result.exit_code == 0
|
||||
assert "content" in result.stdout
|
||||
|
||||
|
||||
def test_chat_send_with_rag_context(mock_config, mock_keyring):
|
||||
"""Test chat with RAG file and collection context."""
|
||||
response_data = {
|
||||
"choices": [
|
||||
{
|
||||
"message": {
|
||||
"content": "Response with RAG context"
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
with patch("openwebui_cli.commands.chat.create_client") as mock_client:
|
||||
mock_http_client = MagicMock()
|
||||
mock_http_client.__enter__.return_value = mock_http_client
|
||||
mock_http_client.__exit__.return_value = None
|
||||
mock_response = Mock()
|
||||
mock_response.status_code = 200
|
||||
mock_response.json.return_value = response_data
|
||||
mock_http_client.post.return_value = mock_response
|
||||
mock_client.return_value = mock_http_client
|
||||
|
||||
result = runner.invoke(
|
||||
app,
|
||||
[
|
||||
"chat", "send",
|
||||
"-m", "test-model",
|
||||
"-p", "Search my docs",
|
||||
"--file", "file-123",
|
||||
"--collection", "coll-456",
|
||||
"--no-stream"
|
||||
],
|
||||
)
|
||||
|
||||
assert result.exit_code == 0
|
||||
# Verify the request was made with files context
|
||||
call_args = mock_http_client.post.call_args
|
||||
assert call_args is not None
|
||||
request_body = call_args.kwargs["json"]
|
||||
assert "files" in request_body
|
||||
assert len(request_body["files"]) == 2
|
||||
Loading…
Add table
Reference in a new issue