Skip to content

Commit eec50ff

Browse files
committed
Friendly, provider-aware error pane for listing failures
- Replace raw "I/O error (os error 60)" with structured `ErrorPane` that shows a warm title, plain-language explanation, provider-specific suggestions, and collapsible technical details - Two-layer Rust mapping: 37 macOS errno codes → `FriendlyError` (Transient/NeedsAction/Serious categories), then path-based enrichment for 19 cloud/mount providers (Dropbox, Google Drive, OneDrive, iCloud, MacDroid, macFUSE/SSHFS, VeraCrypt, and more) - Provider detection uses `~/Library/CloudStorage/` prefixes, `~/Library/Mobile Documents/`, specific `/Volumes/` paths, and `statfs` `f_fstypename` for FUSE mounts - `VolumeError::IoError` now carries `raw_os_error: Option<i32>` so errno survives the `From<std::io::Error>` conversion - `streaming.rs` propagates `VolumeError` directly (instead of wrapping in `std::io::Error`) and builds `FriendlyError` at the emission site - Frontend `ErrorPane.svelte` replaces both `PermissionDeniedPane` and the raw error div. Category-based visual tone, markdown rendering via `snarkdown`, retry button with attempt history for transient errors, "Open System Settings" for permission denied - E2E testability: `inject_error()` on `Volume` trait behind `playwright-e2e` feature flag, with Playwright tests for transient/needs-action/a11y - Removed `@lottiefiles/dotlottie-svelte` (only consumer was deleted `PermissionDeniedPane`)
1 parent 185e984 commit eec50ff

30 files changed

Lines changed: 3013 additions & 208 deletions

CHANGELOG.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,7 @@ The format is based on [keep a changelog](https://keepachangelog.com/en/1.1.0/),
1212
- Striped rows setting — alternating row shading in Full and Brief view modes, auto-adapts to light/dark mode
1313
([faa2534](https://github.com/vdavid/cmdr/commit/faa2534))
1414
- MTP per-file copy progress and mid-file cancellation — progress callback on every USB chunk, instant cancel via USB
15-
SIC abort (~300ms instead of draining the full stream)
16-
([ac5ec4d](https://github.com/vdavid/cmdr/commit/ac5ec4d),
15+
SIC abort (~300ms instead of draining the full stream) ([ac5ec4d](https://github.com/vdavid/cmdr/commit/ac5ec4d),
1716
[a66adf6](https://github.com/vdavid/cmdr/commit/a66adf6))
1817

1918
### Fixed

apps/desktop/coverage-allowlist.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@
5959
"file-explorer/network/NetworkBrowser.svelte": { "reason": "Network component, needs Tauri integration" },
6060
"file-explorer/pane/PaneResizer.svelte": { "reason": "Mouse drag UI component, difficult to unit test" },
6161
"file-explorer/network/NetworkLoginForm.svelte": { "reason": "Network component, needs Tauri integration" },
62-
"file-explorer/pane/PermissionDeniedPane.svelte": { "reason": "Simple UI component" },
62+
"file-explorer/pane/ErrorPane.svelte": { "reason": "UI component, logic tested in error-pane-utils.test.ts" },
6363
"file-explorer/pane/VolumeUnreachableBanner.svelte": {
6464
"reason": "UI banner component, depends on Tauri commands"
6565
},

apps/desktop/package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
"@crabnebula/tauri-plugin-drag": "^2.1.0",
4040
"@leeoniya/ufuzzy": "^1.0.19",
4141
"@logtape/logtape": "^2.0.4",
42-
"@lottiefiles/dotlottie-svelte": "^0.9.8",
4342
"@lucide/svelte": "^0.577.0",
4443
"@tauri-apps/api": "^2.10.1",
4544
"@tauri-apps/plugin-dialog": "^2.7.0",
@@ -48,7 +47,8 @@
4847
"@tauri-apps/plugin-process": "^2.3.1",
4948
"@tauri-apps/plugin-store": "^2.4.2",
5049
"@tauri-apps/plugin-updater": "^2.10.0",
51-
"@tauri-apps/plugin-window-state": "~2.4.1"
50+
"@tauri-apps/plugin-window-state": "~2.4.1",
51+
"snarkdown": "2.0.0"
5252
},
5353
"devDependencies": {
5454
"@crabnebula/tauri-driver": "^2.0.9",

apps/desktop/src-tauri/src/commands/file_system.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -922,6 +922,23 @@ fn emit_synthetic_entry_diff(app: &tauri::AppHandle, entry_path: &Path, parent_p
922922
}
923923
}
924924

925+
// ============================================================================
926+
// E2E test support (feature-gated)
927+
// ============================================================================
928+
929+
/// Injects a listing error into an in-memory volume so the next `list_directory` call
930+
/// returns a `VolumeError::IoError` with the given errno. The error is cleared after
931+
/// one use, enabling retry testing.
932+
#[cfg(feature = "playwright-e2e")]
933+
#[tauri::command]
934+
pub fn inject_listing_error(volume_id: String, error_code: i32) -> Result<(), String> {
935+
let volume = get_volume_manager()
936+
.get(&volume_id)
937+
.ok_or_else(|| format!("Volume `{}` not found", volume_id))?;
938+
volume.inject_error(error_code);
939+
Ok(())
940+
}
941+
925942
/// Expands tilde (~) to the user's home directory.
926943
pub(crate) fn expand_tilde(path: &str) -> String {
927944
if (path.starts_with("~/") || path == "~")

apps/desktop/src-tauri/src/commands/ui.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,7 @@ pub fn show_file_context_menu<R: Runtime>(
3939
/// The `shortcut` is the user's configured shortcut in frontend format (e.g. "⌃⌘C"),
4040
/// or empty string if no shortcut is configured.
4141
#[tauri::command]
42-
pub fn show_breadcrumb_context_menu<R: Runtime>(
43-
window: Window<R>,
44-
shortcut: String,
45-
) -> Result<(), String> {
42+
pub fn show_breadcrumb_context_menu<R: Runtime>(window: Window<R>, shortcut: String) -> Result<(), String> {
4643
let app = window.app_handle();
4744
let accelerator = frontend_shortcut_to_accelerator(&shortcut).unwrap_or_default();
4845
let menu = build_breadcrumb_context_menu(app, &accelerator).map_err(|e| e.to_string())?;

apps/desktop/src-tauri/src/file_system/listing/streaming.rs

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ use std::time::Duration;
1414
use crate::benchmark;
1515
use crate::file_system::listing::caching::{CachedListing, LISTING_CACHE};
1616
use crate::file_system::listing::sorting::{DirectorySortMode, SortColumn, SortOrder, sort_entries};
17+
use crate::file_system::volume::VolumeError;
18+
use crate::file_system::volume::friendly_error::{
19+
FriendlyError, enrich_with_provider, friendly_error_from_volume_error,
20+
};
1721
use crate::file_system::watcher::start_watching;
1822

1923
// ============================================================================
@@ -68,6 +72,8 @@ pub struct ListingCompleteEvent {
6872
pub struct ListingErrorEvent {
6973
pub listing_id: String,
7074
pub message: String,
75+
/// Structured error info when available. `None` for internal errors (task panics).
76+
pub friendly: Option<FriendlyError>,
7177
}
7278

7379
/// Cancelled event payload
@@ -141,6 +147,7 @@ pub async fn list_directory_start_streaming(
141147
// Clone values for the spawned task
142148
let listing_id_for_spawn = listing_id.clone();
143149
let path_owned = path.to_path_buf();
150+
let path_for_error = path.to_path_buf();
144151
let volume_id_owned = volume_id.to_string();
145152
let app_for_spawn = app.clone();
146153

@@ -180,17 +187,22 @@ pub async fn list_directory_start_streaming(
180187
"listing-error",
181188
ListingErrorEvent {
182189
listing_id: listing_id_for_cleanup,
183-
message: format!("Task failed: {}", e),
190+
message: "Something went wrong while reading this folder".to_string(),
191+
friendly: None,
184192
},
185193
);
194+
log::error!("Listing task panicked: {}", e);
186195
}
187196
Ok(Err(e)) => {
188-
// Function returned an error (like volume not found, permission denied)
197+
// Function returned an error (volume not found, permission denied, I/O, etc.)
198+
let mut friendly = friendly_error_from_volume_error(&e, &path_for_error);
199+
enrich_with_provider(&mut friendly, &path_for_error);
189200
let _ = app_for_error.emit(
190201
"listing-error",
191202
ListingErrorEvent {
192203
listing_id: listing_id_for_cleanup,
193204
message: e.to_string(),
205+
friendly: Some(friendly),
194206
},
195207
);
196208
}
@@ -225,7 +237,7 @@ fn read_directory_with_progress(
225237
sort_by: SortColumn,
226238
sort_order: SortOrder,
227239
dir_sort_mode: DirectorySortMode,
228-
) -> Result<(), std::io::Error> {
240+
) -> Result<(), VolumeError> {
229241
use tauri::Emitter;
230242

231243
benchmark::log_event("read_directory_with_progress START");
@@ -260,7 +272,7 @@ fn read_directory_with_progress(
260272
// Get the volume from VolumeManager
261273
let volume = crate::file_system::get_volume_manager()
262274
.get(volume_id)
263-
.ok_or_else(|| std::io::Error::new(std::io::ErrorKind::NotFound, format!("Volume not found: {}", volume_id)))?;
275+
.ok_or_else(|| VolumeError::NotFound(format!("Volume not found: {}", volume_id)))?;
264276

265277
// Read directory entries via Volume abstraction
266278
// Use polling-based cancellation to remain responsive even when filesystem I/O blocks
@@ -310,14 +322,15 @@ fn read_directory_with_progress(
310322
Ok(result) => break result,
311323
Err(mpsc::RecvTimeoutError::Timeout) => continue,
312324
Err(mpsc::RecvTimeoutError::Disconnected) => {
313-
return Err(std::io::Error::other(
314-
"Directory listing thread terminated unexpectedly",
315-
));
325+
return Err(VolumeError::IoError {
326+
message: "Directory listing thread terminated unexpectedly".into(),
327+
raw_os_error: None,
328+
});
316329
}
317330
}
318331
};
319332

320-
let mut entries = entries_result.map_err(|e| std::io::Error::other(e.to_string()))?;
333+
let mut entries = entries_result?;
321334
let read_dir_time = read_start.elapsed();
322335
benchmark::log_event_value("read_dir COMPLETE, entries", entries.len());
323336

apps/desktop/src-tauri/src/file_system/volume/CLAUDE.md

Lines changed: 102 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ Every file system operation (listing, copy, rename, delete, indexing, watching)
1111
| File | Role |
1212
|---|---|
1313
| `mod.rs` | `Volume` trait, `VolumeScanner`, `VolumeWatcher`, `VolumeReadStream` traits, `MutationEvent` enum, shared types (`VolumeError`, `SpaceInfo`, `CopyScanResult`, `ScanConflict`, `SourceItemInfo`) |
14+
| `friendly_error.rs` | User-facing error messages. See [Friendly error system](#friendly-error-system) below. |
1415
| `manager.rs` | `VolumeManager` — thread-safe `RwLock<HashMap>` registry; supports a default volume |
1516
| `local_posix.rs` | `LocalPosixVolume` — real filesystem; delegates listing to `file_system::listing`, indexing to `indexing::scanner`, watching to `indexing::watcher` (FSEvents), copy scanning via `walkdir`. Uses `libc::statvfs` FFI for space info. |
1617
| `mtp.rs` | `MtpVolume` — MTP device storage; synchronous `Volume` trait bridged to async MTP calls via `tokio::runtime::Handle::block_on`. Gated with `#[cfg(any(target_os = "macos", target_os = "linux"))]`. |
@@ -71,6 +72,99 @@ Both paths check the `network.directSmbConnection` setting (global `AtomicBool`)
7172
warning and the volume stays as `LocalPosixVolume`. The "Connect directly" UI action (`upgrade_to_smb_volume` command)
7273
provides a manual upgrade path.
7374

75+
## Friendly error system
76+
77+
`friendly_error.rs` turns raw OS errors into warm, actionable messages so the user feels supported when something goes
78+
wrong. This is one of Cmdr's UX differentiators: where other file managers show "I/O error: Operation timed out (os
79+
error 60)", we show a friendly title, a plain-language explanation, and provider-specific advice ("This folder is managed
80+
by **MacDroid**. Here's what to try: ...").
81+
82+
### Philosophy
83+
84+
**The user should never feel alone with a broken state.** Every error message should feel like the app is putting its
85+
hand on the user's shoulder and saying "Here's what happened, and here's what you can do." We go above and beyond: we
86+
detect which cloud provider or mount tool manages the path, and tailor the suggestion to that specific app. A timeout on
87+
a Dropbox folder gets different advice than a timeout on an SSHFS mount.
88+
89+
Power users also need the raw details (errno name, code) for debugging or bug reports. These are available in a
90+
collapsible "Technical details" section, never hidden but never in your face either.
91+
92+
### Architecture
93+
94+
Two-layer mapping, both in this file:
95+
96+
1. **`friendly_error_from_volume_error(err, path)`** — maps `VolumeError` variants and macOS errno codes (37 codes) to a
97+
`FriendlyError` with category (Transient/NeedsAction/Serious), title, explanation, suggestion, and raw detail.
98+
2. **`enrich_with_provider(error, path)`** — detects 19 cloud/mount providers from path patterns and `statfs` filesystem
99+
type, then overwrites the suggestion with provider-specific advice.
100+
101+
The frontend receives the fully-baked `FriendlyError` struct via the `listing-error` Tauri event and renders it with
102+
category-based visual styling. The frontend never sees errno codes or does OS-specific logic.
103+
104+
### Adding a new error message
105+
106+
When you need to handle a new errno or `VolumeError` variant:
107+
108+
1. Add the match arm in `friendly_error_from_volume_error`
109+
2. Pick the right `ErrorCategory`: **Transient** (retry might work), **NeedsAction** (user must do something),
110+
**Serious** (something is genuinely broken)
111+
3. Write the message following the rules below
112+
4. Add a unit test asserting the category and that the text follows the style rules
113+
5. Run the existing `error_messages_never_contain_error_or_failed` test to catch violations
114+
115+
### Adding a new provider
116+
117+
When a new cloud storage or mount tool becomes popular enough to detect:
118+
119+
1. Add a variant to the `Provider` enum with `display_name()` and `app_name()`
120+
2. Add path detection in `detect_provider` (CloudStorage prefix, specific path, or `statfs` type)
121+
3. Write provider-specific suggestions in `provider_suggestion` for each `ErrorCategory`
122+
4. Add a unit test for path detection and suggestion content
123+
5. Update `volumes/CLAUDE.md` provider table to keep the two lists in sync
124+
125+
### Writing rules for error messages
126+
127+
These are non-negotiable. The existing test suite enforces some of them automatically.
128+
129+
- **NEVER use "error" or "failed"** in titles, explanations, or suggestions. Say "Couldn't read" not "Read error". The
130+
automated test `error_messages_never_contain_error_or_failed` catches this.
131+
- **Active voice, contractions**: "Cmdr couldn't..." not "The operation was unable to..."
132+
- **No trivializing**: no "just", "simply", "easy", "all you have to do"
133+
- **No permissive language**: "Check your connection" not "You might want to check..."
134+
- **Direct and warm**: "Here's what to try:" not "Please attempt the following remediation steps:"
135+
- **No em dashes**: use parentheses, commas, or new sentences
136+
- **Sentence case in titles**: "Connection timed out" not "Connection Timed Out"
137+
- **Bold key terms** with `**` only when it helps scanning (for example, provider names)
138+
- **Platform-native terms**: "System Settings" on macOS, "Finder", "Trash"
139+
- **Keep it short**: max two sentences for explanation, bullets for suggestions
140+
141+
Good example:
142+
```
143+
title: "Connection timed out"
144+
explanation: "Cmdr tried to read this folder but the connection didn't respond in time."
145+
suggestion: "Here's what to try:\n- Check that the device or server is reachable\n- ..."
146+
```
147+
148+
Bad example (every rule violated):
149+
```
150+
title: "I/O Error: Operation Timed Out" // "Error", Title Case
151+
explanation: "An error occurred while the system attempted to access the directory." // passive, "error"
152+
suggestion: "You may want to try simply reconnecting the device." // permissive, trivializing
153+
```
154+
155+
### Provider detection strategies
156+
157+
| Strategy | Providers covered |
158+
|---|---|
159+
| `~/Library/CloudStorage/<Prefix>*` | Dropbox, GoogleDrive, OneDrive, Box, pCloud, Nextcloud, SynologyDrive, Tresorit, ProtonDrive, Sync, Egnyte, MacDroid, plus a generic fallback for unrecognized providers |
160+
| `~/Library/Mobile Documents/` | iCloud Drive |
161+
| `/Volumes/pCloudDrive` | pCloud (FUSE virtual drive) |
162+
| `/Volumes/veracrypt*` | VeraCrypt |
163+
| `~/.CMVolumes/` | CloudMounter |
164+
| `statfs` `f_fstypename` (macOS) | macFUSE/SSHFS/Cryptomator/rclone (`macfuse`, `osxfuse`), pCloud (`pcloudfs`) |
165+
166+
The `statfs` check runs only at error time (not on every listing), so the syscall cost is negligible.
167+
74168
## Integration status
75169

76170
`LocalPosixVolume` is wired into the indexing subsystem. `VolumeManager` is actively used.
@@ -93,7 +187,13 @@ provides a manual upgrade path.
93187
**Why**: The `Volume` trait is synchronous because local filesystem ops are blocking and shouldn't touch the async executor. MTP operations are inherently async (USB bulk transfers), so `block_on` bridges the gap. This is safe because MTP methods are always called from `spawn_blocking` contexts (separate OS thread pool), avoiding nested-runtime panics.
94188

95189
**Decision**: `VolumeError` stores `String` messages, not the original `std::io::Error`
96-
**Why**: `std::io::Error` is not `Clone`, but `VolumeError` needs to be `Clone` for ergonomic error propagation across thread boundaries and for serialization to the frontend. Storing the formatted message loses the original error type but keeps the information that matters for user-facing error messages.
190+
**Why**: `std::io::Error` is not `Clone`, but `VolumeError` needs to be `Clone` for ergonomic error propagation across thread boundaries and for serialization to the frontend. Storing the formatted message loses the original error type but keeps the information that matters for user-facing error messages. The `IoError` variant also carries `raw_os_error: Option<i32>` so the friendly error mapper can match on platform-specific errno codes.
191+
192+
**Decision**: Friendly error mapping is two layers: errno mapping, then provider enrichment
193+
**Why**: Not every provider+errno combination needs custom copy. The base errno message is always useful. Provider enrichment is additive, making the suggestion more specific when we recognize who manages the mount. Keeping them separate avoids a combinatorial explosion of messages.
194+
195+
**Decision**: Friendly error mapping lives in Rust, not the frontend
196+
**Why**: The mapping needs access to the full path (for provider detection) and platform-specific errno codes. Doing it in Rust keeps the frontend thin (principle: smart backend, thin frontend) and avoids duplicating errno knowledge in TypeScript. The frontend receives a ready-to-render `FriendlyError` struct with markdown strings.
97197

98198
**Decision**: `LocalPosixVolume` uses `symlink_metadata` for `exists()` instead of `Path::exists()`
99199
**Why**: `Path::exists()` follows symlinks — a dangling symlink returns `false`, which would make the volume claim a file doesn't exist when it visibly does in a directory listing. `symlink_metadata` detects the symlink itself, matching what the user sees.
@@ -150,6 +250,7 @@ provides a manual upgrade path.
150250

151251
## Testing
152252

253+
- **E2E error injection**: The `Volume` trait has an `inject_error(&self, errno: i32)` method behind the `playwright-e2e` feature flag. `LocalPosixVolume` and `InMemoryVolume` implement it — the next `list_directory` call returns the injected errno, then clears it (single-shot, so retry tests work). Default is no-op.
153254
- `in_memory_test.rs` — unit tests for `InMemoryVolume` (CRUD, sorting, concurrency, stress 50k entries)
154255
- `inmemory_test.rs` — integration tests combining `InMemoryVolume` + `VolumeManager`, streaming state, sort helpers
155256
- `local_posix_test.rs` — real-FS tests (write ops, symlinks, copy, space info) using `std::env::temp_dir()`

0 commit comments

Comments
 (0)