Replace .lock().unwrap() with .expect() for better mutex poisoning diagnostics

[Copilot]

The codebase had 39 occurrences of .lock().unwrap() in source files. These provide no context when mutex poisoning occurs, making debugging difficult.

Changes

• Replace all .lock().unwrap() with .expect("<mutex_name> mutex poisoned") in 12 source files
• Update documentation example in pet-core/src/lib.rs to follow the new pattern
• Test files intentionally unchanged (panicking on test setup is acceptable)

Before

let mut environments = self.environments.lock().unwrap();

After

let mut environments = self.environments.lock().expect("environments mutex poisoned");

Files Modified

• pet-python-utils/src/cache.rs, env.rs
• pet-reporter/src/collect.rs, stdio.rs
• pet-core/src/os_environment.rs, lib.rs
• pet-pyenv/src/lib.rs
• pet-uv/src/lib.rs
• pet-windows-registry/src/lib.rs
• pet/src/jsonrpc.rs, find.rs, lib.rs

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Refactor: Replace .lock().unwrap() with proper error handling or expect()</issue_title>
<issue_description>## Summary
The codebase has 50+ occurrences of .lock().unwrap() which will panic if a thread holding the lock panics (causing mutex poisoning). While this may be acceptable for internal tools, better error handling would improve robustness.

Current Pattern

let mut environments = self.environments.lock().unwrap();

Proposed Improvements

Option 1: Use .expect() with meaningful message

let mut environments = self.environments
    .lock()
    .expect("environments mutex poisoned - previous thread panicked");

Option 2: Handle PoisonError gracefully

let mut environments = self.environments
    .lock()
    .unwrap_or_else(|poisoned| {
        log::warn!("Recovering from poisoned mutex");
        poisoned.into_inner()
    });

Option 3: Use parking_lot::Mutex which doesn't poison

// parking_lot::Mutex doesn't have PoisonError
use parking_lot::Mutex;
let mut environments = self.environments.lock();

Files with Most Occurrences

1. crates/pet-conda/src/lib.rs - ~15 occurrences
2. crates/pet-python-utils/src/cache.rs - ~10 occurrences
3. crates/pet-poetry/src/lib.rs - ~8 occurrences
4. crates/pet-linux-global-python/src/lib.rs - ~5 occurrences
5. crates/pet-reporter/src/cache.rs - ~5 occurrences

Recommendation

For a JSONRPC server that should be reliable:

1. At minimum, replace unwrap() with expect("meaningful message") for better debugging
2. Consider parking_lot crate which has better performance and no poisoning semantics
3. For critical paths (like the reporter), consider graceful recovery

Priority

Low - Current code works but could be more robust.</issue_description>

Comments on the Issue (you are @copilot in this section)

• Fixes Refactor: Replace .lock().unwrap() with proper error handling or expect() #289

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[karthiknadig]

PR Review

@copilot

Summary

This PR addresses issue #289 by replacing .lock().unwrap() with .expect() calls to provide better debugging context when mutex poisoning occurs.

Positive Aspects

1. Meaningful error messages: Each .expect() message clearly identifies which mutex was poisoned (e.g., "environments mutex poisoned", "cache_dir mutex poisoned")

2. Consistent pattern: All messages follow the format "<field_name> mutex poisoned" making it easy to identify the source of issues

3. Documentation updated: The example in pet-core/src/lib.rs was updated to reflect the new pattern

4. Test files correctly excluded: As stated in the PR, test files retain .unwrap() since panics in tests are expected behavior

Minor Issue

Typo in field name: In crates/pet-python-utils/src/cache.rs, there's a pre-existing typo envoronment (should be environment) that's preserved in the expect message:

.expect("envoronment mutex poisoned")

Consider fixing the typo either in this PR or as a follow-up.

Verification

The changes are mechanical and safe:

• unwrap() → expect("message") doesn't change runtime behavior, only improves panic messages
• No logic changes
• All mutex locks still panic on poisoning (just with better context)

Recommendation

Approve - The PR achieves its stated goal. The typo is a pre-existing issue and not a blocker.

[rzhao271]

I forgot this was using Rust.