Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Track decryption failures #4856

Merged
merged 2 commits into from
Jan 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/4719.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Analytics: Track Errors
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,11 @@ internal class MXMegolmDecryption(private val userId: String,
if (requestKeysOnFail) {
requestKeysForEvent(event, false)
}

throw MXCryptoError.Base(
MXCryptoError.ErrorType.UNKNOWN_MESSAGE_INDEX,
"UNKNOWN_MESSAGE_INDEX",
null)
}

val reason = String.format(MXCryptoError.OLM_REASON, throwable.olmException.message)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
/*
* Copyright (c) 2022 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package im.vector.app.features.analytics

import im.vector.app.core.flow.tickerFlow
import im.vector.app.core.time.Clock
import im.vector.app.features.analytics.plan.Error
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.cancel
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.launch
import org.matrix.android.sdk.api.session.crypto.MXCryptoError
import org.matrix.android.sdk.api.session.room.timeline.TimelineEvent
import javax.inject.Inject
import javax.inject.Singleton

private data class DecryptionFailure(
val timeStamp: Long,
val roomId: String,
val failedEventId: String,
val error: MXCryptoError.ErrorType
)

private const val GRACE_PERIOD_MILLIS = 4_000
private const val CHECK_INTERVAL = 2_000L

/**
* Tracks decryption errors that are visible to the user.
* When an error is reported it is not directly tracked via analytics, there is a grace period
* that gives the app a few seconds to get the key to decrypt.
*/
@Singleton
class DecryptionFailureTracker @Inject constructor(
private val vectorAnalytics: VectorAnalytics,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to myself: change to inject the new interface AnalyticsTracker from #4734

private val clock: Clock
) {

private val scope: CoroutineScope = CoroutineScope(SupervisorJob())
private val failures = mutableListOf<DecryptionFailure>()
private val alreadyReported = mutableListOf<String>()

init {
start()
}

fun start() {
tickerFlow(scope, CHECK_INTERVAL)
.onEach {
checkFailures()
}.launchIn(scope)
}

fun stop() {
scope.cancel()
}

fun e2eEventDisplayedInTimeline(event: TimelineEvent) {
scope.launch(Dispatchers.Default) {
val mCryptoError = event.root.mCryptoError
if (mCryptoError != null) {
addDecryptionFailure(DecryptionFailure(clock.epochMillis(), event.roomId, event.eventId, mCryptoError))
} else {
removeFailureForEventId(event.eventId)
}
}
}

/**
* Can be called when the timeline is disposed in order
* to grace those events as they are not anymore displayed on screen
* */
fun onTimeLineDisposed(roomId: String) {
scope.launch(Dispatchers.Default) {
synchronized(failures) {
failures.removeIf { it.roomId == roomId }
}
}
}

private fun addDecryptionFailure(failure: DecryptionFailure) {
// de duplicate
synchronized(failures) {
if (failures.none { it.failedEventId == failure.failedEventId }) {
failures.add(failure)
}
}
}

private fun removeFailureForEventId(eventId: String) {
synchronized(failures) {
failures.removeIf { it.failedEventId == eventId }
}
}

private fun checkFailures() {
val now = clock.epochMillis()
val aggregatedErrors: Map<Error.Name, List<String>>
synchronized(failures) {
val toReport = mutableListOf<DecryptionFailure>()
failures.removeAll { failure ->
(now - failure.timeStamp > GRACE_PERIOD_MILLIS).also {
if (it) {
toReport.add(failure)
}
}
}

aggregatedErrors = toReport
.groupBy { it.error.toAnalyticsErrorName() }
.mapValues {
it.value.map { it.failedEventId }
}
}

aggregatedErrors.forEach { aggregation ->
// there is now way to send the total/sum in posthog, so iterating
aggregation.value
// for now we ignore events already reported even if displayed again?
.filter { alreadyReported.contains(it).not() }
.forEach { failedEventId ->
vectorAnalytics.capture(Error(failedEventId, Error.Domain.E2EE, aggregation.key))
alreadyReported.add(failedEventId)
}
}
}

private fun MXCryptoError.ErrorType.toAnalyticsErrorName(): Error.Name {
return when (this) {
MXCryptoError.ErrorType.UNKNOWN_INBOUND_SESSION_ID -> Error.Name.OlmKeysNotSentError
MXCryptoError.ErrorType.OLM -> {
Error.Name.OlmUnspecifiedError
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small remark: why a different formatting here?

MXCryptoError.ErrorType.UNKNOWN_MESSAGE_INDEX -> Error.Name.OlmIndexError
else -> Error.Name.UnknownError
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import im.vector.app.core.mvrx.runCatchingToAsync
import im.vector.app.core.platform.VectorViewModel
import im.vector.app.core.resources.StringProvider
import im.vector.app.core.utils.BehaviorDataSource
import im.vector.app.features.analytics.DecryptionFailureTracker
import im.vector.app.features.call.conference.ConferenceEvent
import im.vector.app.features.call.conference.JitsiActiveConferenceHolder
import im.vector.app.features.call.conference.JitsiService
Expand Down Expand Up @@ -112,6 +113,7 @@ class RoomDetailViewModel @AssistedInject constructor(
private val directRoomHelper: DirectRoomHelper,
private val jitsiService: JitsiService,
private val activeConferenceHolder: JitsiActiveConferenceHolder,
private val decryptionFailureTracker: DecryptionFailureTracker,
timelineFactory: TimelineFactory
) : VectorViewModel<RoomDetailViewState, RoomDetailAction, RoomDetailViewEvents>(initialState),
Timeline.Listener, ChatEffectManager.Delegate, CallProtocolsChecker.Listener {
Expand Down Expand Up @@ -1083,6 +1085,7 @@ class RoomDetailViewModel @AssistedInject constructor(
override fun onCleared() {
timeline.dispose()
timeline.removeAllListeners()
decryptionFailureTracker.onTimeLineDisposed(room.roomId)
if (vectorPreferences.sendTypingNotifs()) {
room.userStopsTyping()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package im.vector.app.features.home.room.detail.timeline.factory
import im.vector.app.core.epoxy.TimelineEmptyItem
import im.vector.app.core.epoxy.TimelineEmptyItem_
import im.vector.app.core.epoxy.VectorEpoxyModel
import im.vector.app.features.analytics.DecryptionFailureTracker
import im.vector.app.features.home.room.detail.timeline.helper.TimelineEventVisibilityHelper
import org.matrix.android.sdk.api.session.events.model.EventType
import org.matrix.android.sdk.api.session.room.timeline.TimelineEvent
Expand All @@ -34,6 +35,7 @@ class TimelineItemFactory @Inject constructor(private val messageItemFactory: Me
private val widgetItemFactory: WidgetItemFactory,
private val verificationConclusionItemFactory: VerificationItemFactory,
private val callItemFactory: CallItemFactory,
private val decryptionFailureTracker: DecryptionFailureTracker,
private val timelineEventVisibilityHelper: TimelineEventVisibilityHelper) {

/**
Expand Down Expand Up @@ -122,6 +124,10 @@ class TimelineItemFactory @Inject constructor(private val messageItemFactory: Me
Timber.v("Type ${event.root.getClearType()} not handled")
defaultItemFactory.create(params)
}
}.also {
if (it != null && event.isEncrypted()) {
decryptionFailureTracker.e2eEventDisplayedInTimeline(event)
}
}
}
} catch (throwable: Throwable) {
Expand Down