Skip to content

Commit ce5e941

Browse files
DavidLiedleclaude
andcommitted
refactor: Improve error handling and code quality (Phase 2 & 3)
This release focuses on production readiness by improving error handling and code quality across the codebase. Phase 2 - Error Handling Improvements: - Replaced 19 production unwrap() calls with proper error handling - Used .map_err() to convert errors to FerrixError with context - Used .unwrap_or() and .unwrap_or_else() for safe defaults - Used match statements for explicit error handling - Improved error messages with contextual information Files improved: - src/server/recording.rs: Fixed duration_since() unwrap calls - src/plugin/runtime.rs: Fixed lock().unwrap() calls - src/transport/mosh.rs: Fixed try_into() and option unwraps - src/server/hooks.rs: Fixed strip_prefix() unwrap - src/input/chord.rs: Fixed last() unwrap - src/ai/assistant.rs: Fixed partial_cmp() unwrap - src/server/timetravel.rs: Fixed first()/last() unwraps - src/server/versioning.rs: Fixed as_ref() unwrap Phase 3 - Code Quality (Clippy): - Fixed all clippy warnings in production code - Removed unused std::io::Write import - Replaced manual ASCII range check with .is_ascii_lowercase() - Added #[allow(dead_code)] to intentionally unused methods - Fixed test compilation errors for protocol message fields Testing: - All tests compile and pass - Release build succeeds with zero warnings - Production code is now clippy-clean 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent a28fa35 commit ce5e941

15 files changed

Lines changed: 121 additions & 56 deletions

File tree

CHANGELOG.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
## [0.20.8] - 2025-10-09
11+
12+
### Changed
13+
- **Error Handling**: Improved error handling across production code
14+
- Replaced 19 production unwrap() calls with proper error handling
15+
- Uses `.map_err()`, `.unwrap_or()`, and match statements for safe handling
16+
- Added context to error messages for better debugging
17+
- Files improved: recording.rs, plugin/runtime.rs, transport/mosh.rs, server/hooks.rs, input/chord.rs, ai/assistant.rs, server/timetravel.rs, server/versioning.rs
18+
19+
### Fixed
20+
- **Code Quality**: Fixed all clippy warnings in production code
21+
- Removed unused imports
22+
- Simplified manual ASCII range checks to use `.is_ascii_lowercase()`
23+
- Added `#[allow(dead_code)]` to intentionally unused helper methods
24+
- Fixed test compilation errors for protocol message fields
25+
1026
## [0.20.7] - 2025-10-09
1127

1228
### Fixed

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "ferrix"
3-
version = "0.20.7"
3+
version = "0.20.8"
44
edition = "2021"
55
authors = ["David Liedle", "Claude <noreply@anthropic.com>"]
66
description = "A modern terminal multiplexer built with Rust, inspired by tmux and GNU Screen"

src/ai/assistant.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ impl CommandAssistant {
164164
suggestions.extend(self.suggest_optimizations());
165165

166166
// Sort by confidence
167-
suggestions.sort_by(|a, b| b.confidence.partial_cmp(&a.confidence).unwrap());
167+
suggestions.sort_by(|a, b| b.confidence.partial_cmp(&a.confidence).unwrap_or(std::cmp::Ordering::Equal));
168168

169169
// Cache results
170170
self.suggestions_cache.insert(partial_command.to_string(), suggestions.clone());

src/client/mod.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,7 @@ impl Client {
338338
Err(FerrixError::NotConnected)
339339
}
340340

341+
#[allow(dead_code)]
341342
async fn run_attached(&mut self) -> Result<()> {
342343
// Only enable raw mode if we're in an interactive terminal
343344
let is_tty = std::io::stdin().is_terminal();
@@ -671,7 +672,7 @@ impl Client {
671672
if key_event.modifiers == KeyModifiers::CONTROL {
672673
// Convert to lowercase for control character calculation
673674
let lower_c = c.to_ascii_lowercase();
674-
if lower_c >= 'a' && lower_c <= 'z' {
675+
if lower_c.is_ascii_lowercase() {
675676
data.push((lower_c as u8) - b'a' + 1);
676677
} else if c == '@' {
677678
data.push(0); // Ctrl-@ is NUL
@@ -1817,7 +1818,6 @@ impl Client {
18171818
}
18181819

18191820
async fn draw_pane_content(&mut self, pane: &PaneInfo) -> Result<()> {
1820-
use std::io::Write;
18211821
use crossterm::style::{SetForegroundColor, SetBackgroundColor, SetAttribute, ResetColor};
18221822

18231823
// Use a buffer to collect all output before flushing
@@ -2380,9 +2380,7 @@ impl Client {
23802380
execute!(stdout, MoveTo(0, rows - 1))?;
23812381

23822382
// Build status bar content
2383-
let session_name = self.attached_session_name
2384-
.as_ref()
2385-
.map(|s| s.clone())
2383+
let session_name = self.attached_session_name.clone()
23862384
.unwrap_or_else(|| "No Session".to_string());
23872385

23882386
let window_info = if let Some(layout) = &self.current_layout {

src/input/chord.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,8 @@ impl ChordDetector {
146146
let chord: Vec<KeyEvent> = keys.iter().map(|k| {
147147
let parts: Vec<&str> = k.split('+').collect();
148148
let (modifiers, code) = if parts.len() > 1 {
149-
(parts[0..parts.len()-1].join("+"), parts.last().unwrap().to_string())
149+
(parts[0..parts.len()-1].join("+"),
150+
parts.last().map(|s| s.to_string()).unwrap_or_default())
150151
} else {
151152
(String::new(), k.to_string())
152153
};

src/plugin/runtime.rs

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -117,15 +117,17 @@ impl PluginRuntime {
117117

118118
// Instantiate the module
119119
let instance = {
120-
let mut store_guard = store.lock().unwrap();
120+
let mut store_guard = store.lock()
121+
.map_err(|e| FerrixError::Plugin(format!("Failed to lock plugin store: {}", e)))?;
121122
linker.instantiate(&mut *store_guard, &module)
122123
.map_err(|e| FerrixError::Plugin(format!("Failed to instantiate plugin: {}", e)))?
123124
};
124125

125126
// Get exported functions
126127
let mut exports = HashMap::new();
127128
{
128-
let mut store_guard = store.lock().unwrap();
129+
let mut store_guard = store.lock()
130+
.map_err(|e| FerrixError::Plugin(format!("Failed to lock plugin store: {}", e)))?;
129131
for export_name in &manifest.exports {
130132
if let Some(func) = instance.get_func(&mut *store_guard, export_name) {
131133
exports.insert(export_name.clone(), func);
@@ -135,7 +137,8 @@ impl PluginRuntime {
135137

136138
// Initialize the plugin
137139
{
138-
let mut store_guard = store.lock().unwrap();
140+
let mut store_guard = store.lock()
141+
.map_err(|e| FerrixError::Plugin(format!("Failed to lock plugin store: {}", e)))?;
139142
if let Some(init_func) = instance.get_func(&mut *store_guard, "plugin_init") {
140143
init_func.call(&mut *store_guard, &[], &mut [])
141144
.map_err(|e| FerrixError::Plugin(format!("Plugin initialization failed: {}", e)))?;
@@ -174,7 +177,8 @@ impl PluginRuntime {
174177
if let Some(plugin) = plugins.remove(plugin_id) {
175178
// Call cleanup if available
176179
if let Some(cleanup_func) = plugin.exports.get("plugin_cleanup") {
177-
let mut store_guard = plugin.store.lock().unwrap();
180+
let mut store_guard = plugin.store.lock()
181+
.map_err(|e| FerrixError::Plugin(format!("Failed to lock plugin store: {}", e)))?;
178182
cleanup_func.call(&mut *store_guard, &[], &mut [])
179183
.map_err(|e| FerrixError::Plugin(format!("Plugin cleanup failed: {}", e)))?;
180184
}
@@ -207,7 +211,13 @@ impl PluginRuntime {
207211
for (_id, plugin) in plugins.iter() {
208212
if let Some(handle_func) = plugin.exports.get("handle_command") {
209213
// Get a lock on the store and update context
210-
let mut store_guard = plugin.store.lock().unwrap();
214+
let mut store_guard = match plugin.store.lock() {
215+
Ok(guard) => guard,
216+
Err(e) => {
217+
warn!("Failed to lock plugin store: {}", e);
218+
continue;
219+
}
220+
};
211221
store_guard.data_mut().context = context.clone();
212222

213223
// Serialize command to pass to WASM
@@ -248,7 +258,13 @@ impl PluginRuntime {
248258
let hook_name = format!("hook_{:?}", hook).to_lowercase();
249259
if let Some(hook_func) = plugin.exports.get(&hook_name) {
250260
// Get a lock on the store and update context
251-
let mut store_guard = plugin.store.lock().unwrap();
261+
let mut store_guard = match plugin.store.lock() {
262+
Ok(guard) => guard,
263+
Err(e) => {
264+
warn!("Failed to lock plugin store: {}", e);
265+
continue;
266+
}
267+
};
252268
store_guard.data_mut().context = context.clone();
253269

254270
// Call the hook function
@@ -278,7 +294,13 @@ impl PluginRuntime {
278294
for (_id, plugin) in plugins.iter() {
279295
if let Some(event_func) = plugin.exports.get("handle_event") {
280296
// Get a lock on the store and add event to queue
281-
let mut store_guard = plugin.store.lock().unwrap();
297+
let mut store_guard = match plugin.store.lock() {
298+
Ok(guard) => guard,
299+
Err(e) => {
300+
warn!("Failed to lock plugin store: {}", e);
301+
continue;
302+
}
303+
};
282304
store_guard.data_mut().event_queue.push(event.clone());
283305

284306
// Call the event handler function
@@ -416,9 +438,7 @@ impl PluginRuntime {
416438

417439
// Register hooks based on exported functions
418440
for export in &manifest.exports {
419-
if export.starts_with("hook_") {
420-
// Parse hook type from function name
421-
let hook_name = export.strip_prefix("hook_").unwrap();
441+
if let Some(hook_name) = export.strip_prefix("hook_") {
422442

423443
// Map to PluginHook enum (simplified)
424444
let hook = match hook_name {

src/protocol/tests.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ mod protocol_tests {
3030
let session_id = SessionId(Uuid::new_v4());
3131
let message = ClientMessage::CreateSession {
3232
name: Some("test-session".to_string()),
33+
working_dir: None,
3334
};
3435

3536
// Test encoding
@@ -47,7 +48,7 @@ mod protocol_tests {
4748
let session_id = SessionId(Uuid::new_v4());
4849
let message = ServerMessage::SessionCreated {
4950
session_id,
50-
session_name: "test-session".to_string(),
51+
name: "test-session".to_string(),
5152
};
5253

5354
// Test encoding
@@ -65,6 +66,7 @@ mod protocol_tests {
6566
let session_id = SessionId(Uuid::new_v4());
6667
let original_message = ClientMessage::CreateSession {
6768
name: Some("test-session".to_string()),
69+
working_dir: None,
6870
};
6971

7072
// Encode message
@@ -76,8 +78,8 @@ mod protocol_tests {
7678
Some(decoded_message) => {
7779
// Compare messages (would need PartialEq implementation)
7880
match (&original_message, &decoded_message) {
79-
(ClientMessage::CreateSession { name: name1 },
80-
ClientMessage::CreateSession { name: name2 }) => {
81+
(ClientMessage::CreateSession { name: name1, working_dir: _ },
82+
ClientMessage::CreateSession { name: name2, working_dir: _ }) => {
8183
assert_eq!(name1, name2);
8284
}
8385
_ => panic!("Message types don't match"),
@@ -190,6 +192,7 @@ mod protocol_tests {
190192

191193
let message = ClientMessage::CreateSession {
192194
name: Some("test".to_string()),
195+
working_dir: None,
193196
};
194197

195198
// Encode message
@@ -278,13 +281,14 @@ mod protocol_tests {
278281

279282
let message = ClientMessage::CreateSession {
280283
name: Some(format!("session-{}", i)),
284+
working_dir: None,
281285
};
282286

283287
codec.encode(message, &mut buf).unwrap();
284288
assert!(!buf.is_empty());
285289

286290
match codec.decode(&mut buf).unwrap() {
287-
Some(ClientMessage::CreateSession { name }) => {
291+
Some(ClientMessage::CreateSession { name, working_dir: _ }) => {
288292
assert_eq!(name, Some(format!("session-{}", i)));
289293
}
290294
_ => panic!("Wrong message type"),

src/server/hooks.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,8 @@ impl HookEvent {
147147

148148
// Command hooks
149149
name if name.starts_with("after-") => {
150-
let cmd = name.strip_prefix("after-").unwrap();
151-
Some(HookEvent::AfterCommand(cmd.to_string()))
150+
name.strip_prefix("after-")
151+
.map(|cmd| HookEvent::AfterCommand(cmd.to_string()))
152152
}
153153

154154
_ => None,

src/server/recording.rs

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ impl SessionRecorder {
175175
// Calculate duration
176176
let duration = SystemTime::now()
177177
.duration_since(self.start_time)
178-
.unwrap();
178+
.unwrap_or(Duration::from_secs(0));
179179
self.metadata.duration_ms = Some(duration.as_millis() as u64);
180180

181181
// Write final metadata
@@ -245,7 +245,7 @@ impl SessionRecorder {
245245
let event = RecordingEvent::Output {
246246
timestamp: SystemTime::now()
247247
.duration_since(self.start_time)
248-
.unwrap()
248+
.unwrap_or(Duration::from_secs(0))
249249
.as_millis() as u64,
250250
pane_id,
251251
data,
@@ -263,7 +263,7 @@ impl SessionRecorder {
263263
let event = RecordingEvent::Input {
264264
timestamp: SystemTime::now()
265265
.duration_since(self.start_time)
266-
.unwrap()
266+
.unwrap_or(Duration::from_secs(0))
267267
.as_millis() as u64,
268268
pane_id,
269269
data,
@@ -280,7 +280,7 @@ impl SessionRecorder {
280280
let event = RecordingEvent::Resize {
281281
timestamp: SystemTime::now()
282282
.duration_since(self.start_time)
283-
.unwrap()
283+
.unwrap_or(Duration::from_secs(0))
284284
.as_millis() as u64,
285285
pane_id,
286286
cols,
@@ -537,7 +537,9 @@ impl SessionPlayer {
537537
}
538538
});
539539

540-
writeln!(file, "{}", serde_json::to_string(&header).unwrap())
540+
let header_str = serde_json::to_string(&header)
541+
.map_err(|e| FerrixError::Other(format!("Failed to serialize header: {}", e)))?;
542+
writeln!(file, "{}", header_str)
541543
.map_err(|e| FerrixError::Other(format!("Failed to write header: {}", e)))?;
542544

543545
// Write events
@@ -546,7 +548,9 @@ impl SessionPlayer {
546548
let timestamp_sec = *timestamp as f64 / 1000.0;
547549
let text = String::from_utf8_lossy(data);
548550
let event_array = serde_json::json!([timestamp_sec, "o", text]);
549-
writeln!(file, "{}", serde_json::to_string(&event_array).unwrap())
551+
let event_str = serde_json::to_string(&event_array)
552+
.map_err(|e| FerrixError::Other(format!("Failed to serialize event: {}", e)))?;
553+
writeln!(file, "{}", event_str)
550554
.map_err(|e| FerrixError::Other(format!("Failed to write event: {}", e)))?;
551555
}
552556
}
@@ -561,9 +565,10 @@ impl SessionPlayer {
561565

562566
writeln!(file, "# Ferrix Session Recording")?;
563567
writeln!(file, "# Session: {} ({:?})", self.metadata.session_name, self.metadata.session_id)?;
564-
writeln!(file, "# Date: {}", chrono::DateTime::<chrono::Utc>::from_timestamp(
568+
let created_date = chrono::DateTime::<chrono::Utc>::from_timestamp(
565569
self.metadata.created_at as i64, 0
566-
).unwrap())?;
570+
).unwrap_or_else(chrono::Utc::now);
571+
writeln!(file, "# Date: {}", created_date)?;
567572
writeln!(file, "# Duration: {:?}ms\n", self.metadata.duration_ms)?;
568573

569574
for event in &self.events {

0 commit comments

Comments
 (0)