diff --git a/changelog.d/7695.bugfix b/changelog.d/7695.bugfix new file mode 100644 index 0000000000..7ec0805bce --- /dev/null +++ b/changelog.d/7695.bugfix @@ -0,0 +1 @@ +[Rich text editor] Add error tracking for rich text editor diff --git a/vector-app/src/main/java/im/vector/app/core/di/SingletonModule.kt b/vector-app/src/main/java/im/vector/app/core/di/SingletonModule.kt index 28ca761ace..21a46b0757 100644 --- a/vector-app/src/main/java/im/vector/app/core/di/SingletonModule.kt +++ b/vector-app/src/main/java/im/vector/app/core/di/SingletonModule.kt @@ -46,6 +46,7 @@ import im.vector.app.core.utils.AndroidSystemSettingsProvider import im.vector.app.core.utils.SystemSettingsProvider import im.vector.app.features.analytics.AnalyticsTracker import im.vector.app.features.analytics.VectorAnalytics +import im.vector.app.features.analytics.errors.ErrorTracker import im.vector.app.features.analytics.impl.DefaultVectorAnalytics import im.vector.app.features.analytics.metrics.VectorPlugins import im.vector.app.features.invite.AutoAcceptInvites @@ -84,6 +85,9 @@ import javax.inject.Singleton @Binds abstract fun bindVectorAnalytics(analytics: DefaultVectorAnalytics): VectorAnalytics + @Binds + abstract fun bindErrorTracker(analytics: DefaultVectorAnalytics): ErrorTracker + @Binds abstract fun bindAnalyticsTracker(analytics: DefaultVectorAnalytics): AnalyticsTracker diff --git a/vector/src/main/java/im/vector/app/features/analytics/VectorAnalytics.kt b/vector/src/main/java/im/vector/app/features/analytics/VectorAnalytics.kt index 7d11f93883..802ba08092 100644 --- a/vector/src/main/java/im/vector/app/features/analytics/VectorAnalytics.kt +++ b/vector/src/main/java/im/vector/app/features/analytics/VectorAnalytics.kt @@ -16,9 +16,10 @@ package im.vector.app.features.analytics +import im.vector.app.features.analytics.errors.ErrorTracker import kotlinx.coroutines.flow.Flow -interface VectorAnalytics : AnalyticsTracker { +interface VectorAnalytics : AnalyticsTracker, ErrorTracker { /** * Return a Flow of Boolean, true if the user has given their consent. */ diff --git a/vector/src/main/java/im/vector/app/features/analytics/errors/ErrorTracker.kt b/vector/src/main/java/im/vector/app/features/analytics/errors/ErrorTracker.kt new file mode 100644 index 0000000000..8ad6bfffc0 --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/analytics/errors/ErrorTracker.kt @@ -0,0 +1,21 @@ +/* + * 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.errors + +interface ErrorTracker { + fun trackError(throwable: Throwable) +} diff --git a/vector/src/main/java/im/vector/app/features/analytics/impl/DefaultVectorAnalytics.kt b/vector/src/main/java/im/vector/app/features/analytics/impl/DefaultVectorAnalytics.kt index 553d699d86..ca7608166c 100644 --- a/vector/src/main/java/im/vector/app/features/analytics/impl/DefaultVectorAnalytics.kt +++ b/vector/src/main/java/im/vector/app/features/analytics/impl/DefaultVectorAnalytics.kt @@ -41,7 +41,7 @@ private val IGNORED_OPTIONS: Options? = null @Singleton class DefaultVectorAnalytics @Inject constructor( postHogFactory: PostHogFactory, - private val sentryFactory: SentryFactory, + private val sentryAnalytics: SentryAnalytics, analyticsConfig: AnalyticsConfig, private val analyticsStore: AnalyticsStore, private val lateInitUserPropertiesFactory: LateInitUserPropertiesFactory, @@ -97,7 +97,7 @@ class DefaultVectorAnalytics @Inject constructor( setAnalyticsId("") // Close Sentry SDK. - sentryFactory.stopSentry() + sentryAnalytics.stopSentry() } private fun observeAnalyticsId() { @@ -135,8 +135,8 @@ class DefaultVectorAnalytics @Inject constructor( private fun initOrStopSentry() { userConsent?.let { when (it) { - true -> sentryFactory.initSentry() - false -> sentryFactory.stopSentry() + true -> sentryAnalytics.initSentry() + false -> sentryAnalytics.stopSentry() } } } @@ -180,4 +180,10 @@ class DefaultVectorAnalytics @Inject constructor( putAll(this@toPostHogUserProperties.filter { it.value != null }) } } + + override fun trackError(throwable: Throwable) { + sentryAnalytics + .takeIf { userConsent == true } + ?.trackError(throwable) + } } diff --git a/vector/src/main/java/im/vector/app/features/analytics/impl/SentryFactory.kt b/vector/src/main/java/im/vector/app/features/analytics/impl/SentryAnalytics.kt similarity index 88% rename from vector/src/main/java/im/vector/app/features/analytics/impl/SentryFactory.kt rename to vector/src/main/java/im/vector/app/features/analytics/impl/SentryAnalytics.kt index a000f2a77a..21721a31ae 100644 --- a/vector/src/main/java/im/vector/app/features/analytics/impl/SentryFactory.kt +++ b/vector/src/main/java/im/vector/app/features/analytics/impl/SentryAnalytics.kt @@ -18,6 +18,7 @@ package im.vector.app.features.analytics.impl import android.content.Context import im.vector.app.features.analytics.AnalyticsConfig +import im.vector.app.features.analytics.errors.ErrorTracker import im.vector.app.features.analytics.log.analyticsTag import io.sentry.Sentry import io.sentry.SentryOptions @@ -25,10 +26,10 @@ import io.sentry.android.core.SentryAndroid import timber.log.Timber import javax.inject.Inject -class SentryFactory @Inject constructor( +class SentryAnalytics @Inject constructor( private val context: Context, private val analyticsConfig: AnalyticsConfig, -) { +) : ErrorTracker { fun initSentry() { Timber.tag(analyticsTag.value).d("Initializing Sentry") @@ -47,4 +48,8 @@ class SentryFactory @Inject constructor( Timber.tag(analyticsTag.value).d("Stopping Sentry") Sentry.close() } + + override fun trackError(throwable: Throwable) { + Sentry.captureException(throwable) + } } diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/composer/MessageComposerFragment.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/composer/MessageComposerFragment.kt index 97e74785ec..bf9e0ae726 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/composer/MessageComposerFragment.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/composer/MessageComposerFragment.kt @@ -60,6 +60,7 @@ import im.vector.app.core.utils.onPermissionDeniedDialog import im.vector.app.core.utils.registerForPermissionsResult import im.vector.app.databinding.FragmentComposerBinding import im.vector.app.features.VectorFeatures +import im.vector.app.features.analytics.errors.ErrorTracker import im.vector.app.features.attachments.AttachmentType import im.vector.app.features.attachments.AttachmentTypeSelectorBottomSheet import im.vector.app.features.attachments.AttachmentTypeSelectorSharedAction @@ -116,6 +117,7 @@ class MessageComposerFragment : VectorBaseFragment(), A @Inject lateinit var vectorFeatures: VectorFeatures @Inject lateinit var buildMeta: BuildMeta @Inject lateinit var session: Session + @Inject lateinit var errorTracker: ErrorTracker private val roomId: String get() = withState(timelineViewModel) { it.roomId } @@ -171,6 +173,7 @@ class MessageComposerFragment : VectorBaseFragment(), A views.composerLayout.isGone = vectorPreferences.isRichTextEditorEnabled() views.richTextComposerLayout.isVisible = vectorPreferences.isRichTextEditorEnabled() + views.richTextComposerLayout.setOnErrorListener(errorTracker::trackError) messageComposerViewModel.observeViewEvents { when (it) { diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/composer/RichTextComposerLayout.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/composer/RichTextComposerLayout.kt index 48459b5c06..16234c3766 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/composer/RichTextComposerLayout.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/composer/RichTextComposerLayout.kt @@ -49,10 +49,11 @@ import im.vector.app.databinding.ComposerRichTextLayoutBinding import im.vector.app.databinding.ViewRichTextMenuButtonBinding import io.element.android.wysiwyg.EditorEditText import io.element.android.wysiwyg.inputhandlers.models.InlineFormat +import io.element.android.wysiwyg.utils.RustErrorCollector import uniffi.wysiwyg_composer.ActionState import uniffi.wysiwyg_composer.ComposerAction -class RichTextComposerLayout @JvmOverloads constructor( +internal class RichTextComposerLayout @JvmOverloads constructor( context: Context, attrs: AttributeSet? = null, defStyleAttr: Int = 0 @@ -248,10 +249,15 @@ class RichTextComposerLayout @JvmOverloads constructor( updateMenuStateFor(action, state) } } - updateEditTextVisibility() } + fun setOnErrorListener(onError: (e: RichTextEditorException) -> Unit) { + views.richTextComposerEditText.rustErrorCollector = RustErrorCollector { + onError(RichTextEditorException(it)) + } + } + private fun updateEditTextVisibility() { views.richTextComposerEditText.isVisible = isTextFormattingEnabled views.richTextMenu.isVisible = isTextFormattingEnabled diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/composer/RichTextEditorException.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/composer/RichTextEditorException.kt new file mode 100644 index 0000000000..9bdef59ae3 --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/composer/RichTextEditorException.kt @@ -0,0 +1,21 @@ +/* + * 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.home.room.detail.composer + +internal class RichTextEditorException( + cause: Throwable, +) : Exception(cause) diff --git a/vector/src/test/java/im/vector/app/features/analytics/impl/DefaultVectorAnalyticsTest.kt b/vector/src/test/java/im/vector/app/features/analytics/impl/DefaultVectorAnalyticsTest.kt index be53f1b908..3fd0528a19 100644 --- a/vector/src/test/java/im/vector/app/features/analytics/impl/DefaultVectorAnalyticsTest.kt +++ b/vector/src/test/java/im/vector/app/features/analytics/impl/DefaultVectorAnalyticsTest.kt @@ -23,7 +23,7 @@ import im.vector.app.test.fakes.FakeAnalyticsStore import im.vector.app.test.fakes.FakeLateInitUserPropertiesFactory import im.vector.app.test.fakes.FakePostHog import im.vector.app.test.fakes.FakePostHogFactory -import im.vector.app.test.fakes.FakeSentryFactory +import im.vector.app.test.fakes.FakeSentryAnalytics import im.vector.app.test.fixtures.AnalyticsConfigFixture.anAnalyticsConfig import im.vector.app.test.fixtures.aUserProperties import im.vector.app.test.fixtures.aVectorAnalyticsEvent @@ -46,11 +46,11 @@ class DefaultVectorAnalyticsTest { private val fakePostHog = FakePostHog() private val fakeAnalyticsStore = FakeAnalyticsStore() private val fakeLateInitUserPropertiesFactory = FakeLateInitUserPropertiesFactory() - private val fakeSentryFactory = FakeSentryFactory() + private val fakeSentryAnalytics = FakeSentryAnalytics() private val defaultVectorAnalytics = DefaultVectorAnalytics( postHogFactory = FakePostHogFactory(fakePostHog.instance).instance, - sentryFactory = fakeSentryFactory.instance, + sentryAnalytics = fakeSentryAnalytics.instance, analyticsStore = fakeAnalyticsStore.instance, globalScope = CoroutineScope(Dispatchers.Unconfined), analyticsConfig = anAnalyticsConfig(isEnabled = true), @@ -75,7 +75,7 @@ class DefaultVectorAnalyticsTest { fakePostHog.verifyOptOutStatus(optedOut = false) - fakeSentryFactory.verifySentryInit() + fakeSentryAnalytics.verifySentryInit() } @Test @@ -84,7 +84,7 @@ class DefaultVectorAnalyticsTest { fakePostHog.verifyOptOutStatus(optedOut = true) - fakeSentryFactory.verifySentryClose() + fakeSentryAnalytics.verifySentryClose() } @Test @@ -111,7 +111,7 @@ class DefaultVectorAnalyticsTest { fakePostHog.verifyReset() - fakeSentryFactory.verifySentryClose() + fakeSentryAnalytics.verifySentryClose() } @Test @@ -149,6 +149,25 @@ class DefaultVectorAnalyticsTest { fakePostHog.verifyNoEventTracking() } + + @Test + fun `given user has consented, when tracking exception, then submits to sentry`() = runTest { + fakeAnalyticsStore.givenUserContent(consent = true) + val exception = Exception("test") + + defaultVectorAnalytics.trackError(exception) + + fakeSentryAnalytics.verifySentryTrackError(exception) + } + + @Test + fun `given user has not consented, when tracking exception, then does not track to sentry`() = runTest { + fakeAnalyticsStore.givenUserContent(consent = false) + + defaultVectorAnalytics.trackError(Exception("test")) + + fakeSentryAnalytics.verifyNoErrorTracking() + } } private fun VectorAnalyticsScreen.toPostHogProperties(): Properties? { diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeSentryFactory.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeSentryAnalytics.kt similarity index 74% rename from vector/src/test/java/im/vector/app/test/fakes/FakeSentryFactory.kt rename to vector/src/test/java/im/vector/app/test/fakes/FakeSentryAnalytics.kt index 2628f80435..59f41543b0 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakeSentryFactory.kt +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeSentryAnalytics.kt @@ -16,15 +16,15 @@ package im.vector.app.test.fakes -import im.vector.app.features.analytics.impl.SentryFactory +import im.vector.app.features.analytics.impl.SentryAnalytics import io.mockk.every import io.mockk.mockk import io.mockk.verify -class FakeSentryFactory { +class FakeSentryAnalytics { private var isSentryEnabled = false - val instance = mockk().also { + val instance = mockk(relaxUnitFun = true).also { every { it.initSentry() } answers { isSentryEnabled = true } @@ -41,4 +41,13 @@ class FakeSentryFactory { fun verifySentryClose() { verify { instance.stopSentry() } } + + fun verifySentryTrackError(error: Throwable) { + verify { instance.trackError(error) } + } + + fun verifyNoErrorTracking() = + verify(inverse = true) { + instance.trackError(any()) + } }