refactor: replace unwrap/expect with proper error handling

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "sysdig-lsp"
-version = "0.7.4"
+version = "0.7.5"
 edition = "2024"
 authors = [ "Sysdig Inc." ]
 readme = "README.md"

Justfile

@@ -1,3 +1,6 @@
+# Help commands
+default:
+    @just --list
 
 test:
     cargo nextest run

src/app/lsp_server/lsp_server_inner.rs

@@ -245,14 +245,14 @@ where
             )
             .await;
 
-        if documentation_found.is_none() {
+        let Some(documentation) = documentation_found else {
             return Ok(None);
-        }
+        };
 
         Ok(Some(Hover {
             contents: Markup(MarkupContent {
                 kind: Markdown,
-                value: documentation_found.unwrap(),
+                value: documentation,
             }),
             range: None,
         }))

src/domain/scanresult/accepted_risk.rs

@@ -90,7 +90,7 @@ impl AcceptedRisk {
         if self
             .assigned_to_vulnerabilities
             .write()
-            .unwrap()
+            .unwrap_or_else(|e| panic!("RwLock poisoned in accepted_risk.rs: {}", e))
             .insert(WeakHash(Arc::downgrade(&vulnerability)))
         {
             vulnerability.add_accepted_risk(self.clone());
@@ -100,7 +100,7 @@ impl AcceptedRisk {
     pub fn assigned_to_vulnerabilities(self: &Arc<Self>) -> Vec<Arc<Vulnerability>> {
         self.assigned_to_vulnerabilities
             .read()
-            .unwrap()
+            .unwrap_or_else(|e| panic!("RwLock poisoned in accepted_risk.rs: {}", e))
             .iter()
             .filter_map(|v| v.0.upgrade())
             .collect()
@@ -110,7 +110,7 @@ impl AcceptedRisk {
         if self
             .assigned_to_packages
             .write()
-            .unwrap()
+            .unwrap_or_else(|e| panic!("RwLock poisoned in accepted_risk.rs: {}", e))
             .insert(WeakHash(Arc::downgrade(&a_package)))
         {
             a_package.add_accepted_risk(self.clone());
@@ -120,7 +120,7 @@ impl AcceptedRisk {
     pub fn assigned_to_packages(self: &Arc<Self>) -> Vec<Arc<Package>> {
         self.assigned_to_packages
             .read()
-            .unwrap()
+            .unwrap_or_else(|e| panic!("RwLock poisoned in accepted_risk.rs: {}", e))
             .iter()
             .filter_map(|p| p.0.upgrade())
             .collect()

src/domain/scanresult/layer.rs

@@ -60,17 +60,25 @@ impl Layer {
     }
 
     pub(in crate::domain::scanresult) fn add_package(&self, a_package: Arc<Package>) {
-        self.packages.write().unwrap().insert(a_package);
+        self.packages
+            .write()
+            .unwrap_or_else(|e| panic!("RwLock poisoned in layer.rs: {}", e))
+            .insert(a_package);
     }
 
     pub fn packages(&self) -> Vec<Arc<Package>> {
-        self.packages.read().unwrap().iter().cloned().collect()
+        self.packages
+            .read()
+            .unwrap_or_else(|e| panic!("RwLock poisoned in layer.rs: {}", e))
+            .iter()
+            .cloned()
+            .collect()
     }
 
     pub fn vulnerabilities(&self) -> Vec<Arc<Vulnerability>> {
         self.packages
             .read()
-            .unwrap()
+            .unwrap_or_else(|e| panic!("RwLock poisoned in layer.rs: {}", e))
             .iter()
             .flat_map(|p| p.vulnerabilities())
             .collect()

src/domain/scanresult/package.rs

@@ -75,7 +75,7 @@ impl Package {
         if self
             .vulnerabilities
             .write()
-            .unwrap()
+            .unwrap_or_else(|e| panic!("RwLock poisoned in package.rs: {}", e))
             .insert(WeakHash(Arc::downgrade(&vulnerability)))
         {
             vulnerability.add_found_in_package(self.clone());
@@ -85,7 +85,7 @@ impl Package {
     pub fn vulnerabilities(&self) -> Vec<Arc<Vulnerability>> {
         self.vulnerabilities
             .read()
-            .unwrap()
+            .unwrap_or_else(|e| panic!("RwLock poisoned in package.rs: {}", e))
             .iter()
             .filter_map(|v| v.0.upgrade())
             .collect()
@@ -95,7 +95,7 @@ impl Package {
         if self
             .accepted_risks
             .write()
-            .unwrap()
+            .unwrap_or_else(|e| panic!("RwLock poisoned in package.rs: {}", e))
             .insert(WeakHash(Arc::downgrade(&accepted_risk)))
         {
             accepted_risk.add_for_package(self.clone());
@@ -105,7 +105,7 @@ impl Package {
     pub fn accepted_risks(&self) -> Vec<Arc<AcceptedRisk>> {
         self.accepted_risks
             .read()
-            .unwrap()
+            .unwrap_or_else(|e| panic!("RwLock poisoned in package.rs: {}", e))
             .iter()
             .filter_map(|r| r.0.upgrade())
             .collect()
@@ -156,12 +156,15 @@ impl Package {
 
         let mut sorted_candidates = candidate_versions;
         sorted_candidates.sort_by(|a, b| {
-            let score_a = scores.get(a).unwrap();
-            let score_b = scores.get(b).unwrap();
+            // These unwrap_or calls should never execute since we populated scores
+            // for all candidate versions above, but we handle the case defensively
+            let empty_score = HashMap::new();
+            let score_a = scores.get(a).unwrap_or(&empty_score);
+            let score_b = scores.get(b).unwrap_or(&empty_score);
 
             for severity in &severity_order {
-                let count_a = score_a.get(severity).unwrap();
-                let count_b = score_b.get(severity).unwrap();
+                let count_a = score_a.get(severity).unwrap_or(&0);
+                let count_b = score_b.get(severity).unwrap_or(&0);
                 if count_a != count_b {
                     return count_b.cmp(count_a); // Higher count is better
                 }
@@ -212,8 +215,18 @@ impl Clone for Package {
             version: self.version.clone(),
             path: self.path.clone(),
             found_in_layer: self.found_in_layer.clone(),
-            vulnerabilities: RwLock::new(self.vulnerabilities.read().unwrap().clone()),
-            accepted_risks: RwLock::new(self.accepted_risks.read().unwrap().clone()),
+            vulnerabilities: RwLock::new(
+                self.vulnerabilities
+                    .read()
+                    .unwrap_or_else(|e| panic!("RwLock poisoned in package.rs: {}", e))
+                    .clone(),
+            ),
+            accepted_risks: RwLock::new(
+                self.accepted_risks
+                    .read()
+                    .unwrap_or_else(|e| panic!("RwLock poisoned in package.rs: {}", e))
+                    .clone(),
+            ),
         }
     }
 }

src/domain/scanresult/policy.rs

@@ -62,7 +62,7 @@ impl Policy {
         if self
             .bundles
             .write()
-            .unwrap()
+            .unwrap_or_else(|e| panic!("RwLock poisoned in policy.rs: {}", e))
             .insert(WeakHash(Arc::downgrade(policy_bundle)))
         {
             policy_bundle.add_policy(self.clone());
@@ -72,7 +72,7 @@ impl Policy {
     pub fn bundles(&self) -> Vec<Arc<PolicyBundle>> {
         self.bundles
             .read()
-            .unwrap()
+            .unwrap_or_else(|e| panic!("RwLock poisoned in policy.rs: {}", e))
             .iter()
             .filter_map(|b| b.0.upgrade())
             .collect()

src/domain/scanresult/policy_bundle.rs

@@ -37,7 +37,7 @@ impl PolicyBundle {
         if self
             .found_in_policies
             .write()
-            .unwrap()
+            .unwrap_or_else(|e| panic!("RwLock poisoned in policy_bundle.rs: {}", e))
             .insert(WeakHash(Arc::downgrade(&policy)))
         {
             policy.add_bundle(self);
@@ -56,14 +56,17 @@ impl PolicyBundle {
             evaluation_result,
             Arc::downgrade(self),
         ));
-        self.rules.write().unwrap().insert(rule.clone());
+        self.rules
+            .write()
+            .unwrap_or_else(|e| panic!("RwLock poisoned in policy_bundle.rs: {}", e))
+            .insert(rule.clone());
         rule
     }
 
     pub fn found_in_policies(&self) -> Vec<Arc<Policy>> {
         self.found_in_policies
             .read()
-            .unwrap()
+            .unwrap_or_else(|e| panic!("RwLock poisoned in policy_bundle.rs: {}", e))
             .iter()
             .filter_map(|p| p.0.upgrade())
             .collect()
@@ -78,7 +81,12 @@ impl PolicyBundle {
     }
 
     pub fn rules(&self) -> Vec<Arc<PolicyBundleRule>> {
-        self.rules.read().unwrap().iter().cloned().collect()
+        self.rules
+            .read()
+            .unwrap_or_else(|e| panic!("RwLock poisoned in policy_bundle.rs: {}", e))
+            .iter()
+            .cloned()
+            .collect()
     }
 
     pub fn evaluation_result(&self) -> EvaluationResult {

src/domain/scanresult/policy_bundle_rule.rs

@@ -42,7 +42,12 @@ impl Clone for PolicyBundleRule {
             description: self.description.clone(),
             evaluation_result: self.evaluation_result,
             parent: self.parent.clone(),
-            failures: RwLock::new(self.failures.read().unwrap().clone()),
+            failures: RwLock::new(
+                self.failures
+                    .read()
+                    .unwrap_or_else(|e| panic!("RwLock poisoned in policy_bundle_rule.rs: {}", e))
+                    .clone(),
+            ),
         }
     }
 }
@@ -86,7 +91,7 @@ impl PolicyBundleRule {
         let failure = PolicyBundleRuleImageConfigFailure::new(remediation, Arc::downgrade(self));
         self.failures
             .write()
-            .unwrap()
+            .unwrap_or_else(|e| panic!("RwLock poisoned in policy_bundle_rule.rs: {}", e))
             .push(PolicyBundleRuleFailure::ImageConfig(failure.clone()));
         failure
     }
@@ -98,12 +103,15 @@ impl PolicyBundleRule {
         let failure = PolicyBundleRulePkgVulnFailure::new(description, Arc::downgrade(self));
         self.failures
             .write()
-            .unwrap()
+            .unwrap_or_else(|e| panic!("RwLock poisoned in policy_bundle_rule.rs: {}", e))
             .push(PolicyBundleRuleFailure::PkgVuln(failure.clone()));
         failure
     }
 
     pub fn failures(&self) -> Vec<PolicyBundleRuleFailure> {
-        self.failures.read().unwrap().clone()
+        self.failures
+            .read()
+            .unwrap_or_else(|e| panic!("RwLock poisoned in policy_bundle_rule.rs: {}", e))
+            .clone()
     }
 }

src/domain/scanresult/vulnerability.rs

@@ -89,7 +89,7 @@ impl Vulnerability {
         if self
             .found_in_packages
             .write()
-            .unwrap()
+            .unwrap_or_else(|e| panic!("RwLock poisoned in vulnerability.rs: {}", e))
             .insert(WeakHash(Arc::downgrade(&a_package)))
         {
             a_package.add_vulnerability_found(self.clone());
@@ -99,7 +99,7 @@ impl Vulnerability {
     pub fn found_in_packages(&self) -> Vec<Arc<Package>> {
         self.found_in_packages
             .read()
-            .unwrap()
+            .unwrap_or_else(|e| panic!("RwLock poisoned in vulnerability.rs: {}", e))
             .iter()
             .filter_map(|p| p.0.upgrade())
             .collect()
@@ -117,7 +117,7 @@ impl Vulnerability {
         if self
             .accepted_risks
             .write()
-            .unwrap()
+            .unwrap_or_else(|e| panic!("RwLock poisoned in vulnerability.rs: {}", e))
             .insert(WeakHash(Arc::downgrade(&accepted_risk)))
         {
             accepted_risk.add_for_vulnerability(self.clone());
@@ -127,7 +127,7 @@ impl Vulnerability {
     pub fn accepted_risks(&self) -> Vec<Arc<AcceptedRisk>> {
         self.accepted_risks
             .read()
-            .unwrap()
+            .unwrap_or_else(|e| panic!("RwLock poisoned in vulnerability.rs: {}", e))
             .iter()
             .filter_map(|r| r.0.upgrade())
             .collect()

src/infra/docker_image_builder.rs

@@ -46,15 +46,19 @@ impl DockerImageBuilder {
             .pack_containerfile_dir_into_a_tar(containerfile)
             .await?;
 
+        let dockerfile_name = containerfile
+            .file_name()
+            .and_then(|osstr| osstr.to_str())
+            .ok_or_else(|| {
+                DockerImageBuilderError::Generic(
+                    "invalid containerfile path: unable to extract filename".to_string(),
+                )
+            })?;
+
         let image_name = format!("sysdig-lsp-image-build-{}", rand::random::<u8>());
         let mut results = self.docker_client.build_image(
             BuildImageOptionsBuilder::new()
-                .dockerfile(
-                    containerfile
-                        .file_name()
-                        .and_then(|osstr| osstr.to_str())
-                        .unwrap(),
-                )
+                .dockerfile(dockerfile_name)
                 .t(&image_name)
                 .build(),
             None,
@@ -66,12 +70,16 @@ impl DockerImageBuilder {
         ));
         while let Some(result) = results.next().await {
             match result {
-                Ok(BuildInfo { aux, .. }) if aux.is_some() => {
-                    let image_id = aux.unwrap().id.unwrap();
-                    build_info = Ok(ImageBuildResult {
-                        image_name: image_name.clone(),
-                        image_id,
-                    });
+                Ok(BuildInfo {
+                    aux: Some(aux_info @ bollard::secret::ImageId { id: Some(_), .. }),
+                    ..
+                }) => {
+                    if let Some(image_id) = aux_info.id {
+                        build_info = Ok(ImageBuildResult {
+                            image_name: image_name.clone(),
+                            image_id,
+                        });
+                    }
                 }
                 Err(error) => return Err(DockerImageBuilderError::Docker(error)),
                 _ => {}