Data Recovery: a folder error names the folder that is missing or is not a folder

When a folder on the way to the Data Recovery folder was missing and
couldn't be made, or was a file, the error named the area folder or the
copy inside it, for example `<recovery>/1794385145-1: Not a directory
(os error 20)`. Errors on the store's folders and files now name the
first folder on the way, from the top, that is not a folder ("<path> is
not a folder: <OS error>") or is missing ("couldn't make <path>: <OS
error>" while the store makes it, "<path> is missing" otherwise). When
every folder on the way is there, the error names the folder or file it
happened on. When reading a folder reports it missing, listing checks
the folders on the way, so a file there is an error and not an empty
list.

crates/engine/src/cmd/recovery.rs

@@ -274,7 +274,46 @@ impl FolderStore {
         builder.recursive(true);
         #[cfg(unix)]
         std::os::unix::fs::DirBuilderExt::mode(&mut builder, 0o700);
-        builder.create(dir).map_err(|e| format!("{}: {e}", dir.display()))
+        builder.create(dir).map_err(|e| Self::folder_error(dir, &e, true))
+    }
+
+    /// The first folder on the way to `dir` (`dir` included), from the top down, that is not a
+    /// folder (`true`) or is missing (`false`). None when every one of them is a folder or one of
+    /// them can't be checked.
+    fn in_the_way(dir: &std::path::Path) -> Option<(&std::path::Path, bool)> {
+        let mut way: Vec<&std::path::Path> = dir.ancestors().filter(|p| !p.as_os_str().is_empty()).collect();
+        way.reverse();
+        for p in way {
+            match std::fs::metadata(p) {
+                Ok(m) if m.is_dir() => {}
+                Ok(_) => return Some((p, true)),
+                Err(m) if m.kind() == std::io::ErrorKind::NotFound => return Some((p, false)),
+                Err(_) => return None,
+            }
+        }
+        None
+    }
+
+    /// The message for error `e` on folder `dir`, which was being made (`making`) or read. It
+    /// names the first folder on the way to `dir` that is not a folder, or is missing (when
+    /// `making`, the one that couldn't be made), and otherwise `dir`.
+    fn folder_error(dir: &std::path::Path, e: &std::io::Error, making: bool) -> String {
+        match Self::in_the_way(dir) {
+            Some((p, true)) => format!("{} is not a folder: {e}", p.display()),
+            Some((p, false)) if making => format!("couldn't make {}: {e}", p.display()),
+            Some((p, false)) if p != dir => format!("{} is missing", p.display()),
+            _ => format!("{}: {e}", dir.display()),
+        }
+    }
+
+    /// The message for error `e` on file `f`. It names the first folder on the way to `f` that is
+    /// not a folder or is missing, and otherwise `f`.
+    fn file_error(f: &std::path::Path, e: &std::io::Error) -> String {
+        match f.parent().and_then(Self::in_the_way) {
+            Some((p, true)) => format!("{} is not a folder: {e}", p.display()),
+            Some((p, false)) => format!("{} is missing", p.display()),
+            None => format!("{}: {e}", f.display()),
+        }
     }
 
     /// Refuse area folder `dir` unless it is a folder itself, not a link to one, and (on Unix)
@@ -282,7 +321,7 @@ impl FolderStore {
     /// linked to a folder of its own in a shared recovery folder looked free, and claiming it
     /// wrote the copies through the link.
     fn own_area(dir: &std::path::Path) -> std::result::Result<(), String> {
-        let m = std::fs::symlink_metadata(dir).map_err(|e| format!("{}: {e}", dir.display()))?;
+        let m = std::fs::symlink_metadata(dir).map_err(|e| Self::folder_error(dir, &e, false))?;
         if !m.file_type().is_dir() {
             return Err(format!("{}: not a folder of its own", dir.display()));
         }
@@ -294,11 +333,17 @@ impl FolderStore {
     }
 
     /// The plain names of the files (or, `dirs`, folders) in `dir`; none when it doesn't exist.
+    /// A file on the way to `dir` is an error: Windows reports such a folder as missing.
     fn names(dir: &std::path::Path, dirs: bool) -> std::result::Result<Vec<String>, String> {
         let entries = match std::fs::read_dir(dir) {
             Ok(e) => e,
-            Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(vec![]),
-            Err(e) => return Err(format!("{}: {e}", dir.display())),
+            Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
+                return match Self::in_the_way(dir) {
+                    Some((p, true)) => Err(format!("{} is not a folder: {e}", p.display())),
+                    _ => Ok(vec![]),
+                };
+            }
+            Err(e) => return Err(Self::folder_error(dir, &e, false)),
         };
         Ok(entries
             .filter_map(|e| e.ok())
@@ -333,7 +378,7 @@ impl FolderStore {
         match e.kind() {
             std::io::ErrorKind::NotFound => Ok(Lock::Busy),
             std::io::ErrorKind::PermissionDenied if std::fs::symlink_metadata(path).is_ok() => Ok(Lock::Busy),
-            _ => Err(format!("{}: {e}", path.display())),
+            _ => Err(Self::file_error(path, &e)),
         }
     }
 
@@ -345,7 +390,7 @@ impl FolderStore {
             Ok(()) | Err(std::fs::TryLockError::WouldBlock) => Ok(Lock::Busy),
             // The lock file was removed meanwhile, and locking it failed instead.
             Err(std::fs::TryLockError::Error(_)) if !Self::still_named(&file, path) => Ok(Lock::Busy),
-            Err(std::fs::TryLockError::Error(e)) => Err(format!("{}: {e}", path.display())),
+            Err(std::fs::TryLockError::Error(e)) => Err(Self::file_error(path, &e)),
         }
     }
 
@@ -393,10 +438,10 @@ impl RecoveryStore for FolderStore {
         if let Some(dir) = f.parent() {
             Self::own_area(dir)?;
         }
-        if !std::fs::symlink_metadata(&f).map_err(|e| format!("{}: {e}", f.display()))?.file_type().is_file() {
+        if !std::fs::symlink_metadata(&f).map_err(|e| Self::file_error(&f, &e))?.file_type().is_file() {
             return Err(format!("{}: not a plain file", f.display()));
         }
-        std::fs::read(&f).map_err(|e| format!("{}: {e}", f.display()))
+        std::fs::read(&f).map_err(|e| Self::file_error(&f, &e))
     }
     fn write(&self, name: &str, bytes: &[u8]) -> std::result::Result<(), String> {
         let f = self.file(name)?;
@@ -404,12 +449,12 @@ impl RecoveryStore for FolderStore {
             Self::create_area(dir)?;
             Self::own_area(dir)?;
         }
-        vectorcraft_format::write_atomic(&f, bytes).map_err(|e| format!("{}: {e}", f.display()))
+        vectorcraft_format::write_atomic(&f, bytes).map_err(|e| Self::file_error(&f, &e))
     }
     fn remove(&self, name: &str) -> std::result::Result<(), String> {
         let f = self.file(name)?;
         match std::fs::remove_file(&f) {
-            Err(e) if e.kind() != std::io::ErrorKind::NotFound => Err(format!("{}: {e}", f.display())),
+            Err(e) if e.kind() != std::io::ErrorKind::NotFound => Err(Self::file_error(&f, &e)),
             _ => Ok(()),
         }
     }

crates/engine/src/tests/recovery.rs

@@ -483,6 +483,87 @@ fn the_folder_store_reads_no_linked_entries() {
     let _ = std::fs::remove_dir_all(&base);
 }
 
+/// A file where a folder of the store's path should be: the error names that file, not the area
+/// folder or the copy below it, for a write, a lock, a read, a listing, a removal and a recovery
+/// save.
+#[cfg(unix)]
+#[test]
+fn folder_errors_name_a_file_in_the_way() {
+    let base = std::env::temp_dir().join(format!("vc-recovery-file-in-way-{}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&base);
+    std::fs::create_dir_all(&base).unwrap();
+    let file = base.join("file");
+    std::fs::write(&file, b"").unwrap();
+    let folder = file.join("recovery");
+    let store = FolderStore::new(&folder);
+    let mut s = Session::new();
+    s.prefs.recovery_folder = folder.to_string_lossy().to_string();
+    s.execute("file.new", &json!({"width": 50, "height": 50})).unwrap();
+    s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 5, "height": 5})).unwrap();
+    let errors = [
+        ("write", store.write("1-1/Untitled-1-1.vectorcraft", b"copy").err()),
+        ("lock", store.lock("1-2").err()),
+        ("read", store.read("1-1/Untitled-1-1.vectorcraft").err()),
+        ("list", store.list().err()),
+        ("remove", store.remove("1-1/Untitled-1-1.vectorcraft").err()),
+        ("save", s.execute("file.recovery.save", &json!({})).err().map(|e| e.to_string())),
+    ];
+    let _ = std::fs::remove_dir_all(&base);
+    let named = format!("{} is not a folder: ", file.display());
+    let wrong: Vec<_> = errors.iter().filter(|(_, e)| !e.as_deref().is_some_and(|e| e.contains(&named))).collect();
+    assert!(wrong.is_empty(), "{wrong:?}");
+}
+
+/// A folder of the store's path that is missing and can't be made: a write or a lock names it as
+/// the folder it couldn't make, and a read names it as missing. An area folder that can't be made
+/// in a store folder that is there is named the same way.
+#[cfg(unix)]
+#[test]
+fn folder_errors_name_a_missing_folder_that_cannot_be_made() {
+    use std::os::unix::fs::PermissionsExt as _;
+    let base = std::env::temp_dir().join(format!("vc-recovery-missing-{}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&base);
+    let closed = base.join("closed");
+    std::fs::create_dir_all(&closed).unwrap();
+    std::fs::set_permissions(&closed, std::fs::Permissions::from_mode(0o500)).unwrap();
+    let gone = closed.join("gone");
+    let store = FolderStore::new(gone.join("recovery"));
+    let unmade = format!("couldn't make {}: ", gone.display());
+    let area = FolderStore::new(&closed).write("1-1/Untitled-1-1.vectorcraft", b"copy").err();
+    let errors = [
+        ("write", store.write("1-1/Untitled-1-1.vectorcraft", b"copy").err(), unmade.clone()),
+        ("lock", store.lock("1-2").err(), unmade),
+        ("read", store.read("1-1/Untitled-1-1.vectorcraft").err(), format!("{} is missing", gone.display())),
+        ("area", area, format!("couldn't make {}: ", closed.join("1-1").display())),
+    ];
+    std::fs::set_permissions(&closed, std::fs::Permissions::from_mode(0o700)).unwrap();
+    let _ = std::fs::remove_dir_all(&base);
+    // An account that can write anywhere (root) makes the folders, so the test has nothing to check.
+    if errors[0].1.is_none() {
+        return;
+    }
+    let wrong: Vec<_> = errors.iter().filter(|(_, e, named)| !e.as_deref().is_some_and(|e| e.starts_with(named.as_str()))).collect();
+    assert!(wrong.is_empty(), "{wrong:?}");
+}
+
+/// When every folder on the way is there, an error names the folder it happened on.
+#[cfg(unix)]
+#[test]
+fn folder_errors_name_the_folder_when_every_folder_is_there() {
+    use std::os::unix::fs::PermissionsExt as _;
+    let base = std::env::temp_dir().join(format!("vc-recovery-unreadable-{}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&base);
+    let unreadable = base.join("unreadable");
+    std::fs::create_dir_all(&unreadable).unwrap();
+    std::fs::set_permissions(&unreadable, std::fs::Permissions::from_mode(0o300)).unwrap();
+    let listed = FolderStore::new(&unreadable).list();
+    std::fs::set_permissions(&unreadable, std::fs::Permissions::from_mode(0o700)).unwrap();
+    let _ = std::fs::remove_dir_all(&base);
+    // An account that can read any folder (root) lists it, so the test has nothing to check.
+    let Err(err) = listed else { return };
+    assert!(err.starts_with(&format!("{}: ", unreadable.display())), "{err}");
+}
+
 /// A store in memory that notes each removal of an area (its lock file and folder, then its empty
 /// folder) and whether the area was still held then.
 #[derive(Default)]