fix: propagate silently-swallowed errors in the sync engine - #3
Merged
Conversation
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Contributor
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
cedev-1
approved these changes
Jul 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Several
Results in the sync engine were dropped withlet _ = .../.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.rs—full_sync(highest impact): inventory writes were swallowed inside the 3-way reconcile loop.full_syncalready uses?for every other inventory/API call, so these now do too:The
mark_as_conflictedcase is the important one: if that write failed silently, the file was not frozen and could later be overwritten — the exact silent-data-loss scenarioboth_sides_modified_*/conflicted_file_stays_frozenassert against.tasks/upload.rsalready surfaces this same call's error, sosync.rswas inconsistent.tasks/queue.rs—re_enqueue_offline_tasks:update_taskanddispatch_taskerrors 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 whoseoffline_waitingflag couldn't be cleared isn't falsely reported as re-enqueued.tasks/download.rs: when mirroring a remote deletion, a failedremove_filewas.ok()'d and the task returnedOk(())— 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) andcargo test -p cloudreve-sync— all 55 tests pass.Link to Devin session: https://app.devin.ai/sessions/baccafbde57044f9b0a6f246ca385ff5
Requested by: @cedev-1