Skip to content

fix: propagate silently-swallowed errors in the sync engine - #3

Merged
cedev-1 merged 1 commit into
mainfrom
devin/1784657536-error-handling
Jul 21, 2026
Merged

fix: propagate silently-swallowed errors in the sync engine#3
cedev-1 merged 1 commit into
mainfrom
devin/1784657536-error-handling

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Summary

Several Results in the sync engine were dropped with let _ = ... / .ok(), so DB and filesystem failures passed unnoticed — some of them affecting exactly the invariants the test suite guards (no silent data loss, conflicts surfaced). This propagates or at least logs them, matching patterns already used elsewhere in the same modules.

drive/sync.rsfull_sync (highest impact): inventory writes were swallowed inside the 3-way reconcile loop. full_sync already uses ? for every other inventory/API call, so these now do too:

- let _ = mount.inventory.batch_delete_by_path(vec![&path_str]);   // 3 delete branches
- let _ = mount.inventory.upsert(&entry);
- let _ = mount.inventory.mark_as_conflicted(&path_str, Pending);
+ mount.inventory.<op>(...).context("…")?;

The mark_as_conflicted case is the important one: if that write failed silently, the file was not frozen and could later be overwritten — the exact silent-data-loss scenario both_sides_modified_* / conflicted_file_stays_frozen assert against. tasks/upload.rs already surfaces this same call's error, so sync.rs was inconsistent.

tasks/queue.rsre_enqueue_offline_tasks: update_task and dispatch_task errors were dropped while the task was still counted as re-enqueued. Now mirrors the sibling resume loop in the same file (warn + continue, only count on success), so a task whose offline_waiting flag couldn't be cleared isn't falsely reported as re-enqueued.

tasks/download.rs: when mirroring a remote deletion, a failed remove_file was .ok()'d and the task returned Ok(()) — the local file silently survived. Now logs a warning.

uploader/mod.rs: failure to delete an expired upload session was dropped; now logged (control flow unchanged — still best-effort).

Verification

cargo clippy -p cloudreve-sync --tests (no new warnings) and cargo test -p cloudreve-sync — all 55 tests pass.

Link to Devin session: https://app.devin.ai/sessions/baccafbde57044f9b0a6f246ca385ff5
Requested by: @cedev-1

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@cedev-1 cedev-1 self-assigned this Jul 21, 2026
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@cedev-1 cedev-1 added the enhancement New feature or request label Jul 21, 2026
@cedev-1
cedev-1 merged commit 10be0ce into main Jul 21, 2026
1 check passed
@cedev-1
cedev-1 deleted the devin/1784657536-error-handling branch July 26, 2026 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant