smitebot: return Result from command handlers

smitebot/src/commands/bench_exec.rs

@@ -20,6 +20,7 @@ use clap::Args;
 
 use crate::commands::build::{BuildInputs, run_build};
 use crate::config::CampaignConfig;
+use crate::error::CliError;
 use crate::latency_stats::{LatencyStats, avg_duration, mean_stddev};
 use crate::libnyx::{Libnyx, NyxReturn, PAYLOAD_HEADER_SIZE};
 use crate::utils::{pin_to_cpu, setup_nyx};
@@ -105,75 +106,34 @@ pub struct BenchExecArgs {
 }
 
 impl BenchExecCommand {
-    /// Runs the benchmark and returns whether it completed successfully.
-    pub fn execute(args: &BenchExecArgs) -> bool {
-        let iterations = match validate_iterations(args.iterations) {
-            Ok(n) => n,
-            Err(e) => {
-                log::error!("{e}");
-                return false;
-            }
-        };
-        let repeat = match validate_repeat(args.repeat) {
-            Ok(n) => n,
-            Err(e) => {
-                log::error!("{e}");
-                return false;
-            }
-        };
+    /// Runs the benchmark.
+    pub fn execute(args: &BenchExecArgs) -> Result<(), CliError> {
+        let iterations = validate_iterations(args.iterations)?;
+        let repeat = validate_repeat(args.repeat)?;
         // A zero timeout causes Nyx to silently turn off the timing the
         // benchmark exists to measure.
         if args.timeout_secs == 0 {
-            log::error!("Nyx requires --timeout to be at least 1 second");
-            return false;
+            return Err(CliError::Msg(
+                "Nyx requires --timeout to be at least 1 second".to_string(),
+            ));
         }
 
-        let config = match CampaignConfig::load(&args.path) {
-            Ok(c) => c,
-            Err(e) => {
-                log::error!("{e}");
-                return false;
-            }
-        };
+        let config = CampaignConfig::load(&args.path)?;
 
-        let input = match load_input(args) {
-            Ok(input) => input,
-            Err(e) => {
-                log::error!("{e}");
-                return false;
-            }
-        };
-        let max_input_size = match validate_max_input_size(args.max_input_size, input.len()) {
-            Ok(size) => size,
-            Err(e) => {
-                log::error!("{e}");
-                return false;
-            }
-        };
+        let input = load_input(args)?;
+        let max_input_size = validate_max_input_size(args.max_input_size, input.len())?;
 
-        let Some(libnyx_path) = locate_libnyx(&config) else {
-            return false;
-        };
+        let libnyx_path = locate_libnyx(&config)?;
 
-        if !ensure_sharedir(&config, args.no_build) {
-            return false;
-        }
+        ensure_sharedir(&config, args.no_build)?;
 
-        let libnyx = match Libnyx::load(&libnyx_path) {
-            Ok(lib) => lib,
-            Err(e) => {
-                log::error!("failed to load {}: {e}", libnyx_path.display());
-                return false;
-            }
-        };
+        let libnyx = Libnyx::load(&libnyx_path)
+            .map_err(|e| CliError::Msg(format!("failed to load {}: {e}", libnyx_path.display())))?;
 
         // Pin before booting: libnyx spawns QEMU as a child, so the VM inherits
         // this affinity mask. Doing it here also pins the timing loop itself.
         let cpu = args.cpu.unwrap_or(args.worker_id as usize);
-        if let Err(e) = pin_to_cpu(cpu) {
-            log::error!("{e}");
-            return false;
-        }
+        pin_to_cpu(cpu)?;
         log::info!("pinned to CPU {cpu}");
 
         // Each repeat is a fresh VM boot so the aggregate captures boot- and
@@ -183,21 +143,15 @@ impl BenchExecCommand {
             if repeat > 1 {
                 log::info!("benchmark run {}/{repeat}", run_index + 1);
             }
-            match run_once(
+            runs.push(run_once(
                 &libnyx,
                 &config,
                 &input,
                 args,
                 max_input_size,
                 iterations,
                 run_index,
-            ) {
-                Ok(result) => runs.push(result),
-                Err(e) => {
-                    log::error!("{e}");
-                    return false;
-                }
-            }
+            )?);
         }
 
         let report = BenchReport {
@@ -210,7 +164,7 @@ impl BenchExecCommand {
         };
         report.print();
 
-        true
+        Ok(())
     }
 }
 
@@ -481,48 +435,46 @@ fn load_input(args: &BenchExecArgs) -> Result<Vec<u8>, String> {
 ///
 /// AFL++'s Nyx build is required both to prepare the sharedir (setup-nyx.sh)
 /// and to drive the VM at runtime, so it is checked before either.
-fn locate_libnyx(config: &CampaignConfig) -> Option<PathBuf> {
+fn locate_libnyx(config: &CampaignConfig) -> Result<PathBuf, CliError> {
     if !config.aflpp_path.exists() {
-        log::error!("aflpp_path does not exist: {}", config.aflpp_path.display());
-        return None;
+        return Err(CliError::Msg(format!(
+            "aflpp_path does not exist: {}",
+            config.aflpp_path.display()
+        )));
     }
 
     let libnyx_path = config.aflpp_path.join("libnyx.so");
     if !libnyx_path.exists() {
-        log::error!(
+        return Err(CliError::Msg(format!(
             "{} not found; build AFL++ with Nyx support (see nyx_mode/README.md)",
             libnyx_path.display()
-        );
-        return None;
+        )));
     }
 
-    Some(libnyx_path)
+    Ok(libnyx_path)
 }
 
 /// Builds the image and sets up the sharedir, or (when `no_build`) verifies an
 /// existing sharedir is present.
 ///
 /// libnyx writes `config.ron` into the sharedir during setup-nyx.sh; its
 /// absence means the sharedir was never prepared.
-fn ensure_sharedir(config: &CampaignConfig, no_build: bool) -> bool {
+fn ensure_sharedir(config: &CampaignConfig, no_build: bool) -> Result<(), CliError> {
     if no_build {
         let config_ron = config.sharedir.join("config.ron");
         if !config_ron.exists() {
-            log::error!(
+            return Err(CliError::Msg(format!(
                 "Nyx sharedir is not set up ({} missing); drop --no-build, or run \
                  `smitebot start` / scripts/setup-nyx.sh first",
                 config_ron.display()
-            );
-            return false;
+            )));
         }
-        return true;
+        return Ok(());
     }
 
     let image = config.image_tag();
     log::info!("building Docker image {image}");
-    if !run_build(&BuildInputs::from_config(config, &image)) {
-        return false;
-    }
+    run_build(&BuildInputs::from_config(config, &image))?;
 
     log::info!("setting up Nyx sharedir at {}", config.sharedir.display());
     setup_nyx(config, &image)

smitebot/src/commands/build.rs

@@ -7,6 +7,7 @@ use std::process::{Command, ExitStatus};
 use clap::Args;
 
 use crate::config::{self, CampaignConfig, Target};
+use crate::error::CliError;
 
 /// Command handler for `smitebot build`.
 pub struct BuildCommand;
@@ -118,24 +119,17 @@ impl BuildInputs {
 }
 
 impl BuildCommand {
-    /// Builds the requested Smite Docker image and returns whether Docker succeeded.
-    pub fn execute(args: &BuildArgs) -> bool {
+    /// Builds the requested Smite Docker image.
+    pub fn execute(args: &BuildArgs) -> Result<(), CliError> {
         let config = match &args.config {
-            Some(path) => match CampaignConfig::load(path) {
-                Ok(c) => Some(c),
-                Err(e) => {
-                    log::error!("{e}");
-                    return false;
-                }
-            },
+            Some(path) => Some(CampaignConfig::load(path)?),
             None => None,
         };
         let inputs = BuildInputs::resolve(config.as_ref(), args);
         if let Some(err) =
             config::check_scenario_exists(&inputs.smite_dir, inputs.target, &inputs.scenario)
         {
-            log::error!("{err}");
-            return false;
+            return Err(CliError::Msg(err));
         }
         log::info!(
             "building {} with {}",
@@ -146,30 +140,26 @@ impl BuildCommand {
     }
 }
 
-/// Checks that the Dockerfile exists, runs `docker build`, and reports success.
+/// Checks that the Dockerfile exists and runs `docker build`.
 ///
 /// Shared by `smitebot build` and `smitebot start`.
-pub fn run_build(inputs: &BuildInputs) -> bool {
+pub fn run_build(inputs: &BuildInputs) -> Result<(), CliError> {
     if !inputs.dockerfile.exists() {
-        log::error!("Dockerfile not found: {}", inputs.dockerfile.display());
-        return false;
+        return Err(CliError::Msg(format!(
+            "Dockerfile not found: {}",
+            inputs.dockerfile.display()
+        )));
     }
 
-    let status = match run_docker_build(inputs) {
-        Ok(status) => status,
-        Err(e) => {
-            log::error!("failed to run docker build: {e}");
-            return false;
-        }
-    };
+    let status = run_docker_build(inputs)
+        .map_err(|e| CliError::Msg(format!("failed to run docker build: {e}")))?;
 
     if !status.success() {
-        log::error!("docker build failed with {status}");
-        return false;
+        return Err(CliError::Msg(format!("docker build failed with {status}")));
     }
 
     log::info!("built {}", inputs.image);
-    true
+    Ok(())
 }
 
 /// Returns the default image tag used by Smite's manual Docker build flow.

smitebot/src/commands/config.rs

@@ -7,6 +7,7 @@ use clap::Args;
 use serde::Serialize;
 
 use crate::config::CampaignConfig;
+use crate::error::CliError;
 
 /// Command handler for `smitebot config`.
 pub struct ConfigCommand;
@@ -48,7 +49,10 @@ struct ConfigErrorReport {
 
 impl ConfigCommand {
     /// Validates a campaign configuration file and reports the result.
-    pub fn execute(args: &ConfigArgs) -> bool {
+    ///
+    /// The report (plain or JSON) is the artifact and is always printed here, so
+    /// a failure returns [`CliError::Reported`]: `main` only sets the exit code.
+    pub fn execute(args: &ConfigArgs) -> Result<(), CliError> {
         match CampaignConfig::load(&args.path) {
             Ok(config) => {
                 let errors = config.check_paths();
@@ -96,7 +100,11 @@ impl ConfigCommand {
                         eprintln!("error: {error}");
                     }
                 }
-                valid
+                if valid {
+                    Ok(())
+                } else {
+                    Err(CliError::Reported)
+                }
             }
             Err(e) => {
                 if args.json {
@@ -110,7 +118,7 @@ impl ConfigCommand {
                 } else {
                     eprintln!("error: {e}");
                 }
-                false
+                Err(CliError::Reported)
             }
         }
     }