From bc412d6f59621cf9639d8ff06f093d3684589885 Mon Sep 17 00:00:00 2001 From: qin-ctx Date: Wed, 24 Jun 2026 14:09:53 +0800 Subject: [PATCH] fix(cli): preserve structured API error context --- crates/ov_cli/src/base_client.rs | 165 +++++++--- crates/ov_cli/src/client.rs | 103 +++--- crates/ov_cli/src/commands/chat.rs | 47 ++- crates/ov_cli/src/config_agent.rs | 64 +++- crates/ov_cli/src/config_command_ui.rs | 44 ++- crates/ov_cli/src/config_wizard/store.rs | 36 ++- crates/ov_cli/src/error.rs | 33 ++ crates/ov_cli/src/error_classifier.rs | 81 +++-- crates/ov_cli/src/error_ui.rs | 382 +++++++++++++++-------- crates/ov_cli/src/help_ui.rs | 25 ++ crates/ov_cli/src/status_ui.rs | 71 ++++- 11 files changed, 758 insertions(+), 293 deletions(-) diff --git a/crates/ov_cli/src/base_client.rs b/crates/ov_cli/src/base_client.rs index 1cf5e58ef5..b7fedaba6a 100644 --- a/crates/ov_cli/src/base_client.rs +++ b/crates/ov_cli/src/base_client.rs @@ -13,6 +13,40 @@ use indicatif::{ProgressBar, ProgressStyle}; use crate::error::{Error, Result}; +const ERROR_BODY_PREVIEW_CHARS: usize = 4096; + +pub(crate) fn classify_request_error(context: &str, error: reqwest::Error) -> Error { + let message = format!("{context}: {error}"); + if error.is_timeout() { + Error::Timeout(message) + } else if error.is_connect() { + Error::Network(message) + } else { + Error::Request(message) + } +} + +pub(crate) fn classify_response_error(context: &str, error: reqwest::Error) -> Error { + let message = format!("{context}: {error}"); + if error.is_timeout() { + Error::Timeout(message) + } else { + Error::Response(message) + } +} + +fn body_preview(bytes: &[u8]) -> String { + let body = String::from_utf8_lossy(bytes); + let mut preview = body + .chars() + .take(ERROR_BODY_PREVIEW_CHARS) + .collect::(); + if body.chars().count() > ERROR_BODY_PREVIEW_CHARS { + preview.push_str("..."); + } + preview +} + fn parse_ignore_dirs(ignore_dirs: Option<&str>) -> Vec { ignore_dirs .map(|s| { @@ -66,11 +100,12 @@ fn zip_entry_name(relative_path: &Path) -> Result { Ok(normalize_zip_entry_name(name)) } -pub fn api_error_from_envelope(json: &Value, status: StatusCode) -> String { +pub fn api_error_from_envelope(json: &Value, status: StatusCode) -> Error { let error_code = json .get("error") .and_then(|e| e.get("code")) - .and_then(|c| c.as_str()); + .and_then(|c| c.as_str()) + .map(str::to_string); let error_msg = json .get("error") .and_then(|e| e.get("message")) @@ -82,10 +117,27 @@ pub fn api_error_from_envelope(json: &Value, status: StatusCode) -> String { .map(|s| s.to_string()) }) .unwrap_or_else(|| format!("HTTP error {}", status)); + let details = json.get("error").and_then(|e| e.get("details")).cloned(); + + Error::api_structured( + error_msg, + Some(status.as_u16()), + error_code, + details.filter(|value| !value.is_null()), + ) +} - match error_code { - Some(code) => format!("[{}] {}", code, error_msg), - None => error_msg, +pub(crate) fn api_error_from_body(status: StatusCode, bytes: &[u8]) -> Error { + match serde_json::from_slice::(bytes) { + Ok(json) => api_error_from_envelope(&json, status), + Err(_) => Error::api_with_status( + format!( + "HTTP error {}\n\nRaw response body:\n{}", + status, + body_preview(bytes) + ), + status.as_u16(), + ), } } @@ -264,13 +316,19 @@ impl BaseClient { let bytes = response .bytes() .await - .map_err(|e| Error::Network(format!("Failed to read response body: {}", e)))?; + .map_err(|e| classify_response_error("Failed to read response body", e))?; let json: Value = match serde_json::from_slice(&bytes) { Ok(json) => json, Err(e) => { - let body_str = String::from_utf8_lossy(&bytes); - return Err(Error::Network(format!( + let body_str = body_preview(&bytes); + if !status.is_success() { + return Err(Error::api_with_status( + format!("HTTP error {}\n\nRaw response body:\n{}", status, body_str), + status.as_u16(), + )); + } + return Err(Error::Response(format!( "Failed to parse JSON response: {}\n\nRaw response body:\n{}", e, body_str ))); @@ -278,26 +336,12 @@ impl BaseClient { }; if !status.is_success() { - return Err(Error::api_with_status( - api_error_from_envelope(&json, status), - status.as_u16(), - )); + return Err(api_error_from_envelope(&json, status)); } if let Some(error) = json.get("error") { if !error.is_null() { - let code = error - .get("code") - .and_then(|c| c.as_str()) - .unwrap_or("UNKNOWN"); - let message = error - .get("message") - .and_then(|m| m.as_str()) - .unwrap_or("Unknown error"); - return Err(Error::api_with_status( - format!("[{}] {}", code, message), - status.as_u16(), - )); + return Err(api_error_from_envelope(&json, status)); } } @@ -319,7 +363,7 @@ impl BaseClient { ReqwestClient::builder() .timeout(timeout) .build() - .map_err(|e| Error::Network(format!("Failed to build HTTP client: {}", e))) + .map_err(|e| Error::Request(format!("Failed to build HTTP client: {}", e))) } pub(crate) fn create_client_with_connect_timeout( @@ -331,7 +375,7 @@ impl BaseClient { .connect_timeout(connect_timeout) .timeout(timeout) .build() - .map_err(|e| Error::Network(format!("Failed to build HTTP client: {}", e))) + .map_err(|e| Error::Request(format!("Failed to build HTTP client: {}", e))) } pub async fn get( @@ -348,7 +392,7 @@ impl BaseClient { .query(¶ms) .send() .await - .map_err(|e| Error::Network(format!("HTTP request failed: {}", e)))?; + .map_err(|e| classify_request_error("HTTP request failed", e))?; self.handle_response(response).await } @@ -372,7 +416,7 @@ impl BaseClient { let response = request .send() .await - .map_err(|e| Error::Network(format!("HTTP request failed: {}", e)))?; + .map_err(|e| classify_request_error("HTTP request failed", e))?; self.handle_response(response).await } @@ -395,7 +439,7 @@ impl BaseClient { let response = request .send() .await - .map_err(|e| Error::Network(format!("HTTP request failed: {}", e)))?; + .map_err(|e| classify_request_error("HTTP request failed", e))?; self.handle_response(response).await } @@ -415,7 +459,7 @@ impl BaseClient { let response = request .send() .await - .map_err(|e| Error::Network(format!("HTTP request failed: {}", e)))?; + .map_err(|e| classify_request_error("HTTP request failed", e))?; self.handle_response(response).await } @@ -434,7 +478,7 @@ impl BaseClient { .query(¶ms) .send() .await - .map_err(|e| Error::Network(format!("HTTP request failed: {}", e)))?; + .map_err(|e| classify_request_error("HTTP request failed", e))?; self.handle_response(response).await } @@ -458,7 +502,7 @@ impl BaseClient { let response = request .send() .await - .map_err(|e| Error::Network(format!("HTTP request failed: {}", e)))?; + .map_err(|e| classify_request_error("HTTP request failed", e))?; self.handle_response(response).await } @@ -479,7 +523,7 @@ impl BaseClient { .json(body) .send() .await - .map_err(|e| Error::Network(format!("HTTP request failed: {}", e)))?; + .map_err(|e| classify_request_error("HTTP request failed", e))?; self.handle_response(response).await } @@ -500,7 +544,7 @@ impl BaseClient { .json(body) .send() .await - .map_err(|e| Error::Network(format!("HTTP request failed: {}", e)))?; + .map_err(|e| classify_request_error("HTTP request failed", e))?; self.handle_response(response).await } @@ -656,6 +700,47 @@ mod tests { assert_eq!(entry, "scripts/check_bounding_boxes.py"); } + + #[test] + fn api_error_from_body_preserves_unknown_structured_code() { + let body = br#"{ + "status": "error", + "error": { + "code": "PROCESSING_ERROR", + "message": "Parse error: boom", + "details": {"task_id": "task-1"} + } + }"#; + + let error = api_error_from_body(StatusCode::INTERNAL_SERVER_ERROR, body); + + assert!(matches!( + error, + Error::Api { + message, + status: Some(500), + code: Some(code), + details: Some(details), + } if message == "Parse error: boom" + && code == "PROCESSING_ERROR" + && details["task_id"] == "task-1" + )); + } + + #[test] + fn api_error_from_body_keeps_non_json_error_body_as_api_error() { + let error = api_error_from_body(StatusCode::BAD_GATEWAY, b"upstream unavailable"); + + assert!(matches!( + error, + Error::Api { + message, + status: Some(502), + code: None, + .. + } if message.contains("upstream unavailable") + )); + } } // ============ FileUploader ============ @@ -685,7 +770,7 @@ impl<'a> FileUploader<'a> { ignore_dirs: Option<&str>, ) -> Result { if !dir_path.is_dir() { - return Err(Error::Network(format!( + return Err(Error::InvalidPath(format!( "Path {} is not a directory", dir_path.display() ))); @@ -725,7 +810,7 @@ impl<'a> FileUploader<'a> { ignore_dirs: Option<&str>, ) -> Result { if !dir_path.is_dir() { - return Err(Error::Network(format!( + return Err(Error::InvalidPath(format!( "Path {} is not a directory", dir_path.display() ))); @@ -833,7 +918,7 @@ impl<'a> FileUploader<'a> { let part = part .mime_str("application/octet-stream") - .map_err(|e| Error::Network(format!("Failed to set mime type: {}", e)))?; + .map_err(|e| Error::Request(format!("Failed to set mime type: {}", e)))?; let mut form = reqwest::multipart::Form::new().part("file", part); if let Some(upload_mode) = &self.upload_mode { @@ -855,7 +940,7 @@ impl<'a> FileUploader<'a> { .multipart(form) .send() .await - .map_err(|e| Error::Network(format!("File upload failed: {}", e)))?; + .map_err(|e| classify_request_error("File upload failed", e))?; let result: Value = self.client.handle_response(response).await?; result @@ -901,7 +986,7 @@ impl<'a> FileUploader<'a> { let part = part .mime_str("application/octet-stream") - .map_err(|e| Error::Network(format!("Failed to set mime type: {}", e)))?; + .map_err(|e| Error::Request(format!("Failed to set mime type: {}", e)))?; let mut form = reqwest::multipart::Form::new().part("file", part); if let Some(upload_mode) = &self.upload_mode { @@ -923,7 +1008,7 @@ impl<'a> FileUploader<'a> { .multipart(form) .send() .await - .map_err(|e| Error::Network(format!("File upload failed: {}", e)))?; + .map_err(|e| classify_request_error("File upload failed", e))?; pb.finish_with_message("Upload complete"); diff --git a/crates/ov_cli/src/client.rs b/crates/ov_cli/src/client.rs index 63b87e47f5..48b65c971c 100644 --- a/crates/ov_cli/src/client.rs +++ b/crates/ov_cli/src/client.rs @@ -5,6 +5,7 @@ use std::path::Path; pub use crate::base_client::{BaseClient, FileUploader, TimeoutConfig}; +use crate::base_client::{api_error_from_body, classify_request_error, classify_response_error}; use crate::error::{Error, Result}; /// Drop null-valued keys (and an empty `args` object) from a request body before @@ -319,41 +320,23 @@ impl HttpClient { .query(¶ms) .send() .await - .map_err(|e| Error::Network(format!("HTTP request failed: {}", e)))?; + .map_err(|e| classify_request_error("HTTP request failed", e))?; let status = response.status(); if !status.is_success() { let bytes = response .bytes() .await - .map_err(|e| Error::Network(format!("Failed to read error response: {}", e)))?; - - let error_msg = match serde_json::from_slice::(&bytes) { - Ok(json) => json - .get("error") - .and_then(|e| e.get("message")) - .and_then(|m| m.as_str()) - .map(|s| s.to_string()) - .or_else(|| { - json.get("detail") - .and_then(|d| d.as_str()) - .map(|s| s.to_string()) - }) - .unwrap_or_else(|| format!("HTTP error {}", status)), - Err(_) => { - let body_str = String::from_utf8_lossy(&bytes); - format!("HTTP error {}\n\nRaw response body:\n{}", status, body_str) - } - }; + .map_err(|e| classify_response_error("Failed to read error response", e))?; - return Err(Error::api(error_msg)); + return Err(api_error_from_body(status, &bytes)); } response .bytes() .await .map(|b| b.to_vec()) - .map_err(|e| Error::Network(format!("Failed to read response bytes: {}", e))) + .map_err(|e| classify_response_error("Failed to read response bytes", e)) } // ============ Filesystem Methods ============ @@ -1036,40 +1019,22 @@ impl HttpClient { .query(&[("profile", "0")]) .send() .await - .map_err(|e| Error::Network(format!("HTTP request failed: {}", e)))?; + .map_err(|e| classify_request_error("HTTP request failed", e))?; let status = response.status(); if !status.is_success() { let bytes = response .bytes() .await - .map_err(|e| Error::Network(format!("Failed to read error response: {}", e)))?; - - let error_msg = match serde_json::from_slice::(&bytes) { - Ok(json) => json - .get("error") - .and_then(|e| e.get("message")) - .and_then(|m| m.as_str()) - .map(|s| s.to_string()) - .or_else(|| { - json.get("detail") - .and_then(|d| d.as_str()) - .map(|s| s.to_string()) - }) - .unwrap_or_else(|| format!("HTTP error {}", status)), - Err(_) => { - let body_str = String::from_utf8_lossy(&bytes); - format!("HTTP error {}\n\nRaw response body:\n{}", status, body_str) - } - }; + .map_err(|e| classify_response_error("Failed to read error response", e))?; - return Err(Error::api(error_msg)); + return Err(api_error_from_body(status, &bytes)); } let bytes = response .bytes() .await - .map_err(|e| Error::Network(format!("Failed to read response bytes: {}", e)))?; + .map_err(|e| classify_response_error("Failed to read response bytes", e))?; let to_path = Path::new(to); let final_path = if to_path.is_dir() { @@ -1454,6 +1419,7 @@ impl HttpClient { mod tests { use super::{BaseClient, HttpClient, TimeoutConfig}; use crate::base_client::api_error_from_envelope; + use crate::error::Error; use reqwest::StatusCode; use serde_json::json; use std::collections::HashMap; @@ -1634,6 +1600,41 @@ mod tests { assert_eq!(body["agent_id"], json!("legacy-agent")); } + #[tokio::test] + async fn request_timeout_maps_to_timeout_error() { + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("test server should bind"); + let addr = listener.local_addr().expect("test server should have addr"); + tokio::spawn(async move { + let Ok((mut stream, _)) = listener.accept().await else { + return; + }; + let mut buffer = vec![0; 1024]; + let _ = stream.read(&mut buffer).await; + tokio::time::sleep(std::time::Duration::from_millis(500)).await; + }); + + let client = HttpClient::new( + format!("http://{addr}"), + None, + None, + None, + None, + None, + 0.05, + false, + None, + ); + + let error = client + .get::("/health", &[]) + .await + .expect_err("request should time out"); + + assert!(matches!(error, Error::Timeout(_))); + } + #[test] fn standard_error_envelope_formats_api_error() { let body = json!({ @@ -1644,10 +1645,16 @@ mod tests { } }); - assert_eq!( - api_error_from_envelope(&body, StatusCode::INTERNAL_SERVER_ERROR), - "[PROCESSING_ERROR] Parse error: boom" - ); + let error = api_error_from_envelope(&body, StatusCode::INTERNAL_SERVER_ERROR); + assert!(matches!( + error, + Error::Api { + message, + status: Some(500), + code: Some(code), + .. + } if message == "Parse error: boom" && code == "PROCESSING_ERROR" + )); } #[test] diff --git a/crates/ov_cli/src/commands/chat.rs b/crates/ov_cli/src/commands/chat.rs index b64df88be7..a171eab29b 100644 --- a/crates/ov_cli/src/commands/chat.rs +++ b/crates/ov_cli/src/commands/chat.rs @@ -18,6 +18,7 @@ use termimad::MadSkin; use crate::config::Config; use crate::utils; +use crate::base_client::{api_error_from_body, classify_request_error, classify_response_error}; use crate::error::{Error, Result}; const DEFAULT_ENDPOINT: &str = "http://localhost:1933/bot/v1"; @@ -127,7 +128,7 @@ impl ChatCommand { let client = Client::builder() .timeout(Duration::from_secs(300)) .build() - .map_err(|e| Error::Network(format!("Failed to create HTTP client: {}", e)))?; + .map_err(|e| Error::Request(format!("Failed to create HTTP client: {}", e)))?; let health = self.fetch_openviking_health(&client, None).await; let auth = self.resolve_auth(health.as_ref().map(health_auth_mode))?; @@ -305,18 +306,21 @@ User/Admin API key. OpenViking-backed VikingBot memory and file tools may not wo let response = req_builder .send() .await - .map_err(|e| Error::Network(format!("Failed to send request: {}", e)))?; + .map_err(|e| classify_request_error("Failed to send request", e))?; if !response.status().is_success() { let status = response.status(); - let text = response.text().await.unwrap_or_default(); - return Err(Error::api(format!("Request failed ({}): {}", status, text))); + let bytes = response + .bytes() + .await + .map_err(|e| classify_response_error("Failed to read error response", e))?; + return Err(api_error_from_body(status, &bytes)); } let chat_response: ChatResponse = response .json() .await - .map_err(|e| Error::Parse(format!("Failed to parse response: {}", e)))?; + .map_err(|e| classify_response_error("Failed to parse response", e))?; // Print events if any self.print_events(&chat_response.events); @@ -349,12 +353,15 @@ User/Admin API key. OpenViking-backed VikingBot memory and file tools may not wo let response = req_builder .send() .await - .map_err(|e| Error::Network(format!("Failed to send request: {}", e)))?; + .map_err(|e| classify_request_error("Failed to send request", e))?; if !response.status().is_success() { let status = response.status(); - let text = response.text().await.unwrap_or_default(); - return Err(Error::api(format!("Request failed ({}): {}", status, text))); + let bytes = response + .bytes() + .await + .map_err(|e| classify_response_error("Failed to read error response", e))?; + return Err(api_error_from_body(status, &bytes)); } // Process the SSE stream @@ -366,7 +373,7 @@ User/Admin API key. OpenViking-backed VikingBot memory and file tools may not wo while let Some(chunk) = response .chunk() .await - .map_err(|e| Error::Network(format!("Stream error: {}", e)))? + .map_err(|e| classify_response_error("Stream error", e))? { let chunk_str = String::from_utf8_lossy(&chunk); buffer.push_str(&chunk_str); @@ -542,18 +549,21 @@ User/Admin API key. OpenViking-backed VikingBot memory and file tools may not wo let response = req_builder .send() .await - .map_err(|e| Error::Network(format!("Failed to send request: {}", e)))?; + .map_err(|e| classify_request_error("Failed to send request", e))?; if !response.status().is_success() { let status = response.status(); - let text = response.text().await.unwrap_or_default(); - return Err(Error::api(format!("Request failed ({}): {}", status, text))); + let bytes = response + .bytes() + .await + .map_err(|e| classify_response_error("Failed to read error response", e))?; + return Err(api_error_from_body(status, &bytes)); } let chat_response: ChatResponse = response .json() .await - .map_err(|e| Error::Parse(format!("Failed to parse response: {}", e)))?; + .map_err(|e| classify_response_error("Failed to parse response", e))?; // Save session ID if session_id.is_none() { @@ -595,12 +605,15 @@ User/Admin API key. OpenViking-backed VikingBot memory and file tools may not wo let response = req_builder .send() .await - .map_err(|e| Error::Network(format!("Failed to send request: {}", e)))?; + .map_err(|e| classify_request_error("Failed to send request", e))?; if !response.status().is_success() { let status = response.status(); - let text = response.text().await.unwrap_or_default(); - return Err(Error::api(format!("Request failed ({}): {}", status, text))); + let bytes = response + .bytes() + .await + .map_err(|e| classify_response_error("Failed to read error response", e))?; + return Err(api_error_from_body(status, &bytes)); } let mut response = response; @@ -615,7 +628,7 @@ User/Admin API key. OpenViking-backed VikingBot memory and file tools may not wo while let Some(chunk) = response .chunk() .await - .map_err(|e| Error::Network(format!("Stream error: {}", e)))? + .map_err(|e| classify_response_error("Stream error", e))? { let chunk_str = String::from_utf8_lossy(&chunk); buffer.push_str(&chunk_str); diff --git a/crates/ov_cli/src/config_agent.rs b/crates/ov_cli/src/config_agent.rs index 97ebd52fbc..f17880662f 100644 --- a/crates/ov_cli/src/config_agent.rs +++ b/crates/ov_cli/src/config_agent.rs @@ -20,6 +20,7 @@ use crate::{ validate_user_id_value, }, error::Error, + error_classifier::{ApiErrorKind, api_error_kind}, output::OutputFormat, theme, }; @@ -547,6 +548,7 @@ async fn validate_config_for_write( kind: ConfigKind, require_api_key: bool, ) -> AgentResult<()> { + let api_key_is_validated_root_key = root_as_normal(config); if let Some(root_key) = config.root_api_key.as_deref() { let mut root_probe = config.clone(); root_probe.api_key = Some(root_key.to_string()); @@ -561,9 +563,13 @@ async fn validate_config_for_write( } } - let api_key_role = match validate_candidate_config_with_role(config, require_api_key).await { - Ok(role) => role, - Err(error) => return Err(validation_error(kind, error)), + let api_key_role = if api_key_is_validated_root_key { + Some(ApiKeyRole::Root) + } else { + match validate_candidate_config_with_role(config, require_api_key).await { + Ok(role) => role, + Err(error) => return Err(validation_error(kind, error)), + } }; if kind == ConfigKind::Custom @@ -919,18 +925,58 @@ fn validate_optional_actor_peer_id(actor_peer_id: Option<&str>) -> AgentResult<( fn validation_error(kind: ConfigKind, error: Error) -> AgentError { match error { - Error::Api { message, .. } => AgentError::auth(format!( + Error::Api { + message, + code, + status, + .. + } => match api_error_kind(code.as_deref(), status) { + ApiErrorKind::Authentication => AgentError::auth(format!( + "{} {message}", + match kind { + ConfigKind::OpenVikingService => "Check the API key.", + ConfigKind::Custom => "Check the API key, account, and user.", + } + )), + ApiErrorKind::Permission => AgentError::validation(format!( + "{} {message}", + match kind { + ConfigKind::OpenVikingService => { + "OpenViking Service rejected this identity's permissions." + } + ConfigKind::Custom => "The custom server rejected this identity's permissions.", + } + )), + _ => AgentError::validation(format!("Validation failed: {message}")), + }, + Error::Network(message) => AgentError::validation(format!( "{} {message}", match kind { - ConfigKind::OpenVikingService => "Check the API key.", - ConfigKind::Custom => "Check the API key, account, and user.", + ConfigKind::OpenVikingService => "Could not reach OpenViking Service.", + ConfigKind::Custom => "Could not reach the custom server.", } )), - Error::Network(message) => AgentError::validation(format!( + Error::Timeout(message) => AgentError::validation(format!( "{} {message}", match kind { - ConfigKind::OpenVikingService => "Could not reach OpenViking Service.", - ConfigKind::Custom => "Could not reach the custom server.", + ConfigKind::OpenVikingService => + "OpenViking Service did not respond before the timeout.", + ConfigKind::Custom => "The custom server did not respond before the timeout.", + } + )), + Error::Request(message) => AgentError::validation(format!( + "{} {message}", + match kind { + ConfigKind::OpenVikingService => + "Could not send the request to OpenViking Service.", + ConfigKind::Custom => "Could not send the request to the custom server.", + } + )), + Error::ServerUnhealthy(message) => AgentError::validation(format!( + "{} {message}", + match kind { + ConfigKind::OpenVikingService => "OpenViking Service is reachable but unhealthy.", + ConfigKind::Custom => "The custom server is reachable but unhealthy.", } )), Error::Config(message) => AgentError::bad_input(message), diff --git a/crates/ov_cli/src/config_command_ui.rs b/crates/ov_cli/src/config_command_ui.rs index fee08d1ef7..0eac25e5a7 100644 --- a/crates/ov_cli/src/config_command_ui.rs +++ b/crates/ov_cli/src/config_command_ui.rs @@ -5,7 +5,7 @@ use crate::{ config::{Config, display_config_home}, config_wizard::ConfigKind, error::Error, - error_classifier::looks_like_auth_error, + error_classifier::{ApiErrorKind, api_error_kind}, i18n::{Language, copy}, theme, }; @@ -406,6 +406,9 @@ fn unknown_value(value: &str) -> String { enum ValidationFailureKind { Network, Auth, + Permission, + Timeout, + Response, Unhealthy, Other, } @@ -413,9 +416,15 @@ enum ValidationFailureKind { impl ValidationFailureKind { fn from_error(error: &Error) -> Self { match error { - Error::Network(message) if message.contains("unhealthy") => Self::Unhealthy, + Error::ServerUnhealthy(_) => Self::Unhealthy, Error::Network(_) => Self::Network, - Error::Api { message, .. } if looks_like_auth_error(message) => Self::Auth, + Error::Timeout(_) => Self::Timeout, + Error::Response(_) => Self::Response, + Error::Api { code, status, .. } => match api_error_kind(code.as_deref(), *status) { + ApiErrorKind::Authentication => Self::Auth, + ApiErrorKind::Permission => Self::Permission, + _ => Self::Other, + }, _ => Self::Other, } } @@ -423,25 +432,34 @@ impl ValidationFailureKind { fn server_check(self, language: Language) -> String { match self { Self::Network => fail_value(copy(language, "unreachable", "无法连接")), - Self::Auth | Self::Unhealthy => ok_value(copy(language, "reachable", "可连接")), - Self::Other => warn_value(unknown(language)), + Self::Auth | Self::Permission | Self::Unhealthy => { + ok_value(copy(language, "reachable", "可连接")) + } + Self::Response => warn_value(copy(language, "reachable", "可连接")), + Self::Timeout => warn_value(copy(language, "timed out", "请求超时")), + Self::Other => warn_value(copy(language, "not checked", "未检查")), } } fn auth_check(self, language: Language) -> String { match self { Self::Auth => fail_value(copy(language, "rejected", "被拒绝")), - Self::Network => warn_value(copy(language, "not checked", "未检查")), + Self::Permission => fail_value(copy(language, "permission denied", "权限不足")), + Self::Network | Self::Timeout | Self::Response => { + warn_value(copy(language, "not checked", "未检查")) + } Self::Unhealthy => ok_value(copy(language, "accepted", "已通过")), - Self::Other => warn_value(unknown(language)), + Self::Other => warn_value(copy(language, "not checked", "未检查")), } } fn health_check(self, language: Language) -> String { match self { Self::Unhealthy => fail_value(copy(language, "unhealthy", "不健康")), - Self::Network | Self::Auth => warn_value(copy(language, "not checked", "未检查")), - Self::Other => warn_value(unknown(language)), + Self::Network | Self::Auth | Self::Permission | Self::Timeout | Self::Response => { + warn_value(copy(language, "not checked", "未检查")) + } + Self::Other => warn_value(copy(language, "not checked", "未检查")), } } @@ -450,12 +468,20 @@ impl ValidationFailureKind { Language::En => match self { Self::Network => "Could not reach the configured OpenViking server.", Self::Auth => "OpenViking rejected the API key for this config.", + Self::Permission => "OpenViking rejected this config's permissions.", + Self::Timeout => "OpenViking did not respond before the timeout.", + Self::Response => { + "OpenViking responded, but the CLI could not understand the response." + } Self::Unhealthy => "OpenViking is reachable but reported an unhealthy state.", Self::Other => "The active config could not be validated.", }, Language::ZhCn => match self { Self::Network => "无法连接已配置的 OpenViking 服务器。", Self::Auth => "OpenViking 拒绝了这个配置的 API Key。", + Self::Permission => "OpenViking 拒绝了这个配置的权限。", + Self::Timeout => "OpenViking 未能在超时时间内响应。", + Self::Response => "OpenViking 已响应，但 CLI 无法理解响应内容。", Self::Unhealthy => "服务器可连接，但健康状态异常。", Self::Other => "当前配置验证失败。", }, diff --git a/crates/ov_cli/src/config_wizard/store.rs b/crates/ov_cli/src/config_wizard/store.rs index 01c2340cae..ec303d9393 100644 --- a/crates/ov_cli/src/config_wizard/store.rs +++ b/crates/ov_cli/src/config_wizard/store.rs @@ -590,7 +590,7 @@ pub(crate) async fn validate_candidate_config_with_role( .and_then(Value::as_bool) .is_some_and(|healthy| !healthy) { - return Err(Error::Network( + return Err(Error::ServerUnhealthy( "Server responded but reported unhealthy status".to_string(), )); } @@ -613,7 +613,17 @@ async fn detect_api_key_role(client: &BaseClient) -> Result { status: Some(status), .. }) if admin_probe_regular_key_status(status) => Ok(ApiKeyRole::Regular), - Err(Error::Api { message, status }) => Err(Error::Api { message, status }), + Err(Error::Api { + message, + status, + code, + details, + }) => Err(Error::Api { + message, + status, + code, + details, + }), Err(error) => Err(error), } } @@ -653,9 +663,22 @@ fn should_run_authenticated_probe( pub(crate) fn validation_error_copy(kind: ConfigKind, error: &Error) -> String { match error { + Error::ServerUnhealthy(_) => { + "Server is reachable but reported unhealthy status. Check the server logs.".to_string() + } Error::Network(msg) if msg.contains("unhealthy") => { "Server is reachable but reported unhealthy status. Check the server logs.".to_string() } + Error::Timeout(_) => match kind { + ConfigKind::OpenVikingService => { + "OpenViking Service did not respond before the timeout. Check the service status and try again." + .to_string() + } + ConfigKind::Custom => { + "Server did not respond before the timeout. Check the URL, server status, and timeout setting." + .to_string() + } + }, Error::Network(_) => match kind { ConfigKind::OpenVikingService => { "Cannot reach OpenViking Service. Check your network connection.".to_string() @@ -691,9 +714,18 @@ pub(crate) fn validation_error_copy(kind: ConfigKind, error: &Error) -> String { pub(crate) fn validation_error_copy_zh(kind: ConfigKind, error: &Error) -> String { match error { + Error::ServerUnhealthy(_) => "服务器可连接，但健康状态异常。请检查服务器日志。".to_string(), Error::Network(msg) if msg.contains("unhealthy") => { "服务器可连接，但健康状态异常。请检查服务器日志。".to_string() } + Error::Timeout(_) => match kind { + ConfigKind::OpenVikingService => { + "OpenViking 服务未能在超时时间内响应。请检查服务状态后重试。".to_string() + } + ConfigKind::Custom => { + "服务器未能在超时时间内响应。请检查 URL、服务器状态和超时配置。".to_string() + } + }, Error::Network(_) => match kind { ConfigKind::OpenVikingService => { "无法连接 OpenViking 服务。请检查网络连接。".to_string() diff --git a/crates/ov_cli/src/error.rs b/crates/ov_cli/src/error.rs index a73950278a..e0d35f4eb6 100644 --- a/crates/ov_cli/src/error.rs +++ b/crates/ov_cli/src/error.rs @@ -1,3 +1,4 @@ +use serde_json::Value; use thiserror::Error; #[derive(Error, Debug)] @@ -14,10 +15,24 @@ pub enum Error { #[error("Network error: {0}")] Network(String), + #[error("Request timeout: {0}")] + Timeout(String), + + #[error("Request error: {0}")] + Request(String), + + #[error("Response error: {0}")] + Response(String), + + #[error("Server unhealthy: {0}")] + ServerUnhealthy(String), + #[error("API error: {message}")] Api { message: String, status: Option, + code: Option, + details: Option, }, #[error("Client error: {0}")] @@ -50,6 +65,8 @@ impl Error { Self::Api { message: message.into(), status: None, + code: None, + details: None, } } @@ -57,6 +74,22 @@ impl Error { Self::Api { message: message.into(), status: Some(status), + code: None, + details: None, + } + } + + pub fn api_structured( + message: impl Into, + status: Option, + code: Option>, + details: Option, + ) -> Self { + Self::Api { + message: message.into(), + status, + code: code.map(Into::into), + details, } } } diff --git a/crates/ov_cli/src/error_classifier.rs b/crates/ov_cli/src/error_classifier.rs index 8f22880fc4..855c5a7451 100644 --- a/crates/ov_cli/src/error_classifier.rs +++ b/crates/ov_cli/src/error_classifier.rs @@ -1,12 +1,42 @@ -pub(crate) fn looks_like_auth_error(message: &str) -> bool { - let lower = message.to_ascii_lowercase(); - lower.contains("api key") - || lower.contains("unauthorized") - || lower.contains("forbidden") - || lower.contains("authentication") - || lower - .split(|ch: char| !ch.is_ascii_alphanumeric()) - .any(|token| token == "auth") +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum ApiErrorKind { + Authentication, + Permission, + InvalidRequest, + NotFound, + Conflict, + ResourceExhausted, + DeadlineExceeded, + Unavailable, + Other, +} + +pub(crate) fn api_error_kind(code: Option<&str>, status: Option) -> ApiErrorKind { + match code { + Some("UNAUTHENTICATED") => return ApiErrorKind::Authentication, + Some("PERMISSION_DENIED") => return ApiErrorKind::Permission, + Some("INVALID_ARGUMENT" | "INVALID_URI" | "UNSUPPORTED_URI" | "UNSUPPORTED_MODE") => { + return ApiErrorKind::InvalidRequest; + } + Some("NOT_FOUND") => return ApiErrorKind::NotFound, + Some("CONFLICT" | "ALREADY_EXISTS" | "ABORTED") => return ApiErrorKind::Conflict, + Some("RESOURCE_EXHAUSTED") => return ApiErrorKind::ResourceExhausted, + Some("DEADLINE_EXCEEDED") => return ApiErrorKind::DeadlineExceeded, + Some("UNAVAILABLE") => return ApiErrorKind::Unavailable, + _ => {} + } + + match status { + Some(401) => ApiErrorKind::Authentication, + Some(403) => ApiErrorKind::Permission, + Some(400 | 422) => ApiErrorKind::InvalidRequest, + Some(404) => ApiErrorKind::NotFound, + Some(409) => ApiErrorKind::Conflict, + Some(429) => ApiErrorKind::ResourceExhausted, + Some(504) => ApiErrorKind::DeadlineExceeded, + Some(502 | 503) => ApiErrorKind::Unavailable, + _ => ApiErrorKind::Other, + } } /// Detect the kernel's pydantic `extra="forbid"` rejection — e.g. @@ -33,7 +63,7 @@ pub(crate) fn extra_forbidden_field(message: &str) -> Option { #[cfg(test)] mod tests { - use super::{extra_forbidden_field, looks_like_auth_error}; + use super::{ApiErrorKind, api_error_kind, extra_forbidden_field}; #[test] fn extracts_extra_forbidden_field() { @@ -56,22 +86,19 @@ mod tests { } #[test] - fn detects_auth_errors() { - for message in [ - "API key is invalid", - "request was unauthorized", - "forbidden", - "authentication failed", - "auth failed", - ] { - assert!(looks_like_auth_error(message), "{message}"); - } - } - - #[test] - fn avoids_auth_substring_false_positives() { - for message in ["author not found", "authority unavailable"] { - assert!(!looks_like_auth_error(message), "{message}"); - } + fn classifies_known_api_codes_without_message_matching() { + assert_eq!( + api_error_kind(Some("UNAUTHENTICATED"), Some(401)), + ApiErrorKind::Authentication + ); + assert_eq!( + api_error_kind(Some("PERMISSION_DENIED"), Some(403)), + ApiErrorKind::Permission + ); + assert_eq!( + api_error_kind(Some("PROCESSING_ERROR"), Some(500)), + ApiErrorKind::Other + ); + assert_eq!(api_error_kind(None, Some(404)), ApiErrorKind::NotFound); } } diff --git a/crates/ov_cli/src/error_ui.rs b/crates/ov_cli/src/error_ui.rs index 8dd8d7fb6a..ca10ce1277 100644 --- a/crates/ov_cli/src/error_ui.rs +++ b/crates/ov_cli/src/error_ui.rs @@ -5,7 +5,7 @@ use unicode_width::UnicodeWidthStr; use crate::{ error::Error, - error_classifier::{extra_forbidden_field, looks_like_auth_error}, + error_classifier::{ApiErrorKind, api_error_kind, extra_forbidden_field}, i18n::{Language, copy}, terminal_ui::{fit_to_display_width, truncate_to_display_width}, theme, @@ -231,47 +231,67 @@ pub(crate) fn report_for_runtime_error(command: impl Into, error: &Error ErrorAction::new("ov health", copy(language, "Run a quick server health check", "快速检查服务器健康状态")), ErrorAction::new("ov config switch", copy(language, "Switch to another config", "切换到其他配置")), ]), - Error::Api { message, .. } if looks_like_auth_error(message) => ErrorReport::new( - copy(language, "Authentication Error", "认证错误"), - copy(language, "OpenViking rejected the API key for the active config.", "OpenViking 拒绝了当前配置的 API Key。"), + Error::Timeout(message) => ErrorReport::new( + copy(language, "Request Timeout", "请求超时"), + copy( + language, + "OpenViking did not return a response before the configured timeout.", + "OpenViking 未能在当前超时时间内返回响应。", + ), ) - .with_command(command) + .with_command(command.clone()) .with_detail(message) .with_actions(vec![ - ErrorAction::new("ov config", copy(language, "Edit this config", "编辑这个配置")), - ErrorAction::new("ov config switch", copy(language, "Use another config", "使用其他配置")), + ErrorAction::new("ov status", copy(language, "Check OpenViking status", "查看 OpenViking 状态")), + ErrorAction::new("ov config show", copy(language, "Show the active timeout config", "显示当前超时配置")), ]), - Error::Api { message, .. } if extra_forbidden_field(message).is_some() => { - let field = extra_forbidden_field(message).unwrap_or_default(); - ErrorReport::new( - copy(language, "OpenViking API Error", "OpenViking API 错误"), - match language { - Language::En => format!( - "OpenViking rejected an unsupported field \"{field}\". This instance's version likely does not match your CLI (the field may be missing, renamed, or removed)." - ), - Language::ZhCn => format!( - "OpenViking 拒绝了不支持的字段 \"{field}\"：该实例版本可能与当前 CLI 不匹配（字段可能缺失、改名或已被移除）。" - ), - }, - ) - .with_command(command) - .with_detail(message) - .with_actions(vec![ - ErrorAction::new("ov health", copy(language, "Check the instance version", "查看实例版本")), - ErrorAction::new("ov config validate", copy(language, "Check the active config", "检查当前配置")), - ErrorAction::new("ov status", copy(language, "Check OpenViking status", "查看 OpenViking 状态")), - ]) - } - Error::Api { message, .. } => ErrorReport::new( - copy(language, "OpenViking API Error", "OpenViking API 错误"), - api_error_message(language, message), + Error::Request(message) => ErrorReport::new( + copy(language, "Request Error", "请求错误"), + copy( + language, + "The CLI could not build or send this request.", + "CLI 无法构造或发送这个请求。", + ), ) - .with_command(command) + .with_command(command.clone()) + .with_detail(message) + .with_actions(vec![ + ErrorAction::new("ov config show", copy(language, "Show the active config", "显示当前配置")), + ]), + Error::Response(message) => ErrorReport::new( + copy(language, "Response Error", "响应错误"), + copy( + language, + "OpenViking responded, but the CLI could not read or understand the response.", + "OpenViking 已返回响应，但 CLI 无法读取或理解响应内容。", + ), + ) + .with_command(command.clone()) .with_detail(message) .with_actions(vec![ - ErrorAction::new("ov config validate", copy(language, "Check the active config", "检查当前配置")), ErrorAction::new("ov status", copy(language, "Check OpenViking status", "查看 OpenViking 状态")), + ErrorAction::new("ov health", copy(language, "Run a quick server health check", "快速检查服务器健康状态")), + ]), + Error::ServerUnhealthy(message) => ErrorReport::new( + copy(language, "Server Unhealthy", "服务不健康"), + copy( + language, + "OpenViking is reachable, but it reported an unhealthy status.", + "OpenViking 可连接，但服务报告当前状态不健康。", + ), + ) + .with_command(command) + .with_detail(message) + .with_actions(vec![ + ErrorAction::new("ov status", copy(language, "Check component status", "查看组件状态")), + ErrorAction::new("ov health", copy(language, "Run a quick health check", "快速检查健康状态")), ]), + Error::Api { + message, + status, + code, + details, + } => api_error_report(command, language, code.as_deref(), *status, message, details.as_ref()), Error::Client(message) => ErrorReport::new( copy(language, "Command Error", "命令错误"), sentence_case_error(message), @@ -331,6 +351,168 @@ fn config_error_actions(message: &str, language: Language) -> Vec { ] } +fn api_error_report( + command: String, + language: Language, + code: Option<&str>, + status: Option, + message: &str, + details: Option<&serde_json::Value>, +) -> ErrorReport { + if let Some(field) = extra_forbidden_field(message) { + return ErrorReport::new( + copy(language, "OpenViking API Error", "OpenViking API 错误"), + match language { + Language::En => format!( + "OpenViking rejected an unsupported field \"{field}\". This instance's version likely does not match your CLI (the field may be missing, renamed, or removed)." + ), + Language::ZhCn => format!( + "OpenViking 拒绝了不支持的字段 \"{field}\"：该实例版本可能与当前 CLI 不匹配（字段可能缺失、改名或已被移除）。" + ), + }, + ) + .with_command(command) + .with_detail(api_error_detail(code, status, message, details)) + .with_actions(vec![ + ErrorAction::new( + "ov health", + copy(language, "Check the instance version", "查看实例版本"), + ), + ErrorAction::new( + "ov config validate", + copy(language, "Check the active config", "检查当前配置"), + ), + ErrorAction::new( + "ov status", + copy(language, "Check OpenViking status", "查看 OpenViking 状态"), + ), + ]); + } + + let kind = api_error_kind(code, status); + let (title, fallback) = match kind { + ApiErrorKind::Authentication => ( + copy(language, "Authentication Error", "认证错误"), + copy( + language, + "OpenViking rejected the credentials for the active config.", + "OpenViking 拒绝了当前配置的凭证。", + ), + ), + ApiErrorKind::Permission => ( + copy(language, "Permission Error", "权限错误"), + copy( + language, + "OpenViking denied this request for the active identity.", + "OpenViking 拒绝了当前身份执行这个请求。", + ), + ), + ApiErrorKind::InvalidRequest => ( + copy(language, "Request Error", "请求错误"), + copy( + language, + "OpenViking rejected the request parameters.", + "OpenViking 拒绝了请求参数。", + ), + ), + ApiErrorKind::NotFound => ( + copy(language, "Not Found", "资源不存在"), + copy( + language, + "OpenViking could not find the requested resource.", + "OpenViking 找不到请求的资源。", + ), + ), + ApiErrorKind::Conflict => ( + copy(language, "Conflict", "冲突"), + copy( + language, + "OpenViking reported a conflicting operation or state.", + "OpenViking 报告当前操作或状态存在冲突。", + ), + ), + ApiErrorKind::ResourceExhausted => ( + copy(language, "Resource Exhausted", "资源耗尽"), + copy( + language, + "OpenViking reported a quota or rate-limit error.", + "OpenViking 返回了配额或限流错误。", + ), + ), + ApiErrorKind::DeadlineExceeded => ( + copy(language, "Server Timeout", "服务端超时"), + copy( + language, + "OpenViking exceeded its server-side processing deadline.", + "OpenViking 超过了服务端处理时限。", + ), + ), + ApiErrorKind::Unavailable => ( + copy(language, "Service Unavailable", "服务不可用"), + copy( + language, + "OpenViking reported that a required service is unavailable.", + "OpenViking 报告依赖服务当前不可用。", + ), + ), + ApiErrorKind::Other => ( + copy(language, "OpenViking API Error", "OpenViking API 错误"), + copy( + language, + "OpenViking returned an error for this request.", + "OpenViking 返回了请求错误。", + ), + ), + }; + + ErrorReport::new( + title, + api_error_message(language, code, message).unwrap_or_else(|| fallback.to_string()), + ) + .with_command(command) + .with_detail(api_error_detail(code, status, message, details)) + .with_actions(api_error_actions(kind, language)) +} + +fn api_error_actions(kind: ApiErrorKind, language: Language) -> Vec { + match kind { + ApiErrorKind::Authentication => vec![ + ErrorAction::new( + "ov config", + copy(language, "Edit this config", "编辑这个配置"), + ), + ErrorAction::new( + "ov config switch", + copy(language, "Use another config", "使用其他配置"), + ), + ], + ApiErrorKind::Permission => vec![ + ErrorAction::new( + "ov config show", + copy(language, "Show the active identity", "显示当前身份"), + ), + ErrorAction::new( + "ov config switch", + copy(language, "Use another config", "使用其他配置"), + ), + ], + ApiErrorKind::Unavailable | ApiErrorKind::DeadlineExceeded => vec![ + ErrorAction::new( + "ov status", + copy(language, "Check OpenViking status", "查看 OpenViking 状态"), + ), + ErrorAction::new( + "ov health", + copy(language, "Run a quick health check", "快速检查健康状态"), + ), + ], + _ => vec![ErrorAction::new( + "ov status", + copy(language, "Check OpenViking status", "查看 OpenViking 状态"), + )], + } +} + fn is_config_file_load_error(message: &str) -> bool { message.starts_with("Failed to read config file:") || message.starts_with("Failed to parse config file:") @@ -511,79 +693,42 @@ fn is_contextual_top_level_command(command: &str) -> bool { ) } -fn api_error_message(language: Language, raw: &str) -> String { - let Some(summary) = summarize_api_error(raw) else { - return copy( - language, - "OpenViking returned an error for this request.", - "OpenViking 返回了请求错误。", - ) - .to_string(); +fn api_error_message(language: Language, code: Option<&str>, raw: &str) -> Option { + let message = strip_request_id(raw); + let message = message.lines().next().unwrap_or_default().trim(); + let summary = match (code, message.is_empty()) { + (Some(code), true) => code.to_string(), + (Some(code), false) => format!("{code}: {}", ensure_sentence(message)), + (None, true) => return None, + (None, false) => ensure_sentence(message), }; - match language { + Some(match language { Language::En => format!("OpenViking returned an error: {summary}"), Language::ZhCn => format!("OpenViking 返回了请求错误：{summary}"), - } + }) } -fn summarize_api_error(raw: &str) -> Option { - let raw = raw.trim(); - if raw.is_empty() { - return None; - } - - if let Some(summary) = summarize_json_api_error(raw) { - return Some(summary); - } - - let cleaned = strip_request_id(raw); - let cleaned = cleaned.trim(); - if cleaned.is_empty() { - return None; - } - - if let Some((code, message)) = bracketed_error(cleaned) { - return Some(format_summary(Some(code), message)); +fn api_error_detail( + code: Option<&str>, + status: Option, + message: &str, + details: Option<&serde_json::Value>, +) -> String { + let mut lines = Vec::new(); + if let Some(status) = status { + lines.push(format!("HTTP status: {status}")); } - - Some(ensure_sentence( - cleaned.lines().next().unwrap_or(cleaned).trim(), - )) -} - -fn summarize_json_api_error(raw: &str) -> Option { - let value: serde_json::Value = serde_json::from_str(raw).ok()?; - let error = value.get("error").unwrap_or(&value); - let code = error - .get("code") - .and_then(serde_json::Value::as_str) - .filter(|value| !value.trim().is_empty()); - let message = error - .get("message") - .and_then(serde_json::Value::as_str) - .or_else(|| error.get("detail").and_then(serde_json::Value::as_str)) - .filter(|value| !value.trim().is_empty())?; - Some(format_summary(code, message)) -} - -fn bracketed_error(value: &str) -> Option<(&str, &str)> { - let rest = value.strip_prefix('[')?; - let end = rest.find(']')?; - let code = rest[..end].trim(); - let message = rest[end + 1..].trim(); - if code.is_empty() || message.is_empty() { - return None; + if let Some(code) = code { + lines.push(format!("Code: {code}")); } - Some((code, message)) -} - -fn format_summary(code: Option<&str>, message: &str) -> String { - let message = ensure_sentence(strip_request_id(message).trim()); - match code { - Some(code) => format!("{code}: {message}"), - None => message, + lines.push(format!("Message: {message}")); + if let Some(details) = details { + let rendered = + serde_json::to_string_pretty(details).unwrap_or_else(|_| details.to_string()); + lines.push(format!("Details: {rendered}")); } + lines.join("\n") } fn strip_request_id(value: &str) -> &str { @@ -1122,57 +1267,26 @@ Usage: ov config [OPTIONS] [COMMAND] #[test] fn runtime_api_error_hides_raw_detail_by_default() { - let error = Error::api( - "[AuthenticationError] API key invalid. Request ID: 02177930089909800000000000000000" - .to_string(), + let error = Error::api_structured( + "API key invalid. Request ID: 02177930089909800000000000000000", + Some(401), + Some("UNAUTHENTICATED"), + None, ); let report = report_for_runtime_error("ov status", &error); let normal = strip_ansi(&render_report(&report, false)); let verbose = strip_ansi(&render_report(&report, true)); assert!(normal.contains("Authentication Error")); - assert!(normal.contains("OpenViking rejected the API key")); + assert!(normal.contains("UNAUTHENTICATED: API key invalid.")); assert!(normal.contains("ov config")); assert!(!normal.contains("Request ID")); - assert!(!normal.contains("AuthenticationError")); - - assert!(verbose.contains("Detail:")); - assert!(verbose.contains("Request ID")); - } - - #[test] - fn runtime_api_error_shows_sanitized_detail_by_default() { - let error = Error::api( - "[InvalidRequest] resource not found. Request ID: 02177930089909800000000000000000" - .to_string(), - ); - let report = report_for_runtime_error("ov read viking://missing", &error); - let normal = strip_ansi(&render_report(&report, false)); - let verbose = strip_ansi(&render_report(&report, true)); - - assert!(normal.contains("OpenViking API Error")); - assert!(normal.contains("InvalidRequest: resource not found.")); - assert!(!normal.contains("Request ID")); assert!(!normal.contains("02177930089909800000000000000000")); assert!(verbose.contains("Detail:")); assert!(verbose.contains("Request ID")); } - #[test] - fn runtime_api_error_summarizes_json_error_envelope() { - let error = Error::api( - r#"{"error":{"code":"PermissionDenied","message":"root key required","request_id":"abc"}}"# - .to_string(), - ); - let report = report_for_runtime_error("ov admin list-users --sudo", &error); - let normal = strip_ansi(&render_report(&report, false)); - - assert!(normal.contains("PermissionDenied: root key required.")); - assert!(!normal.contains("request_id")); - assert!(!normal.contains("abc")); - } - #[test] fn plain_help_error_points_to_prefixed_help() { let report = super::report_for_plain_help_error("ov config help", "ov config --help"); diff --git a/crates/ov_cli/src/help_ui.rs b/crates/ov_cli/src/help_ui.rs index 96a5fc9553..57e79e8a43 100644 --- a/crates/ov_cli/src/help_ui.rs +++ b/crates/ov_cli/src/help_ui.rs @@ -67,6 +67,7 @@ const CORE_WORKFLOW: &[HelpCommand] = help_commands![ "find", "read", "write", + "set-tags", "add-memory", ]; @@ -383,6 +384,24 @@ const COMMAND_HELP_SPECS: &[CommandHelpSpec] = &[ }, ], }, + CommandHelpSpec { + path: &["set-tags"], + purpose: "Set metadata tags on a resource.", + examples: &[HelpItem { + label: "ov set-tags viking://projects/acme owner=team-a priority=high", + description: "Attach or replace explicit key-value tags.", + }], + next_steps: &[ + HelpItem { + label: "ov stat ", + description: "Confirm the updated metadata.", + }, + HelpItem { + label: "ov find \"query\" --tags owner=team-a", + description: "Use tags to narrow retrieval.", + }, + ], + }, CommandHelpSpec { path: &["get"], purpose: "Download a file resource to a local path.", @@ -1651,6 +1670,7 @@ fn localized_command_purpose(spec: &CommandHelpSpec, language: Language) -> &str ["health"] => "快速检查服务器是否可连接。", ["status"] => "查看 OpenViking 服务器诊断状态。", ["language"] => "选择 OpenViking CLI 显示语言。", + ["set-tags"] => "设置资源的元数据标签。", _ => spec.purpose, } } @@ -1689,6 +1709,11 @@ fn localized_help_item_description<'a>( "ov language" => "打开语言选择器。", "ov language zh-CN" => "将显示语言切换为简体中文。", "ov lang en" => "使用短别名切换为英文显示。", + "ov set-tags viking://projects/acme owner=team-a priority=high" => { + "附加或替换显式键值标签。" + } + "ov stat " => "确认更新后的元数据。", + "ov find \"query\" --tags owner=team-a" => "使用标签缩小检索范围。", "language" => "可选语言代码：en 或 zh-CN。", "name" => "已保存的配置名称。", "--name " => "已保存配置名称。不提供则自动生成。", diff --git a/crates/ov_cli/src/status_ui.rs b/crates/ov_cli/src/status_ui.rs index 8079410bc1..c017df6f7a 100644 --- a/crates/ov_cli/src/status_ui.rs +++ b/crates/ov_cli/src/status_ui.rs @@ -6,7 +6,7 @@ use crate::{ config::{Config, display_config_home}, config_wizard::{ConfigKind, ConfigStore}, error::{Error, Result}, - error_classifier::looks_like_auth_error, + error_classifier::{ApiErrorKind, api_error_kind}, i18n::{Language, copy}, theme, }; @@ -204,17 +204,25 @@ pub(crate) fn render_unreachable_status( #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum StatusFailureKind { Authentication, + Permission, Api, + Timeout, + Unhealthy, Connection, } impl StatusFailureKind { fn from_error(error: Option<&Error>) -> Self { match error { - Some(Error::Api { message, .. }) if looks_like_auth_error(message) => { - Self::Authentication - } - Some(Error::Api { .. }) => Self::Api, + Some(Error::Api { code, status, .. }) => match api_error_kind(code.as_deref(), *status) + { + ApiErrorKind::Authentication => Self::Authentication, + ApiErrorKind::Permission => Self::Permission, + _ => Self::Api, + }, + Some(Error::Timeout(_)) => Self::Timeout, + Some(Error::ServerUnhealthy(_)) => Self::Unhealthy, + Some(Error::Response(_)) => Self::Api, _ => Self::Connection, } } @@ -222,7 +230,10 @@ impl StatusFailureKind { fn status_label(self, language: Language) -> &'static str { match self { Self::Authentication => copy(language, "Authentication failed", "认证失败"), + Self::Permission => copy(language, "Permission denied", "权限不足"), Self::Api => copy(language, "Server error", "服务器错误"), + Self::Timeout => copy(language, "Timed out", "请求超时"), + Self::Unhealthy => copy(language, "Unhealthy", "不健康"), Self::Connection => copy(language, "Unreachable", "无法连接"), } } @@ -230,11 +241,26 @@ impl StatusFailureKind { fn issue_label(self, language: Language) -> &'static str { match self { Self::Authentication => copy(language, "API key rejected", "API Key 被拒绝"), + Self::Permission => copy( + language, + "Active identity lacks permission", + "当前身份权限不足", + ), Self::Api => copy( language, "OpenViking returned an API error", "OpenViking 返回 API 错误", ), + Self::Timeout => copy( + language, + "Request exceeded configured timeout", + "请求超过当前超时时间", + ), + Self::Unhealthy => copy( + language, + "Server is reachable but unhealthy", + "服务器可连接但状态不健康", + ), Self::Connection => copy(language, "Cannot reach server", "无法连接服务器"), } } @@ -255,6 +281,20 @@ impl StatusFailureKind { copy(language, "Validate after updating", "更新后验证"), ), ], + Self::Permission => vec![ + ( + "ov config show", + copy(language, "Show active identity", "显示当前身份"), + ), + ( + "ov config switch", + copy(language, "Use another config", "切换到其他配置"), + ), + ( + "ov config validate", + copy(language, "Validate permissions", "验证权限"), + ), + ], Self::Api => vec![ ( "ov config validate", @@ -269,6 +309,20 @@ impl StatusFailureKind { copy(language, "Run a quick health check", "快速健康检查"), ), ], + Self::Timeout | Self::Unhealthy => vec![ + ( + "ov status --verbose", + copy(language, "Show backend details", "显示后端详情"), + ), + ( + "ov observer queue", + copy(language, "Inspect background queues", "查看后台队列"), + ), + ( + "ov health", + copy(language, "Run a quick health check", "快速健康检查"), + ), + ], Self::Connection => vec![ ( "ov config validate", @@ -886,8 +940,11 @@ mod tests { #[test] fn unreachable_status_distinguishes_auth_failures() { - let error = crate::error::Error::api( - "[AuthenticationError] API key invalid. Request ID: abc".to_string(), + let error = crate::error::Error::api_structured( + "API key invalid. Request ID: abc", + Some(401), + Some("UNAUTHENTICATED"), + None, ); let rendered = super::render_unreachable_status(&sample_config(), Some("local"), 2, Some(&error));