fix(libsync): treat transient network errors as per-file retries, not whole-run aborts#970
Open
bernardgut wants to merge 1 commit into
Open
Conversation
… whole-run aborts classifyError() maps every QNetworkReply error in (NoError, UnknownProxyError] to FatalError, which propagates up to propagator()->abort() and aborts the entire sync run. On a long sync a single timeout / connection reset / temporary network failure on one file kills the whole pass, and the expensive discovery has to restart from scratch. Map the recoverable connectivity errors (timeout, connection refused, host not found, temporary network failure, network session failed, proxy refused/closed/timeout) to a per-file NormalError that sets anotherSyncNeeded, so only that file is retried on the next pass and the run continues. Genuinely fatal cases (TLS handshake, proxy auth, redirect loops, ...) keep FatalError. Tests, both bug-bites-verified (each fails against the pre-fix classifyError): - test/testclassifyerror.cpp unit-pins the classifyError contract. - test/testselfheal.cpp is a FakeFolder integration test proving the self-heal property end-to-end: a transient network error injected on one file must not abort the whole run; the healthy files queued after it still upload. Authored-By: Bernard Gütermann <bernard.gutermann@sekops.ch>
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
1 participant
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.
Splitting #960 in two as requested. This is the first half.
Problem
classifyError() treats every QNetworkReply error up to UnknownProxyError as FatalError, which aborts the whole sync run via propagator()->abort(). So a single timeout or connection reset on one file kills the entire pass and discovery starts over. On long syncs this happens all the time, and the sync never converges.
Fix
Recoverable connectivity errors (timeout, connection refused, host not found, temporary network failure, network session failed, proxy refused/closed/timeout) now return a per-file NormalError and set anotherSyncNeeded. Only that file is retried and the run continues. Genuinely fatal cases (TLS handshake, proxy auth, redirect loops) keep FatalError.
Tests
Both fail against the pre-fix classifyError and pass with it:
Full ctest on the CI image: 32/33. The one failure is testfolderwatcher, which fails on my build host for environment reasons (inotify instance limit exhausted, unrelated to this change).
Together with #955 this makes long syncs converge instead of wedging. None of these fixes are in a release yet. If you would consider a 3.0.4, I am happy to prepare the stable-3.0 backport PRs.