From f1c4ad0e67fd1537aeb6ebea8e5b5e3eca0523ea Mon Sep 17 00:00:00 2001 From: nicos_backbase Date: Fri, 17 Oct 2025 22:39:09 +0200 Subject: [PATCH] chore: refine error handling --- src/commands/clone.rs | 54 ++++++++++++++++++++++++++++++---- src/commands/pr.rs | 59 ++++++++++++++++++++++++++++++------- src/commands/remove.rs | 67 ++++++++++++++++++++++++++++++++++-------- src/commands/run.rs | 59 ++++++++++++++++++++++++++++++------- src/github/api.rs | 26 +++++++++------- 5 files changed, 216 insertions(+), 49 deletions(-) diff --git a/src/commands/clone.rs b/src/commands/clone.rs index 3d29703..1475412 100644 --- a/src/commands/clone.rs +++ b/src/commands/clone.rs @@ -35,35 +35,77 @@ impl Command for CloneCommand { format!("Cloning {} repositories...", repositories.len()).green() ); + let mut errors = Vec::new(); + let mut successful = 0; + if context.parallel { let tasks: Vec<_> = repositories .into_iter() .map(|repo| { + let repo_name = repo.name.clone(); tokio::spawn(async move { - tokio::task::spawn_blocking(move || git::clone_repository(&repo)).await? + let result = + tokio::task::spawn_blocking(move || git::clone_repository(&repo)) + .await?; + Ok::<_, anyhow::Error>((repo_name, result)) }) }) .collect(); for task in tasks { - if let Err(e) = task.await? { - eprintln!("{}", format!("Error: {e}").red()); + match task.await? { + Ok((_, Ok(_))) => successful += 1, + Ok((repo_name, Err(e))) => { + eprintln!("{}", format!("Error: {e}").red()); + errors.push((repo_name, e)); + } + Err(e) => { + eprintln!("{}", format!("Task error: {e}").red()); + errors.push(("unknown".to_string(), e)); + } } } } else { for repo in repositories { - if let Err(e) = tokio::task::spawn_blocking({ + let repo_name = repo.name.clone(); + match tokio::task::spawn_blocking({ let repo = repo.clone(); move || git::clone_repository(&repo) }) .await? { - eprintln!("{}", format!("Error: {e}").red()); + Ok(_) => successful += 1, + Err(e) => { + eprintln!("{}", format!("Error: {e}").red()); + errors.push((repo_name, e)); + } } } } - println!("{}", "Done cloning repositories".green()); + // Report summary + if errors.is_empty() { + println!("{}", "Done cloning repositories".green()); + } else { + println!( + "{}", + format!( + "Completed with {} successful, {} failed", + successful, + errors.len() + ) + .yellow() + ); + + // If all operations failed, return an error to propagate to main + if successful == 0 { + return Err(anyhow::anyhow!( + "All clone operations failed. First error: {}", + errors[0].1 + )); + } + } + Ok(()) } } diff --git a/src/commands/pr.rs b/src/commands/pr.rs index fbfeb54..636d609 100644 --- a/src/commands/pr.rs +++ b/src/commands/pr.rs @@ -59,33 +59,72 @@ impl Command for PrCommand { create_only: self.create_only, }; + let mut errors = Vec::new(); + let mut successful = 0; + if context.parallel { let tasks: Vec<_> = repositories .into_iter() .map(|repo| { let pr_options = pr_options.clone(); - async move { github::create_pull_request(&repo, &pr_options).await } + async move { + ( + repo.name.clone(), + github::create_pull_request(&repo, &pr_options).await, + ) + } }) .collect(); for task in tasks { - if let Err(e) = task.await { - eprintln!("{}", format!("Error: {e}").red()); + let (repo_name, result) = task.await; + match result { + Ok(_) => successful += 1, + Err(e) => { + eprintln!("{}", format!("Error: {e}").red()); + errors.push((repo_name, e)); + } } } } else { for repo in repositories { - if let Err(e) = github::create_pull_request(&repo, &pr_options).await { - eprintln!( - "{} | {}", - repo.name.cyan().bold(), - format!("Error: {e}").red() - ); + match github::create_pull_request(&repo, &pr_options).await { + Ok(_) => successful += 1, + Err(e) => { + eprintln!( + "{} | {}", + repo.name.cyan().bold(), + format!("Error: {e}").red() + ); + errors.push((repo.name.clone(), e)); + } } } } - println!("{}", "Done processing pull requests".green()); + // Report summary + if errors.is_empty() { + println!("{}", "Done processing pull requests".green()); + } else { + println!( + "{}", + format!( + "Completed with {} successful, {} failed", + successful, + errors.len() + ) + .yellow() + ); + + // If all operations failed, return an error to propagate to main + if successful == 0 { + return Err(anyhow::anyhow!( + "All pull request operations failed. First error: {}", + errors[0].1 + )); + } + } + Ok(()) } } diff --git a/src/commands/remove.rs b/src/commands/remove.rs index 20c1486..7e1f8d2 100644 --- a/src/commands/remove.rs +++ b/src/commands/remove.rs @@ -35,13 +35,17 @@ impl Command for RemoveCommand { format!("Removing {} repositories...", repositories.len()).green() ); + let mut errors = Vec::new(); + let mut successful = 0; + if context.parallel { let tasks: Vec<_> = repositories .into_iter() .map(|repo| { + let repo_name = repo.name.clone(); tokio::spawn(async move { let target_dir = repo.get_target_dir(); - tokio::task::spawn_blocking(move || { + let result = tokio::task::spawn_blocking(move || { if std::path::Path::new(&target_dir).exists() { fs::remove_dir_all(&target_dir).map_err(anyhow::Error::from) } else { @@ -49,36 +53,73 @@ impl Command for RemoveCommand { Ok(()) } }) - .await? + .await?; + Ok::<_, anyhow::Error>((repo_name, result)) }) }) .collect(); for task in tasks { - if let Err(e) = task.await? { - eprintln!("{}", format!("Error: {e}").red()); + match task.await? { + Ok((_, Ok(_))) => successful += 1, + Ok((repo_name, Err(e))) => { + eprintln!("{}", format!("Error: {e}").red()); + errors.push((repo_name, e)); + } + Err(e) => { + eprintln!("{}", format!("Task error: {e}").red()); + errors.push(("unknown".to_string(), e)); + } } } } else { for repo in repositories { let target_dir = repo.get_target_dir(); if std::path::Path::new(&target_dir).exists() { - if let Err(e) = fs::remove_dir_all(&target_dir) { - eprintln!( - "{} | {}", - repo.name.cyan().bold(), - format!("Error: {e}").red() - ); - } else { - println!("{} | {}", repo.name.cyan().bold(), "Removed".green()); + match fs::remove_dir_all(&target_dir) { + Ok(_) => { + println!("{} | {}", repo.name.cyan().bold(), "Removed".green()); + successful += 1; + } + Err(e) => { + eprintln!( + "{} | {}", + repo.name.cyan().bold(), + format!("Error: {e}").red() + ); + errors.push((repo.name.clone(), e.into())); + } } } else { println!("{} | Directory does not exist", repo.name.cyan().bold()); + successful += 1; // Count as success since the desired state is achieved } } } - println!("{}", "Done removing repositories".green()); + // Report summary + if errors.is_empty() { + println!("{}", "Done removing repositories".green()); + } else { + println!( + "{}", + format!( + "Completed with {} successful, {} failed", + successful, + errors.len() + ) + .yellow() + ); + + // If all operations failed, return an error to propagate to main + if successful == 0 { + return Err(anyhow::anyhow!( + "All removal operations failed. First error: {}", + errors[0].1 + )); + } + } + Ok(()) } } diff --git a/src/commands/run.rs b/src/commands/run.rs index f827d0a..4d3f672 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -45,6 +45,9 @@ impl Command for RunCommand { let runner = CommandRunner::new(); + let mut errors = Vec::new(); + let mut successful = 0; + if context.parallel { let tasks: Vec<_> = repositories .into_iter() @@ -52,31 +55,67 @@ impl Command for RunCommand { let runner = &runner; let command = self.command.clone(); let log_dir = self.log_dir.clone(); - async move { runner.run_command(&repo, &command, Some(&log_dir)).await } + async move { + ( + repo.name.clone(), + runner.run_command(&repo, &command, Some(&log_dir)).await, + ) + } }) .collect(); for task in tasks { - if let Err(e) = task.await { - eprintln!("{}", format!("Error: {e}").red()); + let (repo_name, result) = task.await; + match result { + Ok(_) => successful += 1, + Err(e) => { + eprintln!("{}", format!("Error: {e}").red()); + errors.push((repo_name, e)); + } } } } else { for repo in repositories { - if let Err(e) = runner + match runner .run_command(&repo, &self.command, Some(&self.log_dir)) .await { - eprintln!( - "{} | {}", - repo.name.cyan().bold(), - format!("Error: {e}").red() - ); + Ok(_) => successful += 1, + Err(e) => { + eprintln!( + "{} | {}", + repo.name.cyan().bold(), + format!("Error: {e}").red() + ); + errors.push((repo.name.clone(), e)); + } } } } - println!("{}", "Done running commands".green()); + // Report summary + if errors.is_empty() { + println!("{}", "Done running commands".green()); + } else { + println!( + "{}", + format!( + "Completed with {} successful, {} failed", + successful, + errors.len() + ) + .yellow() + ); + + // If all operations failed, return an error to propagate to main + if successful == 0 { + return Err(anyhow::anyhow!( + "All command executions failed. First error: {}", + errors[0].1 + )); + } + } + Ok(()) } } diff --git a/src/github/api.rs b/src/github/api.rs index 41d55ae..9febbb1 100644 --- a/src/github/api.rs +++ b/src/github/api.rs @@ -50,13 +50,23 @@ pub async fn create_pull_request(repo: &Repository, options: &PrOptions) -> Resu git::push_branch(&repo_path, &branch_name)?; // Create PR via GitHub API - create_github_pr(repo, &branch_name, options).await?; + let pr_url = create_github_pr(repo, &branch_name, options).await?; + println!( + "{} | {} {}", + repo.name.cyan().bold(), + "Pull request created:".green(), + pr_url + ); } Ok(()) } -async fn create_github_pr(repo: &Repository, branch_name: &str, options: &PrOptions) -> Result<()> { +async fn create_github_pr( + repo: &Repository, + branch_name: &str, + options: &PrOptions, +) -> Result { let client = GitHubClient::new(Some(options.token.clone())); // Extract owner and repo name from URL @@ -81,13 +91,9 @@ async fn create_github_pr(repo: &Repository, branch_name: &str, options: &PrOpti )) .await?; - let pr_url = result["html_url"].as_str().unwrap_or("unknown"); - println!( - "{} | {} {}", - repo.name.cyan().bold(), - "Pull request created:".green(), - pr_url - ); + let pr_url = result["html_url"] + .as_str() + .ok_or_else(|| anyhow::anyhow!("No html_url in GitHub API response"))?; - Ok(()) + Ok(pr_url.to_string()) }