Skip to content

Commit 7ed362b

Browse files
bmartyElementBot
bmarty
and
ElementBot
authored
Push: improve Push history screen, log and stored data (#4601)
* Add adb tools to help with doze mode and app standby * Add info about the device state when an error occurs in push. * Keep more events in the DB. * Push history: add confirmation dialog when resetting the data * Push history: add a filter to see only the errors * Update screenshots * Push history: print out invalid/ignored data received. * Increase log level for push, to make such log more visible. It also appears that sometimes Timber.d are not present in the rageshakes. * Log priority * Do not include device state for invalid/ignored event. * Fix tests. * Fix format issue. * Fix mistake in code blocks and do not filter when not necessary. * Improve formatting and add missing unit test. * Reduce nesting of blocks. --------- Co-authored-by: ElementBot <[email protected]>
1 parent 653416f commit 7ed362b

File tree

30 files changed

+388
-58
lines changed

30 files changed

+388
-58
lines changed

‎libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/history/DefaultPushHistoryService.kt‎

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,13 @@
77

88
package io.element.android.libraries.push.impl.history
99

10+
import android.content.Context
11+
import android.os.Build
12+
import android.os.PowerManager
13+
import androidx.core.content.getSystemService
1014
import com.squareup.anvil.annotations.ContributesBinding
1115
import io.element.android.libraries.di.AppScope
16+
import io.element.android.libraries.di.ApplicationContext
1217
import io.element.android.libraries.matrix.api.core.EventId
1318
import io.element.android.libraries.matrix.api.core.RoomId
1419
import io.element.android.libraries.matrix.api.core.SessionId
@@ -21,15 +26,38 @@ import javax.inject.Inject
2126
class DefaultPushHistoryService @Inject constructor(
2227
private val pushDatabase: PushDatabase,
2328
private val systemClock: SystemClock,
29+
@ApplicationContext context: Context,
2430
) : PushHistoryService {
31+
private val powerManager = context.getSystemService<PowerManager>()
32+
private val packageName = context.packageName
33+
2534
override fun onPushReceived(
2635
providerInfo: String,
2736
eventId: EventId?,
2837
roomId: RoomId?,
2938
sessionId: SessionId?,
3039
hasBeenResolved: Boolean,
40+
includeDeviceState: Boolean,
3141
comment: String?,
3242
) {
43+
val finalComment = buildString {
44+
append(comment.orEmpty())
45+
if (includeDeviceState && powerManager != null) {
46+
// Add info about device state
47+
append("\n")
48+
append(" - Idle: ${powerManager.isDeviceIdleMode}\n")
49+
append(" - Power Save Mode: ${powerManager.isPowerSaveMode}\n")
50+
append(" - Ignoring Battery Optimizations: ${powerManager.isIgnoringBatteryOptimizations(packageName)}\n")
51+
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
52+
append(" - Device Light Idle Mode: ${powerManager.isDeviceLightIdleMode}\n")
53+
append(" - Low Power Standby Enabled: ${powerManager.isLowPowerStandbyEnabled}\n")
54+
}
55+
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) {
56+
append(" - Exempt from Low Power Standby: ${powerManager.isExemptFromLowPowerStandby}\n")
57+
}
58+
}
59+
}.takeIf { it.isNotEmpty() }
60+
3361
pushDatabase.pushHistoryQueries.insertPushHistory(
3462
PushHistory(
3563
pushDate = systemClock.epochMillis(),
@@ -38,11 +66,11 @@ class DefaultPushHistoryService @Inject constructor(
3866
roomId = roomId?.value,
3967
sessionId = sessionId?.value,
4068
hasBeenResolved = if (hasBeenResolved) 1 else 0,
41-
comment = comment,
69+
comment = finalComment,
4270
)
4371
)
4472

45-
// Keep only the last 100 events
46-
pushDatabase.pushHistoryQueries.removeOldest(100)
73+
// Keep only the last 1_000 events
74+
pushDatabase.pushHistoryQueries.removeOldest(1_000)
4775
}
4876
}

‎libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/history/PushHistoryService.kt‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,19 +22,22 @@ interface PushHistoryService {
2222
roomId: RoomId?,
2323
sessionId: SessionId?,
2424
hasBeenResolved: Boolean,
25+
includeDeviceState: Boolean,
2526
comment: String?,
2627
)
2728
}
2829

2930
fun PushHistoryService.onInvalidPushReceived(
3031
providerInfo: String,
32+
data: String,
3133
) = onPushReceived(
3234
providerInfo = providerInfo,
3335
eventId = null,
3436
roomId = null,
3537
sessionId = null,
3638
hasBeenResolved = false,
37-
comment = "Invalid push data",
39+
includeDeviceState = false,
40+
comment = "Invalid or ignored push data:\n$data",
3841
)
3942

4043
fun PushHistoryService.onUnableToRetrieveSession(
@@ -48,6 +51,7 @@ fun PushHistoryService.onUnableToRetrieveSession(
4851
roomId = roomId,
4952
sessionId = null,
5053
hasBeenResolved = false,
54+
includeDeviceState = true,
5155
comment = "Unable to retrieve session: $reason",
5256
)
5357

@@ -63,6 +67,7 @@ fun PushHistoryService.onUnableToResolveEvent(
6367
roomId = roomId,
6468
sessionId = sessionId,
6569
hasBeenResolved = false,
70+
includeDeviceState = true,
6671
comment = "Unable to resolve event: $reason",
6772
)
6873

@@ -78,6 +83,7 @@ fun PushHistoryService.onSuccess(
7883
roomId = roomId,
7984
sessionId = sessionId,
8085
hasBeenResolved = true,
86+
includeDeviceState = false,
8187
comment = buildString {
8288
append("Success")
8389
if (comment.isNullOrBlank().not()) {
@@ -94,5 +100,6 @@ fun PushHistoryService.onDiagnosticPush(
94100
roomId = null,
95101
sessionId = null,
96102
hasBeenResolved = true,
103+
includeDeviceState = false,
97104
comment = "Diagnostic push",
98105
)

‎libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,9 @@ class DefaultPushHandler @Inject constructor(
7272
}
7373
}
7474

75-
override suspend fun handleInvalid(providerInfo: String) {
75+
override suspend fun handleInvalid(providerInfo: String, data: String) {
7676
incrementPushDataStore.incrementPushCounter()
77-
pushHistoryService.onInvalidPushReceived(providerInfo)
77+
pushHistoryService.onInvalidPushReceived(providerInfo, data)
7878
}
7979

8080
/**

‎libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/history/FakePushHistoryService.kt‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,17 @@ class FakePushHistoryService(
1919
RoomId?,
2020
SessionId?,
2121
Boolean,
22+
Boolean,
2223
String?
23-
) -> Unit = { _, _, _, _, _, _ -> lambdaError() }
24+
) -> Unit = { _, _, _, _, _, _, _ -> lambdaError() }
2425
) : PushHistoryService {
2526
override fun onPushReceived(
2627
providerInfo: String,
2728
eventId: EventId?,
2829
roomId: RoomId?,
2930
sessionId: SessionId?,
3031
hasBeenResolved: Boolean,
32+
includeDeviceState: Boolean,
3133
comment: String?,
3234
) {
3335
onPushReceivedResult(
@@ -36,6 +38,7 @@ class FakePushHistoryService(
3638
roomId,
3739
sessionId,
3840
hasBeenResolved,
41+
includeDeviceState,
3942
comment
4043
)
4144
}

‎libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandlerTest.kt‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -60,20 +60,20 @@ class DefaultPushHandlerTest {
6060
@Test
6161
fun `check handleInvalid behavior`() = runTest {
6262
val incrementPushCounterResult = lambdaRecorder<Unit> {}
63-
val onPushReceivedResult = lambdaRecorder<String, EventId?, RoomId?, SessionId?, Boolean, String?, Unit> { _, _, _, _, _, _ -> }
63+
val onPushReceivedResult = lambdaRecorder<String, EventId?, RoomId?, SessionId?, Boolean, Boolean, String?, Unit> { _, _, _, _, _, _, _ -> }
6464
val pushHistoryService = FakePushHistoryService(
6565
onPushReceivedResult = onPushReceivedResult,
6666
)
6767
val defaultPushHandler = createDefaultPushHandler(
6868
incrementPushCounterResult = incrementPushCounterResult,
6969
pushHistoryService = pushHistoryService,
7070
)
71-
defaultPushHandler.handleInvalid(A_PUSHER_INFO)
71+
defaultPushHandler.handleInvalid(A_PUSHER_INFO, "data")
7272
incrementPushCounterResult.assertions()
7373
.isCalledOnce()
7474
onPushReceivedResult.assertions()
7575
.isCalledOnce()
76-
.with(value(A_PUSHER_INFO), value(null), value(null), value(null), value(false), value("Invalid push data"))
76+
.with(value(A_PUSHER_INFO), value(null), value(null), value(null), value(false), value(false), value("Invalid or ignored push data:\ndata"))
7777
}
7878

7979
@Test
@@ -85,7 +85,7 @@ class DefaultPushHandlerTest {
8585
}
8686
val onNotifiableEventReceived = lambdaRecorder<NotifiableEvent, Unit> {}
8787
val incrementPushCounterResult = lambdaRecorder<Unit> {}
88-
val onPushReceivedResult = lambdaRecorder<String, EventId?, RoomId?, SessionId?, Boolean, String?, Unit> { _, _, _, _, _, _ -> }
88+
val onPushReceivedResult = lambdaRecorder<String, EventId?, RoomId?, SessionId?, Boolean, Boolean, String?, Unit> { _, _, _, _, _, _, _ -> }
8989
val pushHistoryService = FakePushHistoryService(
9090
onPushReceivedResult = onPushReceivedResult,
9191
)
@@ -133,7 +133,7 @@ class DefaultPushHandlerTest {
133133
unread = 0,
134134
clientSecret = A_SECRET,
135135
)
136-
val onPushReceivedResult = lambdaRecorder<String, EventId?, RoomId?, SessionId?, Boolean, String?, Unit> { _, _, _, _, _, _ -> }
136+
val onPushReceivedResult = lambdaRecorder<String, EventId?, RoomId?, SessionId?, Boolean, Boolean, String?, Unit> { _, _, _, _, _, _, _ -> }
137137
val pushHistoryService = FakePushHistoryService(
138138
onPushReceivedResult = onPushReceivedResult,
139139
)
@@ -176,7 +176,7 @@ class DefaultPushHandlerTest {
176176
unread = 0,
177177
clientSecret = A_SECRET,
178178
)
179-
val onPushReceivedResult = lambdaRecorder<String, EventId?, RoomId?, SessionId?, Boolean, String?, Unit> { _, _, _, _, _, _ -> }
179+
val onPushReceivedResult = lambdaRecorder<String, EventId?, RoomId?, SessionId?, Boolean, Boolean, String?, Unit> { _, _, _, _, _, _, _ -> }
180180
val pushHistoryService = FakePushHistoryService(
181181
onPushReceivedResult = onPushReceivedResult,
182182
)
@@ -221,7 +221,7 @@ class DefaultPushHandlerTest {
221221
unread = 0,
222222
clientSecret = A_SECRET,
223223
)
224-
val onPushReceivedResult = lambdaRecorder<String, EventId?, RoomId?, SessionId?, Boolean, String?, Unit> { _, _, _, _, _, _ -> }
224+
val onPushReceivedResult = lambdaRecorder<String, EventId?, RoomId?, SessionId?, Boolean, Boolean, String?, Unit> { _, _, _, _, _, _, _ -> }
225225
val pushHistoryService = FakePushHistoryService(
226226
onPushReceivedResult = onPushReceivedResult,
227227
)
@@ -263,7 +263,7 @@ class DefaultPushHandlerTest {
263263
unread = 0,
264264
clientSecret = A_SECRET,
265265
)
266-
val onPushReceivedResult = lambdaRecorder<String, EventId?, RoomId?, SessionId?, Boolean, String?, Unit> { _, _, _, _, _, _ -> }
266+
val onPushReceivedResult = lambdaRecorder<String, EventId?, RoomId?, SessionId?, Boolean, Boolean, String?, Unit> { _, _, _, _, _, _, _ -> }
267267
val pushHistoryService = FakePushHistoryService(
268268
onPushReceivedResult = onPushReceivedResult,
269269
)
@@ -290,7 +290,7 @@ class DefaultPushHandlerTest {
290290
.isNeverCalled()
291291
onPushReceivedResult.assertions()
292292
.isCalledOnce()
293-
.with(any(), value(AN_EVENT_ID), value(A_ROOM_ID), value(A_USER_ID), value(false), any())
293+
.with(any(), value(AN_EVENT_ID), value(A_ROOM_ID), value(A_USER_ID), value(false), value(true), any())
294294
}
295295

296296
@Test
@@ -314,7 +314,7 @@ class DefaultPushHandlerTest {
314314
> { _, _, _, _, _, _, _, _ -> }
315315
val elementCallEntryPoint = FakeElementCallEntryPoint(handleIncomingCallResult = handleIncomingCallLambda)
316316
val onNotifiableEventReceived = lambdaRecorder<NotifiableEvent, Unit> {}
317-
val onPushReceivedResult = lambdaRecorder<String, EventId?, RoomId?, SessionId?, Boolean, String?, Unit> { _, _, _, _, _, _ -> }
317+
val onPushReceivedResult = lambdaRecorder<String, EventId?, RoomId?, SessionId?, Boolean, Boolean, String?, Unit> { _, _, _, _, _, _, _ -> }
318318
val pushHistoryService = FakePushHistoryService(
319319
onPushReceivedResult = onPushReceivedResult,
320320
)
@@ -359,7 +359,7 @@ class DefaultPushHandlerTest {
359359
Unit,
360360
> { _, _, _, _, _, _, _, _ -> }
361361
val elementCallEntryPoint = FakeElementCallEntryPoint(handleIncomingCallResult = handleIncomingCallLambda)
362-
val onPushReceivedResult = lambdaRecorder<String, EventId?, RoomId?, SessionId?, Boolean, String?, Unit> { _, _, _, _, _, _ -> }
362+
val onPushReceivedResult = lambdaRecorder<String, EventId?, RoomId?, SessionId?, Boolean, Boolean, String?, Unit> { _, _, _, _, _, _, _ -> }
363363
val pushHistoryService = FakePushHistoryService(
364364
onPushReceivedResult = onPushReceivedResult,
365365
)
@@ -403,7 +403,7 @@ class DefaultPushHandlerTest {
403403
Unit,
404404
> { _, _, _, _, _, _, _, _ -> }
405405
val elementCallEntryPoint = FakeElementCallEntryPoint(handleIncomingCallResult = handleIncomingCallLambda)
406-
val onPushReceivedResult = lambdaRecorder<String, EventId?, RoomId?, SessionId?, Boolean, String?, Unit> { _, _, _, _, _, _ -> }
406+
val onPushReceivedResult = lambdaRecorder<String, EventId?, RoomId?, SessionId?, Boolean, Boolean, String?, Unit> { _, _, _, _, _, _, _ -> }
407407
val pushHistoryService = FakePushHistoryService(
408408
onPushReceivedResult = onPushReceivedResult,
409409
)
@@ -444,7 +444,7 @@ class DefaultPushHandlerTest {
444444
)
445445
val onRedactedEventReceived = lambdaRecorder<ResolvedPushEvent.Redaction, Unit> { }
446446
val incrementPushCounterResult = lambdaRecorder<Unit> {}
447-
val onPushReceivedResult = lambdaRecorder<String, EventId?, RoomId?, SessionId?, Boolean, String?, Unit> { _, _, _, _, _, _ -> }
447+
val onPushReceivedResult = lambdaRecorder<String, EventId?, RoomId?, SessionId?, Boolean, Boolean, String?, Unit> { _, _, _, _, _, _, _ -> }
448448
val pushHistoryService = FakePushHistoryService(
449449
onPushReceivedResult = onPushReceivedResult,
450450
)
@@ -476,7 +476,7 @@ class DefaultPushHandlerTest {
476476
clientSecret = A_SECRET,
477477
)
478478
val diagnosticPushHandler = DiagnosticPushHandler()
479-
val onPushReceivedResult = lambdaRecorder<String, EventId?, RoomId?, SessionId?, Boolean, String?, Unit> { _, _, _, _, _, _ -> }
479+
val onPushReceivedResult = lambdaRecorder<String, EventId?, RoomId?, SessionId?, Boolean, Boolean, String?, Unit> { _, _, _, _, _, _, _ -> }
480480
val pushHistoryService = FakePushHistoryService(
481481
onPushReceivedResult = onPushReceivedResult,
482482
)

‎libraries/push/test/src/main/kotlin/io/element/android/libraries/push/test/FakePushService.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ class FakePushService(
8989
pushCounterFlow.value = counter
9090
}
9191

92-
override suspend fun resetPushHistory() {
92+
override suspend fun resetPushHistory() = simulateLongTask {
9393
resetPushHistoryResult()
9494
}
9595
}

‎libraries/push/test/src/main/kotlin/io/element/android/libraries/push/test/test/FakePushHandler.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,13 @@ import io.element.android.tests.testutils.lambda.lambdaError
1313

1414
class FakePushHandler(
1515
private val handleResult: (PushData, String) -> Unit = { _, _ -> lambdaError() },
16-
private val handleInvalidResult: (String) -> Unit = { lambdaError() },
16+
private val handleInvalidResult: (String, String) -> Unit = { _, _ -> lambdaError() },
1717
) : PushHandler {
1818
override suspend fun handle(pushData: PushData, providerInfo: String) {
1919
handleResult(pushData, providerInfo)
2020
}
2121

22-
override suspend fun handleInvalid(providerInfo: String) {
23-
handleInvalidResult(providerInfo)
22+
override suspend fun handleInvalid(providerInfo: String, data: String) {
23+
handleInvalidResult(providerInfo, data)
2424
}
2525
}

‎libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/PushHandler.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,6 @@ interface PushHandler {
1515

1616
suspend fun handleInvalid(
1717
providerInfo: String,
18+
data: String,
1819
)
1920
}

‎libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/VectorFirebaseMessagingService.kt‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,23 @@ class VectorFirebaseMessagingService : FirebaseMessagingService() {
3131
}
3232

3333
override fun onNewToken(token: String) {
34-
Timber.tag(loggerTag.value).d("New Firebase token")
34+
Timber.tag(loggerTag.value).w("New Firebase token")
3535
coroutineScope.launch {
3636
firebaseNewTokenHandler.handle(token)
3737
}
3838
}
3939

4040
override fun onMessageReceived(message: RemoteMessage) {
41-
Timber.tag(loggerTag.value).d("New Firebase message")
41+
Timber.tag(loggerTag.value).w("New Firebase message. Priority: ${message.priority}/${message.originalPriority}")
4242
coroutineScope.launch {
4343
val pushData = pushParser.parse(message.data)
4444
if (pushData == null) {
4545
Timber.tag(loggerTag.value).w("Invalid data received from Firebase")
4646
pushHandler.handleInvalid(
4747
providerInfo = FirebaseConfig.NAME,
48+
data = message.data.keys.joinToString("\n") {
49+
"$it: ${message.data[it]}"
50+
},
4851
)
4952
} else {
5053
pushHandler.handle(

‎libraries/pushproviders/firebase/src/test/kotlin/io/element/android/libraries/pushproviders/firebase/VectorFirebaseMessagingServiceTest.kt‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,24 @@ import org.robolectric.RobolectricTestRunner
3232
class VectorFirebaseMessagingServiceTest {
3333
@Test
3434
fun `test receiving invalid data`() = runTest {
35-
val lambda = lambdaRecorder<String, Unit> {}
35+
val lambda = lambdaRecorder<String, String, Unit> { _, _ -> }
3636
val vectorFirebaseMessagingService = createVectorFirebaseMessagingService(
3737
pushHandler = FakePushHandler(handleInvalidResult = lambda)
3838
)
39-
vectorFirebaseMessagingService.onMessageReceived(RemoteMessage(Bundle()))
39+
vectorFirebaseMessagingService.onMessageReceived(
40+
message = RemoteMessage(
41+
Bundle().apply {
42+
putString("a", "A")
43+
putString("b", "B")
44+
}
45+
)
46+
)
4047
runCurrent()
4148
lambda.assertions().isCalledOnce()
49+
.with(
50+
value(FirebaseConfig.NAME),
51+
value("a: A\nb: B"),
52+
)
4253
}
4354

4455
@Test

0 commit comments

Comments
 (0)