[Backup] Bug fixes around backup cancellation and background #1173
Conversation
This file contains 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
We save the current upload id in a global variable to be able to cancel an upload. On iOS, we saved it in the "begin" callback of RNFS.uploadFiles function. We can save it sooner, before the start of the upload, so before the "begin" callback, so let's do it to avoid having an not up to date current upload id.
When the user was canceling a backup, we rejected with an error object where detail property was "Upload canceled" and then we checked later if the detail property was "User cancelled upload" to do stuff related to cancellation. It was obviously not working. Let's fix this by creating a CancellationError extending from Error to never reproduce this kind of issue. CancellationError is also now a standalone error and not a subset of upload errors.
No upstream PR has been made because package is inactive for 2 years.
Upload cancellation was not supported correctly on iOS. When an upload was canceled, it was behaving like a NetworkError. Thanks to the previous commit where we introduced a patch-package to react-native-fs, we can handle the cancellation correctly.
…ckup" button When the user try to stop the backup by clicking on the "Stop backup" button, it was possible that a last media was uploaded. So the user clicked "Stop button" with "10/100 medias uploaded" displayed, and it stopped on "11/100 medias uploaded". When we click on the "Stop backup" button, we : - set a "shouldStop" boolean to true - cancel the current upload if it exists and stop backup directly The "shouldStop" boolean was only checked at the beginning of the upload loop. The upload loop consists in : 1. "shouldStop" check 2. metadata + dedup stuff <-- if we stop upload during this phase, we still do the upload and we stop upload at the next iteration 3. upload stuff In this commit, we add a "shouldStop" check between 2. and 3.
Background backup is mostly unsupported. We were checking at the beginning of the upload loop if we were in the background to stop the backup if needed. This was highly inefficient because when going to background the app could be freezed by the operating system before reaching the next loop cycle. Now we listen to AppState changes to stop the backup. "stopBackup" can now be called with two reason leading to two error messages sent to the user : - stopped by the user - stop because going to background
zatteo
force-pushed
the
fix/background-management-backup
branch
from
27fc6b3 to
0add571
Compare
Ldoppea
approved these changes
Feb 20, 2024
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.
Checklist
Before merging this PR, the following things must have been done if relevant: