Skip to content

Commit

Permalink
feat: Show information about notification fetches on "About" screen (#…
Browse files Browse the repository at this point in the history
…454)

Some users report that Pachli is not retrieving/displaying notifications
in a timely fashion.

To assist in diagnosing these errors, provide an additional set of tabs
on the "About" screen that contain information about how Pachli is
fetching notifications, and if not, why not.

Allow the user to save notification related logs and other details to a
file that can be attached to an e-mail or bug report.

Recording data:

- Provide a `NotificationConfig` singleton with properties to record
different aspects of the notification configuration. Update these
properties as different notification actions occur.

- Store logs in a `LogEntryEntity` table. Log events of interest with a
new `Timber` `LogEntryTree` that is planted in all cases.

- Add `PruneLogEntryEntityWorker` to trim saved logs to the last 48
hours.

Display:

- Add a `NotificationFragment` to `AboutActivity`. It hosts two other
fragments in tabs to show details from `NotificationConfig` and the
relevant logs, as well as controls for interacting with them.

Bug fixes:

- Filter out notifications with a null tag when processing active
notifications, prevents an NPE crash

Other changes:

- Log more details when errors occur so the bug reports are more helpful
  • Loading branch information
nikclayton authored Feb 17, 2024
1 parent 3fb6994 commit 23e3cf1
Show file tree
Hide file tree
Showing 42 changed files with 3,449 additions and 56 deletions.
4 changes: 2 additions & 2 deletions app/src/main/java/app/pachli/MainActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ import app.pachli.appstore.EventHub
import app.pachli.appstore.MainTabsChangedEvent
import app.pachli.appstore.ProfileEditedEvent
import app.pachli.components.compose.ComposeActivity.Companion.canHandleMimeType
import app.pachli.components.notifications.androidNotificationsAreEnabled
import app.pachli.components.notifications.createNotificationChannelsForAccount
import app.pachli.components.notifications.disableAllNotifications
import app.pachli.components.notifications.enablePushNotificationsWithFallback
import app.pachli.components.notifications.notificationsAreEnabled
import app.pachli.components.notifications.showMigrationNoticeIfNecessary
import app.pachli.core.activity.AccountSelectionListener
import app.pachli.core.activity.BottomSheetActivity
Expand Down Expand Up @@ -1020,7 +1020,7 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, MenuProvider {
accountManager,
sharedPreferencesRepository,
)
if (notificationsAreEnabled(this, accountManager)) {
if (androidNotificationsAreEnabled(this, accountManager)) {
lifecycleScope.launch {
enablePushNotificationsWithFallback(this@MainActivity, mastodonApi, accountManager)
}
Expand Down
19 changes: 18 additions & 1 deletion app/src/main/java/app/pachli/PachliApplication.kt
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import androidx.work.ExistingPeriodicWorkPolicy
import androidx.work.PeriodicWorkRequestBuilder
import androidx.work.WorkManager
import app.pachli.components.notifications.createWorkerNotificationChannel
import app.pachli.core.activity.LogEntryTree
import app.pachli.core.activity.TreeRing
import app.pachli.core.activity.initCrashReporter
import app.pachli.core.preferences.AppTheme
Expand All @@ -35,6 +36,7 @@ import app.pachli.core.preferences.SharedPreferencesRepository
import app.pachli.util.LocaleManager
import app.pachli.util.setAppNightMode
import app.pachli.worker.PruneCacheWorker
import app.pachli.worker.PruneLogEntryEntityWorker
import app.pachli.worker.WorkerFactory
import autodispose2.AutoDisposePlugins
import dagger.hilt.android.HiltAndroidApp
Expand All @@ -59,6 +61,9 @@ class PachliApplication : Application() {
@Inject
lateinit var sharedPreferencesRepository: SharedPreferencesRepository

@Inject
lateinit var logEntryTree: LogEntryTree

override fun attachBaseContext(base: Context?) {
super.attachBaseContext(base)

Expand Down Expand Up @@ -86,6 +91,7 @@ class PachliApplication : Application() {
BuildConfig.DEBUG -> Timber.plant(Timber.DebugTree())
BuildConfig.FLAVOR_color == "orange" -> Timber.plant(TreeRing)
}
Timber.plant(logEntryTree)

// Migrate shared preference keys and defaults from version to version.
val oldVersion = sharedPreferencesRepository.getInt(PrefKeys.SCHEMA_VERSION, NEW_INSTALL_SCHEMA_VERSION)
Expand Down Expand Up @@ -117,15 +123,26 @@ class PachliApplication : Application() {
.build(),
)

val workManager = WorkManager.getInstance(this)
// Prune the database every ~ 12 hours when the device is idle.
val pruneCacheWorker = PeriodicWorkRequestBuilder<PruneCacheWorker>(12, TimeUnit.HOURS)
.setConstraints(Constraints.Builder().setRequiresDeviceIdle(true).build())
.build()
WorkManager.getInstance(this).enqueueUniquePeriodicWork(
workManager.enqueueUniquePeriodicWork(
PruneCacheWorker.PERIODIC_WORK_TAG,
ExistingPeriodicWorkPolicy.KEEP,
pruneCacheWorker,
)

// Delete old logs every ~ 12 hours when the device is idle.
val pruneLogEntryEntityWorker = PeriodicWorkRequestBuilder<PruneLogEntryEntityWorker>(12, TimeUnit.HOURS)
.setConstraints(Constraints.Builder().setRequiresDeviceIdle(true).build())
.build()
workManager.enqueueUniquePeriodicWork(
PruneLogEntryEntityWorker.PERIODIC_WORK_TAG,
ExistingPeriodicWorkPolicy.KEEP,
pruneLogEntryEntityWorker,
)
}

private fun upgradeSharedPreferences(oldVersion: Int, newVersion: Int) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,17 @@ import android.app.NotificationManager
import android.content.Context
import androidx.annotation.WorkerThread
import app.pachli.core.accounts.AccountManager
import app.pachli.core.activity.NotificationConfig
import app.pachli.core.common.string.isLessThan
import app.pachli.core.database.model.AccountEntity
import app.pachli.core.network.model.Links
import app.pachli.core.network.model.Marker
import app.pachli.core.network.model.Notification
import app.pachli.core.network.retrofit.MastodonApi
import com.github.michaelbull.result.Err
import com.github.michaelbull.result.Ok
import dagger.hilt.android.qualifiers.ApplicationContext
import java.time.Instant
import javax.inject.Inject
import kotlin.math.min
import kotlin.time.Duration.Companion.milliseconds
Expand All @@ -49,7 +53,13 @@ class NotificationFetcher @Inject constructor(
@ApplicationContext private val context: Context,
) {
suspend fun fetchAndShow() {
Timber.d("NotificationFetcher.fetchAndShow() started")
for (account in accountManager.getAllAccountsOrderedByActive()) {
Timber.d(
"Checking %s$, notificationsEnabled = %s",
account.fullName,
account.notificationsEnabled,
)
if (account.notificationsEnabled) {
try {
val notificationManager = context.getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager
Expand All @@ -67,6 +77,7 @@ class NotificationFetcher @Inject constructor(
// Err on the side of removing *older* notifications to make room for newer
// notifications.
val currentAndroidNotifications = notificationManager.activeNotifications
.filter { it.tag != null }
.sortedWith(compareBy({ it.tag.length }, { it.tag })) // oldest notifications first

// Check to see if any notifications need to be removed
Expand Down Expand Up @@ -135,6 +146,7 @@ class NotificationFetcher @Inject constructor(
* than the marker.
*/
private suspend fun fetchNewNotifications(account: AccountEntity): List<Notification> {
Timber.d("fetchNewNotifications(%s)", account.fullName)
val authHeader = String.format("Bearer %s", account.accessToken)

// Figure out where to read from. Choose the most recent notification ID from:
Expand All @@ -158,12 +170,24 @@ class NotificationFetcher @Inject constructor(
// Fetch all outstanding notifications
val notifications = buildList {
while (minId != null) {
val now = Instant.now()
Timber.d("Fetching notifications from server")
val response = mastodonApi.notificationsWithAuth(
authHeader,
account.domain,
minId = minId,
)
if (!response.isSuccessful) break
if (!response.isSuccessful) {
val error = response.errorBody()?.string()
Timber.e("Fetching notifications from server failed: %s", error)
NotificationConfig.lastFetchNewNotifications[account.fullName] = Pair(now, Err(error ?: "Unknown error"))
break
}
NotificationConfig.lastFetchNewNotifications[account.fullName] = Pair(now, Ok(Unit))
Timber.i(
"Fetching notifications from server succeeded, returned %d notifications",
response.body()?.size,
)

// Notifications are returned in the page in order, newest first,
// (https://github.com/mastodon/documentation/issues/1226), insert the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import app.pachli.BuildConfig
import app.pachli.MainActivity
import app.pachli.R
import app.pachli.core.accounts.AccountManager
import app.pachli.core.activity.NotificationConfig
import app.pachli.core.common.string.unicodeWrap
import app.pachli.core.database.model.AccountEntity
import app.pachli.core.designsystem.R as DR
Expand Down Expand Up @@ -112,7 +113,6 @@ private const val EXTRA_NOTIFICATION_TYPE =
* Takes a given Mastodon notification and creates a new Android notification or updates the
* existing Android notification.
*
*
* The Android notification has it's tag set to the Mastodon notification ID, and it's ID set
* to the ID of the account that received the notification.
*
Expand Down Expand Up @@ -557,34 +557,64 @@ fun deleteNotificationChannelsForAccount(account: AccountEntity, context: Contex
}
}

fun notificationsAreEnabled(context: Context, accountManager: AccountManager): Boolean {
/**
* @return True if at least one account has Android notifications enabled.
*/
fun androidNotificationsAreEnabled(context: Context, accountManager: AccountManager): Boolean {
Timber.d("Checking if Android notifications are enabled")

return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
Timber.d(
"%d >= %d, checking notification manager",
Build.VERSION.SDK_INT,
Build.VERSION_CODES.O,
)
// on Android >= O notifications are enabled if at least one channel is enabled
val notificationManager =
context.getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager
if (notificationManager.areNotificationsEnabled()) {
for (channel in notificationManager.notificationChannels) {
Timber.d(
"Checking NotificationChannel %s / importance: %s",
channel.id,
channel.importance,
)
if (channel != null && channel.importance > NotificationManager.IMPORTANCE_NONE) {
Timber.d("NotificationsEnabled")
Timber.d("Channel notification importance > %d, enabling notifications", NotificationManager.IMPORTANCE_NONE)
NotificationConfig.androidNotificationsEnabled = true
return true
} else {
Timber.d("Channel notification importance <= %d, skipping", NotificationManager.IMPORTANCE_NONE)
}
}
}
Timber.d("NotificationsDisabled")
Timber.i("Notifications disabled because no notification channels are enabled")
NotificationConfig.androidNotificationsEnabled = false
false
} else {
// on Android < O notifications are enabled if at least one account has notification enabled
accountManager.areNotificationsEnabled()
Timber.d(
"%d < %d, checking account manager",
Build.VERSION.SDK_INT,
Build.VERSION_CODES.O,
)
val result = accountManager.areAndroidNotificationsEnabled()
Timber.d("Did any accounts have notifications enabled?: %s", result)
NotificationConfig.androidNotificationsEnabled = result
return result
}
}

fun enablePullNotifications(context: Context) {
Timber.i("Enabling pull notifications for all accounts")
val workManager = WorkManager.getInstance(context)
workManager.cancelAllWorkByTag(NOTIFICATION_PULL_TAG)

// Periodic work requests are supposed to start running soon after being enqueued. In
// practice that may not be soon enough, so create and enqueue an expedited one-time
// request to get new notifications immediately.
Timber.d("Enqueing immediate notification worker")
val fetchNotifications: WorkRequest = OneTimeWorkRequest.Builder(NotificationWorker::class.java)
.setExpedited(OutOfQuotaPolicy.DROP_WORK_REQUEST)
.setConstraints(Constraints.Builder().setRequiredNetworkType(NetworkType.CONNECTED).build())
Expand All @@ -603,11 +633,13 @@ fun enablePullNotifications(context: Context) {
.build()
workManager.enqueue(workRequest)
Timber.d("enabled notification checks with %d ms interval", PeriodicWorkRequest.MIN_PERIODIC_INTERVAL_MILLIS)
NotificationConfig.notificationMethod = NotificationConfig.Method.Pull
}

fun disablePullNotifications(context: Context) {
WorkManager.getInstance(context).cancelAllWorkByTag(NOTIFICATION_PULL_TAG)
Timber.d("disabled notification checks")
Timber.w("Disabling pull notifications for all accounts")
NotificationConfig.notificationMethod = NotificationConfig.Method.Unknown
}

fun clearNotificationsForAccount(context: Context, account: AccountEntity) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,20 @@ import android.view.View
import androidx.appcompat.app.AlertDialog
import app.pachli.R
import app.pachli.core.accounts.AccountManager
import app.pachli.core.activity.NotificationConfig
import app.pachli.core.database.model.AccountEntity
import app.pachli.core.navigation.LoginActivityIntent
import app.pachli.core.navigation.LoginActivityIntent.LoginMode
import app.pachli.core.network.model.Notification
import app.pachli.core.network.retrofit.MastodonApi
import app.pachli.core.preferences.SharedPreferencesRepository
import app.pachli.util.CryptoUtil
import at.connyduck.calladapter.networkresult.fold
import at.connyduck.calladapter.networkresult.onFailure
import at.connyduck.calladapter.networkresult.onSuccess
import com.github.michaelbull.result.Err
import com.github.michaelbull.result.Ok
import com.github.michaelbull.result.Result
import com.google.android.material.snackbar.Snackbar
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext
Expand All @@ -43,6 +48,7 @@ private const val KEY_MIGRATION_NOTICE_DISMISSED = "migration_notice_dismissed"
private fun anyAccountNeedsMigration(accountManager: AccountManager): Boolean =
accountManager.accounts.any(::accountNeedsMigration)

/** @return True if the account does not have the `push` OAuth scope, false otherwise */
private fun accountNeedsMigration(account: AccountEntity): Boolean =
!account.oauthScopes.contains("push")

Expand Down Expand Up @@ -100,9 +106,14 @@ private fun showMigrationExplanationDialog(
private suspend fun enableUnifiedPushNotificationsForAccount(context: Context, api: MastodonApi, accountManager: AccountManager, account: AccountEntity) {
if (isUnifiedPushNotificationEnabledForAccount(account)) {
// Already registered, update the subscription to match notification settings
updateUnifiedPushSubscription(context, api, accountManager, account)
val result = updateUnifiedPushSubscription(context, api, accountManager, account)
NotificationConfig.notificationMethodAccount[account.fullName] = when (result) {
is Err -> NotificationConfig.Method.PushError(result.error)
is Ok -> NotificationConfig.Method.Push
}
} else {
UnifiedPush.registerAppWithDialog(context, account.id.toString(), features = arrayListOf(UnifiedPush.FEATURE_BYTES_MESSAGE))
NotificationConfig.notificationMethodAccount[account.fullName] = NotificationConfig.Method.Push
}
}

Expand All @@ -118,19 +129,35 @@ fun disableUnifiedPushNotificationsForAccount(context: Context, account: Account
fun isUnifiedPushNotificationEnabledForAccount(account: AccountEntity): Boolean =
account.unifiedPushUrl.isNotEmpty()

/** True if one or more UnifiedPush distributors are available */
private fun isUnifiedPushAvailable(context: Context): Boolean =
UnifiedPush.getDistributors(context).isNotEmpty()

fun canEnablePushNotifications(context: Context, accountManager: AccountManager): Boolean =
isUnifiedPushAvailable(context) && !anyAccountNeedsMigration(accountManager)
fun canEnablePushNotifications(context: Context, accountManager: AccountManager): Boolean {
val unifiedPushAvailable = isUnifiedPushAvailable(context)
val anyAccountNeedsMigration = anyAccountNeedsMigration(accountManager)

NotificationConfig.unifiedPushAvailable = unifiedPushAvailable
NotificationConfig.anyAccountNeedsMigration = anyAccountNeedsMigration

return unifiedPushAvailable && !anyAccountNeedsMigration
}

suspend fun enablePushNotificationsWithFallback(context: Context, api: MastodonApi, accountManager: AccountManager) {
Timber.d("Enabling push notifications with fallback")
if (!canEnablePushNotifications(context, accountManager)) {
Timber.d("Cannot enable push notifications, switching to pull")
NotificationConfig.notificationMethod = NotificationConfig.Method.Pull
accountManager.accounts.map {
NotificationConfig.notificationMethodAccount[it.fullName] = NotificationConfig.Method.Pull
}
// No UP distributors
enablePullNotifications(context)
return
}

NotificationConfig.notificationMethod = NotificationConfig.Method.Push

val nm = context.getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager

accountManager.accounts.forEach {
Expand All @@ -153,6 +180,7 @@ private fun disablePushNotifications(context: Context, accountManager: AccountMa
}

fun disableAllNotifications(context: Context, accountManager: AccountManager) {
Timber.d("Disabling all notifications")
disablePushNotifications(context, accountManager)
disablePullNotifications(context)
}
Expand Down Expand Up @@ -207,18 +235,21 @@ suspend fun registerUnifiedPushEndpoint(
}

// Synchronize the enabled / disabled state of notifications with server-side subscription
suspend fun updateUnifiedPushSubscription(context: Context, api: MastodonApi, accountManager: AccountManager, account: AccountEntity) {
withContext(Dispatchers.IO) {
api.updatePushNotificationSubscription(
suspend fun updateUnifiedPushSubscription(context: Context, api: MastodonApi, accountManager: AccountManager, account: AccountEntity): Result<Unit, Throwable> {
return withContext(Dispatchers.IO) {
return@withContext api.updatePushNotificationSubscription(
"Bearer ${account.accessToken}",
account.domain,
buildSubscriptionData(context, account),
).onSuccess {
).fold({
Timber.d("UnifiedPush subscription updated for account %d", account.id)

account.pushServerKey = it.serverKey
accountManager.saveAccount(account)
}
Ok(Unit)
}, {
Timber.e(it, "Could not enable UnifiedPush subscription for account %d", account.id)
Err(it)
})
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ package app.pachli.components.preference
import android.os.Bundle
import androidx.preference.PreferenceFragmentCompat
import app.pachli.R
import app.pachli.components.notifications.androidNotificationsAreEnabled
import app.pachli.components.notifications.disablePullNotifications
import app.pachli.components.notifications.enablePullNotifications
import app.pachli.components.notifications.notificationsAreEnabled
import app.pachli.core.accounts.AccountManager
import app.pachli.core.database.model.AccountEntity
import app.pachli.core.preferences.PrefKeys
Expand All @@ -48,7 +48,7 @@ class NotificationPreferencesFragment : PreferenceFragmentCompat() {
isChecked = activeAccount.notificationsEnabled
setOnPreferenceChangeListener { _, newValue ->
updateAccount { it.notificationsEnabled = newValue as Boolean }
if (notificationsAreEnabled(context, accountManager)) {
if (androidNotificationsAreEnabled(context, accountManager)) {
enablePullNotifications(context)
} else {
disablePullNotifications(context)
Expand Down
Loading

0 comments on commit 23e3cf1

Please sign in to comment.