Skip to content

Commit

Permalink
Add error tracking for rich text editor
Browse files Browse the repository at this point in the history
  • Loading branch information
jonnyandrew committed Dec 2, 2022
1 parent d580d4c commit 08d5975
Show file tree
Hide file tree
Showing 11 changed files with 82 additions and 19 deletions.
1 change: 1 addition & 0 deletions changelog.d/7695.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[Rich text editor] Add error tracking for rich text editor
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import com.posthog.android.Properties
import im.vector.app.core.di.NamedGlobalScope
import im.vector.app.features.analytics.AnalyticsConfig
import im.vector.app.features.analytics.VectorAnalytics
import im.vector.app.features.analytics.errors.ErrorTracker
import im.vector.app.features.analytics.itf.VectorAnalyticsEvent
import im.vector.app.features.analytics.itf.VectorAnalyticsScreen
import im.vector.app.features.analytics.log.analyticsTag
Expand All @@ -41,12 +42,12 @@ 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,
@NamedGlobalScope private val globalScope: CoroutineScope
) : VectorAnalytics {
) : VectorAnalytics, ErrorTracker by sentryAnalytics {

private val posthog: PostHog? = when {
analyticsConfig.isEnabled -> postHogFactory.createPosthog()
Expand Down Expand Up @@ -97,7 +98,7 @@ class DefaultVectorAnalytics @Inject constructor(
setAnalyticsId("")

// Close Sentry SDK.
sentryFactory.stopSentry()
sentryAnalytics.stopSentry()
}

private fun observeAnalyticsId() {
Expand Down Expand Up @@ -135,8 +136,8 @@ class DefaultVectorAnalytics @Inject constructor(
private fun initOrStopSentry() {
userConsent?.let {
when (it) {
true -> sentryFactory.initSentry()
false -> sentryFactory.stopSentry()
true -> sentryAnalytics.initSentry()
false -> sentryAnalytics.stopSentry()
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,18 @@ 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
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")
Expand All @@ -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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -116,6 +117,7 @@ class MessageComposerFragment : VectorBaseFragment<FragmentComposerBinding>(), 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 }

Expand Down Expand Up @@ -171,6 +173,7 @@ class MessageComposerFragment : VectorBaseFragment<FragmentComposerBinding>(), A

views.composerLayout.isGone = vectorPreferences.isRichTextEditorEnabled()
views.richTextComposerLayout.isVisible = vectorPreferences.isRichTextEditorEnabled()
views.richTextComposerLayout.setOnErrorListener(errorTracker::trackError)

messageComposerViewModel.observeViewEvents {
when (it) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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),
Expand All @@ -75,7 +75,7 @@ class DefaultVectorAnalyticsTest {

fakePostHog.verifyOptOutStatus(optedOut = false)

fakeSentryFactory.verifySentryInit()
fakeSentryAnalytics.verifySentryInit()
}

@Test
Expand All @@ -84,7 +84,7 @@ class DefaultVectorAnalyticsTest {

fakePostHog.verifyOptOutStatus(optedOut = true)

fakeSentryFactory.verifySentryClose()
fakeSentryAnalytics.verifySentryClose()
}

@Test
Expand All @@ -111,7 +111,7 @@ class DefaultVectorAnalyticsTest {

fakePostHog.verifyReset()

fakeSentryFactory.verifySentryClose()
fakeSentryAnalytics.verifySentryClose()
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<SentryFactory>().also {
val instance = mockk<SentryAnalytics>().also {
every { it.initSentry() } answers {
isSentryEnabled = true
}
Expand Down

0 comments on commit 08d5975

Please sign in to comment.