From 7768a095739f1dce659a4216b43e4827772eb010 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Wed, 7 Dec 2022 18:48:06 +0600 Subject: [PATCH 1/4] 1981-Dont_show_notification_on_sync_failure_on_no_network issue: https://gitlab.e.foundation/e/backlog/-/issues/1981 Sometimes sync is called when the network is not available. Or sync is called on the time network is just shown as available but not ready, in this case, ConnectException is thrown. For this cases, we don't want to show failure notifications to users. --- .../davdroid/syncadapter/SyncManager.kt | 18 +++++++++--- .../at/bitfire/davdroid/ui/NetworkUtils.kt | 29 +++++++++++++++++++ 2 files changed, 43 insertions(+), 4 deletions(-) create mode 100644 app/src/main/java/at/bitfire/davdroid/ui/NetworkUtils.kt diff --git a/app/src/main/java/at/bitfire/davdroid/syncadapter/SyncManager.kt b/app/src/main/java/at/bitfire/davdroid/syncadapter/SyncManager.kt index 34b4f4710..f00e87845 100644 --- a/app/src/main/java/at/bitfire/davdroid/syncadapter/SyncManager.kt +++ b/app/src/main/java/at/bitfire/davdroid/syncadapter/SyncManager.kt @@ -31,6 +31,7 @@ import at.bitfire.davdroid.log.Logger import at.bitfire.davdroid.resource.* import at.bitfire.davdroid.settings.AccountSettings import at.bitfire.davdroid.ui.DebugInfoActivity +import at.bitfire.davdroid.ui.NetworkUtils import at.bitfire.davdroid.ui.NotificationUtils import at.bitfire.davdroid.ui.account.SettingsActivity import at.bitfire.ical4android.CalendarStorageException @@ -51,7 +52,9 @@ import org.dmfs.tasks.contract.TaskContract import java.io.IOException import java.io.InterruptedIOException import java.lang.ref.WeakReference +import java.net.ConnectException import java.net.HttpURLConnection +import java.net.UnknownHostException import java.security.cert.CertificateException import java.util.* import java.util.concurrent.LinkedBlockingQueue @@ -737,12 +740,19 @@ abstract class SyncManager, out CollectionType: L } } + // sometimes sync is kicked in when no network is not available. + // Or sync is scheduled & kicked just the time network becomes available but not ready; in this case it throws ConnectException. + // In these cases, we don't want to show notification to users. + if (!NetworkUtils.isConnectedToNetwork(context) || e is ConnectException) { + return + } + val contentIntent: Intent var viewItemAction: NotificationCompat.Action? = null - if ((account.type == context.getString(R.string.account_type) || - account.type == context.getString(R.string.eelo_account_type) || - account.type == context.getString(R.string.google_account_type)) && - (e is UnauthorizedException || e is NotFoundException)) { + + if ((account.type == context.getString(R.string.account_type) || account.type == context.getString(R.string.eelo_account_type) || account.type == context.getString(R.string.google_account_type)) + && (e is UnauthorizedException || e is NotFoundException)) { + contentIntent = Intent(context, SettingsActivity::class.java) contentIntent.putExtra(SettingsActivity.EXTRA_ACCOUNT, if (authority == ContactsContract.AUTHORITY) diff --git a/app/src/main/java/at/bitfire/davdroid/ui/NetworkUtils.kt b/app/src/main/java/at/bitfire/davdroid/ui/NetworkUtils.kt new file mode 100644 index 000000000..f36e74e4b --- /dev/null +++ b/app/src/main/java/at/bitfire/davdroid/ui/NetworkUtils.kt @@ -0,0 +1,29 @@ +/* + * Copyright MURENA SAS 2022 + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package at.bitfire.davdroid.ui + +import android.content.Context +import android.net.ConnectivityManager + +object NetworkUtils { + + fun isConnectedToNetwork(context: Context): Boolean { + val cm = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager + val nw = cm.activeNetwork ?: return false + return cm.getNetworkCapabilities(nw) != null + } +} \ No newline at end of file -- GitLab From 9fcbe1f261731846340b2ee210051a8078be6e77 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Mon, 12 Dec 2022 16:50:28 +0600 Subject: [PATCH 2/4] Group up exception notifications for accounts issue: https://gitlab.e.foundation/e/backlog/-/issues/1981 On sync failure, app create new notification based on tag & fix notificationId (SyncManager.kt::794L). The tag is created in combination of (authorityType+accountId). So every authority failure has separate notifications. We want to show single notification for single account sync failure. So We have updated the tag value to accountId. --- .../main/java/at/bitfire/davdroid/resource/LocalAddressBook.kt | 2 +- app/src/main/java/at/bitfire/davdroid/resource/LocalCalendar.kt | 2 +- .../java/at/bitfire/davdroid/resource/LocalJtxCollection.kt | 2 +- app/src/main/java/at/bitfire/davdroid/resource/LocalTaskList.kt | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/at/bitfire/davdroid/resource/LocalAddressBook.kt b/app/src/main/java/at/bitfire/davdroid/resource/LocalAddressBook.kt index 142ed8505..526ad2c4d 100644 --- a/app/src/main/java/at/bitfire/davdroid/resource/LocalAddressBook.kt +++ b/app/src/main/java/at/bitfire/davdroid/resource/LocalAddressBook.kt @@ -125,7 +125,7 @@ open class LocalAddressBook( } override val tag: String - get() = "contacts-${account.name}" + get() = account.name override val title = account.name!! diff --git a/app/src/main/java/at/bitfire/davdroid/resource/LocalCalendar.kt b/app/src/main/java/at/bitfire/davdroid/resource/LocalCalendar.kt index bb6e29b53..bf22df955 100644 --- a/app/src/main/java/at/bitfire/davdroid/resource/LocalCalendar.kt +++ b/app/src/main/java/at/bitfire/davdroid/resource/LocalCalendar.kt @@ -86,7 +86,7 @@ class LocalCalendar private constructor( } override val tag: String - get() = "events-${account.name}-$id" + get() = account.name override val title: String get() = displayName ?: id.toString() diff --git a/app/src/main/java/at/bitfire/davdroid/resource/LocalJtxCollection.kt b/app/src/main/java/at/bitfire/davdroid/resource/LocalJtxCollection.kt index 53760c46f..3a35f14b9 100644 --- a/app/src/main/java/at/bitfire/davdroid/resource/LocalJtxCollection.kt +++ b/app/src/main/java/at/bitfire/davdroid/resource/LocalJtxCollection.kt @@ -43,7 +43,7 @@ class LocalJtxCollection(account: Account, client: ContentProviderClient, id: Lo } override val tag: String - get() = "jtx-${account.name}-$id" + get() = account.name override val title: String get() = displayname ?: id.toString() override var lastSyncState: SyncState? diff --git a/app/src/main/java/at/bitfire/davdroid/resource/LocalTaskList.kt b/app/src/main/java/at/bitfire/davdroid/resource/LocalTaskList.kt index 20ea115a1..911ecbf64 100644 --- a/app/src/main/java/at/bitfire/davdroid/resource/LocalTaskList.kt +++ b/app/src/main/java/at/bitfire/davdroid/resource/LocalTaskList.kt @@ -65,7 +65,7 @@ class LocalTaskList private constructor( } override val tag: String - get() = "tasks-${account.name}-$id" + get() = account.name override val title: String get() = name ?: id.toString() -- GitLab From f0de8b612adb07438992b8f52c1719624a4226f1 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Thu, 15 Dec 2022 13:28:35 +0600 Subject: [PATCH 3/4] Enable retry performSync on unhandled operations We want enable option to retry right failed (unhandled) sync operation. The sync wait-time will be calculated using fibonacci series. (for ex: by default wait 5,8,13,21 seconds to retry). If after all retry attemps, operation still failed, show user exception. This is for better UX. --- .../EeloCalendarsSyncAdapterService.kt | 6 +-- .../EeloContactsSyncAdapterService.kt | 2 +- .../EeloTasksSyncAdapterService.kt | 6 +-- .../GoogleCalendarsSyncAdapterService.kt | 6 +-- .../GoogleContactsSyncAdapterService.kt | 2 +- .../GoogleTasksSyncAdapterService.kt | 6 +-- .../davdroid/syncadapter/SyncManager.kt | 42 +++++++++++++++++-- 7 files changed, 52 insertions(+), 18 deletions(-) diff --git a/app/src/main/java/at/bitfire/davdroid/syncadapter/EeloCalendarsSyncAdapterService.kt b/app/src/main/java/at/bitfire/davdroid/syncadapter/EeloCalendarsSyncAdapterService.kt index 26e1a9f04..60ba1088c 100644 --- a/app/src/main/java/at/bitfire/davdroid/syncadapter/EeloCalendarsSyncAdapterService.kt +++ b/app/src/main/java/at/bitfire/davdroid/syncadapter/EeloCalendarsSyncAdapterService.kt @@ -121,16 +121,16 @@ class EeloCalendarsSyncAdapterService : SyncAdapterService() { ) object : AsyncTask() { override fun doInBackground(vararg params: Void): Void? { - it.performSync() + it.performSyncWithRetry() return null } }.execute() } } else { - it.performSync() + it.performSyncWithRetry() } } else { - it.performSync() + it.performSyncWithRetry() } } } diff --git a/app/src/main/java/at/bitfire/davdroid/syncadapter/EeloContactsSyncAdapterService.kt b/app/src/main/java/at/bitfire/davdroid/syncadapter/EeloContactsSyncAdapterService.kt index b5c0419d8..52ca8b17f 100644 --- a/app/src/main/java/at/bitfire/davdroid/syncadapter/EeloContactsSyncAdapterService.kt +++ b/app/src/main/java/at/bitfire/davdroid/syncadapter/EeloContactsSyncAdapterService.kt @@ -83,7 +83,7 @@ class EeloContactsSyncAdapterService: SyncAdapterService() { Logger.log.info("Taking settings from: ${addressBook.mainAccount}") ContactsSyncManager(context, account, accountSettings, httpClient.value, extras, authority, syncResult, provider, addressBook).let { - it.performSync() + it.performSyncWithRetry() } } catch(e: Exception) { Logger.log.log(Level.SEVERE, "Couldn't sync contacts", e) diff --git a/app/src/main/java/at/bitfire/davdroid/syncadapter/EeloTasksSyncAdapterService.kt b/app/src/main/java/at/bitfire/davdroid/syncadapter/EeloTasksSyncAdapterService.kt index 2dbbd4c80..7afed9507 100644 --- a/app/src/main/java/at/bitfire/davdroid/syncadapter/EeloTasksSyncAdapterService.kt +++ b/app/src/main/java/at/bitfire/davdroid/syncadapter/EeloTasksSyncAdapterService.kt @@ -136,16 +136,16 @@ class EeloTasksSyncAdapterService : SyncAdapterService() { ) object : AsyncTask() { override fun doInBackground(vararg params: Void): Void? { - it.performSync() + it.performSyncWithRetry() return null } }.execute() }) } else { - it.performSync() + it.performSyncWithRetry() } } else { - it.performSync() + it.performSyncWithRetry() } } } diff --git a/app/src/main/java/at/bitfire/davdroid/syncadapter/GoogleCalendarsSyncAdapterService.kt b/app/src/main/java/at/bitfire/davdroid/syncadapter/GoogleCalendarsSyncAdapterService.kt index 643a649cd..95c8d5fcd 100644 --- a/app/src/main/java/at/bitfire/davdroid/syncadapter/GoogleCalendarsSyncAdapterService.kt +++ b/app/src/main/java/at/bitfire/davdroid/syncadapter/GoogleCalendarsSyncAdapterService.kt @@ -124,16 +124,16 @@ class GoogleCalendarsSyncAdapterService : SyncAdapterService() { ) object : AsyncTask() { override fun doInBackground(vararg params: Void): Void? { - it.performSync() + it.performSyncWithRetry() return null } }.execute() }) } else { - it.performSync() + it.performSyncWithRetry() } } else { - it.performSync() + it.performSyncWithRetry() } } } diff --git a/app/src/main/java/at/bitfire/davdroid/syncadapter/GoogleContactsSyncAdapterService.kt b/app/src/main/java/at/bitfire/davdroid/syncadapter/GoogleContactsSyncAdapterService.kt index fd8c3ade1..c15def3b6 100644 --- a/app/src/main/java/at/bitfire/davdroid/syncadapter/GoogleContactsSyncAdapterService.kt +++ b/app/src/main/java/at/bitfire/davdroid/syncadapter/GoogleContactsSyncAdapterService.kt @@ -107,7 +107,7 @@ class GoogleContactsSyncAdapterService : SyncAdapterService() { syncResult, provider, addressBook - ).performSync() + ).performSyncWithRetry() } catch (e: Exception) { Logger.log.log(Level.SEVERE, "Couldn't sync contacts", e) } diff --git a/app/src/main/java/at/bitfire/davdroid/syncadapter/GoogleTasksSyncAdapterService.kt b/app/src/main/java/at/bitfire/davdroid/syncadapter/GoogleTasksSyncAdapterService.kt index 8a3d0713f..66e180fb8 100644 --- a/app/src/main/java/at/bitfire/davdroid/syncadapter/GoogleTasksSyncAdapterService.kt +++ b/app/src/main/java/at/bitfire/davdroid/syncadapter/GoogleTasksSyncAdapterService.kt @@ -135,16 +135,16 @@ class GoogleTasksSyncAdapterService : SyncAdapterService() { ) object : AsyncTask() { override fun doInBackground(vararg params: Void): Void? { - it.performSync() + it.performSyncWithRetry() return null } }.execute() }) } else { - it.performSync() + it.performSyncWithRetry() } } else { - it.performSync() + it.performSyncWithRetry() } } } diff --git a/app/src/main/java/at/bitfire/davdroid/syncadapter/SyncManager.kt b/app/src/main/java/at/bitfire/davdroid/syncadapter/SyncManager.kt index f00e87845..196afa80d 100644 --- a/app/src/main/java/at/bitfire/davdroid/syncadapter/SyncManager.kt +++ b/app/src/main/java/at/bitfire/davdroid/syncadapter/SyncManager.kt @@ -54,7 +54,6 @@ import java.io.InterruptedIOException import java.lang.ref.WeakReference import java.net.ConnectException import java.net.HttpURLConnection -import java.net.UnknownHostException import java.security.cert.CertificateException import java.util.* import java.util.concurrent.LinkedBlockingQueue @@ -133,8 +132,21 @@ abstract class SyncManager, out CollectionType: L val workDispatcher = getWorkDispatcher() + /** + * Call performSync with default retry values + */ + fun performSyncWithRetry() { + performSync(5, 8, 21) + } - fun performSync() { + /** + * Perform sync operation. + * On unhandled exceptions, retry following fibonnacci sequence (if user pass valid retry times. + * @param retryAfter optional param, in seconds. On unhandled exception `onSync`, if value > 0 && <= maxRetryTime, wait until this value & retry + * @param secondRetryAfter optional param, in seconds. Used to calculate fibonnacci sequence for rety on unhandled exception + * @param maxRetryTime optional param, in seconds. On unhandled exception, max time the method should retry. + */ + fun performSync(retryAfter: Int = Int.MIN_VALUE, secondRetryAfter: Int = Int.MIN_VALUE, maxRetryTime: Int = Int.MIN_VALUE) { // dismiss previous error notifications notificationManager.cancel(notificationTag, NotificationUtils.NOTIFY_SYNC_ERROR) @@ -255,7 +267,6 @@ abstract class SyncManager, out CollectionType: L } else Logger.log.info("Remote collection didn't change, no reason to sync") - }, { e, local, remote -> when (e) { // sync was cancelled or account has been removed: re-throw to SyncAdapterService @@ -283,12 +294,35 @@ abstract class SyncManager, out CollectionType: L } // all others - else -> + else -> { + if (retrySyncOperation(retryAfter, secondRetryAfter, maxRetryTime, e)) { + return@unwrapExceptions + } + notifyException(e, local, remote) + } + } }) } + private fun retrySyncOperation(retryAfter: Int, secondRetryAfter: Int, maxRetryTime: Int, e: Throwable): Boolean { + if (retryAfter > 0 && secondRetryAfter > 0 && retryAfter <= maxRetryTime) { + try { + Logger.log.severe("Faced unhandled exception $e, Will retry sync") + Logger.log.info("Retry sync after $retryAfter seconds") + Thread.sleep(retryAfter * 1000L) + performSync(secondRetryAfter, retryAfter + secondRetryAfter, maxRetryTime) + } catch (ex: Throwable) { + Logger.log.warning("Retry sync interrupted. $ex") + } + + return true + } + + return false + } + /** * Prepares synchronization. Sets the lateinit properties [collectionURL] and [davCollection]. -- GitLab From 5ceed740f76ab0456d8010eb553d24bd4e3ae28b Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Thu, 15 Dec 2022 13:47:51 +0600 Subject: [PATCH 4/4] Move network connection checking logic to performSync method --- .../davdroid/syncadapter/SyncManager.kt | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/at/bitfire/davdroid/syncadapter/SyncManager.kt b/app/src/main/java/at/bitfire/davdroid/syncadapter/SyncManager.kt index 196afa80d..2213d229f 100644 --- a/app/src/main/java/at/bitfire/davdroid/syncadapter/SyncManager.kt +++ b/app/src/main/java/at/bitfire/davdroid/syncadapter/SyncManager.kt @@ -293,8 +293,22 @@ abstract class SyncManager, out CollectionType: L } } + // sometimes sync is scheduled & kicked just the time network becomes available but not ready; in this case it throws ConnectException. + // In this case, we don't want to show notification to users. + is ConnectException -> { + Logger.log.severe("Failed to connect server for sync $e") + return@unwrapExceptions + } + // all others else -> { + // sometimes sync is kicked in when no network is not available. + // In this case, we don't want to show notification to users. + if (!NetworkUtils.isConnectedToNetwork(context)) { + Logger.log.warning("No internet connection. Skipping sync operation") + return@unwrapExceptions + } + if (retrySyncOperation(retryAfter, secondRetryAfter, maxRetryTime, e)) { return@unwrapExceptions } @@ -774,13 +788,6 @@ abstract class SyncManager, out CollectionType: L } } - // sometimes sync is kicked in when no network is not available. - // Or sync is scheduled & kicked just the time network becomes available but not ready; in this case it throws ConnectException. - // In these cases, we don't want to show notification to users. - if (!NetworkUtils.isConnectedToNetwork(context) || e is ConnectException) { - return - } - val contentIntent: Intent var viewItemAction: NotificationCompat.Action? = null -- GitLab