fix(library): notify when sync fails before library update

Report sync failures that previously failed silently (e.g. null remote
backup) and show a library update error notification when a chained sync
blocks the update, including the sync failure reason.

Co-authored-by: Cuong-Tran <cuong-tran@users.noreply.github.com>

app/src/main/java/eu/kanade/tachiyomi/data/library/LibraryUpdateJob.kt

@@ -182,6 +182,17 @@ class LibraryUpdateJob(private val context: Context, workerParams: WorkerParamet
 
         return withIOContext {
             try {
+                LibraryUpdateSyncResult.consumeFailure()?.let { syncError ->
+                    notifier.showUpdateErrorNotification(
+                        failed = 1,
+                        errorMessage = context.stringResource(
+                            MR.strings.library_update_skipped_sync_error,
+                            syncError,
+                        ),
+                    )
+                    return@withIOContext Result.success()
+                }
+
                 when (target) {
                     Target.CHAPTERS -> updateChapterList()
                     // SY -->
@@ -844,6 +855,7 @@ class LibraryUpdateJob(private val context: Context, workerParams: WorkerParamet
                 // Define the SyncDataJob
                 val syncDataJob = OneTimeWorkRequestBuilder<SyncDataJob>()
                     .addTag(SyncDataJob.TAG_MANUAL)
+                    .addTag(SyncDataJob.TAG_BEFORE_LIBRARY_UPDATE)
                     .build()
 
                 // Chain SyncDataJob to run before LibraryUpdateJob

app/src/main/java/eu/kanade/tachiyomi/data/library/LibraryUpdateNotifier.kt

@@ -153,7 +153,7 @@ class LibraryUpdateNotifier(
      *
      * @param failed Number of entries that failed to update.
      */
-    fun showUpdateErrorNotification(failed: Int) {
+    fun showUpdateErrorNotification(failed: Int, errorMessage: String? = null) {
         if (failed == 0) {
             return
         }
@@ -163,7 +163,7 @@ class LibraryUpdateNotifier(
             Notifications.CHANNEL_LIBRARY_ERROR,
         ) {
             setContentTitle(context.stringResource(MR.strings.notification_update_error, failed))
-            setContentText(context.stringResource(MR.strings.action_show_errors))
+            setContentText(errorMessage ?: context.stringResource(MR.strings.action_show_errors))
             setSmallIcon(R.drawable.ic_komikku)
             setColor(ContextCompat.getColor(context, R.color.ic_launcher))
 

app/src/main/java/eu/kanade/tachiyomi/data/library/LibraryUpdateSyncResult.kt

@@ -0,0 +1,20 @@
+package eu.kanade.tachiyomi.data.library
+
+/**
+ * Holds a sync failure message to be reported by [LibraryUpdateJob] when a library update
+ * was chained to run after [eu.kanade.tachiyomi.data.sync.SyncDataJob].
+ */
+object LibraryUpdateSyncResult {
+    @Volatile
+    private var pendingError: String? = null
+
+    fun setFailure(message: String) {
+        pendingError = message
+    }
+
+    fun consumeFailure(): String? {
+        val error = pendingError
+        pendingError = null
+        return error
+    }
+}

app/src/main/java/eu/kanade/tachiyomi/data/sync/SyncDataJob.kt

@@ -14,14 +14,18 @@ import androidx.work.WorkQuery
 import androidx.work.WorkerParameters
 import eu.kanade.domain.sync.SyncPreferences
 import eu.kanade.tachiyomi.data.SyncStatus
+import eu.kanade.tachiyomi.data.library.LibraryUpdateSyncResult
 import eu.kanade.tachiyomi.data.notification.Notifications
 import eu.kanade.tachiyomi.util.system.isOnline
 import eu.kanade.tachiyomi.util.system.isRunning
 import eu.kanade.tachiyomi.util.system.setForegroundSafely
 import eu.kanade.tachiyomi.util.system.workManager
+import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.runBlocking
 import logcat.LogPriority
+import tachiyomi.core.common.i18n.stringResource
 import tachiyomi.core.common.util.system.logcat
+import tachiyomi.i18n.MR
 import uy.kohesive.injekt.Injekt
 import uy.kohesive.injekt.api.get
 import java.util.concurrent.TimeUnit
@@ -53,12 +57,22 @@ class SyncDataJob(private val context: Context, workerParams: WorkerParameters)
         setForegroundSafely()
 
         return try {
-            SyncManager(context).syncData()
+            when (val result = SyncManager(context).syncData()) {
+                SyncDataResult.Success -> Result.success()
+                is SyncDataResult.Failure -> {
+                    handleSyncFailure(result.message, result.errorAlreadyNotified)
                     Result.success()
+                }
+            }
+        } catch (e: CancellationException) {
+            throw e
         } catch (e: Exception) {
             logcat(LogPriority.ERROR, e)
-            notifier.showSyncError(e.message)
-            Result.success() // try again next time
+            handleSyncFailure(
+                e.message ?: context.stringResource(MR.strings.unknown_error),
+                errorAlreadyNotified = false,
+            )
+            Result.success()
         } finally {
             // KMK -->
             syncStatus.stop()
@@ -78,10 +92,19 @@ class SyncDataJob(private val context: Context, workerParams: WorkerParameters)
         )
     }
 
+    private fun handleSyncFailure(message: String, errorAlreadyNotified: Boolean) {
+        if (tags.contains(TAG_BEFORE_LIBRARY_UPDATE)) {
+            LibraryUpdateSyncResult.setFailure(message)
+        } else if (!errorAlreadyNotified) {
+            notifier.showSyncError(message)
+        }
+    }
+
     companion object {
         private const val TAG_JOB = "SyncDataJob"
         private const val TAG_AUTO = "$TAG_JOB:auto"
         const val TAG_MANUAL = "$TAG_JOB:manual"
+        const val TAG_BEFORE_LIBRARY_UPDATE = "$TAG_JOB:before-library-update"
 
         fun isRunning(context: Context): Boolean {
             return context.workManager.isRunning(TAG_JOB)

app/src/main/java/eu/kanade/tachiyomi/data/sync/SyncDataResult.kt

@@ -0,0 +1,28 @@
+package eu.kanade.tachiyomi.data.sync
+
+/**
+ * Captures the most recent sync failure reported by a [eu.kanade.tachiyomi.data.sync.service.SyncService]
+ * implementation so [SyncManager] can propagate the message without losing context.
+ */
+internal object SyncFailureState {
+    private var failure: Pair<String, Boolean>? = null
+
+    fun report(message: String, errorAlreadyNotified: Boolean = true) {
+        failure = message to errorAlreadyNotified
+    }
+
+    fun consume(): Pair<String, Boolean>? = failure.also { failure = null }
+}
+
+sealed interface SyncDataResult {
+    data object Success : SyncDataResult
+
+    /**
+     * @param message Human-readable failure reason.
+     * @param errorAlreadyNotified `true` when a sync service already showed a sync error notification.
+     */
+    data class Failure(
+        val message: String,
+        val errorAlreadyNotified: Boolean = false,
+    ) : SyncDataResult
+}

app/src/main/java/eu/kanade/tachiyomi/data/sync/SyncManager.kt

@@ -19,7 +19,9 @@ import kotlinx.serialization.json.Json
 import kotlinx.serialization.protobuf.ProtoBuf
 import logcat.LogPriority
 import logcat.logcat
+import tachiyomi.core.common.i18n.stringResource
 import tachiyomi.core.common.util.system.logcat
+import tachiyomi.i18n.MR
 import tachiyomi.data.Chapters
 import tachiyomi.data.DatabaseHandler
 import tachiyomi.data.manga.MangaMapper.mapManga
@@ -72,7 +74,9 @@ class SyncManager(
      * This function retrieves local data (favorites, manga, extensions, and categories)
      * from the database using the BackupManager, then synchronizes the data with a sync service.
      */
-    suspend fun syncData() {
+    suspend fun syncData(): SyncDataResult {
+        SyncFailureState.consume()
+
         // Reset isSyncing in case it was left over or failed syncing during restore.
         handler.await(inTransaction = true) {
             mangasQueries.resetIsSyncing()
@@ -153,12 +157,22 @@ class SyncManager(
             }
         }
 
-        val remoteBackup = syncService?.doSync(syncData)
+        if (syncService == null) {
+            return SyncDataResult.Failure(
+                message = context.stringResource(MR.strings.sync_failed_generic),
+            )
+        }
+
+        val remoteBackup = syncService.doSync(syncData)
 
         if (remoteBackup == null) {
-            logcat(LogPriority.DEBUG) { "Skip restore due to network issues" }
-            // should we call showSyncError?
-            return
+            logcat(LogPriority.DEBUG) { "Skip restore due to sync failure" }
+            val (message, errorAlreadyNotified) = SyncFailureState.consume()
+                ?: (context.stringResource(MR.strings.sync_failed_generic) to false)
+            return SyncDataResult.Failure(
+                message = message,
+                errorAlreadyNotified = errorAlreadyNotified,
+            )
         }
 
         if (remoteBackup === syncData.backup) {
@@ -166,13 +180,14 @@ class SyncManager(
             logcat(LogPriority.DEBUG) { "Skip restore due to remote was overwrite from local" }
             syncPreferences.lastSyncTimestamp().set(Date().time)
             notifier.showSyncSuccess("Sync completed successfully")
-            return
+            return SyncDataResult.Success
         }
 
         // Stop the sync early if the remote backup is null or empty
         if (remoteBackup.backupManga.isEmpty()) {
-            notifier.showSyncError("No data found on remote server.")
-            return
+            val message = "No data found on remote server."
+            notifier.showSyncError(message)
+            return SyncDataResult.Failure(message = message, errorAlreadyNotified = true)
         }
 
         // Check if it's first sync based on lastSyncTimestamp
@@ -180,7 +195,7 @@ class SyncManager(
             // It's first sync no need to restore data. (just update remote data)
             syncPreferences.lastSyncTimestamp().set(Date().time)
             notifier.showSyncSuccess("Updated remote data successfully")
-            return
+            return SyncDataResult.Success
         }
 
         val (filteredFavorites, nonFavorites) = filterFavoritesAndNonFavorites(remoteBackup)
@@ -208,7 +223,7 @@ class SyncManager(
             // update the sync timestamp
             syncPreferences.lastSyncTimestamp().set(Date().time)
             notifier.showSyncSuccess("Sync completed successfully")
-            return
+            return SyncDataResult.Success
         }
 
         val backupUri = writeSyncDataToCache(context, newSyncData)
@@ -228,8 +243,12 @@ class SyncManager(
 
             // update the sync timestamp
             syncPreferences.lastSyncTimestamp().set(Date().time)
+            return SyncDataResult.Success
         } else {
             logcat(LogPriority.ERROR) { "Failed to write sync data to file" }
+            return SyncDataResult.Failure(
+                message = context.stringResource(MR.strings.sync_failed_generic),
+            )
         }
     }
 

app/src/main/java/eu/kanade/tachiyomi/data/sync/service/GoogleDriveSyncService.kt

@@ -18,6 +18,8 @@ import com.google.api.services.drive.DriveScopes
 import com.google.api.services.drive.model.File
 import eu.kanade.domain.sync.SyncPreferences
 import eu.kanade.tachiyomi.data.backup.models.Backup
+import eu.kanade.tachiyomi.data.sync.SyncFailureState
+import eu.kanade.tachiyomi.data.sync.SyncNotifier
 import kotlinx.coroutines.launch
 import kotlinx.serialization.json.Json
 import kotlinx.serialization.protobuf.ProtoBuf
@@ -63,6 +65,8 @@ class GoogleDriveSyncService(context: Context, json: Json, syncPreferences: Sync
 
     private val googleDriveService = GoogleDriveService(context)
 
+    private val notifier = SyncNotifier(context)
+
     private val protoBuf: ProtoBuf = Injekt.get()
 
     override suspend fun doSync(syncData: SyncData): Backup? {
@@ -96,7 +100,10 @@ class GoogleDriveSyncService(context: Context, json: Json, syncPreferences: Sync
             pushSyncData(syncData)
             return syncData.backup
         } catch (e: Exception) {
-            logcat(LogPriority.ERROR, "SyncService") { "Error syncing: ${e.message}" }
+            val message = e.message ?: context.stringResource(MR.strings.unknown_error)
+            logcat(LogPriority.ERROR, "SyncService") { "Error syncing: $message" }
+            notifier.showSyncError(message)
+            SyncFailureState.report(message)
             return null
         }
     }

app/src/main/java/eu/kanade/tachiyomi/data/sync/service/SyncYomiSyncService.kt

@@ -3,6 +3,7 @@ package eu.kanade.tachiyomi.data.sync.service
 import android.content.Context
 import eu.kanade.domain.sync.SyncPreferences
 import eu.kanade.tachiyomi.data.backup.models.Backup
+import eu.kanade.tachiyomi.data.sync.SyncFailureState
 import eu.kanade.tachiyomi.data.sync.SyncNotifier
 import eu.kanade.tachiyomi.network.GET
 import eu.kanade.tachiyomi.network.POST
@@ -87,10 +88,11 @@ class SyncYomiSyncService(
             if (success) {
                 reportSyncEvent(SyncEventStatus.SYNC_SUCCESS)
             } else {
-                reportSyncEvent(SyncEventStatus.SYNC_FAILED, "Failed to push sync data")
-                // KMK -->
+                val message = "Failed to push sync data"
+                reportSyncEvent(SyncEventStatus.SYNC_FAILED, message)
+                notifier.showSyncError(message)
+                SyncFailureState.report(message)
                 return null
-                // KMK <--
             }
 
             return finalSyncData.backup
@@ -99,9 +101,11 @@ class SyncYomiSyncService(
                 reportSyncEvent(SyncEventStatus.SYNC_CANCELLED, e.message)
                 throw e
             }
-            logcat(LogPriority.ERROR) { "Error syncing: ${e.message}" }
-            notifier.showSyncError(e.message)
-            reportSyncEvent(SyncEventStatus.SYNC_ERROR, e.message)
+            val message = e.message ?: context.stringResource(MR.strings.unknown_error)
+            logcat(LogPriority.ERROR) { "Error syncing: $message" }
+            notifier.showSyncError(message)
+            SyncFailureState.report(message)
+            reportSyncEvent(SyncEventStatus.SYNC_ERROR, message)
             return null
         }
     }

app/src/main/java/eu/kanade/tachiyomi/data/sync/service/WebDavSyncService.kt

@@ -3,6 +3,7 @@ package eu.kanade.tachiyomi.data.sync.service
 import android.content.Context
 import eu.kanade.domain.sync.SyncPreferences
 import eu.kanade.tachiyomi.data.backup.models.Backup
+import eu.kanade.tachiyomi.data.sync.SyncFailureState
 import eu.kanade.tachiyomi.data.sync.SyncNotifier
 import eu.kanade.tachiyomi.network.GET
 import eu.kanade.tachiyomi.network.PUT
@@ -117,7 +118,9 @@ class WebDavSyncService(
             return finalSyncData.backup
         } catch (e: Exception) {
             xLogE("WebDAV sync error:", e)
-            notifier.showSyncError(e.message)
+            val message = e.message ?: context.stringResource(MR.strings.unknown_error)
+            notifier.showSyncError(message)
+            SyncFailureState.report(message)
             return null
         }
     }
@@ -182,6 +185,7 @@ class WebDavSyncService(
                     "Please retry syncing or check your WebDAV folder."
                 xLogW(message)
                 notifier.showSyncError(message)
+                SyncFailureState.report(message)
             }
             else -> {
                 val bodyStr = response.body.string()

i18n/src/commonMain/moko-resources/base/strings.xml

@@ -923,6 +923,8 @@
     <string name="notification_chapters_single_and_more">Chapter %1$s and %2$d more</string>
     <string name="notification_chapters_multiple">Chapters %1$s</string>
     <string name="notification_update_error">%1$d update(s) failed</string>
+    <string name="library_update_skipped_sync_error">Library update skipped: %1$s</string>
+    <string name="sync_failed_generic">Could not sync with the remote service</string>
     <string name="learn_more">Tap to learn more</string>
     <string name="notification_cover_update_failed">Failed to update cover</string>
     <string name="notification_first_add_to_library">Please add the entry to your library before doing this</string>