fix(inventory_agent) : Agent now retreives correct dmidecode fields, fixed uuid generation which is unacceptable, fixed storage drive parsing, much better error handling, much more strict behavior which also leads to more complete output as missing fields will raise errors unless explicitely optional

Cargo.lock

@@ -2178,7 +2178,6 @@ dependencies = [
  "serde",
  "serde_json",
  "sysinfo",
- "uuid",
 ]
 
 [[package]]

harmony_inventory_agent/Cargo.toml

@@ -10,4 +10,3 @@ serde.workspace = true
 serde_json.workspace = true
 log.workspace = true
 env_logger.workspace = true
-uuid.workspace = true

harmony_inventory_agent/src/hwinfo.rs

@@ -1,3 +1,4 @@
+use log::debug;
 use serde::{Deserialize, Serialize};
 use serde_json::Value;
 use std::fs;
@@ -140,8 +141,14 @@ impl PhysicalHost {
             cmd.stdout(std::process::Stdio::null());
             cmd.stderr(std::process::Stdio::null());
 
+            if let Ok(status) = cmd.status() {
+                if !status.success() {
                     missing_tools.push(*tool);
                 }
+            } else {
+                missing_tools.push(*tool);
+            }
+        }
 
         if !missing_tools.is_empty() {
             let missing_str = missing_tools
@@ -174,11 +181,11 @@ impl PhysicalHost {
         true
     }
 
-    fn gather_storage_drives() -> Vec<StorageDrive> {
+    fn gather_storage_drives() -> Result<Vec<StorageDrive>, String> {
         let mut drives = Vec::new();
 
         // Use lsblk with JSON output for robust parsing
-        if let Ok(output) = Command::new("lsblk")
+        let output = Command::new("lsblk")
             .args([
                 "-d",
                 "-o",
@@ -189,16 +196,30 @@ impl PhysicalHost {
                 "--json",
             ])
             .output()
-            && output.status.success()
+            .map_err(|e| format!("Failed to execute lsblk: {}", e))?;
-            && let Ok(json) = serde_json::from_slice::<Value>(&output.stdout)
+
-            && let Some(blockdevices) = json.get("blockdevices").and_then(|v| v.as_array())
+        if !output.status.success() {
-        {
+            return Err(format!(
+                "lsblk command failed: {}",
+                String::from_utf8_lossy(&output.stderr)
+            ));
+        }
+
+        let json: Value = serde_json::from_slice(&output.stdout)
+            .map_err(|e| format!("Failed to parse lsblk JSON output: {}", e))?;
+
+        let blockdevices = json
+            .get("blockdevices")
+            .and_then(|v| v.as_array())
+            .ok_or("Invalid lsblk JSON: missing 'blockdevices' array")?;
+
         for device in blockdevices {
             let name = device
                 .get("name")
                 .and_then(|v| v.as_str())
-                    .unwrap_or("")
+                .ok_or("Missing 'name' in lsblk device")?
                 .to_string();
+
             if name.is_empty() {
                 continue;
             }
@@ -215,13 +236,16 @@ impl PhysicalHost {
                 .map(|s| s.trim().to_string())
                 .unwrap_or_default();
 
-                let size_str = device.get("size").and_then(|v| v.as_str()).unwrap_or("0");
+            let size_str = device
-                let size_bytes = Self::parse_size(size_str).unwrap_or(0);
+                .get("size")
+                .and_then(|v| v.as_str())
+                .ok_or("Missing 'size' in lsblk device")?;
+            let size_bytes = Self::parse_size(size_str)?;
 
             let rotational = device
                 .get("rota")
                 .and_then(|v| v.as_bool())
-                    .unwrap_or(false);
+                .ok_or("Missing 'rota' in lsblk device")?;
 
             let wwn = device
                 .get("wwn")
@@ -231,56 +255,67 @@ impl PhysicalHost {
 
             let device_path = Path::new("/sys/block").join(&name);
 
+            let logical_block_size = Self::read_sysfs_u32(
+                &device_path.join("queue/logical_block_size"),
+            )
+            .map_err(|e| format!("Failed to read logical block size for {}: {}", name, e))?;
+
+            let physical_block_size = Self::read_sysfs_u32(
+                &device_path.join("queue/physical_block_size"),
+            )
+            .map_err(|e| format!("Failed to read physical block size for {}: {}", name, e))?;
+
+            let interface_type = Self::get_interface_type(&name, &device_path)?;
+            let smart_status = Self::get_smart_status(&name)?;
+
             let mut drive = StorageDrive {
                 name: name.clone(),
                 model,
                 serial,
                 size_bytes,
-                    logical_block_size: Self::read_sysfs_u32(
+                logical_block_size,
-                        &device_path.join("queue/logical_block_size"),
+                physical_block_size,
-                    )
-                    .unwrap_or(512),
-                    physical_block_size: Self::read_sysfs_u32(
-                        &device_path.join("queue/physical_block_size"),
-                    )
-                    .unwrap_or(512),
                 rotational,
                 wwn,
-                    interface_type: Self::get_interface_type(&name, &device_path),
+                interface_type,
-                    smart_status: Self::get_smart_status(&name),
+                smart_status,
             };
 
             // Enhance with additional sysfs info if available
             if device_path.exists() {
                 if drive.model.is_empty() {
-                        drive.model = Self::read_sysfs_string(&device_path.join("device/model"));
+                    drive.model = Self::read_sysfs_string(&device_path.join("device/model"))
+                        .map_err(|e| format!("Failed to read model for {}: {}", name, e))?;
                 }
                 if drive.serial.is_empty() {
-                        drive.serial = Self::read_sysfs_string(&device_path.join("device/serial"));
+                    drive.serial = Self::read_sysfs_string(&device_path.join("device/serial"))
+                        .map_err(|e| format!("Failed to read serial for {}: {}", name, e))?;
                 }
             }
 
             drives.push(drive);
         }
+
+        Ok(drives)
     }
 
-        drives
+    fn gather_storage_controller() -> Result<StorageController, String> {
-    }
-
-    fn gather_storage_controller() -> StorageController {
         let mut controller = StorageController {
             name: "Unknown".to_string(),
             driver: "Unknown".to_string(),
         };
 
         // Use lspci with JSON output if available
-        if let Ok(output) = Command::new("lspci")
+        let output = Command::new("lspci")
             .args(["-nn", "-d", "::0100", "-J"]) // Storage controllers class with JSON
             .output()
-            && output.status.success()
+            .map_err(|e| format!("Failed to execute lspci: {}", e))?;
-            && let Ok(json) = serde_json::from_slice::<Value>(&output.stdout)
+
-            && let Some(devices) = json.as_array()
+        if output.status.success() {
-        {
+            let json: Value = serde_json::from_slice(&output.stdout)
+                .map_err(|e| format!("Failed to parse lspci JSON output: {}", e))?;
+
+            if let Some(devices) = json.as_array() {
                 for device in devices {
                     if let Some(device_info) = device.as_object()
                         && let Some(name) = device_info
@@ -294,14 +329,16 @@ impl PhysicalHost {
                     }
                 }
             }
+        }
 
-        // Fallback to text output if JSON fails
+        // Fallback to text output if JSON fails or no device found
-        if controller.name == "Unknown"
+        if controller.name == "Unknown" {
-            && let Ok(output) = Command::new("lspci")
+            let output = Command::new("lspci")
                 .args(["-nn", "-d", "::0100"]) // Storage controllers class
                 .output()
-            && output.status.success()
+                .map_err(|e| format!("Failed to execute lspci (fallback): {}", e))?;
-        {
+
+            if output.status.success() {
                 let output_str = String::from_utf8_lossy(&output.stdout);
                 if let Some(line) = output_str.lines().next() {
                     let parts: Vec<&str> = line.split(':').collect();
@@ -310,11 +347,14 @@ impl PhysicalHost {
                     }
                 }
             }
+        }
 
         // Try to get driver info from lsmod
-        if let Ok(output) = Command::new("lsmod").output()
+        let output = Command::new("lsmod")
-            && output.status.success()
+            .output()
-        {
+            .map_err(|e| format!("Failed to execute lsmod: {}", e))?;
+
+        if output.status.success() {
             let output_str = String::from_utf8_lossy(&output.stdout);
             for line in output_str.lines() {
                 if line.contains("ahci")
@@ -331,16 +371,28 @@ impl PhysicalHost {
             }
         }
 
-        controller
+        Ok(controller)
     }
 
-    fn gather_memory_modules() -> Vec<MemoryModule> {
+    fn gather_memory_modules() -> Result<Vec<MemoryModule>, String> {
         let mut modules = Vec::new();
 
-        if let Ok(output) = Command::new("dmidecode").arg("--type").arg("17").output()
+        let output = Command::new("dmidecode")
-            && output.status.success()
+            .arg("--type")
-        {
+            .arg("17")
-            let output_str = String::from_utf8_lossy(&output.stdout);
+            .output()
+            .map_err(|e| format!("Failed to execute dmidecode: {}", e))?;
+
+        if !output.status.success() {
+            return Err(format!(
+                "dmidecode command failed: {}",
+                String::from_utf8_lossy(&output.stderr)
+            ));
+        }
+
+        let output_str = String::from_utf8(output.stdout)
+            .map_err(|e| format!("Failed to parse dmidecode output: {}", e))?;
+
         let sections: Vec<&str> = output_str.split("Memory Device").collect();
 
         for section in sections.into_iter().skip(1) {
@@ -386,12 +438,11 @@ impl PhysicalHost {
                 modules.push(module);
             }
         }
+
+        Ok(modules)
     }
 
-        modules
+    fn gather_cpus(sys: &System) -> Result<Vec<CPU>, String> {
-    }
-
-    fn gather_cpus(sys: &System) -> Vec<CPU> {
         let mut cpus = Vec::new();
         let global_cpu = sys.global_cpu_info();
 
@@ -403,23 +454,29 @@ impl PhysicalHost {
             frequency_mhz: global_cpu.frequency(),
         });
 
-        cpus
+        Ok(cpus)
     }
 
     fn gather_chipset() -> Result<Chipset, String> {
         Ok(Chipset {
-            name: Self::read_dmi("board-product-name")?,
+            name: Self::read_dmi("baseboard-product-name")?,
-            vendor: Self::read_dmi("board-manufacturer")?,
+            vendor: Self::read_dmi("baseboard-manufacturer")?,
         })
     }
 
-    fn gather_network_interfaces() -> Vec<NetworkInterface> {
+    fn gather_network_interfaces() -> Result<Vec<NetworkInterface>, String> {
         let mut interfaces = Vec::new();
         let sys_net_path = Path::new("/sys/class/net");
 
-        if let Ok(entries) = fs::read_dir(sys_net_path) {
+        let entries = fs::read_dir(sys_net_path)
-            for entry in entries.flatten() {
+            .map_err(|e| format!("Failed to read /sys/class/net: {}", e))?;
-                let iface_name = entry.file_name().into_string().unwrap_or_default();
+
+        for entry in entries {
+            let entry = entry.map_err(|e| format!("Failed to read directory entry: {}", e))?;
+            let iface_name = entry
+                .file_name()
+                .into_string()
+                .map_err(|_| "Invalid UTF-8 in interface name")?;
             let iface_path = entry.path();
 
             // Skip virtual interfaces
@@ -439,16 +496,42 @@ impl PhysicalHost {
                 continue;
             }
 
-                let mac_address = Self::read_sysfs_string(&iface_path.join("address"));
+            let mac_address = Self::read_sysfs_string(&iface_path.join("address"))
-                let speed_mbps = Self::read_sysfs_u32(&iface_path.join("speed"));
+                .map_err(|e| format!("Failed to read MAC address for {}: {}", iface_name, e))?;
-                let operstate = Self::read_sysfs_string(&iface_path.join("operstate"));
+
-                let mtu = Self::read_sysfs_u32(&iface_path.join("mtu")).unwrap_or(1500);
+            let speed_mbps = if iface_path.join("speed").exists() {
-                let driver = Self::read_sysfs_string(&iface_path.join("device/driver/module"));
+                match Self::read_sysfs_u32(&iface_path.join("speed")) {
-                let firmware_version =
+                    Ok(speed) => Some(speed),
-                    Self::read_sysfs_opt_string(&iface_path.join("device/firmware_version"));
+                    Err(e) => {
+                        debug!(
+                            "Failed to read speed for {}: {} . This is expected to fail on wifi interfaces.",
+                            iface_name, e
+                        );
+                        None
+                    }
+                }
+            } else {
+                None
+            };
+
+            let operstate = Self::read_sysfs_string(&iface_path.join("operstate"))
+                .map_err(|e| format!("Failed to read operstate for {}: {}", iface_name, e))?;
+
+            let mtu = Self::read_sysfs_u32(&iface_path.join("mtu"))
+                .map_err(|e| format!("Failed to read MTU for {}: {}", iface_name, e))?;
+
+            let driver =
+                Self::read_sysfs_symlink_basename(&iface_path.join("device/driver/module"))
+                    .map_err(|e| format!("Failed to read driver for {}: {}", iface_name, e))?;
+
+            let firmware_version = Self::read_sysfs_opt_string(
+                &iface_path.join("device/firmware_version"),
+            )
+            .map_err(|e| format!("Failed to read firmware version for {}: {}", iface_name, e))?;
 
             // Get IP addresses using ip command with JSON output
-                let (ipv4_addresses, ipv6_addresses) = Self::get_interface_ips_json(&iface_name);
+            let (ipv4_addresses, ipv6_addresses) = Self::get_interface_ips_json(&iface_name)
+                .map_err(|e| format!("Failed to get IP addresses for {}: {}", iface_name, e))?;
 
             interfaces.push(NetworkInterface {
                 name: iface_name,
@@ -462,173 +545,219 @@ impl PhysicalHost {
                 firmware_version,
             });
         }
+
+        Ok(interfaces)
     }
 
-        interfaces
+    fn gather_management_interface() -> Result<Option<ManagementInterface>, String> {
-    }
-
-    fn gather_management_interface() -> Option<ManagementInterface> {
-        // Try to detect common management interfaces
         if Path::new("/dev/ipmi0").exists() {
-            Some(ManagementInterface {
+            Ok(Some(ManagementInterface {
                 kind: "IPMI".to_string(),
                 address: None,
-                firmware: Self::read_dmi("bios-version"),
+                firmware: Some(Self::read_dmi("bios-version")?),
-            })
+            }))
         } else if Path::new("/sys/class/misc/mei").exists() {
-            Some(ManagementInterface {
+            Ok(Some(ManagementInterface {
                 kind: "Intel ME".to_string(),
                 address: None,
                 firmware: None,
-            })
+            }))
         } else {
-            None
+            Ok(None)
         }
     }
 
-    fn get_host_uuid() -> String {
+    fn get_host_uuid() -> Result<String, String> {
-        Self::read_dmi("system-uuid").unwrap()
+        Self::read_dmi("system-uuid")
     }
 
     // Helper methods
-    fn read_sysfs_string(path: &Path) -> String {
+    fn read_sysfs_string(path: &Path) -> Result<String, String> {
         fs::read_to_string(path)
-            .unwrap_or_default()
-            .trim()
-            .to_string()
-    }
-
-    fn read_sysfs_opt_string(path: &Path) -> Option<String> {
-        fs::read_to_string(path)
-            .ok()
             .map(|s| s.trim().to_string())
-            .filter(|s| !s.is_empty())
+            .map_err(|e| format!("Failed to read {}: {}", path.display(), e))
     }
 
-    fn read_sysfs_u32(path: &Path) -> Option<u32> {
+    fn read_sysfs_opt_string(path: &Path) -> Result<Option<String>, String> {
+        match fs::read_to_string(path) {
+            Ok(s) => {
+                let s = s.trim().to_string();
+                Ok(if s.is_empty() { None } else { Some(s) })
+            }
+            Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None),
+            Err(e) => Err(format!("Failed to read {}: {}", path.display(), e)),
+        }
+    }
+
+    fn read_sysfs_u32(path: &Path) -> Result<u32, String> {
         fs::read_to_string(path)
-            .ok()
+            .map_err(|e| format!("Failed to read {}: {}", path.display(), e))?
-            .and_then(|s| s.trim().parse().ok())
+            .trim()
+            .parse()
+            .map_err(|e| format!("Failed to parse {}: {}", path.display(), e))
+    }
+
+    fn read_sysfs_symlink_basename(path: &Path) -> Result<String, String> {
+        match fs::read_link(path) {
+            Ok(target_path) => match target_path.file_name() {
+                Some(name_osstr) => match name_osstr.to_str() {
+                    Some(name_str) => Ok(name_str.to_string()),
+                    None => Err(format!(
+                        "Symlink target basename is not valid UTF-8: {}",
+                        target_path.display()
+                    )),
+                },
+                None => Err(format!(
+                    "Symlink target has no basename: {} -> {}",
+                    path.display(),
+                    target_path.display()
+                )),
+            },
+            Err(e) if e.kind() == std::io::ErrorKind::NotFound => Err(format!(
+                "Could not resolve symlink for path : {}",
+                path.display()
+            )),
+            Err(e) => Err(format!("Failed to read symlink {}: {}", path.display(), e)),
+        }
     }
 
-    // Valid string keywords are:
-    // bios-vendor
-    // bios-version
-    // bios-release-date
-    // bios-revision
-    // firmware-revision
-    // system-manufacturer
-    // system-product-name
-    // system-version
-    // system-serial-number
-    // system-uuid
-    // system-sku-number
-    // system-family
-    // baseboard-manufacturer
-    // baseboard-product-name
-    // baseboard-version
-    // baseboard-serial-number
-    // baseboard-asset-tag
-    // chassis-manufacturer
-    // chassis-type
-    // chassis-version
-    // chassis-serial-number
-    // chassis-asset-tag
-    // processor-family
-    // processor-manufacturer
-    // processor-version
-    // processor-frequency
     fn read_dmi(field: &str) -> Result<String, String> {
-        match Command::new("dmidecode").arg("-s").arg(field).output() {
+        let output = Command::new("dmidecode")
-            Ok(output) => {
+            .arg("-s")
-                let stdout = String::from_utf8(output.stdout).expect("Output should parse as utf8");
+            .arg(field)
-                if output.status.success() && stdout.is_empty() {
+            .output()
-                    return Ok(stdout);
+            .map_err(|e| format!("Failed to execute dmidecode for field {}: {}", field, e))?;
-                } else {
+
+        if !output.status.success() {
             return Err(format!(
-                        "dmidecode command failed for field {field} : {stdout}"
+                "dmidecode command failed for field {}: {}",
+                field,
+                String::from_utf8_lossy(&output.stderr)
             ));
         }
-            }
-            Err(e) => Err(format!("dmidecode command failed for field {field} : {e}")),
-        }
-    }
 
-    fn get_interface_type(device_name: &str, device_path: &Path) -> String {
+        String::from_utf8(output.stdout)
-        if device_name.starts_with("nvme") {
-            "NVMe".to_string()
-        } else if device_name.starts_with("sd") {
-            "SATA".to_string()
-        } else if device_name.starts_with("hd") {
-            "IDE".to_string()
-        } else if device_name.starts_with("vd") {
-            "VirtIO".to_string()
-        } else {
-            // Try to determine from device path
-            Self::read_sysfs_string(&device_path.join("device/subsystem"))
-                .split('/')
-                .next_back()
-                .unwrap_or("Unknown")
-                .to_string()
-        }
-    }
-
-    fn get_smart_status(device_name: &str) -> Option<String> {
-        Command::new("smartctl")
-            .arg("-H")
-            .arg(format!("/dev/{}", device_name))
-            .output()
-            .ok()
-            .filter(|output| output.status.success())
-            .and_then(|output| String::from_utf8(output.stdout).ok())
-            .and_then(|s| {
-                s.lines()
-                    .find(|line| line.contains("SMART overall-health self-assessment"))
-                    .and_then(|line| line.split(':').nth(1))
             .map(|s| s.trim().to_string())
+            .map_err(|e| {
+                format!(
+                    "Failed to parse dmidecode output for field {}: {}",
+                    field, e
+                )
             })
     }
 
-    fn parse_size(size_str: &str) -> Option<u64> {
+    fn get_interface_type(device_name: &str, device_path: &Path) -> Result<String, String> {
-        if size_str.ends_with('T') {
+        if device_name.starts_with("nvme") {
-            size_str[..size_str.len() - 1]
+            Ok("NVMe".to_string())
-                .parse::<u64>()
+        } else if device_name.starts_with("sd") {
-                .ok()
+            Ok("SATA".to_string())
-                .map(|t| t * 1024 * 1024 * 1024 * 1024)
+        } else if device_name.starts_with("hd") {
-        } else if size_str.ends_with('G') {
+            Ok("IDE".to_string())
-            size_str[..size_str.len() - 1]
+        } else if device_name.starts_with("vd") {
-                .parse::<u64>()
+            Ok("VirtIO".to_string())
-                .ok()
-                .map(|g| g * 1024 * 1024 * 1024)
-        } else if size_str.ends_with('M') {
-            size_str[..size_str.len() - 1]
-                .parse::<u64>()
-                .ok()
-                .map(|m| m * 1024 * 1024)
-        } else if size_str.ends_with('K') {
-            size_str[..size_str.len() - 1]
-                .parse::<u64>()
-                .ok()
-                .map(|k| k * 1024)
-        } else if size_str.ends_with('B') {
-            size_str[..size_str.len() - 1].parse::<u64>().ok()
         } else {
-            size_str.parse::<u64>().ok()
+            // Try to determine from device path
+            let subsystem = Self::read_sysfs_string(&device_path.join("device/subsystem"))?;
+            Ok(subsystem
+                .split('/')
+                .next_back()
+                .unwrap_or("Unknown")
+                .to_string())
         }
     }
 
-    fn get_interface_ips_json(iface_name: &str) -> (Vec<String>, Vec<String>) {
+    fn get_smart_status(device_name: &str) -> Result<Option<String>, String> {
+        let output = Command::new("smartctl")
+            .arg("-H")
+            .arg(format!("/dev/{}", device_name))
+            .output()
+            .map_err(|e| format!("Failed to execute smartctl for {}: {}", device_name, e))?;
+
+        if !output.status.success() {
+            return Ok(None);
+        }
+
+        let stdout = String::from_utf8(output.stdout)
+            .map_err(|e| format!("Failed to parse smartctl output for {}: {}", device_name, e))?;
+
+        for line in stdout.lines() {
+            if line.contains("SMART overall-health self-assessment") {
+                if let Some(status) = line.split(':').nth(1) {
+                    return Ok(Some(status.trim().to_string()));
+                }
+            }
+        }
+
+        Ok(None)
+    }
+
+    fn parse_size(size_str: &str) -> Result<u64, String> {
+        debug!("Parsing size_str '{size_str}'");
+        let size;
+        if size_str.ends_with('T') {
+            size = size_str[..size_str.len() - 1]
+                .parse::<f64>()
+                .map(|t| t * 1024.0 * 1024.0 * 1024.0 * 1024.0)
+                .map_err(|e| format!("Failed to parse T size '{}': {}", size_str, e))
+        } else if size_str.ends_with('G') {
+            size = size_str[..size_str.len() - 1]
+                .parse::<f64>()
+                .map(|g| g * 1024.0 * 1024.0 * 1024.0)
+                .map_err(|e| format!("Failed to parse G size '{}': {}", size_str, e))
+        } else if size_str.ends_with('M') {
+            size = size_str[..size_str.len() - 1]
+                .parse::<f64>()
+                .map(|m| m * 1024.0 * 1024.0)
+                .map_err(|e| format!("Failed to parse M size '{}': {}", size_str, e))
+        } else if size_str.ends_with('K') {
+            size = size_str[..size_str.len() - 1]
+                .parse::<f64>()
+                .map(|k| k * 1024.0)
+                .map_err(|e| format!("Failed to parse K size '{}': {}", size_str, e))
+        } else if size_str.ends_with('B') {
+            size = size_str[..size_str.len() - 1]
+                .parse::<f64>()
+                .map_err(|e| format!("Failed to parse B size '{}': {}", size_str, e))
+        } else {
+            size = size_str
+                .parse::<f64>()
+                .map_err(|e| format!("Failed to parse size '{}': {}", size_str, e))
+        }
+
+        size.map(|s| s as u64)
+    }
+
+    fn get_interface_ips_json(iface_name: &str) -> Result<(Vec<String>, Vec<String>), String> {
         let mut ipv4 = Vec::new();
         let mut ipv6 = Vec::new();
 
         // Get IPv4 addresses using JSON output
-        if let Ok(output) = Command::new("ip")
+        let output = Command::new("ip")
             .args(["-j", "-4", "addr", "show", iface_name])
             .output()
-            && output.status.success()
+            .map_err(|e| {
-            && let Ok(json) = serde_json::from_slice::<Value>(&output.stdout)
+                format!(
-            && let Some(addrs) = json.as_array()
+                    "Failed to execute ip command for IPv4 on {}: {}",
-        {
+                    iface_name, e
+                )
+            })?;
+
+        if !output.status.success() {
+            return Err(format!(
+                "ip command for IPv4 on {} failed: {}",
+                iface_name,
+                String::from_utf8_lossy(&output.stderr)
+            ));
+        }
+
+        let json: Value = serde_json::from_slice(&output.stdout).map_err(|e| {
+            format!(
+                "Failed to parse ip JSON output for IPv4 on {}: {}",
+                iface_name, e
+            )
+        })?;
+
+        if let Some(addrs) = json.as_array() {
             for addr_info in addrs {
                 if let Some(addr_info_obj) = addr_info.as_object()
                     && let Some(addr_info) =
@@ -646,13 +775,32 @@ impl PhysicalHost {
         }
 
         // Get IPv6 addresses using JSON output
-        if let Ok(output) = Command::new("ip")
+        let output = Command::new("ip")
             .args(["-j", "-6", "addr", "show", iface_name])
             .output()
-            && output.status.success()
+            .map_err(|e| {
-            && let Ok(json) = serde_json::from_slice::<Value>(&output.stdout)
+                format!(
-            && let Some(addrs) = json.as_array()
+                    "Failed to execute ip command for IPv6 on {}: {}",
-        {
+                    iface_name, e
+                )
+            })?;
+
+        if !output.status.success() {
+            return Err(format!(
+                "ip command for IPv6 on {} failed: {}",
+                iface_name,
+                String::from_utf8_lossy(&output.stderr)
+            ));
+        }
+
+        let json: Value = serde_json::from_slice(&output.stdout).map_err(|e| {
+            format!(
+                "Failed to parse ip JSON output for IPv6 on {}: {}",
+                iface_name, e
+            )
+        })?;
+
+        if let Some(addrs) = json.as_array() {
             for addr_info in addrs {
                 if let Some(addr_info_obj) = addr_info.as_object()
                     && let Some(addr_info) =
@@ -672,6 +820,6 @@ impl PhysicalHost {
             }
         }
 
-        (ipv4, ipv6)
+        Ok((ipv4, ipv6))
     }
 }