From 03793fad36480273ebd702e80f41f4baf513647c Mon Sep 17 00:00:00 2001 From: diogo464 Date: Thu, 19 Jun 2025 09:40:05 +0100 Subject: Implement PidFileReadError enum for better error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add PidFileReadError enum with FileNotFound, FileInvalid, and IoError variants - Implement Display and Error traits for proper error handling - Update PidFile::read_from_file() to return Result - Update all call sites to handle specific error types: - is_process_running(): Handle FileNotFound/FileInvalid as "not running" - stop_daemon(): Handle FileNotFound as "not running", FileInvalid as cleanup needed - list_daemons(): Handle FileInvalid with specific error messages - status_daemon(): Handle FileNotFound as "NOT FOUND", FileInvalid as detailed error - clean_orphaned_files(): Handle FileInvalid as cleanup candidate - Remove redundant Path::exists() checks (error type provides this info) - Fix test timing issue by adding sleep in test_run_creates_files - More idiomatic Rust error handling with specific error types 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- src/main.rs | 130 ++++++++++++++++++++++++++++++++++++++++++++--------------- tests/cli.rs | 3 ++ 2 files changed, 101 insertions(+), 32 deletions(-) diff --git a/src/main.rs b/src/main.rs index 4ab1dd9..246af67 100644 --- a/src/main.rs +++ b/src/main.rs @@ -9,6 +9,36 @@ use std::sync::mpsc::channel; use std::thread; use std::time::Duration; +/// Error types for reading PID files +#[derive(Debug)] +pub enum PidFileReadError { + /// The PID file does not exist + FileNotFound, + /// The PID file exists but has invalid content + FileInvalid(String), + /// IO error occurred while reading + IoError(std::io::Error), +} + +impl std::fmt::Display for PidFileReadError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + PidFileReadError::FileNotFound => write!(f, "PID file not found"), + PidFileReadError::FileInvalid(reason) => write!(f, "PID file invalid: {}", reason), + PidFileReadError::IoError(err) => write!(f, "IO error reading PID file: {}", err), + } + } +} + +impl std::error::Error for PidFileReadError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + PidFileReadError::IoError(err) => Some(err), + _ => None, + } + } +} + /// Represents the contents of a PID file #[derive(Debug, Clone)] struct PidFile { @@ -35,23 +65,34 @@ impl PidFile { } /// Read PID file from a file - fn read_from_file>(path: P) -> Result { - let contents = std::fs::read_to_string(path)?; + fn read_from_file>(path: P) -> Result { + let contents = match std::fs::read_to_string(&path) { + Ok(contents) => contents, + Err(err) => { + return if err.kind() == std::io::ErrorKind::NotFound { + Err(PidFileReadError::FileNotFound) + } else { + Err(PidFileReadError::IoError(err)) + }; + } + }; + let lines: Vec<&str> = contents.lines().collect(); if lines.is_empty() { - return Err(anyhow::anyhow!("PID file is empty")); + return Err(PidFileReadError::FileInvalid("PID file is empty".to_string())); } - let pid = lines[0] - .trim() - .parse::() - .context("Failed to parse PID from first line")?; + let pid = lines[0].trim().parse::().map_err(|_| { + PidFileReadError::FileInvalid("Invalid PID on first line".to_string()) + })?; let command: Vec = lines[1..].iter().map(|line| line.to_string()).collect(); if command.is_empty() { - return Err(anyhow::anyhow!("No command found in PID file")); + return Err(PidFileReadError::FileInvalid( + "No command found in PID file".to_string(), + )); } Ok(Self { pid, command }) @@ -256,14 +297,11 @@ fn run_daemon(id: &str, command: &[String]) -> Result<()> { } fn is_process_running(pid_file: &str) -> Result { - // Try to read the PID file - if !Path::new(pid_file).exists() { - return Ok(false); // No PID file means no running process - } - let pid_file_data = match PidFile::read_from_file(pid_file) { Ok(data) => data, - Err(_) => return Ok(false), // Invalid PID file + Err(PidFileReadError::FileNotFound) => return Ok(false), // No PID file means no running process + Err(PidFileReadError::FileInvalid(_)) => return Ok(false), // Invalid PID file means no running process + Err(PidFileReadError::IoError(err)) => return Err(err.into()), // Propagate IO errors }; // Check if process is still running using kill -0 @@ -280,15 +318,18 @@ fn stop_daemon(id: &str, timeout: u64) -> Result<()> { // Check if PID file exists and read PID data let pid_file_data = match PidFile::read_from_file(&pid_file) { Ok(data) => data, - Err(_) => { - if Path::new(&pid_file).exists() { - println!("Process '{}': invalid PID file, removing it", id); - std::fs::remove_file(&pid_file)?; - } else { - println!("Process '{}' is not running (no PID file found)", id); - } + Err(PidFileReadError::FileNotFound) => { + println!("Process '{}' is not running (no PID file found)", id); + return Ok(()); + } + Err(PidFileReadError::FileInvalid(_)) => { + println!("Process '{}': invalid PID file, removing it", id); + std::fs::remove_file(&pid_file)?; return Ok(()); } + Err(PidFileReadError::IoError(err)) => { + return Err(anyhow::anyhow!("Failed to read PID file: {}", err)); + } }; let pid = pid_file_data.pid; @@ -614,13 +655,34 @@ fn list_daemons(quiet: bool) -> Result<()> { println!("{:<20} {:<8} {:<10} {}", id, pid_file_data.pid, status, command); } } - Err(_) => { + Err(PidFileReadError::FileNotFound) => { + // This shouldn't happen since we found the file, but handle gracefully + if quiet { + println!("{}:NOTFOUND:ERROR", id); + } else { + println!( + "{:<20} {:<8} {:<10} {}", + id, "NOTFOUND", "ERROR", "PID file disappeared" + ); + } + } + Err(PidFileReadError::FileInvalid(reason)) => { if quiet { println!("{}:INVALID:ERROR", id); } else { println!( "{:<20} {:<8} {:<10} {}", - id, "INVALID", "ERROR", "Invalid PID file" + id, "INVALID", "ERROR", reason + ); + } + } + Err(PidFileReadError::IoError(_)) => { + if quiet { + println!("{}:ERROR:ERROR", id); + } else { + println!( + "{:<20} {:<8} {:<10} {}", + id, "ERROR", "ERROR", "Cannot read PID file" ); } } @@ -642,12 +704,6 @@ fn status_daemon(id: &str) -> Result<()> { println!("Daemon: {}", id); println!("PID file: {}", pid_file); - // Check if PID file exists - if !Path::new(&pid_file).exists() { - println!("Status: NOT FOUND (no PID file)"); - return Ok(()); - } - // Read PID data from file match PidFile::read_from_file(&pid_file) { Ok(pid_file_data) => { @@ -676,8 +732,14 @@ fn status_daemon(id: &str) -> Result<()> { println!("Note: Use 'demon clean' to remove orphaned files"); } } - Err(e) => { - println!("Status: ERROR (cannot read PID file: {})", e); + Err(PidFileReadError::FileNotFound) => { + println!("Status: NOT FOUND (no PID file)"); + } + Err(PidFileReadError::FileInvalid(reason)) => { + println!("Status: ERROR (invalid PID file: {})", reason); + } + Err(PidFileReadError::IoError(err)) => { + println!("Status: ERROR (cannot read PID file: {})", err); } } @@ -741,7 +803,11 @@ fn clean_orphaned_files() -> Result<()> { ); } } - Err(_) => { + Err(PidFileReadError::FileNotFound) => { + // This shouldn't happen since we found the file, but handle gracefully + tracing::warn!("PID file {} disappeared during processing", path_str); + } + Err(PidFileReadError::FileInvalid(_)) | Err(PidFileReadError::IoError(_)) => { println!("Cleaning up invalid PID file: {}", path_str); if let Err(e) = std::fs::remove_file(&path) { tracing::warn!("Failed to remove invalid PID file {}: {}", path_str, e); diff --git a/tests/cli.rs b/tests/cli.rs index e99e876..2903f61 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -57,6 +57,9 @@ fn test_run_creates_files() { assert!(temp_dir.path().join("test.stdout").exists()); assert!(temp_dir.path().join("test.stderr").exists()); + // Give the process a moment to complete + std::thread::sleep(Duration::from_millis(100)); + // Check that stdout contains our output let stdout_content = fs::read_to_string(temp_dir.path().join("test.stdout")).unwrap(); assert_eq!(stdout_content.trim(), "hello"); -- cgit