Fix ANRs when receiving push notifications (#6696)
In Sentry there are some reports of methods called when notifications are fetched that end up having ANRs. This looked weird because everything is asynchronous... but it's still running with a `Main` dispatcher. Using the `Default/computation` one instead should be the right call.
This commit is contained in:
parent
c59d46a766
commit
cd0fc5190e
2 changed files with 13 additions and 5 deletions
|
|
@ -11,6 +11,7 @@ package io.element.android.libraries.push.impl.push
|
||||||
import dev.zacsweers.metro.AppScope
|
import dev.zacsweers.metro.AppScope
|
||||||
import dev.zacsweers.metro.ContributesBinding
|
import dev.zacsweers.metro.ContributesBinding
|
||||||
import dev.zacsweers.metro.SingleIn
|
import dev.zacsweers.metro.SingleIn
|
||||||
|
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
|
||||||
import io.element.android.libraries.core.log.logger.LoggerTag
|
import io.element.android.libraries.core.log.logger.LoggerTag
|
||||||
import io.element.android.libraries.core.meta.BuildMeta
|
import io.element.android.libraries.core.meta.BuildMeta
|
||||||
import io.element.android.libraries.push.impl.db.PushRequest
|
import io.element.android.libraries.push.impl.db.PushRequest
|
||||||
|
|
@ -35,6 +36,7 @@ import kotlinx.coroutines.CoroutineScope
|
||||||
import kotlinx.coroutines.currentCoroutineContext
|
import kotlinx.coroutines.currentCoroutineContext
|
||||||
import kotlinx.coroutines.flow.first
|
import kotlinx.coroutines.flow.first
|
||||||
import kotlinx.coroutines.launch
|
import kotlinx.coroutines.launch
|
||||||
|
import kotlinx.coroutines.withContext
|
||||||
import timber.log.Timber
|
import timber.log.Timber
|
||||||
|
|
||||||
private val loggerTag = LoggerTag("PushHandler", LoggerTag.PushLoggerTag)
|
private val loggerTag = LoggerTag("PushHandler", LoggerTag.PushLoggerTag)
|
||||||
|
|
@ -53,6 +55,7 @@ class DefaultPushHandler(
|
||||||
private val workManagerScheduler: WorkManagerScheduler,
|
private val workManagerScheduler: WorkManagerScheduler,
|
||||||
private val syncPendingNotificationsRequestFactory: SyncPendingNotificationsRequestBuilder.Factory,
|
private val syncPendingNotificationsRequestFactory: SyncPendingNotificationsRequestBuilder.Factory,
|
||||||
resultProcessor: NotificationResultProcessor,
|
resultProcessor: NotificationResultProcessor,
|
||||||
|
private val dispatchers: CoroutineDispatchers,
|
||||||
) : PushHandler {
|
) : PushHandler {
|
||||||
init {
|
init {
|
||||||
resultProcessor.start()
|
resultProcessor.start()
|
||||||
|
|
@ -64,7 +67,7 @@ class DefaultPushHandler(
|
||||||
* @param pushData the data received in the push.
|
* @param pushData the data received in the push.
|
||||||
* @param providerInfo the provider info.
|
* @param providerInfo the provider info.
|
||||||
*/
|
*/
|
||||||
override suspend fun handle(pushData: PushData, providerInfo: String): Boolean {
|
override suspend fun handle(pushData: PushData, providerInfo: String): Boolean = withContext(dispatchers.computation) {
|
||||||
// Start measuring how long it takes to display a notification from when the push is received
|
// Start measuring how long it takes to display a notification from when the push is received
|
||||||
Timber.d("Calculating push-to-notification for event ${pushData.eventId}")
|
Timber.d("Calculating push-to-notification for event ${pushData.eventId}")
|
||||||
val parent = analyticsService.startLongRunningTransaction(AnalyticsLongRunningTransaction.PushToNotification(pushData.eventId.value))
|
val parent = analyticsService.startLongRunningTransaction(AnalyticsLongRunningTransaction.PushToNotification(pushData.eventId.value))
|
||||||
|
|
@ -81,7 +84,7 @@ class DefaultPushHandler(
|
||||||
}
|
}
|
||||||
|
|
||||||
// Diagnostic Push
|
// Diagnostic Push
|
||||||
return if (pushData.eventId == DefaultTestPush.TEST_EVENT_ID) {
|
if (pushData.eventId == DefaultTestPush.TEST_EVENT_ID) {
|
||||||
pushHistoryService.onDiagnosticPush(providerInfo)
|
pushHistoryService.onDiagnosticPush(providerInfo)
|
||||||
diagnosticPushHandler.handlePush()
|
diagnosticPushHandler.handlePush()
|
||||||
false
|
false
|
||||||
|
|
@ -90,7 +93,7 @@ class DefaultPushHandler(
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
override suspend fun handleInvalid(providerInfo: String, data: String) {
|
override suspend fun handleInvalid(providerInfo: String, data: String) = withContext(dispatchers.computation) {
|
||||||
incrementPushDataStore.incrementPushCounter()
|
incrementPushDataStore.incrementPushCounter()
|
||||||
pushHistoryService.onInvalidPushReceived(providerInfo, data)
|
pushHistoryService.onInvalidPushReceived(providerInfo, data)
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -11,6 +11,7 @@
|
||||||
package io.element.android.libraries.push.impl.push
|
package io.element.android.libraries.push.impl.push
|
||||||
|
|
||||||
import app.cash.turbine.test
|
import app.cash.turbine.test
|
||||||
|
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
|
||||||
import io.element.android.libraries.core.meta.BuildMeta
|
import io.element.android.libraries.core.meta.BuildMeta
|
||||||
import io.element.android.libraries.matrix.api.core.EventId
|
import io.element.android.libraries.matrix.api.core.EventId
|
||||||
import io.element.android.libraries.matrix.api.core.RoomId
|
import io.element.android.libraries.matrix.api.core.RoomId
|
||||||
|
|
@ -40,7 +41,9 @@ import io.element.android.services.toolbox.test.systemclock.FakeSystemClock
|
||||||
import io.element.android.tests.testutils.lambda.lambdaError
|
import io.element.android.tests.testutils.lambda.lambdaError
|
||||||
import io.element.android.tests.testutils.lambda.lambdaRecorder
|
import io.element.android.tests.testutils.lambda.lambdaRecorder
|
||||||
import io.element.android.tests.testutils.lambda.value
|
import io.element.android.tests.testutils.lambda.value
|
||||||
|
import io.element.android.tests.testutils.testCoroutineDispatchers
|
||||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||||
|
import kotlinx.coroutines.test.TestScope
|
||||||
import kotlinx.coroutines.test.advanceTimeBy
|
import kotlinx.coroutines.test.advanceTimeBy
|
||||||
import kotlinx.coroutines.test.runCurrent
|
import kotlinx.coroutines.test.runCurrent
|
||||||
import kotlinx.coroutines.test.runTest
|
import kotlinx.coroutines.test.runTest
|
||||||
|
|
@ -212,7 +215,7 @@ class DefaultPushHandlerTest {
|
||||||
.isCalledOnce()
|
.isCalledOnce()
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun createDefaultPushHandler(
|
private fun TestScope.createDefaultPushHandler(
|
||||||
incrementPushCounterResult: () -> Unit = { lambdaError() },
|
incrementPushCounterResult: () -> Unit = { lambdaError() },
|
||||||
userPushStore: FakeUserPushStore = FakeUserPushStore(),
|
userPushStore: FakeUserPushStore = FakeUserPushStore(),
|
||||||
pushClientSecret: PushClientSecret = FakePushClientSecret(),
|
pushClientSecret: PushClientSecret = FakePushClientSecret(),
|
||||||
|
|
@ -227,6 +230,7 @@ class DefaultPushHandlerTest {
|
||||||
start = {},
|
start = {},
|
||||||
stop = {},
|
stop = {},
|
||||||
),
|
),
|
||||||
|
dispatchers: CoroutineDispatchers = testCoroutineDispatchers(),
|
||||||
): DefaultPushHandler {
|
): DefaultPushHandler {
|
||||||
return DefaultPushHandler(
|
return DefaultPushHandler(
|
||||||
incrementPushDataStore = object : IncrementPushDataStore {
|
incrementPushDataStore = object : IncrementPushDataStore {
|
||||||
|
|
@ -246,7 +250,8 @@ class DefaultPushHandlerTest {
|
||||||
resultProcessor = resultProcessor,
|
resultProcessor = resultProcessor,
|
||||||
syncPendingNotificationsRequestFactory = SyncPendingNotificationsRequestBuilder.Factory {
|
syncPendingNotificationsRequestFactory = SyncPendingNotificationsRequestBuilder.Factory {
|
||||||
FakeSyncPendingNotificationsRequestBuilder()
|
FakeSyncPendingNotificationsRequestBuilder()
|
||||||
}
|
},
|
||||||
|
dispatchers = dispatchers,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue