fix(fuse,cli): keep a pass going past a failed listing, report skipped restores

A listing that failed anywhere in the remote walk ended the whole pass,
so a first sync of a large folder on a flaky link walked the tree again
and again and never downloaded anything (B271, issue #46). A failed
listing below the root now records that folder as a gap: nothing under
it is deleted, the rest of the pass applies, and the report names it.

A restore that skipped every folder was logged as a successful activity
(B269). The daemon now gets the restored and skipped folders back, logs
any skip as a failed activity with the reasons, and sends the Rewatch
for folders already restored when the database insert fails partway.
`pdfs devices join` points to `pdfs activity` for the outcome.
This commit is contained in:
nvrl committed 2026-10-31 11:11:01 +01:00
1 parent 9460c93b50
commit 6f2b4bb80a
6 files changed
+160 -29

No files matched your search

+5 -1
View File
@@ -1458,7 +1458,11 @@ fn cmd_sync_restore(yes: bool, device: Option<String>) -> Result<()> {
items: chosen,
},
None => CtlRequest::RestoreSyncFolders { items: chosen },
})?)
})?)?;
// The daemon acks before it works, so the outcome, and any folder it had
// to skip, only shows up later.
println!("Run `pdfs activity` to see which folders were restored.");
Ok(())
}
/// Expand a leading `~` in a hand-typed path. The daemon requires an absolute
+10 -2
View File
@@ -1526,9 +1526,17 @@ fn handle_control_conn(core: &Core, username: &str, mountpoint: &Path, stream: U
let core = core.clone();
std::thread::spawn(move || {
match core.restore_sync_folders(&items, device.as_deref()) {
Ok(message) => {
Ok(outcome) => {
core.touch_profile();
core.log_activity(ActivityKind::Download, &message, "", true);
if !outcome.complete() {
warn!(skipped = ?outcome.skipped, "restore skipped folders");
}
core.log_activity(
ActivityKind::Download,
outcome.summary(),
outcome.skipped.join("; "),
outcome.complete(),
);
}
Err(e) => {
warn!(error = %e, "restore failed");
+42 -10
View File
@@ -388,7 +388,7 @@ impl Core {
&self,
items: &[pdfs_core::control::RestoreItem],
device: Option<&str>,
) -> CoreResult<String> {
) -> CoreResult<RestoreOutcome> {
if items.is_empty() {
return Err(CoreError::invalid("nothing to restore"));
}
@@ -434,10 +434,19 @@ impl Core {
continue;
}
};
let id = self
.db
.sync_folder_add(&local.to_string_lossy(), &item.remote_uid, &device.share_id)
.map_err(|e| CoreError::internal(format!("db: {e:?}")))?;
let id = match self.db.sync_folder_add(
&local.to_string_lossy(),
&item.remote_uid,
&device.share_id,
) {
Ok(id) => id,
Err(e) => {
// The folders restored before this one still need their
// watches.
let _ = self.sync_tx.send(sync::SyncMsg::Rewatch);
return Err(CoreError::internal(format!("db: {e:?}")));
}
};
// `ondemand` is applied through the normal request path, which
// mounts the FUSE session and handles the folder being busy — the
// restore does not get its own copy of that logic.
@@ -454,12 +463,35 @@ impl Core {
for reason in &skipped {
info!(reason, "restore skipped a folder");
}
let mut message = format!("restored {restored} folder(s)");
if !skipped.is_empty() {
message.push_str(&format!("; skipped: {}", skipped.join(", ")));
}
info!(restored, skipped = skipped.len(), "restore complete");
Ok(message)
Ok(RestoreOutcome { restored, skipped })
}
}
/// What a restore did: how many folders it attached, and why it left out the
/// others.
pub(crate) struct RestoreOutcome {
pub(crate) restored: usize,
pub(crate) skipped: Vec<String>,
}
impl RestoreOutcome {
/// The activity entry's headline.
pub(crate) fn summary(&self) -> String {
match (self.restored, self.skipped.len()) {
(n, 0) => format!("restored {n} folder(s)"),
(0, k) => format!("restored no folders; skipped {k}"),
(n, k) => format!("restored {n} folder(s); skipped {k}"),
}
}
/// Whether the restore did all it was asked. A skipped folder is a
/// failure the user has to see: the reply went out as "restoring folders"
/// before any work, and a restore that skipped every folder used to be
/// logged as a success, so nothing anywhere said that nothing would
/// download (`docs/BUGS.md` B269).
pub(crate) fn complete(&self) -> bool {
self.skipped.is_empty()
}
}
+45 -5
View File
@@ -1135,14 +1135,22 @@ impl Core {
let uids = self
.rt
.block_on(self.drive.enumerate_folder_children_node_uids(folder))
.map_err(|e| format!("enumerate {folder}: {e}"))?;
.map_err(|e| format!("enumerate {folder}: {e}"));
let uids = match unlisted_gap(walk, prefix, uids)? {
Some(uids) => uids,
None => return Ok(()),
};
if uids.is_empty() {
return Ok(());
}
let nodes = self
.rt
.block_on(self.drive.enumerate_nodes_light(&uids))
.map_err(|e| format!("resolve nodes: {e}"))?;
.map_err(|e| format!("resolve nodes: {e}"));
let nodes = match unlisted_gap(walk, prefix, nodes)? {
Some(nodes) => nodes,
None => return Ok(()),
};
// The SDK leaves out a node it cannot decrypt, and says so only in its
// log. Its name went with it, so all this can record is the folder.
if nodes.len() < uids.len() {
@@ -1186,10 +1194,14 @@ impl Core {
let sized: HashMap<String, (i64, Option<String>)> = if stale.is_empty() {
HashMap::new()
} else {
self.rt
let full = self
.rt
.block_on(self.drive.enumerate_nodes(&stale))
.map_err(|e| format!("resolve nodes: {e}"))?
.iter()
.map_err(|e| format!("resolve nodes: {e}"));
let Some(full) = unlisted_gap(walk, prefix, full)? else {
return Ok(());
};
full.iter()
.map(|n| {
(
n.uid.to_string(),
@@ -2007,6 +2019,34 @@ struct RemoteWalk<'a> {
gaps: &'a mut RemoteGaps,
}
/// A listing call's result in [`Core::walk_remote`]. A failure below the root
/// records `prefix` as unlisted and returns `None`, so the walk goes on
/// without that folder. A failure at the root still fails the pass, since
/// nothing would be left to compare.
///
/// One listing error, after the SDK's own retries, used to fail the whole
/// walk. Nothing is applied before the walk ends, so on a link that drops a
/// request now and then a large first sync started over from nothing on
/// every pass and never got past it (`docs/BUGS.md` B271).
fn unlisted_gap<T>(
walk: &mut RemoteWalk<'_>,
prefix: &str,
result: Result<T, String>,
) -> Result<Option<T>, PassAbort> {
match result {
Ok(value) => Ok(Some(value)),
Err(error) if prefix.is_empty() => Err(error.into()),
Err(error) => {
warn!(
folder = prefix,
error, "sync: a folder on Drive could not be listed; leaving it out of this pass"
);
walk.gaps.unlisted(prefix);
Ok(None)
}
}
}
/// Whether the file at `path` is no longer what the scan saw as `planned`.
/// A file that is gone has nothing left to lose, so it has not changed.
fn changed_since_scan(path: &Path, planned: &LocalItem) -> bool {
+39 -2
View File
@@ -236,6 +236,9 @@ pub(super) struct RemoteGaps {
short: HashSet<String>,
/// Paths seen, but whose entry could not be read.
unread: HashSet<String>,
/// Folders whose listing failed outright, after the SDK's retries. Nothing
/// under one is known, so all of it is left out of the pass (B271).
unlisted: HashSet<String>,
}
impl RemoteGaps {
@@ -249,8 +252,13 @@ impl RemoteGaps {
self.unread.insert(rel);
}
/// The listing of `folder` failed, so nothing under it was walked.
pub(super) fn unlisted(&mut self, folder: &str) {
self.unlisted.insert(folder.to_string());
}
pub(super) fn is_empty(&self) -> bool {
self.short.is_empty() && self.unread.is_empty()
self.short.is_empty() && self.unread.is_empty() && self.unlisted.is_empty()
}
/// Whether `rel` may be missing from `remote` only because the walk could
@@ -272,7 +280,7 @@ impl RemoteGaps {
let mut at = parent_rel(rel);
loop {
if remote_dirs.contains_key(at) {
return self.short.contains(at);
return self.short.contains(at) || self.unlisted.contains(at);
}
if at.is_empty() {
return false;
@@ -295,6 +303,9 @@ impl RemoteGaps {
.iter()
.map(|rel| format!("{rel} (cannot be read on Proton Drive)")),
)
.chain(self.unlisted.iter().map(|rel| {
format!("{rel} (could not be listed on Proton Drive; tried again next pass)")
}))
.collect();
lines.sort_unstable();
lines
@@ -844,6 +855,32 @@ mod file_plan_tests {
assert!(gaps.hides("top.txt", &remote, &dirs));
}
/// A folder whose listing failed was walked no further: nothing under it
/// reads as deleted on Drive, and a sibling that listed still does
/// (`docs/BUGS.md` B271).
#[test]
fn a_folder_that_failed_to_list_hides_everything_under_it() {
let remote = HashMap::from([
("docs".to_string(), folder()),
("pics".to_string(), folder()),
]);
let dirs = HashMap::from([
(String::new(), uid("root")),
("docs".to_string(), uid("docs")),
("pics".to_string(), uid("pics")),
]);
let mut gaps = RemoteGaps::default();
gaps.unlisted("docs");
assert!(gaps.hides("docs/a.txt", &remote, &dirs));
assert!(gaps.hides("docs/sub/deep.txt", &remote, &dirs));
assert!(!gaps.hides("docs", &remote, &dirs));
assert!(!gaps.hides("pics/gone.jpg", &remote, &dirs));
assert_eq!(
gaps.report(),
["docs (could not be listed on Proton Drive; tried again next pass)"]
);
}
#[test]
fn a_file_whose_size_could_not_be_read_is_hidden() {
let remote: HashMap<String, RemoteItem> = HashMap::new();