Added retries, back-off and initial delay for sync

[ArnyminerZ]

Closes #72

[rfc2822]

Thanks. I'd like to wait a bit, maybe we can find out the real cause for the problem (it's also important for when we switch DAVx5 to mostly WorkManager).

[yan12125]

This branch does not retry failed requests for me until I changed something. Looks like it doesn't work because exceptions are not propagated? I simulate network failures by adding the domain for calendars to my AdAway block list.

With following changes, failed requests are retried 5 times and there are 5 failure notifications. Probably the notification can be moved so that it's trigger only once after 5 failures.

diff --git a/app/src/main/java/at/bitfire/icsdroid/ProcessEventsTask.kt b/app/src/main/java/at/bitfire/icsdroid/ProcessEventsTask.kt
index b46f50e..58fe5af 100644
--- a/app/src/main/java/at/bitfire/icsdroid/ProcessEventsTask.kt
+++ b/app/src/main/java/at/bitfire/icsdroid/ProcessEventsTask.kt
@@ -62,6 +62,7 @@ class ProcessEventsTask(
         } catch(e: Exception) {
             Log.e(Constants.TAG, "Couldn't sync calendar", e)
             calendar.updateStatusError(e.localizedMessage ?: e.toString())
+            throw e
         }
         Log.i(Constants.TAG, "iCalendar file completely processed")
     }
@@ -185,6 +186,8 @@ class ProcessEventsTask(
             notificationManager.notify(calendar.id.toString(), 0, notification.build())
 
             calendar.updateStatusError(message)
+
+            throw ex
         }
     }
 
diff --git a/app/src/main/java/at/bitfire/icsdroid/SyncWorker.kt b/app/src/main/java/at/bitfire/icsdroid/SyncWorker.kt
index 5e5cd31..9a6b69c 100644
--- a/app/src/main/java/at/bitfire/icsdroid/SyncWorker.kt
+++ b/app/src/main/java/at/bitfire/icsdroid/SyncWorker.kt
@@ -97,14 +97,16 @@ class SyncWorker(
                 return withContext(Dispatchers.Default) {
                     performSync(AppAccount.get(applicationContext), providerClient, forceResync)
                 }
+            } catch (e: Exception) {
+                return if (runAttemptCount >= MAX_ATTEMPTS)
+                    Result.failure()
+                else
+                    Result.retry()
             } finally {
                 providerClient.closeCompat()
             }
         }
-        return if (runAttemptCount >= MAX_ATTEMPTS)
-            Result.failure()
-        else
-            Result.retry()
+        return Result.failure()
     }
 
     private suspend fun performSync(account: Account, provider: ContentProviderClient, forceResync: Boolean): Result {

[ArnyminerZ]

@yan12125 yup, that makes sense, however, I will rewrite a little bit your code. Now that I have synchronized with the main branch,

[ArnyminerZ]

@rfc2822 can you take a look?

[yan12125]

Thanks for the update. I still need this change to get failed requests retried:

diff --git a/app/src/main/java/at/bitfire/icsdroid/ProcessEventsTask.kt b/app/src/main/java/at/bitfire/icsdroid/ProcessEventsTask.kt
index 79f8c0e..513c1b5 100644
--- a/app/src/main/java/at/bitfire/icsdroid/ProcessEventsTask.kt
+++ b/app/src/main/java/at/bitfire/icsdroid/ProcessEventsTask.kt
@@ -199,6 +199,8 @@ class ProcessEventsTask(
             notificationManager.notify(subscription.id.toString(), 0, notification.build())
 
             subscriptionsDao.updateStatusError(subscription.id, message)
+
+            throw ex
         }
     }
 

[yan12125]

Hi, could you merge with the 'dev' branch again?

[yan12125]

There are merge conflicts again :(

[yan12125]

Thanks for the update. Seems the latest change misses a variable declaration:

@@ -118,6 +122,8 @@ class SyncWorker(
         val onlyMigrate = inputData.getBoolean(ONLY_MIGRATE, false)
         Log.i(TAG, "Synchronizing (forceReSync=$forceReSync,onlyMigrate=$onlyMigrate)")
 
+        var result: Result = Result.success()
+
         provider =
             try {
                 LocalCalendar.getCalendarProvider(applicationContext)

[yan12125]

I started a branch at https://github.com/yan12125/icsx5/commits/72-couldnt-connect-to-server and added some fixes/improvements on top of this PR:

• Notify failures only after 5 retries: currently, a notification is sent after each failure, so there are 5 notifications if all retries failed, which is kind of annoying.
• Add back fixes introduced in https://github.com/bitfireAT/icsx5/pull/133: a change related to calendar IDs is lost on this PR, and thus events may be saved to the wrong calendar or cannot be saved.
• Really propagate errors: the change I proposed above (Added retries, back-off and initial delay for sync #83 (comment))

With these changes, syncing is reliable even with an unstable server. I have been using my branch for 2 weeks and got no issue. Could you consider adding my changes to this PR?

[rfc2822]

Thanks; seems to be fixed by Samsung in the meanwhile, so I'll close this