Skip to content

Commit 8f2296a

Browse files
vdavidvdavid
committed
Refactor: Typed errors through volume layer
- Add typed variants to `VolumeError` and `MtpConnectionError` so MTP errors stop being erased into `IoError(String)` and guessed back via string matching - `map_volume_error` is now pure variant→variant mapping, zero string parsing - `classify_io_error` and `linux_copy::map_io_error` check `raw_os_error()` first, falling back to string heuristics only for wrapped library errors
1 parent b894908 commit 8f2296a

7 files changed

Lines changed: 125 additions & 69 deletions

File tree

‎apps/desktop/src-tauri/src/file_system/volume/mod.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,14 @@ pub enum VolumeError {
7272
AlreadyExists(String),
7373
/// Not supported by this volume type.
7474
NotSupported,
75+
/// Device went away mid-operation.
76+
DeviceDisconnected(String),
77+
/// Device or volume is read-only.
78+
ReadOnly(String),
79+
/// Device storage is full.
80+
StorageFull { message: String },
81+
/// Connection timed out.
82+
ConnectionTimeout(String),
7583
IoError(String),
7684
}
7785

@@ -82,6 +90,10 @@ impl std::fmt::Display for VolumeError {
8290
Self::PermissionDenied(path) => write!(f, "Permission denied: {}", path),
8391
Self::AlreadyExists(path) => write!(f, "Already exists: {}", path),
8492
Self::NotSupported => write!(f, "Operation not supported"),
93+
Self::DeviceDisconnected(msg) => write!(f, "Device disconnected: {}", msg),
94+
Self::ReadOnly(msg) => write!(f, "Read-only: {}", msg),
95+
Self::StorageFull { message } => write!(f, "Storage full: {}", message),
96+
Self::ConnectionTimeout(msg) => write!(f, "Connection timed out: {}", msg),
8597
Self::IoError(msg) => write!(f, "I/O error: {}", msg),
8698
}
8799
}

‎apps/desktop/src-tauri/src/file_system/volume/mtp.rs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,13 @@ fn map_mtp_error(e: MtpConnectionError) -> VolumeError {
602602
VolumeError::NotFound(e.to_string())
603603
}
604604
MtpConnectionError::ObjectNotFound { path, .. } => VolumeError::NotFound(path),
605-
MtpConnectionError::ExclusiveAccess { .. } => VolumeError::PermissionDenied(e.to_string()),
605+
MtpConnectionError::ExclusiveAccess { .. } | MtpConnectionError::PermissionDenied { .. } => {
606+
VolumeError::PermissionDenied(e.to_string())
607+
}
608+
MtpConnectionError::Disconnected { .. } => VolumeError::DeviceDisconnected(e.to_string()),
609+
MtpConnectionError::Timeout { .. } => VolumeError::ConnectionTimeout(e.to_string()),
610+
MtpConnectionError::StorageFull { .. } => VolumeError::StorageFull { message: e.to_string() },
611+
MtpConnectionError::StoreReadOnly { .. } => VolumeError::ReadOnly(e.to_string()),
606612
_ => VolumeError::IoError(e.to_string()),
607613
}
608614
}

‎apps/desktop/src-tauri/src/file_system/write_operations/delete.rs‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use super::types::{
1313
DryRunResult, IoResultExt, WriteCancelledEvent, WriteCompleteEvent, WriteOperationConfig, WriteOperationError,
1414
WriteOperationPhase, WriteOperationType, WriteProgressEvent, WriteSourceItemDoneEvent,
1515
};
16+
use super::volume_copy::map_volume_error;
1617
use crate::file_system::volume::Volume;
1718

1819
// ============================================================================
@@ -202,19 +203,13 @@ fn scan_volume_recursive(
202203
});
203204
}
204205

205-
let is_dir = volume.is_directory(path).map_err(|e| WriteOperationError::IoError {
206-
path: path.display().to_string(),
207-
message: e.to_string(),
208-
})?;
206+
let is_dir = volume.is_directory(path).map_err(|e| map_volume_error(&path.display().to_string(), e))?;
209207

210208
if is_dir {
211209
// Recurse into children first — list_directory returns FileEntry with size,
212210
// so we use child.size directly instead of calling get_metadata (which returns
213211
// NotSupported on MTP).
214-
let children = volume.list_directory(path).map_err(|e| WriteOperationError::IoError {
215-
path: path.display().to_string(),
216-
message: e.to_string(),
217-
})?;
212+
let children = volume.list_directory(path).map_err(|e| map_volume_error(&path.display().to_string(), e))?;
218213

219214
for child in &children {
220215
let child_path = PathBuf::from(&child.path);
@@ -396,10 +391,7 @@ pub(super) fn delete_volume_files_with_progress(
396391
});
397392
}
398393

399-
volume.delete(&entry.path).map_err(|e| WriteOperationError::IoError {
400-
path: entry.path.display().to_string(),
401-
message: e.to_string(),
402-
})?;
394+
volume.delete(&entry.path).map_err(|e| map_volume_error(&entry.path.display().to_string(), e))?;
403395

404396
files_done += 1;
405397
bytes_done += entry.size;

‎apps/desktop/src-tauri/src/file_system/write_operations/linux_copy.rs‎

Lines changed: 34 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -143,30 +143,40 @@ fn map_io_error(err: std::io::Error, source: &Path, destination: &Path) -> Write
143143
path: destination.display().to_string(),
144144
},
145145
_ => {
146-
// ENOSPC = 28 on Linux
147-
if let Some(os_err) = err.raw_os_error()
148-
&& os_err == libc::ENOSPC
149-
{
150-
return WriteOperationError::InsufficientSpace {
151-
required: 0,
152-
available: 0,
153-
volume_name: None,
154-
};
155-
}
156-
// ENAMETOOLONG = 36 on Linux
157-
if let Some(os_err) = err.raw_os_error()
158-
&& os_err == libc::ENAMETOOLONG
159-
{
160-
return WriteOperationError::NameTooLong {
161-
path: destination.display().to_string(),
162-
};
163-
}
164-
let lower = err.to_string().to_lowercase();
165-
if lower.contains("read-only") {
166-
return WriteOperationError::ReadOnlyDevice {
167-
path: destination.display().to_string(),
168-
device_name: None,
169-
};
146+
if let Some(os_err) = err.raw_os_error() {
147+
match os_err {
148+
libc::ENOSPC => {
149+
return WriteOperationError::InsufficientSpace {
150+
required: 0,
151+
available: 0,
152+
volume_name: None,
153+
};
154+
}
155+
// These use destination path (classify_io_error can't pick the right one)
156+
libc::ENAMETOOLONG => {
157+
return WriteOperationError::NameTooLong {
158+
path: destination.display().to_string(),
159+
};
160+
}
161+
libc::EROFS => {
162+
return WriteOperationError::ReadOnlyDevice {
163+
path: destination.display().to_string(),
164+
device_name: None,
165+
};
166+
}
167+
libc::ENOTCONN | libc::ENETDOWN | libc::ENETUNREACH
168+
| libc::EHOSTUNREACH | libc::ETIMEDOUT => {
169+
return WriteOperationError::ConnectionInterrupted {
170+
path: source.display().to_string(),
171+
};
172+
}
173+
libc::ENODEV => {
174+
return WriteOperationError::DeviceDisconnected {
175+
path: source.display().to_string(),
176+
};
177+
}
178+
_ => {}
179+
}
170180
}
171181
WriteOperationError::IoError {
172182
path: source.display().to_string(),

‎apps/desktop/src-tauri/src/file_system/write_operations/types.rs‎

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -309,18 +309,40 @@ pub enum WriteOperationError {
309309
}
310310

311311
/// Classifies a raw `std::io::Error` into a specific `WriteOperationError` variant based on its
312-
/// `ErrorKind` and message content. Used by both `IoResultExt::with_path` and the `From` impl.
312+
/// errno code, `ErrorKind`, and message content. Used by both `IoResultExt::with_path` and the
313+
/// `From` impl.
313314
fn classify_io_error(e: &std::io::Error, path: String) -> WriteOperationError {
315+
// Prefer errno when available (local FS operations always have one)
316+
#[cfg(unix)]
317+
if let Some(code) = e.raw_os_error() {
318+
match code {
319+
libc::EROFS => {
320+
return WriteOperationError::ReadOnlyDevice {
321+
path,
322+
device_name: None,
323+
}
324+
}
325+
libc::ENAMETOOLONG => return WriteOperationError::NameTooLong { path },
326+
libc::ENOTCONN | libc::ENETDOWN | libc::ENETUNREACH | libc::EHOSTUNREACH
327+
| libc::ETIMEDOUT => return WriteOperationError::ConnectionInterrupted { path },
328+
libc::ENODEV => return WriteOperationError::DeviceDisconnected { path },
329+
_ => {} // Fall through to ErrorKind/message-based classification
330+
}
331+
}
332+
314333
let msg = e.to_string();
315334
let lower = msg.to_lowercase();
316335

317-
// Message-based heuristics (apply regardless of ErrorKind — checked first because they're
318-
// more specific than the generic kind-based mapping below)
336+
// Message-based heuristics as fallback for errors without raw OS codes
337+
// (synthetic/wrapped errors from libraries)
319338
if lower.contains("disconnect") || lower.contains("no such device") {
320339
return WriteOperationError::DeviceDisconnected { path };
321340
}
322341
if lower.contains("read-only") || lower.contains("read only") {
323-
return WriteOperationError::ReadOnlyDevice { path, device_name: None };
342+
return WriteOperationError::ReadOnlyDevice {
343+
path,
344+
device_name: None,
345+
};
324346
}
325347
if lower.contains("connection") || lower.contains("timed out") || lower.contains("timeout") {
326348
return WriteOperationError::ConnectionInterrupted { path };
@@ -329,7 +351,10 @@ fn classify_io_error(e: &std::io::Error, path: String) -> WriteOperationError {
329351
return WriteOperationError::NameTooLong { path };
330352
}
331353
if lower.contains("invalid") && lower.contains("name") {
332-
return WriteOperationError::InvalidName { path, message: msg };
354+
return WriteOperationError::InvalidName {
355+
path,
356+
message: msg,
357+
};
333358
}
334359

335360
// ErrorKind-based fallback, with one kind-specific heuristic
@@ -340,10 +365,16 @@ fn classify_io_error(e: &std::io::Error, path: String) -> WriteOperationError {
340365
if lower.contains("immutable") || lower.contains("operation not permitted") {
341366
return WriteOperationError::FileLocked { path };
342367
}
343-
WriteOperationError::PermissionDenied { path, message: msg }
368+
WriteOperationError::PermissionDenied {
369+
path,
370+
message: msg,
371+
}
344372
}
345373
std::io::ErrorKind::AlreadyExists => WriteOperationError::DestinationExists { path },
346-
_ => WriteOperationError::IoError { path, message: msg },
374+
_ => WriteOperationError::IoError {
375+
path,
376+
message: msg,
377+
},
347378
}
348379
}
349380

‎apps/desktop/src-tauri/src/file_system/write_operations/volume_copy.rs‎

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -892,7 +892,7 @@ async fn move_within_same_volume(
892892
}
893893

894894
/// Maps VolumeError to WriteOperationError, attaching path context where the original error lacks one.
895-
fn map_volume_error(context_path: &str, e: VolumeError) -> WriteOperationError {
895+
pub(super) fn map_volume_error(context_path: &str, e: VolumeError) -> WriteOperationError {
896896
match e {
897897
VolumeError::NotFound(path) => WriteOperationError::SourceNotFound { path },
898898
VolumeError::PermissionDenied(msg) => WriteOperationError::PermissionDenied {
@@ -904,28 +904,25 @@ fn map_volume_error(context_path: &str, e: VolumeError) -> WriteOperationError {
904904
path: context_path.to_string(),
905905
message: "Operation not supported by this volume type".to_string(),
906906
},
907-
VolumeError::IoError(msg) => {
908-
let lower = msg.to_lowercase();
909-
if lower.contains("disconnect") || lower.contains("no such device") || lower.contains("not found") {
910-
WriteOperationError::DeviceDisconnected {
911-
path: context_path.to_string(),
912-
}
913-
} else if lower.contains("read-only") || lower.contains("read only") {
914-
WriteOperationError::ReadOnlyDevice {
915-
path: context_path.to_string(),
916-
device_name: None,
917-
}
918-
} else if lower.contains("connection") || lower.contains("timed out") || lower.contains("timeout") {
919-
WriteOperationError::ConnectionInterrupted {
920-
path: context_path.to_string(),
921-
}
922-
} else {
923-
WriteOperationError::IoError {
924-
path: context_path.to_string(),
925-
message: msg,
926-
}
927-
}
928-
}
907+
VolumeError::DeviceDisconnected(_) => WriteOperationError::DeviceDisconnected {
908+
path: context_path.to_string(),
909+
},
910+
VolumeError::ReadOnly(_) => WriteOperationError::ReadOnlyDevice {
911+
path: context_path.to_string(),
912+
device_name: None,
913+
},
914+
VolumeError::StorageFull { .. } => WriteOperationError::InsufficientSpace {
915+
required: 0,
916+
available: 0,
917+
volume_name: None,
918+
},
919+
VolumeError::ConnectionTimeout(_) => WriteOperationError::ConnectionInterrupted {
920+
path: context_path.to_string(),
921+
},
922+
VolumeError::IoError(msg) => WriteOperationError::IoError {
923+
path: context_path.to_string(),
924+
message: msg,
925+
},
929926
}
930927
}
931928

‎apps/desktop/src-tauri/src/mtp/connection/errors.rs‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ pub enum MtpConnectionError {
3333
StorageFull {
3434
device_id: String,
3535
},
36+
StoreReadOnly {
37+
device_id: String,
38+
},
3639
/// Linux: USB device file not accessible (missing udev rules).
3740
PermissionDenied {
3841
device_id: String,
@@ -81,6 +84,9 @@ impl std::fmt::Display for MtpConnectionError {
8184
Self::StorageFull { device_id } => {
8285
write!(f, "Storage full on device: {device_id}")
8386
}
87+
Self::StoreReadOnly { device_id } => {
88+
write!(f, "Device is read-only: {device_id}")
89+
}
8490
Self::PermissionDenied { device_id } => {
8591
write!(f, "Permission denied for device: {device_id}")
8692
}
@@ -124,9 +130,8 @@ pub(super) fn map_mtp_error(e: mtp_rs::Error, device_id: &str) -> MtpConnectionE
124130
ResponseCode::StoreFull => MtpConnectionError::StorageFull {
125131
device_id: device_id.to_string(),
126132
},
127-
ResponseCode::StoreReadOnly => MtpConnectionError::Other {
133+
ResponseCode::StoreReadOnly => MtpConnectionError::StoreReadOnly {
128134
device_id: device_id.to_string(),
129-
message: "This device is read-only. You can copy files from it, but not to it.".to_string(),
130135
},
131136
ResponseCode::InvalidObjectHandle | ResponseCode::InvalidParentObject => {
132137
MtpConnectionError::ObjectNotFound {
@@ -288,6 +293,9 @@ mod tests {
288293
MtpConnectionError::StorageFull {
289294
device_id: "test".to_string(),
290295
},
296+
MtpConnectionError::StoreReadOnly {
297+
device_id: "test".to_string(),
298+
},
291299
MtpConnectionError::PermissionDenied {
292300
device_id: "test".to_string(),
293301
},

0 commit comments

Comments
 (0)
⚡