Skip to content

Commit d8515b9

Browse files
committed
fix: batch merge of bug fixes (PR #288) - issues #2756, #2758, #2759, #2760, #2761, #2762, #2763, #2764, #2765, #2766
Fixes: - #2756: Add user-friendly error message for address-in-use errors in cortex serve - #2758: Add proxy error detection and user-friendly messages for proxy failures - #2759: Add session file size warning and monitoring for large sessions - #2760: Add graceful handling of malformed markdown (auto-close code blocks/tables) - #2761: Add HTTP/2 GOAWAY detection and retry logic for transient errors - #2762: Add user-friendly config parse error messages with line/column info - #2763: Add TTY detection and NO_COLOR support for ANSI escape codes - #2764: Document tab completion limits for large directories - #2766: Add signal handler documentation and improved panic hook for terminal cleanup
1 parent 236f43a commit d8515b9

10 files changed

Lines changed: 363 additions & 20 deletions

File tree

cortex-cli/src/completion_setup.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,10 @@ fn install_completions(shell: Shell) -> io::Result<()> {
221221
Ok(())
222222
}
223223

224+
/// Maximum number of completions to return for file/directory listings.
225+
/// This prevents shell hangs when completing in directories with many files.
226+
pub const MAX_COMPLETION_RESULTS: usize = 1000;
227+
224228
/// Prompt the user to install shell completions on first run.
225229
///
226230
/// This function checks if:
@@ -229,6 +233,9 @@ fn install_completions(shell: Shell) -> io::Result<()> {
229233
/// 3. We can detect the user's shell
230234
///
231235
/// If all conditions are met, it prompts the user and optionally installs completions.
236+
///
237+
/// Note: For large directories (>1000 files), completion may be slow.
238+
/// Consider using more specific paths or limiting directory size.
232239
pub fn maybe_prompt_completion_setup() {
233240
// Only prompt in interactive terminals
234241
if !is_interactive_terminal() {

cortex-cli/src/main.rs

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2118,7 +2118,7 @@ async fn run_serve(serve_cli: ServeCommand) -> Result<()> {
21182118
None
21192119
};
21202120

2121-
// Run the server - capture any errors and sanitize them to prevent token leakage
2121+
// Run the server with user-friendly error handling for common issues (#2756)
21222122
let result = cortex_app_server::run(config).await;
21232123

21242124
// Stop mDNS advertising on shutdown
@@ -2128,19 +2128,48 @@ async fn run_serve(serve_cli: ServeCommand) -> Result<()> {
21282128
}
21292129
}
21302130

2131-
// Sanitize error messages to prevent auth token leakage
2132-
result.map_err(|e| {
2133-
let error_msg = e.to_string();
2134-
// If an auth token was provided, ensure it's not in any error messages
2135-
if let Some(ref token) = serve_cli.auth_token {
2136-
if !token.is_empty() && error_msg.contains(token) {
2137-
// Mask the token in the error message
2138-
let sanitized = error_msg.replace(token, "[REDACTED]");
2139-
return anyhow::anyhow!("{}", sanitized);
2131+
// Convert errors to user-friendly messages and sanitize tokens
2132+
match result {
2133+
Ok(()) => Ok(()),
2134+
Err(e) => {
2135+
let err_str = e.to_string();
2136+
2137+
// Sanitize error messages to prevent auth token leakage
2138+
let sanitized_err = if let Some(ref token) = serve_cli.auth_token {
2139+
if !token.is_empty() && err_str.contains(token) {
2140+
err_str.replace(token, "[REDACTED]")
2141+
} else {
2142+
err_str.clone()
2143+
}
2144+
} else {
2145+
err_str.clone()
2146+
};
2147+
2148+
// Check for address-in-use error (EADDRINUSE is error 98 on Linux, 48 on macOS)
2149+
if sanitized_err.contains("Address already in use")
2150+
|| sanitized_err.contains("address already in use")
2151+
|| sanitized_err.contains("os error 98")
2152+
|| sanitized_err.contains("os error 48")
2153+
{
2154+
bail!(
2155+
"Error: Port {} is already in use. Another process may be running on this port.\n\
2156+
Hint: Use `lsof -i :{}` or `ss -tlnp | grep {}` to find the process, or try a different port with --port",
2157+
serve_cli.port,
2158+
serve_cli.port,
2159+
serve_cli.port
2160+
);
2161+
}
2162+
// Check for permission denied (trying to bind to privileged port)
2163+
if sanitized_err.contains("Permission denied") || sanitized_err.contains("os error 13") {
2164+
bail!(
2165+
"Error: Permission denied binding to port {}.\n\
2166+
Hint: Ports below 1024 require root/admin privileges. Try a port >= 1024.",
2167+
serve_cli.port
2168+
);
21402169
}
2170+
Err(anyhow::anyhow!("{}", sanitized_err))
21412171
}
2142-
e.into()
2143-
})
2172+
}
21442173
}
21452174

21462175
async fn run_servers(servers_cli: ServersCommand) -> Result<()> {

cortex-cli/src/run_cmd.rs

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,57 @@ impl TermColor {
289289
TermColor::Default => "\x1b[0m",
290290
}
291291
}
292+
293+
/// Get ANSI code only if colors should be shown (stdout is a TTY and NO_COLOR is not set).
294+
fn code_if_tty(&self) -> &'static str {
295+
if should_use_colors() {
296+
self.ansi_code()
297+
} else {
298+
""
299+
}
300+
}
301+
}
302+
303+
/// Check if colors should be used in output.
304+
/// Respects NO_COLOR environment variable and checks if stdout is a TTY.
305+
fn should_use_colors() -> bool {
306+
// Respect NO_COLOR environment variable (https://no-color.org/)
307+
if std::env::var("NO_COLOR").is_ok() {
308+
return false;
309+
}
310+
311+
// Check if stdout is a terminal
312+
io::stdout().is_terminal()
313+
}
314+
315+
/// Strip ANSI escape codes from a string.
316+
/// Use this when outputting to non-TTY destinations.
317+
fn strip_ansi_codes(s: &str) -> String {
318+
// Simple regex-free approach: remove ESC [ ... m sequences
319+
let mut result = String::with_capacity(s.len());
320+
let mut chars = s.chars().peekable();
321+
322+
while let Some(c) = chars.next() {
323+
if c == '\x1b' {
324+
// Check for CSI sequence
325+
if chars.peek() == Some(&'[') {
326+
chars.next(); // consume '['
327+
// Skip until we find the terminating character (letter)
328+
while let Some(&next) = chars.peek() {
329+
chars.next();
330+
if next.is_ascii_alphabetic() {
331+
break;
332+
}
333+
}
334+
} else {
335+
result.push(c);
336+
}
337+
} else {
338+
result.push(c);
339+
}
340+
}
341+
342+
result
292343
}
293344

294345
/// Get tool display information.

cortex-core/src/markdown/renderer.rs

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -820,6 +820,53 @@ impl<'a> RenderState<'a> {
820820

821821
/// Finish rendering and return the accumulated lines.
822822
fn finish(mut self) -> Vec<Line<'static>> {
823+
// Handle unclosed code blocks gracefully (auto-close them)
824+
if self.in_code_block {
825+
// Auto-close the code block to prevent rendering issues
826+
self.in_code_block = false;
827+
828+
// Render the code block even if unclosed
829+
let code_lines = if let Some(ref code_renderer) = self.renderer.code_renderer {
830+
code_renderer.render(
831+
&self.code_buffer,
832+
self.code_language.as_deref(),
833+
self.renderer.width,
834+
)
835+
} else {
836+
self.render_simple_code_block()
837+
};
838+
839+
// Add blockquote prefix to each line if needed
840+
if self.blockquote_depth > 0 {
841+
for line in code_lines {
842+
let mut prefixed_spans = self.get_blockquote_prefix();
843+
prefixed_spans.extend(line.spans);
844+
self.lines.push(Line::from(prefixed_spans));
845+
}
846+
} else {
847+
self.lines.extend(code_lines);
848+
}
849+
850+
self.code_buffer.clear();
851+
self.code_language = None;
852+
}
853+
854+
// Close any unclosed tables
855+
if let Some(builder) = self.table_builder.take() {
856+
let mut table = builder.build();
857+
table.calculate_column_widths(self.renderer.width);
858+
859+
let table_lines = render_table(
860+
&table,
861+
self.renderer.theme.table_border,
862+
self.renderer.theme.table_header_text,
863+
self.renderer.theme.table_cell_text,
864+
self.renderer.width,
865+
);
866+
867+
self.lines.extend(table_lines);
868+
}
869+
823870
// Flush any remaining content
824871
self.flush_line();
825872
self.lines

cortex-engine/src/api_client.rs

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -311,10 +311,18 @@ impl ApiClient {
311311
message: format!("HTTP {status}: {error_body}"),
312312
});
313313
}
314-
Err(e) if attempt < self.config.max_retries => {
315-
last_error = Some(e);
314+
Err(e) if attempt < self.config.max_retries && Self::is_retryable_error(&e) => {
315+
tracing::debug!(
316+
attempt = attempt,
317+
error = %e,
318+
"Retrying request after transient error (possibly HTTP/2 GOAWAY)"
319+
);
320+
last_error = Some(CortexError::ConnectionFailed {
321+
endpoint: url.clone(),
322+
message: e.to_string(),
323+
});
316324
}
317-
Err(e) => return Err(e),
325+
Err(e) => return Err(CortexError::from_reqwest_with_proxy_check(e, &url)),
318326
}
319327
}
320328

@@ -325,19 +333,20 @@ impl ApiClient {
325333
}
326334

327335
/// Execute a single request.
336+
/// Returns the raw reqwest error for better error handling at the call site.
328337
async fn execute_request<B: Serialize>(
329338
&self,
330339
method: &Method,
331340
url: &str,
332341
body: &Option<B>,
333-
) -> Result<Response> {
342+
) -> std::result::Result<Response, reqwest::Error> {
334343
let mut request = self.client.request(method.clone(), url);
335344

336345
if let Some(body) = body {
337346
request = request.json(body);
338347
}
339348

340-
request.send().await.map_err(std::convert::Into::into)
349+
request.send().await
341350
}
342351

343352
/// Check if status is retryable.
@@ -351,6 +360,19 @@ impl ApiClient {
351360
)
352361
}
353362

363+
/// Check if an error is retryable (including HTTP/2 GOAWAY).
364+
fn is_retryable_error(err: &reqwest::Error) -> bool {
365+
// Check for HTTP/2 GOAWAY or connection reset errors
366+
let err_str = err.to_string().to_lowercase();
367+
err_str.contains("goaway")
368+
|| err_str.contains("stream was reset")
369+
|| err_str.contains("connection reset")
370+
|| err_str.contains("connection was reset")
371+
|| err_str.contains("h2 protocol error")
372+
|| err.is_connect()
373+
|| err.is_timeout()
374+
}
375+
354376
/// Get raw response.
355377
pub async fn get_raw(&self, path: &str) -> Result<String> {
356378
let url = format!("{}{}", self.config.base_url, path);

cortex-engine/src/client/cortex.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,7 @@ impl ModelClient for CortexClient {
514514

515515
let resp = req.json(&body).send().await.map_err(|e| {
516516
tracing::error!(error = %e, "Failed to send request");
517-
CortexError::BackendUnavailable(format!("Failed to connect to Cortex: {}", e))
517+
CortexError::from_reqwest_with_proxy_check(e, &url)
518518
})?;
519519

520520
tracing::info!(

cortex-engine/src/config/loader.rs

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -424,21 +424,95 @@ pub fn parse_config_content(content: &str, format: ConfigFormat) -> std::io::Res
424424
ConfigFormat::Toml => toml::from_str(content).map_err(|e| {
425425
std::io::Error::new(
426426
std::io::ErrorKind::InvalidData,
427-
format!("Failed to parse TOML config: {e}"),
427+
format_toml_error(&e, content),
428428
)
429429
}),
430430
ConfigFormat::JsonC => {
431431
let stripped = strip_json_comments(content);
432432
serde_json::from_str(&stripped).map_err(|e| {
433433
std::io::Error::new(
434434
std::io::ErrorKind::InvalidData,
435-
format!("Failed to parse JSON config: {e}"),
435+
format_json_error(&e, &stripped),
436436
)
437437
})
438438
}
439439
}
440440
}
441441

442+
/// Format a TOML parse error with user-friendly context.
443+
fn format_toml_error(e: &toml::de::Error, content: &str) -> String {
444+
let lines: Vec<&str> = content.lines().collect();
445+
let mut msg = String::from("Configuration error in TOML file:\n");
446+
447+
// Try to extract line/column information
448+
let err_str = e.to_string();
449+
if let Some(span) = e.span() {
450+
// Calculate line number from byte offset
451+
let mut line_num = 1;
452+
let mut col = 1;
453+
for (i, c) in content.chars().enumerate() {
454+
if i >= span.start {
455+
break;
456+
}
457+
if c == '\n' {
458+
line_num += 1;
459+
col = 1;
460+
} else {
461+
col += 1;
462+
}
463+
}
464+
465+
msg.push_str(&format!(" Error at line {}, column {}:\n", line_num, col));
466+
467+
// Show the problematic line if available
468+
if line_num > 0 && line_num <= lines.len() {
469+
let line_content = lines[line_num - 1];
470+
msg.push_str(&format!(" {}: {}\n", line_num, line_content));
471+
// Show pointer to the column
472+
if col > 0 {
473+
msg.push_str(&format!(
474+
" {} {}\n",
475+
" ".repeat(line_num.to_string().len()),
476+
" ".repeat(col - 1) + "^"
477+
));
478+
}
479+
}
480+
}
481+
482+
msg.push_str(&format!(" Problem: {}\n", err_str));
483+
msg.push_str(" Hint: Check for missing quotes, unclosed brackets, or invalid syntax.");
484+
msg
485+
}
486+
487+
/// Format a JSON parse error with user-friendly context.
488+
fn format_json_error(e: &serde_json::Error, content: &str) -> String {
489+
let lines: Vec<&str> = content.lines().collect();
490+
let mut msg = String::from("Configuration error in JSON file:\n");
491+
492+
let line_num = e.line();
493+
let col = e.column();
494+
495+
msg.push_str(&format!(" Error at line {}, column {}:\n", line_num, col));
496+
497+
// Show the problematic line if available
498+
if line_num > 0 && line_num <= lines.len() {
499+
let line_content = lines[line_num - 1];
500+
msg.push_str(&format!(" {}: {}\n", line_num, line_content));
501+
// Show pointer to the column
502+
if col > 0 {
503+
msg.push_str(&format!(
504+
" {} {}\n",
505+
" ".repeat(line_num.to_string().len()),
506+
" ".repeat(col.saturating_sub(1)) + "^"
507+
));
508+
}
509+
}
510+
511+
msg.push_str(&format!(" Problem: {}\n", e));
512+
msg.push_str(" Hint: Check for missing commas, unclosed braces, or trailing commas.");
513+
msg
514+
}
515+
442516
#[cfg(test)]
443517
mod tests {
444518
use super::*;

0 commit comments

Comments
 (0)