From 957b12f338edde5c1e089187bbb6effab80736f4 Mon Sep 17 00:00:00 2001 From: Koitharu Date: Fri, 27 Jun 2025 18:39:10 +0300 Subject: [PATCH] Improve error reporting from notifications --- .../kotatsu/alternatives/ui/AutoFixService.kt | 26 ++++++----- .../kotatsu/backups/data/BackupRepository.kt | 6 ++- .../backups/ui/BaseBackupRestoreService.kt | 10 ++--- .../backups/ui/backup/BackupService.kt | 14 +++++- .../kotatsu/core/ErrorReporterReceiver.kt | 45 ++++++++++++++++++- .../kotatsu/local/ui/ImportService.kt | 22 ++++----- 6 files changed, 91 insertions(+), 32 deletions(-) diff --git a/app/src/main/kotlin/org/koitharu/kotatsu/alternatives/ui/AutoFixService.kt b/app/src/main/kotlin/org/koitharu/kotatsu/alternatives/ui/AutoFixService.kt index 2ebcb595f..eeffa2a4d 100644 --- a/app/src/main/kotlin/org/koitharu/kotatsu/alternatives/ui/AutoFixService.kt +++ b/app/src/main/kotlin/org/koitharu/kotatsu/alternatives/ui/AutoFixService.kt @@ -19,6 +19,7 @@ import org.koitharu.kotatsu.R import org.koitharu.kotatsu.alternatives.domain.AutoFixUseCase import org.koitharu.kotatsu.core.ErrorReporterReceiver import org.koitharu.kotatsu.core.model.getTitle +import org.koitharu.kotatsu.core.model.isNsfw import org.koitharu.kotatsu.core.nav.AppRouter import org.koitharu.kotatsu.core.ui.CoroutineIntentService import org.koitharu.kotatsu.core.util.ext.checkNotificationPermission @@ -58,7 +59,7 @@ class AutoFixService : CoroutineIntentService() { autoFixUseCase.invoke(mangaId) } if (applicationContext.checkNotificationPermission(CHANNEL_ID)) { - val notification = buildNotification(result) + val notification = buildNotification(startId, result) notificationManager.notify(TAG, startId, notification) } } @@ -67,7 +68,7 @@ class AutoFixService : CoroutineIntentService() { override fun IntentJobContext.onError(error: Throwable) { if (applicationContext.checkNotificationPermission(CHANNEL_ID)) { - val notification = runBlocking { buildNotification(Result.failure(error)) } + val notification = runBlocking { buildNotification(startId, Result.failure(error)) } notificationManager.notify(TAG, startId, notification) } } @@ -108,7 +109,7 @@ class AutoFixService : CoroutineIntentService() { ) } - private suspend fun buildNotification(result: Result>): Notification { + private suspend fun buildNotification(startId: Int, result: Result>): Notification { val notification = NotificationCompat.Builder(applicationContext, CHANNEL_ID) .setPriority(NotificationCompat.PRIORITY_DEFAULT) .setDefaults(0) @@ -135,7 +136,11 @@ class AutoFixService : CoroutineIntentService() { false, ), ).setVisibility( - if (replacement.isNsfw) NotificationCompat.VISIBILITY_SECRET else NotificationCompat.VISIBILITY_PUBLIC, + if (replacement.isNsfw()) { + NotificationCompat.VISIBILITY_SECRET + } else { + NotificationCompat.VISIBILITY_PUBLIC + }, ) notification .setContentTitle(applicationContext.getString(R.string.fixed)) @@ -165,12 +170,13 @@ class AutoFixService : CoroutineIntentService() { error.getDisplayMessage(applicationContext.resources) }, ).setSmallIcon(android.R.drawable.stat_notify_error) - ErrorReporterReceiver.getPendingIntent(applicationContext, error)?.let { reportIntent -> - notification.addAction( - R.drawable.ic_alert_outline, - applicationContext.getString(R.string.report), - reportIntent, - ) + ErrorReporterReceiver.getNotificationAction( + context = applicationContext, + e = error, + notificationId = startId, + notificationTag = TAG, + )?.let { action -> + notification.addAction(action) } } return notification.build() diff --git a/app/src/main/kotlin/org/koitharu/kotatsu/backups/data/BackupRepository.kt b/app/src/main/kotlin/org/koitharu/kotatsu/backups/data/BackupRepository.kt index 3c6a089f5..ae57a3fc9 100644 --- a/app/src/main/kotlin/org/koitharu/kotatsu/backups/data/BackupRepository.kt +++ b/app/src/main/kotlin/org/koitharu/kotatsu/backups/data/BackupRepository.kt @@ -183,8 +183,10 @@ class BackupRepository @Inject constructor( data.onStart { putNextEntry(ZipEntry(section.entryName)) write("[") - }.onCompletion { - write("]") + }.onCompletion { error -> + if (error == null) { + write("]") + } closeEntry() flush() }.collectIndexed { index, value -> diff --git a/app/src/main/kotlin/org/koitharu/kotatsu/backups/ui/BaseBackupRestoreService.kt b/app/src/main/kotlin/org/koitharu/kotatsu/backups/ui/BaseBackupRestoreService.kt index bcb6f7aaa..d7ed489fb 100644 --- a/app/src/main/kotlin/org/koitharu/kotatsu/backups/ui/BaseBackupRestoreService.kt +++ b/app/src/main/kotlin/org/koitharu/kotatsu/backups/ui/BaseBackupRestoreService.kt @@ -84,13 +84,9 @@ abstract class BaseBackupRestoreService : CoroutineIntentService() { .setBigText(title, message) .setSmallIcon(android.R.drawable.stat_notify_error) result.failures.firstNotNullOfOrNull { error -> - ErrorReporterReceiver.getPendingIntent(applicationContext, error) - }?.let { reportIntent -> - notification.addAction( - R.drawable.ic_alert_outline, - applicationContext.getString(R.string.report), - reportIntent, - ) + ErrorReporterReceiver.getNotificationAction(applicationContext, error, startId, notificationTag) + }?.let { action -> + notification.addAction(action) } } diff --git a/app/src/main/kotlin/org/koitharu/kotatsu/backups/ui/backup/BackupService.kt b/app/src/main/kotlin/org/koitharu/kotatsu/backups/ui/backup/BackupService.kt index 122cc213d..4c39570b7 100644 --- a/app/src/main/kotlin/org/koitharu/kotatsu/backups/ui/backup/BackupService.kt +++ b/app/src/main/kotlin/org/koitharu/kotatsu/backups/ui/backup/BackupService.kt @@ -10,6 +10,7 @@ import android.widget.Toast import androidx.annotation.CheckResult import androidx.core.app.NotificationCompat import androidx.core.content.ContextCompat +import androidx.documentfile.provider.DocumentFile import dagger.hilt.android.AndroidEntryPoint import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.cancelAndJoin @@ -61,8 +62,17 @@ class BackupService : BaseBackupRestoreService() { } else { null } - ZipOutputStream(contentResolver.openOutputStream(destination)).use { output -> - repository.createBackup(output, progress) + try { + ZipOutputStream(contentResolver.openOutputStream(destination)).use { output -> + repository.createBackup(output, progress) + } + } catch (e: Throwable) { + try { + DocumentFile.fromSingleUri(applicationContext, destination)?.delete() + } catch (e2: Throwable) { + e.addSuppressed(e2) + } + throw e } progressUpdateJob?.cancelAndJoin() contentResolver.notifyChange(destination, null) diff --git a/app/src/main/kotlin/org/koitharu/kotatsu/core/ErrorReporterReceiver.kt b/app/src/main/kotlin/org/koitharu/kotatsu/core/ErrorReporterReceiver.kt index 770248c7b..5fedc9b20 100644 --- a/app/src/main/kotlin/org/koitharu/kotatsu/core/ErrorReporterReceiver.kt +++ b/app/src/main/kotlin/org/koitharu/kotatsu/core/ErrorReporterReceiver.kt @@ -5,9 +5,12 @@ import android.content.BroadcastReceiver import android.content.Context import android.content.Intent import android.os.BadParcelableException +import androidx.core.app.NotificationCompat +import androidx.core.app.NotificationManagerCompat import androidx.core.app.PendingIntentCompat import androidx.core.net.toUri import org.koitharu.kotatsu.BuildConfig +import org.koitharu.kotatsu.R import org.koitharu.kotatsu.core.nav.AppRouter import org.koitharu.kotatsu.core.util.ext.getSerializableExtraCompat import org.koitharu.kotatsu.core.util.ext.printStackTraceDebug @@ -17,18 +20,58 @@ class ErrorReporterReceiver : BroadcastReceiver() { override fun onReceive(context: Context?, intent: Intent?) { val e = intent?.getSerializableExtraCompat(AppRouter.KEY_ERROR) ?: return + val notificationId = intent.getIntExtra(EXTRA_NOTIFICATION_ID, 0) + if (notificationId != 0 && context != null) { + val notificationTag = intent.getStringExtra(EXTRA_NOTIFICATION_TAG) + NotificationManagerCompat.from(context).cancel(notificationTag, notificationId) + } e.report() } companion object { private const val ACTION_REPORT = "${BuildConfig.APPLICATION_ID}.action.REPORT_ERROR" + private const val EXTRA_NOTIFICATION_ID = "notify.id" + private const val EXTRA_NOTIFICATION_TAG = "notify.tag" + + fun getPendingIntent(context: Context, e: Throwable): PendingIntent? = getPendingIntentInternal( + context = context, + e = e, + notificationId = 0, + notificationTag = null, + ) + + fun getNotificationAction( + context: Context, + e: Throwable, + notificationId: Int, + notificationTag: String?, + ): NotificationCompat.Action? { + val intent = getPendingIntentInternal( + context = context, + e = e, + notificationId = notificationId, + notificationTag = notificationTag, + ) ?: return null + return NotificationCompat.Action( + R.drawable.ic_alert_outline, + context.getString(R.string.report), + intent, + ) + } - fun getPendingIntent(context: Context, e: Throwable): PendingIntent? = try { + private fun getPendingIntentInternal( + context: Context, + e: Throwable, + notificationId: Int, + notificationTag: String?, + ): PendingIntent? = try { val intent = Intent(context, ErrorReporterReceiver::class.java) intent.setAction(ACTION_REPORT) intent.setData("err://${e.hashCode()}".toUri()) intent.putExtra(AppRouter.KEY_ERROR, e) + intent.putExtra(EXTRA_NOTIFICATION_ID, notificationId) + intent.putExtra(EXTRA_NOTIFICATION_TAG, notificationTag) PendingIntentCompat.getBroadcast(context, 0, intent, 0, false) } catch (e: BadParcelableException) { e.printStackTraceDebug() diff --git a/app/src/main/kotlin/org/koitharu/kotatsu/local/ui/ImportService.kt b/app/src/main/kotlin/org/koitharu/kotatsu/local/ui/ImportService.kt index 8214ddd57..b51877e02 100644 --- a/app/src/main/kotlin/org/koitharu/kotatsu/local/ui/ImportService.kt +++ b/app/src/main/kotlin/org/koitharu/kotatsu/local/ui/ImportService.kt @@ -18,6 +18,7 @@ import dagger.hilt.android.AndroidEntryPoint import kotlinx.coroutines.runBlocking import org.koitharu.kotatsu.R import org.koitharu.kotatsu.core.ErrorReporterReceiver +import org.koitharu.kotatsu.core.model.isNsfw import org.koitharu.kotatsu.core.nav.AppRouter import org.koitharu.kotatsu.core.ui.CoroutineIntentService import org.koitharu.kotatsu.core.util.ext.checkNotificationPermission @@ -57,7 +58,7 @@ class ImportService : CoroutineIntentService() { importer.import(uri).manga } if (applicationContext.checkNotificationPermission(CHANNEL_ID)) { - val notification = buildNotification(result) + val notification = buildNotification(startId, result) notificationManager.notify(TAG, startId, notification) } } @@ -65,7 +66,7 @@ class ImportService : CoroutineIntentService() { override fun IntentJobContext.onError(error: Throwable) { if (applicationContext.checkNotificationPermission(CHANNEL_ID)) { - val notification = runBlocking { buildNotification(Result.failure(error)) } + val notification = runBlocking { buildNotification(startId, Result.failure(error)) } notificationManager.notify(TAG, startId, notification) } } @@ -101,7 +102,7 @@ class ImportService : CoroutineIntentService() { ) } - private suspend fun buildNotification(result: Result): Notification { + private suspend fun buildNotification(startId: Int, result: Result): Notification { val notification = NotificationCompat.Builder(applicationContext, CHANNEL_ID) .setPriority(NotificationCompat.PRIORITY_DEFAULT) .setDefaults(0) @@ -127,7 +128,7 @@ class ImportService : CoroutineIntentService() { false, ), ).setVisibility( - if (manga.isNsfw) NotificationCompat.VISIBILITY_SECRET else NotificationCompat.VISIBILITY_PUBLIC, + if (manga.isNsfw()) NotificationCompat.VISIBILITY_SECRET else NotificationCompat.VISIBILITY_PUBLIC, ) notification.setContentTitle(applicationContext.getString(R.string.import_completed)) .setContentText(applicationContext.getString(R.string.import_completed_hint)) @@ -138,12 +139,13 @@ class ImportService : CoroutineIntentService() { notification.setContentTitle(applicationContext.getString(R.string.error_occurred)) .setContentText(error.getDisplayMessage(applicationContext.resources)) .setSmallIcon(android.R.drawable.stat_notify_error) - ErrorReporterReceiver.getPendingIntent(applicationContext, error)?.let { reportIntent -> - notification.addAction( - R.drawable.ic_alert_outline, - applicationContext.getString(R.string.report), - reportIntent, - ) + ErrorReporterReceiver.getNotificationAction( + context = applicationContext, + e = error, + notificationId = startId, + notificationTag = TAG, + )?.let { action -> + notification.addAction(action) } } return notification.build()