max / audiofiles
- Co-Authored-By
- Claude Opus 5 (1M context) <noreply@anthropic.com>
5 files changed,
+67 insertions,
-18 deletions
| @@ -5,7 +5,7 @@ | |||
| 5 | 5 | ||
| 6 | 6 | use std::path::PathBuf; | |
| 7 | 7 | ||
| 8 | - | use tracing::{error, instrument}; | |
| 8 | + | use tracing::{error, instrument, warn}; | |
| 9 | 9 | ||
| 10 | 10 | use audiofiles_core::db::Database; | |
| 11 | 11 | use audiofiles_core::store::SampleStore; | |
| @@ -114,11 +114,22 @@ | |||
| 114 | 114 | } | |
| 115 | 115 | }; | |
| 116 | 116 | ||
| 117 | - | // WAL checkpoint for clean state. | |
| 118 | - | let _ = worker | |
| 117 | + | // WAL checkpoint for clean state. The row deletes above are already | |
| 118 | + | // committed, so a failed checkpoint neither aborts nor needs a retry: it | |
| 119 | + | // only means the WAL stays large until the next one. It is still a real | |
| 120 | + | // failure, so it counts toward `errors` rather than vanishing, and the | |
| 121 | + | // report the user sees matches what happened. | |
| 122 | + | let errors = match worker | |
| 119 | 123 | .db | |
| 120 | 124 | .conn() | |
| 121 | - | .execute_batch("PRAGMA wal_checkpoint(TRUNCATE)"); | |
| 125 | + | .execute_batch("PRAGMA wal_checkpoint(TRUNCATE)") | |
| 126 | + | { | |
| 127 | + | Ok(()) => errors, | |
| 128 | + | Err(e) => { | |
| 129 | + | warn!("Cleanup worker failed to checkpoint the WAL: {e}"); | |
| 130 | + | errors + 1 | |
| 131 | + | } | |
| 132 | + | }; | |
| 122 | 133 | ||
| 123 | 134 | ctx.emit(CleanupEvent::Complete { removed, errors }); | |
| 124 | 135 | } |
| @@ -4,6 +4,7 @@ | |||
| 4 | 4 | Arc, BrowserState, CollectionId, ColumnConfig, ContentsRef, PathBuf, SearchFilter, SortColumn, | |
| 5 | 5 | SortDirection, error, warn, | |
| 6 | 6 | }; | |
| 7 | + | use crate::backend::{BackendError, BackendResult}; | |
| 7 | 8 | ||
| 8 | 9 | impl BrowserState { | |
| 9 | 10 | // --- Misc helpers --- | |
| @@ -582,36 +583,53 @@ | |||
| 582 | 583 | } | |
| 583 | 584 | } | |
| 584 | 585 | ||
| 585 | - | /// Enable or disable the VFS mirror. Persists the setting. | |
| 586 | - | pub fn set_mirror_enabled(&mut self, enabled: bool) { | |
| 586 | + | /// Enable or disable the VFS mirror, persisting the setting. | |
| 587 | + | /// | |
| 588 | + | /// The in-memory flag is applied either way, so the session behaves as the | |
| 589 | + | /// user asked; the error says the choice will not survive a restart, or | |
| 590 | + | /// that the old symlink tree is still on disk. Both are things the user can | |
| 591 | + | /// act on, so they are returned rather than dropped. | |
| 592 | + | pub fn set_mirror_enabled(&mut self, enabled: bool) -> BackendResult<()> { | |
| 587 | 593 | self.mirror.mirror_enabled = enabled; | |
| 588 | - | let _ = self.backend.set_config( | |
| 594 | + | let persisted = self.backend.set_config( | |
| 589 | 595 | crate::backend::ConfigKey::MirrorEnabled, | |
| 590 | 596 | if enabled { "1" } else { "0" }, | |
| 591 | 597 | ); | |
| 592 | 598 | ||
| 593 | - | if enabled { | |
| 599 | + | let removed = if enabled { | |
| 594 | 600 | // Run initial sync immediately. | |
| 595 | 601 | self.mirror.mirror_dirty = true; | |
| 596 | 602 | self.sync_mirror_if_dirty(); | |
| 603 | + | Ok(()) | |
| 597 | 604 | } else { | |
| 598 | - | let _ = audiofiles_core::vfs_mirror::remove_mirror(&self.mirror.mirror_path); | |
| 599 | - | } | |
| 605 | + | audiofiles_core::vfs_mirror::remove_mirror(&self.mirror.mirror_path) | |
| 606 | + | .map_err(BackendError::from) | |
| 607 | + | }; | |
| 608 | + | ||
| 609 | + | persisted.and(removed) | |
| 600 | 610 | } | |
| 601 | 611 | ||
| 602 | - | /// Set the mirror path. Persists the setting. | |
| 603 | - | pub fn set_mirror_path(&mut self, path: PathBuf) { | |
| 612 | + | /// Set the mirror path, persisting the setting. | |
| 613 | + | /// | |
| 614 | + | /// Same contract as [`Self::set_mirror_enabled`]: the new path takes effect | |
| 615 | + | /// in this session regardless, and the error reports that the old mirror | |
| 616 | + | /// could not be torn down or that the new path was not written to config. | |
| 617 | + | pub fn set_mirror_path(&mut self, path: PathBuf) -> BackendResult<()> { | |
| 604 | 618 | // If mirror is enabled, remove old mirror before switching. | |
| 605 | - | if self.mirror.mirror_enabled { | |
| 606 | - | let _ = audiofiles_core::vfs_mirror::remove_mirror(&self.mirror.mirror_path); | |
| 607 | - | } | |
| 619 | + | let removed = if self.mirror.mirror_enabled { | |
| 620 | + | audiofiles_core::vfs_mirror::remove_mirror(&self.mirror.mirror_path) | |
| 621 | + | .map_err(BackendError::from) | |
| 622 | + | } else { | |
| 623 | + | Ok(()) | |
| 624 | + | }; | |
| 608 | 625 | self.mirror.mirror_path = path; | |
| 609 | - | let _ = self.backend.set_config( | |
| 626 | + | let persisted = self.backend.set_config( | |
| 610 | 627 | crate::backend::ConfigKey::MirrorPath, | |
| 611 | 628 | &self.mirror.mirror_path.to_string_lossy(), | |
| 612 | 629 | ); | |
| 613 | 630 | if self.mirror.mirror_enabled { | |
| 614 | 631 | self.mirror.mirror_dirty = true; | |
| 615 | 632 | } | |
| 633 | + | removed.and(persisted) | |
| 616 | 634 | } | |
| 617 | 635 | } |
| @@ -480,6 +480,7 @@ | |||
| 480 | 480 | mirror_enabled, | |
| 481 | 481 | mirror_path, | |
| 482 | 482 | mirror_dirty: mirror_enabled, | |
| 483 | + | last_error: None, | |
| 483 | 484 | }, | |
| 484 | 485 | sync: SyncUiState::default(), | |
| 485 | 486 | settings: SettingsUiState { |
| @@ -600,6 +600,11 @@ | |||
| 600 | 600 | pub mirror_enabled: bool, | |
| 601 | 601 | pub mirror_path: std::path::PathBuf, | |
| 602 | 602 | pub mirror_dirty: bool, | |
| 603 | + | /// Last failure from a mirror settings change (config write or teardown of | |
| 604 | + | /// the old symlink tree). Held so the settings panel can show it: these | |
| 605 | + | /// writes fail in ways the user has to know about, since the setting will | |
| 606 | + | /// not survive a restart. | |
| 607 | + | pub last_error: Option<String>, | |
| 603 | 608 | } | |
| 604 | 609 | ||
| 605 | 610 | /// Collections sidebar: list, active selection, and create/rename inputs. |
| @@ -1037,7 +1037,10 @@ | |||
| 1037 | 1037 | ) | |
| 1038 | 1038 | .changed() | |
| 1039 | 1039 | { | |
| 1040 | - | state.set_mirror_enabled(mirror); | |
| 1040 | + | state.mirror.last_error = state | |
| 1041 | + | .set_mirror_enabled(mirror) | |
| 1042 | + | .err() | |
| 1043 | + | .map(|e| e.to_string()); | |
| 1041 | 1044 | } | |
| 1042 | 1045 | // Always surface the mirror path so the user knows where the | |
| 1043 | 1046 | // symlink tree will live before enabling, and can change it | |
| @@ -1058,10 +1061,21 @@ | |||
| 1058 | 1061 | state | |
| 1059 | 1062 | .dialogs | |
| 1060 | 1063 | .pick_folder("Choose library mirror location", |s, p| { | |
| 1061 | - | s.set_mirror_path(p); | |
| 1064 | + | s.mirror.last_error = | |
| 1065 | + | s.set_mirror_path(p).err().map(|e| e.to_string()); | |
| 1062 | 1066 | }); | |
| 1063 | 1067 | } | |
| 1064 | 1068 | }); | |
| 1069 | + | // A mirror setting that failed to persist looks identical to one | |
| 1070 | + | // that took, so say so rather than leaving the checkbox lying. | |
| 1071 | + | if let Some(err) = state.mirror.last_error.clone() { | |
| 1072 | + | widgets::warning_banner( | |
| 1073 | + | ui, | |
| 1074 | + | &format!( | |
| 1075 | + | "Mirror setting not saved: {err}. It applies to this session only." | |
| 1076 | + | ), | |
| 1077 | + | ); | |
| 1078 | + | } | |
| 1065 | 1079 | } | |
| 1066 | 1080 | }); | |
| 1067 | 1081 | } |