fix(openvm-prof): replace unwraps with better error strings (#1833)

- replaced `unwrap` calls with error strings so it's easy to identify
what's missing

Author: shuklaayush

crates/prof/src/aggregate.rs

@@ -170,7 +170,9 @@ impl AggregateMetrics {
             if stats.is_none() {
                 continue;
             }
-            let stats = stats.unwrap();
+            let stats = stats.unwrap_or_else(|| {
+                panic!("Missing proof time statistics for group '{}'", group_name)
+            });
             let mut sum = stats.sum;
             let mut max = stats.max;
             // convert ms to s
@@ -185,9 +187,17 @@ impl AggregateMetrics {
             if !group_name.contains("keygen") {
                 // Proving time in keygen group is dummy and not part of total.
                 total_proof_time.val += sum.val;
-                *total_proof_time.diff.as_mut().unwrap() += sum.diff.unwrap_or(0.0);
+                *total_proof_time
+                    .diff
+                    .as_mut()
+                    .expect("total_proof_time.diff should be initialized") +=
+                    sum.diff.unwrap_or(0.0);
                 total_par_proof_time.val += max.val;
-                *total_par_proof_time.diff.as_mut().unwrap() += max.diff.unwrap_or(0.0);
+                *total_par_proof_time
+                    .diff
+                    .as_mut()
+                    .expect("total_par_proof_time.diff should be initialized") +=
+                    max.diff.unwrap_or(0.0);
 
                 // Account for the serial execute_metered and execute_e1 for app outside of segments
                 if group_name != "leaf"
@@ -203,17 +213,33 @@ impl AggregateMetrics {
                         total_proof_time.val += execute_metered_stats.avg.val / 1000.0;
                         total_par_proof_time.val += execute_metered_stats.avg.val / 1000.0;
                         if let Some(diff) = execute_metered_stats.avg.diff {
-                            *total_proof_time.diff.as_mut().unwrap() += diff / 1000.0;
-                            *total_par_proof_time.diff.as_mut().unwrap() += diff / 1000.0;
+                            *total_proof_time
+                                .diff
+                                .as_mut()
+                                .expect("total_proof_time.diff should be initialized") +=
+                                diff / 1000.0;
+                            *total_par_proof_time
+                                .diff
+                                .as_mut()
+                                .expect("total_par_proof_time.diff should be initialized") +=
+                                diff / 1000.0;
                         }
                     }
 
                     if let Some(execute_e1_stats) = execute_e1_stats {
                         total_proof_time.val += execute_e1_stats.avg.val / 1000.0;
                         total_par_proof_time.val += execute_e1_stats.avg.val / 1000.0;
                         if let Some(diff) = execute_e1_stats.avg.diff {
-                            *total_proof_time.diff.as_mut().unwrap() += diff / 1000.0;
-                            *total_par_proof_time.diff.as_mut().unwrap() += diff / 1000.0;
+                            *total_proof_time
+                                .diff
+                                .as_mut()
+                                .expect("total_proof_time.diff should be initialized") +=
+                                diff / 1000.0;
+                            *total_par_proof_time
+                                .diff
+                                .as_mut()
+                                .expect("total_par_proof_time.diff should be initialized") +=
+                                diff / 1000.0;
                         }
                     }
                 }
@@ -251,7 +277,13 @@ impl AggregateMetrics {
             .into_iter()
             .map(|group_name| {
                 let key = group_name.clone();
-                let value = self.by_group.get(group_name).unwrap().clone();
+                let value = self
+                    .by_group
+                    .get(group_name)
+                    .unwrap_or_else(|| {
+                        panic!("Group '{}' should exist in by_group map", group_name)
+                    })
+                    .clone();
                 (key, value)
             })
             .collect()
@@ -363,7 +395,9 @@ impl AggregateMetrics {
             if stats.is_none() {
                 continue;
             }
-            let stats = stats.unwrap();
+            let stats = stats.unwrap_or_else(|| {
+                panic!("Missing proof time statistics for group '{}'", group_name)
+            });
             let mut sum = stats.sum;
             let mut max = stats.max;
             // convert ms to s
@@ -394,7 +428,12 @@ impl AggregateMetrics {
         self.by_group
             .keys()
             .find(|k| group_weight(k) == 0)
-            .unwrap_or_else(|| self.by_group.keys().next().unwrap())
+            .unwrap_or_else(|| {
+                self.by_group
+                    .keys()
+                    .next()
+                    .expect("by_group should contain at least one group")
+            })
             .clone()
     }
 }

crates/prof/src/lib.rs

@@ -95,7 +95,12 @@ impl MetricDb {
 
             let label_values: Vec<String> = label_keys
                 .iter()
-                .map(|key| label_dict.get(key).unwrap().clone())
+                .map(|key| {
+                    label_dict
+                        .get(key)
+                        .unwrap_or_else(|| panic!("Label key '{}' should exist in label_dict", key))
+                        .clone()
+                })
                 .collect();
 
             // Add to dict_by_label_types

crates/prof/src/main.rs

@@ -84,8 +84,9 @@ fn main() -> Result<()> {
             // If this is a new benchmark, prev_path will not exist
             if let Ok(prev_db) = MetricDb::new(&prev_path) {
                 let prev_grouped = GroupedMetrics::new(&prev_db, "group")?;
-                prev_aggregated = Some(prev_grouped.aggregate());
-                aggregated.set_diff(prev_aggregated.as_ref().unwrap());
+                let prev_grouped_aggregated = prev_grouped.aggregate();
+                aggregated.set_diff(&prev_grouped_aggregated);
+                prev_aggregated = Some(prev_grouped_aggregated);
             }
         }
         if name.is_empty() {

crates/prof/src/summary.rs

@@ -56,8 +56,14 @@ impl GithubSummary {
             .zip_eq(md_paths.iter())
             .zip_eq(names)
             .map(|(((aggregated, prev_aggregated), md_path), name)| {
-                let md_filename = md_path.file_name().unwrap().to_str().unwrap();
-                let mut row = aggregated.get_summary_row(md_filename).unwrap();
+                let md_filename = md_path
+                    .file_name()
+                    .expect("Path should have a filename")
+                    .to_str()
+                    .expect("Filename should be valid UTF-8");
+                let mut row = aggregated.get_summary_row(md_filename).unwrap_or_else(|| {
+                    panic!("Failed to get summary row for file '{}'", md_filename)
+                });
                 if let Some(prev_aggregated) = prev_aggregated {
                     // md_filename doesn't matter
                     if let Some(prev_row) = prev_aggregated.get_summary_row(md_filename) {