Merge pull request #2633 from vector-im/feature/fix_initial_sync_retry

Sync: fix initial sync retry

CHANGES.md

@@ -15,7 +15,7 @@ Bugfix 🐛:
  - Room Topic not displayed correctly after visiting a link (#2551)
  - Hiding membership events works the exact opposite (#2603)
  - Tapping drawer having more than 1 room in notifications gives "malformed link" error (#2605)
-
+ - Initial sync is not retried correctly when there is some network error. (#2632)
 Translations 🗣:
  -
 

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt

@@ -36,6 +36,7 @@ import org.matrix.android.sdk.internal.task.TaskExecutor
 import org.matrix.android.sdk.internal.util.BackgroundDetectionObserver
 import org.matrix.android.sdk.internal.util.MatrixCoroutineDispatchers
 import timber.log.Timber
+import java.net.SocketTimeoutException
 import java.util.concurrent.atomic.AtomicBoolean
 
 /**
@@ -68,14 +69,12 @@ abstract class SyncService : Service() {
 
     override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int {
         Timber.i("## Sync: onStartCommand [$this] $intent with action: ${intent?.action}")
-
-        // We should start we have to ensure we fulfill contract to show notification
-        // for foreground service (as per design for this service)
-        // TODO can we check if it's really in foreground
-        onStart(isInitialSync)
         when (intent?.action) {
             ACTION_STOP -> {
                 Timber.i("## Sync: stop command received")
+                // We should start we have to ensure we fulfill contract to show notification
+                // for foreground service (as per design for this service)
+                onStart(isInitialSync)
                 // If it was periodic we ensure that it will not reschedule itself
                 preventReschedule = true
                 // we don't want to cancel initial syncs, let it finish
@@ -85,11 +84,12 @@ abstract class SyncService : Service() {
             }
             else        -> {
                 val isInit = initialize(intent)
+                onStart(isInitialSync)
                 if (isInit) {
                     periodic = intent?.getBooleanExtra(EXTRA_PERIODIC, false) ?: false
                     val onNetworkBack = intent?.getBooleanExtra(EXTRA_NETWORK_BACK_RESTART, false) ?: false
                     Timber.d("## Sync: command received, periodic: $periodic  networkBack: $onNetworkBack")
-                    if (onNetworkBack && !backgroundDetectionObserver.isInBackground) {
+                    if (!isInitialSync && onNetworkBack && !backgroundDetectionObserver.isInBackground) {
                         // the restart after network occurs while the app is in foreground
                         // so just stop. It will be restarted when entering background
                         preventReschedule = true
@@ -165,10 +165,16 @@ abstract class SyncService : Service() {
                 preventReschedule = true
             }
             if (throwable is Failure.NetworkConnection) {
-                // Network is off, no need to reschedule endless alarms :/
+                // Timeout is not critical, so retry as soon as possible.
+                val retryDelay = if (isInitialSync || throwable.cause is SocketTimeoutException) {
+                    0
+                } else {
+                    syncDelaySeconds
+                }
+                // Network might be off, no need to reschedule endless alarms :/
                 preventReschedule = true
-                // Instead start a work to restart background sync when network is back
+                // Instead start a work to restart background sync when network is on
-                onNetworkError(sessionId ?: "", isInitialSync, syncTimeoutSeconds, syncDelaySeconds)
+                onNetworkError(sessionId ?: "", isInitialSync, syncTimeoutSeconds, retryDelay)
             }
             // JobCancellation could be caught here when onDestroy cancels the coroutine context
             if (isRunning.get()) stopMe()

vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt

@@ -21,7 +21,6 @@ import android.app.PendingIntent
 import android.content.Context
 import android.content.Intent
 import android.os.Build
-import androidx.core.content.ContextCompat.getSystemService
 import androidx.core.content.getSystemService
 import androidx.work.Constraints
 import androidx.work.Data
@@ -49,22 +48,19 @@ class VectorSyncService : SyncService() {
             }
         }
 
-        fun newPeriodicIntent(context: Context, sessionId: String, timeoutSeconds: Int, delayInSeconds: Int): Intent {
+        fun newPeriodicIntent(
+                context: Context,
+                sessionId: String,
+                timeoutSeconds: Int,
+                delayInSeconds: Int,
+                networkBack: Boolean = false
+        ): Intent {
             return Intent(context, VectorSyncService::class.java).also {
                 it.putExtra(EXTRA_SESSION_ID, sessionId)
                 it.putExtra(EXTRA_TIMEOUT_SECONDS, timeoutSeconds)
                 it.putExtra(EXTRA_PERIODIC, true)
                 it.putExtra(EXTRA_DELAY_SECONDS, delayInSeconds)
-            }
+                it.putExtra(EXTRA_NETWORK_BACK_RESTART, networkBack)
-        }
-
-        fun newPeriodicNetworkBackIntent(context: Context, sessionId: String, timeoutSeconds: Int, delayInSeconds: Int): Intent {
-            return Intent(context, VectorSyncService::class.java).also {
-                it.putExtra(EXTRA_SESSION_ID, sessionId)
-                it.putExtra(EXTRA_TIMEOUT_SECONDS, timeoutSeconds)
-                it.putExtra(EXTRA_PERIODIC, true)
-                it.putExtra(EXTRA_DELAY_SECONDS, delayInSeconds)
-                it.putExtra(EXTRA_NETWORK_BACK_RESTART, true)
             }
         }
 
@@ -93,12 +89,12 @@ class VectorSyncService : SyncService() {
     }
 
     override fun onRescheduleAsked(sessionId: String, isInitialSync: Boolean, timeout: Int, delay: Int) {
-        reschedule(sessionId, timeout, delay)
+        rescheduleSyncService(sessionId, timeout, delay)
     }
 
     override fun onNetworkError(sessionId: String, isInitialSync: Boolean, timeout: Int, delay: Int) {
         Timber.d("## Sync: A network error occured during sync")
-        val uploadWorkRequest: WorkRequest =
+        val rescheduleSyncWorkRequest: WorkRequest =
                 OneTimeWorkRequestBuilder<RestartWhenNetworkOn>()
                         .setInputData(Data.Builder()
                                 .putString("sessionId", sessionId)
@@ -115,7 +111,7 @@ class VectorSyncService : SyncService() {
         Timber.d("## Sync: Schedule a work to restart service when network will be on")
         WorkManager
                 .getInstance(applicationContext)
-                .enqueue(uploadWorkRequest)
+                .enqueue(rescheduleSyncWorkRequest)
     }
 
     override fun onDestroy() {
@@ -128,11 +124,25 @@ class VectorSyncService : SyncService() {
         notificationManager.cancel(NotificationUtils.NOTIFICATION_ID_FOREGROUND_SERVICE)
     }
 
-    private fun reschedule(sessionId: String, timeout: Int, delay: Int) {
+    class RestartWhenNetworkOn(appContext: Context, workerParams: WorkerParameters) :
+            Worker(appContext, workerParams) {
+        override fun doWork(): Result {
+            val sessionId = inputData.getString("sessionId") ?: return Result.failure()
+            val timeout = inputData.getInt("timeout", 6)
+            val delay = inputData.getInt("delay", 60)
+            applicationContext.rescheduleSyncService(sessionId, timeout, delay, true)
+            // Indicate whether the work finished successfully with the Result
+            return Result.success()
+        }
+    }
+}
+
+private fun Context.rescheduleSyncService(sessionId: String, timeout: Int, delay: Int, networkBack: Boolean = false) {
+    val periodicIntent = VectorSyncService.newPeriodicIntent(this, sessionId, timeout, delay, networkBack)
     val pendingIntent = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
-            PendingIntent.getForegroundService(this, 0, newPeriodicIntent(this, sessionId, timeout, delay), 0)
+        PendingIntent.getForegroundService(this, 0, periodicIntent, 0)
     } else {
-            PendingIntent.getService(this, 0, newPeriodicIntent(this, sessionId, timeout, delay), 0)
+        PendingIntent.getService(this, 0, periodicIntent, 0)
     }
     val firstMillis = System.currentTimeMillis() + delay * 1000L
     val alarmMgr = getSystemService<AlarmManager>()!!
@@ -141,29 +151,4 @@ class VectorSyncService : SyncService() {
     } else {
         alarmMgr.set(AlarmManager.RTC_WAKEUP, firstMillis, pendingIntent)
     }
-    }
-
-    class RestartWhenNetworkOn(appContext: Context, workerParams: WorkerParameters) :
-            Worker(appContext, workerParams) {
-        override fun doWork(): Result {
-            val sessionId = inputData.getString("sessionId") ?: return Result.failure()
-            val timeout = inputData.getInt("timeout", 6)
-            val delay = inputData.getInt("delay", 60)
-
-            val pendingIntent = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
-                PendingIntent.getForegroundService(applicationContext, 0, newPeriodicNetworkBackIntent(applicationContext, sessionId, timeout, delay), 0)
-            } else {
-                PendingIntent.getService(applicationContext, 0, newPeriodicNetworkBackIntent(applicationContext, sessionId, timeout, delay), 0)
-            }
-            val firstMillis = System.currentTimeMillis() + delay * 1000L
-            val alarmMgr = getSystemService<AlarmManager>(applicationContext, AlarmManager::class.java)!!
-            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
-                alarmMgr.setAndAllowWhileIdle(AlarmManager.RTC_WAKEUP, firstMillis, pendingIntent)
-            } else {
-                alarmMgr.set(AlarmManager.RTC_WAKEUP, firstMillis, pendingIntent)
-            }
-            // Indicate whether the work finished successfully with the Result
-            return Result.success()
-        }
-    }
 }