Add max retry checks for FetchNotificationsWorker#6219
Add max retry checks for FetchNotificationsWorker#6219jmartinesp wants to merge 3 commits intodevelopfrom
FetchNotificationsWorker#6219Conversation
|
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #6219 +/- ##
===========================================
- Coverage 81.42% 81.40% -0.02%
===========================================
Files 2570 2570
Lines 69778 69826 +48
Branches 8950 8959 +9
===========================================
+ Hits 56817 56844 +27
- Misses 9640 9656 +16
- Partials 3321 3326 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| if (workerParams.runAttemptCount < MAX_RETRY_ATTEMPTS) { | ||
| Timber.tag(TAG).w("No network, retrying later") | ||
| return@withContext Result.retry() | ||
| } else { | ||
| Timber.tag(TAG).w("No network available and reached max retry attempts (${workerParams.runAttemptCount}/$MAX_RETRY_ATTEMPTS)") | ||
| return@withContext Result.failure() | ||
| } |
There was a problem hiding this comment.
Maybe if there is no network we should not count as a retry?
There was a problem hiding this comment.
I don't think we can make an exception just for this case, to be honest 🫤 .
There was a problem hiding this comment.
Maybe we could let the worker wait for "network" instead of retrying?
There was a problem hiding this comment.
I don't think the workers can be alive in background for long. We could try, but I'm sure that would have some kind of penalty with the future scheduling.
There was a problem hiding this comment.
Maybe we can make the retry backoff exponential instead of linear: my thoughts on this was to have it linear so it's retried after a short while, but I was thinking about a temporary issue in the HS, not the connection failing.
…initely Also, re-schedule those requests that failed because of a network connection hiccup
|
2 participants
Content
Add a maximum of 3 retries for fetching notifications with
WorkManager.Motivation and context
Not having this can potentially result in a work request being re-scheduled indefinitely.
Part of #6209
Tests
I don't think this can be easily tested.
Tested devices
Checklist