diff --git a/cli/src/connection.rs b/cli/src/connection.rs index f6a7a39..320fc0b 100644 --- a/cli/src/connection.rs +++ b/cli/src/connection.rs @@ -116,6 +116,24 @@ fn get_pid_path(session: &str) -> PathBuf { get_socket_dir().join(format!("{}.pid", session)) } +/// Clean up stale socket and PID files for a session +fn cleanup_stale_files(session: &str) { + let pid_path = get_pid_path(session); + let _ = fs::remove_file(&pid_path); + + #[cfg(unix)] + { + let socket_path = get_socket_path(session); + let _ = fs::remove_file(&socket_path); + } + + #[cfg(windows)] + { + let port_path = get_port_path(session); + let _ = fs::remove_file(&port_path); + } +} + #[cfg(windows)] fn get_port_path(session: &str) -> PathBuf { get_socket_dir().join(format!("{}.port", session)) @@ -198,12 +216,22 @@ pub fn ensure_daemon( profile: Option<&str>, state: Option<&str>, ) -> Result { + // Check if daemon is running AND responsive if is_daemon_running(session) && daemon_ready(session) { - return Ok(DaemonResult { - already_running: true, - }); + // Double-check it's actually responsive by waiting and checking again + // This handles the race condition where daemon is shutting down + // (daemon has a 100ms shutdown delay, so we wait longer) + thread::sleep(Duration::from_millis(150)); + if daemon_ready(session) { + return Ok(DaemonResult { + already_running: true, + }); + } } + // Clean up any stale socket/pid files before starting fresh + cleanup_stale_files(session); + // Ensure socket directory exists let socket_dir = get_socket_dir(); if !socket_dir.exists() { @@ -393,12 +421,65 @@ fn connect(session: &str) -> Result { } pub fn send_command(cmd: Value, session: &str) -> Result { + // Retry logic for transient errors (EAGAIN/EWOULDBLOCK/connection issues) + const MAX_RETRIES: u32 = 5; + const RETRY_DELAY_MS: u64 = 200; + + let mut last_error = String::new(); + + for attempt in 0..MAX_RETRIES { + if attempt > 0 { + thread::sleep(Duration::from_millis(RETRY_DELAY_MS * (attempt as u64))); + } + + match send_command_once(&cmd, session) { + Ok(response) => return Ok(response), + Err(e) => { + if is_transient_error(&e) { + last_error = e; + continue; + } + // Non-transient error, fail immediately + return Err(e); + } + } + } + + Err(format!( + "{} (after {} retries - daemon may be busy or unresponsive)", + last_error, MAX_RETRIES + )) +} + +/// Check if an error is transient and worth retrying. +/// Transient errors include: +/// - EAGAIN/EWOULDBLOCK (os error 35 on macOS, 11 on Linux) +/// - EOF errors (daemon closed connection before responding) +/// - Connection reset/broken pipe (daemon crashed or restarting) +/// - Connection refused/socket not found (daemon still starting) +fn is_transient_error(error: &str) -> bool { + error.contains("os error 35") // EAGAIN on macOS + || error.contains("os error 11") // EAGAIN on Linux + || error.contains("WouldBlock") + || error.contains("Resource temporarily unavailable") + || error.contains("EOF") + || error.contains("line 1 column 0") // Empty JSON response + || error.contains("Connection reset") + || error.contains("Broken pipe") + || error.contains("os error 54") // Connection reset by peer (macOS) + || error.contains("os error 104") // Connection reset by peer (Linux) + || error.contains("os error 2") // No such file or directory (socket gone) + || error.contains("os error 61") // Connection refused (macOS) + || error.contains("os error 111") // Connection refused (Linux) +} + +fn send_command_once(cmd: &Value, session: &str) -> Result { let mut stream = connect(session)?; stream.set_read_timeout(Some(Duration::from_secs(30))).ok(); stream.set_write_timeout(Some(Duration::from_secs(5))).ok(); - let mut json_str = serde_json::to_string(&cmd).map_err(|e| e.to_string())?; + let mut json_str = serde_json::to_string(cmd).map_err(|e| e.to_string())?; json_str.push('\n'); stream @@ -510,4 +591,98 @@ mod tests { result.to_string_lossy().contains("home") || result.to_string_lossy().contains("Users") ); } + + // === Transient Error Detection Tests === + + #[test] + fn test_is_transient_error_eagain_macos() { + assert!(is_transient_error( + "Failed to read: Resource temporarily unavailable (os error 35)" + )); + } + + #[test] + fn test_is_transient_error_eagain_linux() { + assert!(is_transient_error( + "Failed to read: Resource temporarily unavailable (os error 11)" + )); + } + + #[test] + fn test_is_transient_error_would_block() { + assert!(is_transient_error("operation WouldBlock")); + } + + #[test] + fn test_is_transient_error_resource_unavailable() { + assert!(is_transient_error("Resource temporarily unavailable")); + } + + #[test] + fn test_is_transient_error_eof() { + assert!(is_transient_error( + "Invalid response: EOF while parsing a value at line 1 column 0" + )); + } + + #[test] + fn test_is_transient_error_empty_json() { + assert!(is_transient_error( + "Invalid response: expected value at line 1 column 0" + )); + } + + #[test] + fn test_is_transient_error_connection_reset() { + assert!(is_transient_error("Connection reset by peer")); + } + + #[test] + fn test_is_transient_error_broken_pipe() { + assert!(is_transient_error("Broken pipe")); + } + + #[test] + fn test_is_transient_error_connection_reset_macos() { + assert!(is_transient_error( + "Failed to send: Connection reset by peer (os error 54)" + )); + } + + #[test] + fn test_is_transient_error_connection_reset_linux() { + assert!(is_transient_error( + "Failed to send: Connection reset by peer (os error 104)" + )); + } + + #[test] + fn test_is_transient_error_socket_not_found() { + assert!(is_transient_error( + "Failed to connect: No such file or directory (os error 2)" + )); + } + + #[test] + fn test_is_transient_error_connection_refused_macos() { + assert!(is_transient_error( + "Failed to connect: Connection refused (os error 61)" + )); + } + + #[test] + fn test_is_transient_error_connection_refused_linux() { + assert!(is_transient_error( + "Failed to connect: Connection refused (os error 111)" + )); + } + + #[test] + fn test_is_transient_error_non_transient() { + // These should NOT be considered transient + assert!(!is_transient_error("Unknown command: foo")); + assert!(!is_transient_error("Invalid JSON syntax")); + assert!(!is_transient_error("Permission denied")); + assert!(!is_transient_error("Daemon not found")); + } }