Repository navigation
fix(sync): let one bad roster file, object or snapshot stop only itself - #1830
Merged
Merged
Conversation
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
8 of 16 tasks
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
A roster file, log object, key or blob over its size limit, or refused for itself, ended the whole sync. It now stops only that item, and it is read again on a later sync. That means an oversized item and a 4xx answer other than 401, 407, 408 and 429, including 403. A linked profile whose snapshot can't be read yet waits for a later sync while the others go on.
Everything else still fails the sync: the network, 5xx, 3xx, 401, 407, 408, 429 and a refused login. The saved cursor and tag don't move, so the next sync reads the item. This is the same list as Flow Desktop's #459. To tell an oversized answer from other bad answers, as Desktop does,
StoreException.TooLargeis new, andMalformedno longer stops only one item.Roster files on a file store are now read one at a time, lowest
seqfirst. Only entries valid against their chain are held, using the check the fork rule already applies (ValidEntries, moved out ofvalidTree). One pull reads at most 16,384 files and 64 MiB, the limits Flow Desktop uses.Related issue
None.
Change type
Validation
./gradlew --offline ... :app:testGithubDebugUnitTest --tests 'io.github.aedev.flow.sync2.*': 313 tests, 0 failed../gradlew --offline ... ktlintCheck: passed.python3 .github/scripts/nightly_notes.py --check: passed.OneBadItemTest: 12 passed. An earlier commit's mutation run covered the chain check, read limit, retry, snapshot wait, rate-limit rule and item rule. It wasn't repeated for those.New in the last commit, and failing before it: 302, 401, 407, 408, 429, 500 and 503 on an object, a chain box, a roster file and a log file fail the sync, and the next read gets the item; 400, 403, 410, 451 and an oversized answer stop only the item.
Each of these made tests fail when tried: 401, 407, 408 or 429 left out of the list, 403 added, 5xx or 3xx counted as about the item, every status or no status counted as about the item, and an oversized answer failing the sync.
./gradlew :app:assembleGithubDebug: not run locally, CI builds it../gradlew :app:testGithubDebugUnitTest: full suite not run locally, CI runs it.I manually tested the affected behavior on an Android device or emulator: not done.
Test device and Android version: none.
Risk and compatibility
Background sync only. A server error shows as a refused request, which WorkManager doesn't retry early, so the next sync is the next scheduled one or Sync now. A chain box that's oversized or refused for itself leaves objects under that older epoch unreadable, so those logs stop at the first such object. The 64 MiB and 16,384 file limits are chosen far above a real roster, not measured. A folder that fills that read limit with junk ahead of real entries leaves this device on an older roster until the junk goes. A store could already hide entries.