Skip to content

Commit 2b1810b

Browse files
committed
docs(cli): add comprehensive error documentation
Code quality improvements based on architect/security review: - Replace .expect() with .ok_or_else() in common.rs:183 (prevents panic from user input, returns proper error) - Add # Errors sections to server.rs::run() - Add # Errors sections to formatters.rs functions (4 functions) - Remove #![allow(clippy::missing_errors_doc)] suppression - Update build_server_config docs (remove Panics, expand Errors) Security audit found only 1 production .expect() - now fixed. All 502 tests pass.
1 parent caa9acc commit 2b1810b

File tree

4 files changed

+28
-10
lines changed

4 files changed

+28
-10
lines changed

crates/mcp-cli/src/commands/common.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -108,12 +108,9 @@ pub fn load_server_from_config(name: &str) -> Result<(ServerId, ServerConfig)> {
108108
///
109109
/// # Errors
110110
///
111-
/// Returns an error if environment variables or headers are not in KEY=VALUE format.
112-
///
113-
/// # Panics
114-
///
115-
/// Panics if `server` is `None` when using stdio transport (i.e., when neither
116-
/// `http` nor `sse` is provided). This is enforced by CLI argument validation.
111+
/// Returns an error if:
112+
/// - Environment variables or headers are not in KEY=VALUE format
113+
/// - `server` is `None` when using stdio transport (neither `http` nor `sse` provided)
117114
///
118115
/// # Examples
119116
///
@@ -180,7 +177,8 @@ pub fn build_server_config(
180177
(id, builder.build())
181178
} else {
182179
// Stdio transport (default)
183-
let command = server.expect("server is required for stdio transport");
180+
let command = server
181+
.ok_or_else(|| anyhow::anyhow!("server command is required for stdio transport"))?;
184182
let id = ServerId::new(&command);
185183
let mut builder: ServerConfigBuilder = ServerConfig::builder().command(command);
186184

crates/mcp-cli/src/commands/server.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,12 @@ impl ServerManager {
286286
///
287287
/// # Errors
288288
///
289-
/// Returns an error if server operation fails.
289+
/// Returns an error if:
290+
/// - Claude Desktop configuration file cannot be found or read
291+
/// - Configuration file contains invalid JSON
292+
/// - Requested server is not found in configuration (for Info action)
293+
/// - Server introspection fails (for Info action)
294+
/// - Output formatting fails
290295
///
291296
/// # Examples
292297
///

crates/mcp-cli/src/formatters.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,20 @@ pub mod json {
5555
/// Format data as JSON.
5656
///
5757
/// Uses pretty-printing with 2-space indentation.
58+
///
59+
/// # Errors
60+
///
61+
/// Returns an error if serialization fails.
5862
pub fn format<T: Serialize>(data: &T) -> Result<String> {
5963
let json = serde_json::to_string_pretty(data)?;
6064
Ok(json)
6165
}
6266

6367
/// Format data as compact JSON (no formatting).
68+
///
69+
/// # Errors
70+
///
71+
/// Returns an error if serialization fails.
6472
pub fn format_compact<T: Serialize>(data: &T) -> Result<String> {
6573
let json = serde_json::to_string(data)?;
6674
Ok(json)
@@ -75,6 +83,10 @@ pub mod text {
7583
///
7684
/// Uses JSON representation but without colors or fancy formatting.
7785
/// Suitable for piping to other commands or scripts.
86+
///
87+
/// # Errors
88+
///
89+
/// Returns an error if serialization fails.
7890
pub fn format<T: Serialize>(data: &T) -> Result<String> {
7991
// For text mode, use JSON without pretty printing
8092
json::format_compact(data)
@@ -88,6 +100,10 @@ pub mod pretty {
88100
/// Format data as colorized, human-readable output.
89101
///
90102
/// Uses colors and formatting for better terminal readability.
103+
///
104+
/// # Errors
105+
///
106+
/// Returns an error if serialization fails.
91107
pub fn format<T: Serialize>(data: &T) -> Result<String> {
92108
// Convert to JSON value first for inspection
93109
let value = serde_json::to_value(data)?;

crates/mcp-cli/src/main.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@
55
#![allow(clippy::unused_async)]
66
#![allow(clippy::cast_possible_truncation)]
77
// u128->u64 for millis is safe in practice
8-
// TODO(phase-7.3): Add comprehensive error documentation to all public CLI functions
9-
#![allow(clippy::missing_errors_doc)]
8+
// Error documentation added to all public CLI functions
109
#![allow(clippy::needless_collect)]
1110
#![allow(clippy::unnecessary_wraps)] // API design requires Result for consistency across commands
1211
#![allow(clippy::unnecessary_literal_unwrap)]

0 commit comments

Comments
 (0)