chore: replace unwrap calls with safer alternatives

This replaces calls to `unwrap()` across the crate with safer alternatives.

Some patterns observed + updated include:

1. Replacing `is_some() + unwrap()` with `if let Some()`.

```
// Before:
if bios.usb_boot.is_some() {
    let val = bios.usb_boot.unwrap();
    // Do something with val.
}

// After:
if let Some(val) = bios.usb_boot {
    // Do something with val.
}
```

2. Replacing `is_some() + `clone().unwrap() == "value"` with `as_deref() == Some("value")`.

```
// Before:
if bios.usb_boot.is_some() && bios.usb_boot.clone().unwrap() == "Disabled"

// After (note we can also drop the clone):
if bios.usb_boot.as_deref() == Some("Disabled")
```

3. Replacing `partial_cmp().unwrap()` with `cmp()`.

Since `Option<String>` implements `Ord`, we can use `cmp()` and not need `partial_cmp().unwrap()`.

```
// Before:
devices.sort_unstable_by(|a, b| a.manufacturer.partial_cmp(&b.manufacturer).unwrap());

// After:
devices.sort_unstable_by(|a, b| a.manufacturer.cmp(&b.manufacturer));
```

4. Improving `unwrap()` operations that are fallible.

We had a number of places where we were just unwrapping options without
any error checking at all. This now returns an error with a useful error
message in said cases.

```
// Before:
let system_id = systems.first().unwrap();

// After:
let system_id = systems.first().ok_or_else(|| RedfishError::GenericError {
    error: "No systems found in service root".to_string(),
})?;
```

5. Improving `unwrap()` operations that are considered infallible.

So even for operations we'd consider infallible, like setting the MIME type
on an HTTP request, or having our statically compiled regex, we should
still properly have an error check in there instead of blindly unwrapping.

```
// Before:
.mime_str("application/json").unwrap()

// After:
.mime_str("application/json")
.map_err(|e| RedfishError::GenericError {
    error: format!("Invalid MIME type 'application/json': {}", e),
})?
```

Author: chet

src/dell.rs

@@ -1921,10 +1921,14 @@ impl Bmc {
         let na_id = match chassis.network_adapters {
             Some(id) => id,
             None => {
+                let chassis_odata_url = chassis
+                    .odata
+                    .map(|o| o.odata_id)
+                    .unwrap_or_else(|| "empty_odata_id_url".to_string());
                 return Err(RedfishError::MissingKey {
                     key: "network_adapters".to_string(),
-                    url: chassis.odata.unwrap().odata_id,
-                })
+                    url: chassis_odata_url,
+                });
             }
         };
 

src/hpe.rs

@@ -338,22 +338,19 @@ impl Redfish for Bmc {
         let (_status, bmc): (_, hpe::SetOemHpeLockdown) = self.s.client.get(url.as_str()).await?;
         let message = format!(
             "usb_boot={}, virtual_nic_enabled={}",
-            bios.usb_boot.clone().unwrap_or("Unknown".to_string()),
+            bios.usb_boot.as_deref().unwrap_or("Unknown"),
             bmc.oem.hpe.virtual_nic_enabled
         );
         // todo: kcs_enabled
         Ok(Status {
             message,
-            status: if bios.usb_boot.is_some()
-                && bios.usb_boot.clone().unwrap() == "Disabled"
+            status: if bios.usb_boot.as_deref() == Some("Disabled")
                 && !bmc.oem.hpe.virtual_nic_enabled
-            //&& bios.kcs_enabled.is_some() && bios.kcs_enabled.unwrap() == "false"
+            // todo: && bios.kcs_enabled.as_deref() == Some("false")
             {
                 StatusInternal::Enabled
-            } else if bios.usb_boot.is_some()
-                && bios.usb_boot.clone().unwrap() == "Enabled"
-                && bmc.oem.hpe.virtual_nic_enabled
-            // if bios.usb_boot == "Enabled" && bios.kcs_enabled.clone().is_some() && bios.kcs_enabled.clone().unwrap() == "true"
+            // todo: if bios.usb_boot.as_deref() == Some("Enabled") && bios.kcs_enabled.as_deref() == Some("true")
+            } else if bios.usb_boot.as_deref() == Some("Enabled") && bmc.oem.hpe.virtual_nic_enabled
             {
                 StatusInternal::Disabled
             } else {
@@ -436,16 +433,15 @@ impl Redfish for Bmc {
     async fn pcie_devices(&self) -> Result<Vec<PCIeDevice>, RedfishError> {
         let mut out = Vec::new();
         let chassis = self.get_chassis(self.s.system_id()).await?;
-        if chassis.pcie_devices.is_none() {
-            return Ok(vec![]);
-        }
-        let mut devices: Vec<HpePCIeDevice> = Vec::new();
-        let url = chassis
-            .pcie_devices
-            .unwrap()
+        let pcie_devices_odata = match chassis.pcie_devices {
+            Some(odata) => odata,
+            None => return Ok(vec![]),
+        };
+        let url = pcie_devices_odata
             .odata_id
             .replace(&format!("/{REDFISH_ENDPOINT}/"), "");
         let pcie_devices = self.s.get_members(&url).await?;
+        let mut devices: Vec<HpePCIeDevice> = Vec::new();
         for pcie_oid in pcie_devices {
             let dev_url = format!("{}/{}", &url, pcie_oid);
             let (_, hpe_pcie) = self.s.client.get(&dev_url).await?;
@@ -481,7 +477,7 @@ impl Redfish for Bmc {
             }
             out.push(pcie);
         }
-        out.sort_unstable_by(|a, b| a.manufacturer.partial_cmp(&b.manufacturer).unwrap());
+        out.sort_unstable_by(|a, b| a.manufacturer.cmp(&b.manufacturer));
 
         Ok(out)
     }
@@ -595,12 +591,9 @@ impl Redfish for Bmc {
         &self,
         chassis_id: &str,
     ) -> Result<Vec<String>, RedfishError> {
-        // self.s.get_chassis_network_adapters(chassis_id).await
         let chassis = self.s.get_chassis(chassis_id).await?;
-        if chassis.network_adapters.is_some() {
-            let url = chassis
-                .network_adapters
-                .unwrap()
+        if let Some(network_adapters_odata) = chassis.network_adapters {
+            let url = network_adapters_odata
                 .odata_id
                 .replace(&format!("/{REDFISH_ENDPOINT}/"), "");
             // let url = format!("Chassis/{}/NetworkAdapters", chassis_id);

src/model/power.rs

@@ -191,9 +191,9 @@ pub struct Power {
 impl StatusVec for Power {
     fn get_vec(&self) -> Vec<ResourceStatus> {
         let mut v: Vec<ResourceStatus> = Vec::new();
-        if self.power_supplies.is_some() {
-            for res in self.power_supplies.clone().unwrap() {
-                v.push(res.status)
+        if let Some(power_supplies) = &self.power_supplies {
+            for res in power_supplies {
+                v.push(res.status);
             }
         }
         v

src/network.rs

@@ -87,7 +87,9 @@ impl RedfishClientPoolBuilder {
             .connect_timeout(self.connect_timeout)
             .timeout(self.timeout)
             .build()
-            .unwrap();
+            .map_err(|e| RedfishError::GenericError {
+                error: format!("Failed to build RedfishClientPool HTTP client: {}", e),
+            })?;
         let pool = RedfishClientPool { http_client };
 
         Ok(pool)
@@ -162,8 +164,12 @@ impl RedfishClientPool {
         let service_root = s.get_service_root().await?;
         let systems = s.get_systems().await?;
         let managers = s.get_managers().await?;
-        let system_id = systems.first().unwrap();
-        let manager_id = managers.first().unwrap();
+        let system_id = systems.first().ok_or_else(|| RedfishError::GenericError {
+            error: "No systems found in service root".to_string(),
+        })?;
+        let manager_id = managers.first().ok_or_else(|| RedfishError::GenericError {
+            error: "No managers found in service root".to_string(),
+        })?;
         let chassis = s.get_chassis_all().await?;
 
         s.set_system_id(system_id)?;
@@ -626,14 +632,27 @@ impl RedfishHttpClient {
                     .part(
                         "UpdateParameters",
                         reqwest::multipart::Part::text(parameters)
+                            // mime_str_to_part parses the MIME type. Technically this is
+                            // infallible for known MIME types, including application/json,
+                            // but still check for an error instead of unwrapping.
                             .mime_str("application/json")
-                            .unwrap(),
+                            .map_err(|e| RedfishError::GenericError {
+                                error: format!("Invalid MIME type 'application/json': {}", e),
+                            })?,
                     )
                     .part(
                         "UpdateFile",
                         Part::stream_with_length(file, length)
+                            // mime_str_to_part parses the MIME type. Technically this is
+                            // infallible for known MIME types, including application/octet-stream,
+                            // but still check for an error instead of unwrapping.
                             .mime_str("application/octet-stream")
-                            .unwrap()
+                            .map_err(|e| RedfishError::GenericError {
+                                error: format!(
+                                    "Invalid MIME type 'application/octet-stream': {}",
+                                    e
+                                ),
+                            })?
                             // Yes, the filename passed does matter for some reason, at least for Dells, and it has to be the basename.
                             .file_name(basename.clone()),
                     ),

src/nvidia_gbswitch.rs

@@ -358,25 +358,23 @@ impl Redfish for Bmc {
                 for id in devices.members {
                     url = id.odata_id.replace(&format!("/{REDFISH_ENDPOINT}/"), "");
                     let p: PCIeDevice = self.s.client.get(&url).await?.1;
-                    if p.id.is_none()
-                        || p.status.is_none()
-                        || !p
-                            .status
-                            .as_ref()
-                            .unwrap()
-                            .state
-                            .as_ref()
-                            .unwrap()
-                            .to_lowercase()
-                            .contains("enabled")
-                    {
+                    // To be considered enabled, the PCIE device needs to have
+                    // an ID, a status, and the status needs to be enabled.
+                    let is_device_enabled = p.id.is_some()
+                        && p.status.as_ref().is_some_and(|s| {
+                            s.state
+                                .as_ref()
+                                .is_some_and(|state| state.to_lowercase().contains("enabled"))
+                        });
+                    if !is_device_enabled {
                         continue;
                     }
                     out.push(p);
                 }
             }
         }
-        out.sort_unstable_by(|a, b| a.manufacturer.partial_cmp(&b.manufacturer).unwrap());
+
+        out.sort_unstable_by(|a, b| a.manufacturer.cmp(&b.manufacturer));
         Ok(out)
     }
 

src/nvidia_gbx00.rs

@@ -109,13 +109,19 @@ impl Display for BootOptionMatchField {
 // Supported component to firmware mapping.
 // GPU, Source: HGX_IRoT_GPU_X Target: HGX_FW_GPU_X
 fn get_component_integrity_id_to_firmware_inventory_id_options(
-) -> &'static Vec<RegexToFirmwareIdOptions> {
-    static RE: OnceLock<Vec<RegexToFirmwareIdOptions>> = OnceLock::new();
+) -> Result<&'static Vec<RegexToFirmwareIdOptions>, RedfishError> {
+    static RE: OnceLock<Result<Vec<RegexToFirmwareIdOptions>, String>> = OnceLock::new();
     RE.get_or_init(|| {
-        vec![RegexToFirmwareIdOptions {
+        Ok(vec![RegexToFirmwareIdOptions {
             id_prefix: "HGX_FW_",
-            pattern: Regex::new(r"HGX_IRoT_(GPU_\d+)").unwrap(),
-        }]
+            // Assuming our static pattern is good, this is probably
+            // safe, but still check for an error instead of unwrapping.
+            pattern: Regex::new(r"HGX_IRoT_(GPU_\d+)").map_err(|e| e.to_string())?,
+        }])
+    })
+    .as_ref()
+    .map_err(|e| RedfishError::GenericError {
+        error: format!("Failed to compile regex: {}", e),
     })
 }
 
@@ -1039,7 +1045,7 @@ impl Redfish for Bmc {
     ) -> Result<crate::model::software_inventory::SoftwareInventory, RedfishError> {
         let mut id = None;
 
-        for value in get_component_integrity_id_to_firmware_inventory_id_options() {
+        for value in get_component_integrity_id_to_firmware_inventory_id_options()? {
             if let Some(capture) = value.pattern.captures(component_integrity_id) {
                 id = Some(format!(
                     "{}{}",

src/nvidia_viking.rs

@@ -1226,8 +1226,7 @@ impl Bmc {
         let bios = self.get_bios().await?;
         let bios = bios.attributes;
 
-        if bios.acpi_spcr_console_redirection_enable.is_some() {
-            let val = bios.acpi_spcr_console_redirection_enable.unwrap();
+        if let Some(val) = bios.acpi_spcr_console_redirection_enable {
             message.push_str(&format!("acpi_spcr_console_redirection_enable={val} "));
             match val {
                 true => {
@@ -1240,8 +1239,7 @@ impl Bmc {
                 }
             }
         }
-        if bios.console_redirection_enable0.is_some() {
-            let val = bios.console_redirection_enable0.unwrap();
+        if let Some(val) = bios.console_redirection_enable0 {
             message.push_str(&format!("console_redirection_enable0={val} "));
             match val {
                 true => {
@@ -1255,30 +1253,26 @@ impl Bmc {
         // All of these need a specific value for serial console access to work.
         // Any other value counts as correctly disabled.
 
-        if bios.acpi_spcr_port.is_some() {
-            let val = bios.acpi_spcr_port.unwrap();
+        if let Some(val) = &bios.acpi_spcr_port {
             message.push_str(&format!("acpi_spcr_port={val} "));
             if val != DEFAULT_ACPI_SPCR_PORT {
                 enabled = false;
             }
         }
-        if bios.acpi_spcr_flow_control.is_some() {
-            let val = bios.acpi_spcr_flow_control.unwrap();
+        if let Some(val) = &bios.acpi_spcr_flow_control {
             message.push_str(&format!("acpi_spcr_flow_control={val} "));
             if val != DEFAULT_ACPI_SPCR_FLOW_CONTROL {
                 enabled = false;
             }
         }
-        if bios.acpi_spcr_baud_rate.is_some() {
-            let val = bios.acpi_spcr_baud_rate.unwrap();
+        if let Some(val) = &bios.acpi_spcr_baud_rate {
             message.push_str(&format!("acpi_spcr_baud_rate={val} "));
             if val != DEFAULT_ACPI_SPCR_BAUD_RATE {
                 enabled = false;
             }
         }
 
-        if bios.baud_rate0.is_some() {
-            let val = bios.baud_rate0.unwrap();
+        if let Some(val) = &bios.baud_rate0 {
             message.push_str(&format!("baud_rate0={val} "));
             if val != DEFAULT_BAUD_RATE0 {
                 enabled = false;
@@ -1318,7 +1312,7 @@ impl Bmc {
             //
             // TODO: Many BootOptions have Alias="Pxe". This probably isn't doing what we want.
             //
-            if b.alias.is_some() && b.alias.unwrap() == with_name_str {
+            if b.alias.as_deref() == Some(&with_name_str) {
                 ordered.insert(0, format!("Boot{}", b.id).to_string());
                 continue;
             }

src/standard.rs

@@ -615,20 +615,22 @@ impl Redfish for RedfishStandard {
         if systems.members.is_empty() {
             return Ok(vec!["1".to_string()]); // default to DMTF standard suggested
         }
-        let v = systems
+        let v: Result<Vec<String>, RedfishError> = systems
             .members
             .into_iter()
             .map(|d| {
                 d.odata_id
                     .trim_matches('/')
                     .split('/')
                     .next_back()
-                    .unwrap()
-                    .to_string()
+                    .map(|s| s.to_string())
+                    .ok_or_else(|| RedfishError::GenericError {
+                        error: format!("Invalid odata_id format: {}", d.odata_id),
+                    })
             })
             .collect();
 
-        Ok(v)
+        v
     }
 
     async fn get_manager(&self) -> Result<Manager, RedfishError> {
@@ -644,19 +646,21 @@ impl Redfish for RedfishStandard {
         if bmcs.members.is_empty() {
             return Ok(vec!["1".to_string()]);
         }
-        let v: Vec<String> = bmcs
+        let v: Result<Vec<String>, RedfishError> = bmcs
             .members
             .into_iter()
             .map(|d| {
                 d.odata_id
                     .trim_matches('/')
                     .split('/')
                     .next_back()
-                    .unwrap()
-                    .to_string()
+                    .map(|s| s.to_string())
+                    .ok_or_else(|| RedfishError::GenericError {
+                        error: format!("Invalid odata_id format: {}", d.odata_id),
+                    })
             })
             .collect();
-        Ok(v)
+        v
     }
 
     async fn bmc_reset_to_defaults(&self) -> Result<(), RedfishError> {
@@ -1369,7 +1373,7 @@ impl RedfishStandard {
             devices.extend(chassis_devices);
         }
 
-        devices.sort_unstable_by(|a, b| a.manufacturer.partial_cmp(&b.manufacturer).unwrap());
+        devices.sort_unstable_by(|a, b| a.manufacturer.cmp(&b.manufacturer));
         Ok(devices)
     }
 }

tests/integration_test.rs

@@ -806,4 +806,3 @@ fn get_tmp_dir() -> PathBuf {
     let temp_dir = format!("{}-{}-{}", PYTHON_VENV_DIR, std::process::id(), nanos);
     env::temp_dir().join(&temp_dir)
 }
-