refactor: replace .lock().unwrap() with .expect() for better error messages

Co-authored-by: karthiknadig <3840081+karthiknadig@users.noreply.github.com>

Author: Copilot

crates/pet-core/src/lib.rs

@@ -88,9 +88,7 @@ pub trait Locator: Send + Sync {
     /// impl Locator for MyLocator {
     ///     fn configure(&self, config: &Configuration) {
     ///         if let Some(dirs) = &config.workspace_directories {
-    ///             // Using unwrap() is acceptable here as mutex poisoning indicates
-    ///             // a panic in another thread, which is unrecoverable in this context.
-    ///             *self.workspace_dirs.lock().unwrap() = dirs.clone();
+    ///             *self.workspace_dirs.lock().expect("workspace_dirs mutex poisoned") = dirs.clone();
     ///         }
     ///     }
     ///     // ... other required methods

crates/pet-core/src/os_environment.rs

@@ -47,18 +47,26 @@ impl Environment for EnvironmentApi {
         get_env_var(key)
     }
     fn get_know_global_search_locations(&self) -> Vec<PathBuf> {
-        if self.global_search_locations.lock().unwrap().is_empty() {
+        if self
+            .global_search_locations
+            .lock()
+            .expect("global_search_locations mutex poisoned")
+            .is_empty()
+        {
             let mut paths =
                 env::split_paths(&self.get_env_var("PATH".to_string()).unwrap_or_default())
                     .filter(|p| p.exists())
                     .collect::<Vec<PathBuf>>();
             trace!("Env PATH: {:?}", paths);
             self.global_search_locations
                 .lock()
-                .unwrap()
+                .expect("global_search_locations mutex poisoned")
                 .append(&mut paths);
         }
-        self.global_search_locations.lock().unwrap().clone()
+        self.global_search_locations
+            .lock()
+            .expect("global_search_locations mutex poisoned")
+            .clone()
     }
 }
 
@@ -74,7 +82,12 @@ impl Environment for EnvironmentApi {
         get_env_var(key)
     }
     fn get_know_global_search_locations(&self) -> Vec<PathBuf> {
-        if self.global_search_locations.lock().unwrap().is_empty() {
+        if self
+            .global_search_locations
+            .lock()
+            .expect("global_search_locations mutex poisoned")
+            .is_empty()
+        {
             let mut paths =
                 env::split_paths(&self.get_env_var("PATH".to_string()).unwrap_or_default())
                     .collect::<Vec<PathBuf>>();
@@ -126,10 +139,13 @@ impl Environment for EnvironmentApi {
 
             self.global_search_locations
                 .lock()
-                .unwrap()
+                .expect("global_search_locations mutex poisoned")
                 .append(&mut paths);
         }
-        self.global_search_locations.lock().unwrap().clone()
+        self.global_search_locations
+            .lock()
+            .expect("global_search_locations mutex poisoned")
+            .clone()
     }
 }
 

crates/pet-pyenv/src/lib.rs

@@ -48,12 +48,18 @@ impl PyEnv {
         }
     }
     fn clear(&self) {
-        self.manager.lock().unwrap().take();
-        self.versions_dir.lock().unwrap().take();
+        self.manager.lock().expect("manager mutex poisoned").take();
+        self.versions_dir
+            .lock()
+            .expect("versions_dir mutex poisoned")
+            .take();
     }
     fn get_manager_versions_dir(&self) -> (Option<EnvManager>, Option<PathBuf>) {
-        let mut managers = self.manager.lock().unwrap();
-        let mut versions = self.versions_dir.lock().unwrap();
+        let mut managers = self.manager.lock().expect("manager mutex poisoned");
+        let mut versions = self
+            .versions_dir
+            .lock()
+            .expect("versions_dir mutex poisoned");
         if managers.is_none() || versions.is_none() {
             let pyenv_info = PyEnvInfo::from(&self.env_vars);
             trace!("PyEnv Info {:?}", pyenv_info);

crates/pet-python-utils/src/cache.rs

@@ -60,34 +60,59 @@ impl CacheImpl {
     }
 
     fn get_cache_directory(&self) -> Option<PathBuf> {
-        self.cache_dir.lock().unwrap().clone()
+        self.cache_dir
+            .lock()
+            .expect("cache_dir mutex poisoned")
+            .clone()
     }
 
     /// Once a cache directory has been set, you cannot change it.
     /// No point supporting such a scenario.
     fn set_cache_directory(&self, cache_dir: PathBuf) {
-        if let Some(cache_dir) = self.cache_dir.lock().unwrap().clone() {
+        if let Some(cache_dir) = self
+            .cache_dir
+            .lock()
+            .expect("cache_dir mutex poisoned")
+            .clone()
+        {
             warn!(
                 "Cache directory has already been set to {:?}. Cannot change it now.",
                 cache_dir
             );
             return;
         }
         trace!("Setting cache directory to {:?}", cache_dir);
-        self.cache_dir.lock().unwrap().replace(cache_dir);
+        self.cache_dir
+            .lock()
+            .expect("cache_dir mutex poisoned")
+            .replace(cache_dir);
     }
     fn clear(&self) -> io::Result<()> {
         trace!("Clearing cache");
-        self.locks.lock().unwrap().clear();
-        if let Some(cache_directory) = self.cache_dir.lock().unwrap().clone() {
+        self.locks.lock().expect("locks mutex poisoned").clear();
+        if let Some(cache_directory) = self
+            .cache_dir
+            .lock()
+            .expect("cache_dir mutex poisoned")
+            .clone()
+        {
             std::fs::remove_dir_all(cache_directory)
         } else {
             Ok(())
         }
     }
     fn create_cache(&self, executable: PathBuf) -> LockableCacheEntry {
-        let cache_directory = self.cache_dir.lock().unwrap().clone();
-        match self.locks.lock().unwrap().entry(executable.clone()) {
+        let cache_directory = self
+            .cache_dir
+            .lock()
+            .expect("cache_dir mutex poisoned")
+            .clone();
+        match self
+            .locks
+            .lock()
+            .expect("locks mutex poisoned")
+            .entry(executable.clone())
+        {
             Entry::Occupied(lock) => lock.get().clone(),
             Entry::Vacant(lock) => {
                 let cache = Box::new(CacheEntryImpl::create(cache_directory.clone(), executable))
@@ -122,7 +147,12 @@ impl CacheEntryImpl {
     }
     pub fn verify_in_memory_cache(&self) {
         // Check if any of the exes have changed since we last cached this.
-        for symlink_info in self.symlinks.lock().unwrap().iter() {
+        for symlink_info in self
+            .symlinks
+            .lock()
+            .expect("symlinks mutex poisoned")
+            .iter()
+        {
             if let Ok(metadata) = symlink_info.0.metadata() {
                 let mtime_changed = metadata.modified().ok() != Some(symlink_info.1);
                 // Only check ctime if we have it stored (may be None on Linux)
@@ -139,7 +169,10 @@ impl CacheEntryImpl {
                         metadata.modified().ok(),
                         metadata.created().ok()
                     );
-                    self.envoronment.lock().unwrap().take();
+                    self.envoronment
+                        .lock()
+                        .expect("envoronment mutex poisoned")
+                        .take();
                     if let Some(cache_directory) = &self.cache_directory {
                         delete_cache_file(cache_directory, &self.executable);
                     }
@@ -155,15 +188,23 @@ impl CacheEntry for CacheEntryImpl {
 
         // New scope to drop lock immediately after we have the value.
         {
-            if let Some(env) = self.envoronment.lock().unwrap().clone() {
+            if let Some(env) = self
+                .envoronment
+                .lock()
+                .expect("envoronment mutex poisoned")
+                .clone()
+            {
                 return Some(env);
             }
         }
 
         if let Some(ref cache_directory) = self.cache_directory {
             let (env, mut symlinks) = get_cache_from_file(cache_directory, &self.executable)?;
-            self.envoronment.lock().unwrap().replace(env.clone());
-            let mut locked_symlinks = self.symlinks.lock().unwrap();
+            self.envoronment
+                .lock()
+                .expect("envoronment mutex poisoned")
+                .replace(env.clone());
+            let mut locked_symlinks = self.symlinks.lock().expect("symlinks mutex poisoned");
             locked_symlinks.clear();
             locked_symlinks.append(&mut symlinks);
             Some(env)
@@ -190,13 +231,13 @@ impl CacheEntry for CacheEntryImpl {
         symlinks.dedup();
 
         {
-            let mut locked_symlinks = self.symlinks.lock().unwrap();
+            let mut locked_symlinks = self.symlinks.lock().expect("symlinks mutex poisoned");
             locked_symlinks.clear();
             locked_symlinks.append(&mut symlinks.clone());
         }
         self.envoronment
             .lock()
-            .unwrap()
+            .expect("envoronment mutex poisoned")
             .replace(environment.clone());
 
         trace!("Caching interpreter info for {:?}", self.executable);
@@ -213,7 +254,7 @@ impl CacheEntry for CacheEntryImpl {
         let known_symlinks: HashSet<PathBuf> = self
             .symlinks
             .lock()
-            .unwrap()
+            .expect("symlinks mutex poisoned")
             .clone()
             .iter()
             .map(|x| x.0.clone())

crates/pet-python-utils/src/env.rs

@@ -56,7 +56,7 @@ impl ResolvedPythonEnv {
             && environment.arch == arch
         {
             let cache = create_cache(self.executable.clone());
-            let entry = cache.lock().unwrap();
+            let entry = cache.lock().expect("cache mutex poisoned");
             entry.track_symlinks(symlinks)
         } else {
             error!(
@@ -75,7 +75,7 @@ impl ResolvedPythonEnv {
         // cache: &dyn Cache,
     ) -> Option<Self> {
         let cache = create_cache(executable.to_path_buf());
-        let entry = cache.lock().unwrap();
+        let entry = cache.lock().expect("cache mutex poisoned");
         if let Some(env) = entry.get() {
             Some(env)
         } else if let Some(env) = get_interpreter_details(executable) {

crates/pet-reporter/src/collect.rs

@@ -29,11 +29,17 @@ impl Reporter for CollectReporter {
         //
     }
     fn report_manager(&self, manager: &EnvManager) {
-        self.managers.lock().unwrap().push(manager.clone());
+        self.managers
+            .lock()
+            .expect("managers mutex poisoned")
+            .push(manager.clone());
     }
 
     fn report_environment(&self, env: &PythonEnvironment) {
-        self.environments.lock().unwrap().push(env.clone());
+        self.environments
+            .lock()
+            .expect("environments mutex poisoned")
+            .push(env.clone());
     }
 }
 

crates/pet-reporter/src/stdio.rs

@@ -28,8 +28,11 @@ pub struct Summary {
 
 impl StdioReporter {
     pub fn get_summary(&self) -> Summary {
-        let managers = self.managers.lock().unwrap();
-        let environments = self.environments.lock().unwrap();
+        let managers = self.managers.lock().expect("managers mutex poisoned");
+        let environments = self
+            .environments
+            .lock()
+            .expect("environments mutex poisoned");
         Summary {
             managers: managers.clone(),
             environments: environments.clone(),
@@ -41,7 +44,7 @@ impl Reporter for StdioReporter {
         //
     }
     fn report_manager(&self, manager: &EnvManager) {
-        let mut managers = self.managers.lock().unwrap();
+        let mut managers = self.managers.lock().expect("managers mutex poisoned");
         let count = managers.get(&manager.tool).unwrap_or(&0) + 1;
         managers.insert(manager.tool, count);
         if self.print_list {
@@ -53,7 +56,10 @@ impl Reporter for StdioReporter {
         if self.kind.is_some() && env.kind != self.kind {
             return;
         }
-        let mut environments = self.environments.lock().unwrap();
+        let mut environments = self
+            .environments
+            .lock()
+            .expect("environments mutex poisoned");
         let count = environments.get(&env.kind).unwrap_or(&0) + 1;
         environments.insert(env.kind, count);
         if self.print_list {

crates/pet-uv/src/lib.rs

@@ -82,7 +82,10 @@ impl Locator for Uv {
 
     fn configure(&self, config: &Configuration) {
         if let Some(workspace_directories) = config.workspace_directories.as_ref() {
-            let mut ws = self.workspace_directories.lock().unwrap();
+            let mut ws = self
+                .workspace_directories
+                .lock()
+                .expect("workspace_directories mutex poisoned");
             ws.clear();
             ws.extend(workspace_directories.iter().cloned());
         }
@@ -137,7 +140,11 @@ impl Locator for Uv {
 
     fn find(&self, reporter: &dyn Reporter) {
         // look through workspace directories for uv-managed projects and any of their workspaces
-        let workspaces = self.workspace_directories.lock().unwrap().clone();
+        let workspaces = self
+            .workspace_directories
+            .lock()
+            .expect("workspace_directories mutex poisoned")
+            .clone();
         for workspace in workspaces {
             // TODO: maybe check for workspace in parent folders?
             for env in list_envs_in_directory(&workspace) {

crates/pet-windows-registry/src/lib.rs

@@ -31,7 +31,10 @@ impl WindowsRegistry {
     }
     #[cfg(windows)]
     fn find_with_cache(&self, reporter: Option<&dyn Reporter>) -> Option<LocatorResult> {
-        let mut result = self.search_result.lock().unwrap();
+        let mut result = self
+            .search_result
+            .lock()
+            .expect("search_result mutex poisoned");
         if let Some(result) = result.clone() {
             return Some(result);
         }
@@ -43,7 +46,10 @@ impl WindowsRegistry {
     }
     #[cfg(windows)]
     fn clear(&self) {
-        let mut search_result = self.search_result.lock().unwrap();
+        let mut search_result = self
+            .search_result
+            .lock()
+            .expect("search_result mutex poisoned");
         search_result.take();
     }
 }

crates/pet/src/find.rs

@@ -248,7 +248,7 @@ pub fn find_and_report_envs(
                 .insert("Workspaces", start.elapsed());
         });
     });
-    summary.lock().unwrap().total = start.elapsed();
+    summary.lock().expect("summary mutex poisoned").total = start.elapsed();
 
     summary
 }