diff --git a/app/metrics.yaml b/app/metrics.yaml index f42c383f87..278c28d472 100644 --- a/app/metrics.yaml +++ b/app/metrics.yaml @@ -2527,6 +2527,7 @@ error_page: A user encountered an error page extra_keys: error_type: + type: string description: "The error type of the error page encountered" bugs: - https://github.com/mozilla-mobile/fenix/issues/1242 diff --git a/app/src/androidTest/java/org/mozilla/fenix/AppRequestInterceptor.kt b/app/src/androidTest/java/org/mozilla/fenix/AppRequestInterceptor.kt index ef0986a8ab..b60ef45a3f 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/AppRequestInterceptor.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/AppRequestInterceptor.kt @@ -12,7 +12,7 @@ import mozilla.components.browser.errorpages.ErrorPages import mozilla.components.browser.errorpages.ErrorType import mozilla.components.concept.engine.EngineSession import mozilla.components.concept.engine.request.RequestInterceptor -import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.GleanMetrics.ErrorPage import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.isOnline import org.mozilla.fenix.helpers.TestHelper.appContext @@ -80,7 +80,7 @@ class AppRequestInterceptor(private val context: Context) : RequestInterceptor { val improvedErrorType = improveErrorType(errorType) val riskLevel = getRiskLevel(improvedErrorType) - context.components.analytics.metrics.track(Event.ErrorPageVisited(improvedErrorType)) + ErrorPage.visitedError.record(ErrorPage.VisitedErrorExtra(improvedErrorType.name)) val errorPageUri = ErrorPages.createUrlEncodedErrorPage( context = context, diff --git a/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt b/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt index 517a855924..e02143e9ab 100644 --- a/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt +++ b/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt @@ -12,7 +12,7 @@ import mozilla.components.browser.errorpages.ErrorPages import mozilla.components.browser.errorpages.ErrorType import mozilla.components.concept.engine.EngineSession import mozilla.components.concept.engine.request.RequestInterceptor -import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.GleanMetrics.ErrorPage import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.isOnline import java.lang.ref.WeakReference @@ -63,7 +63,7 @@ class AppRequestInterceptor( val improvedErrorType = improveErrorType(errorType) val riskLevel = getRiskLevel(improvedErrorType) - context.components.analytics.metrics.track(Event.ErrorPageVisited(improvedErrorType)) + ErrorPage.visitedError.record(ErrorPage.VisitedErrorExtra(improvedErrorType.name)) val errorPageUri = ErrorPages.createUrlEncodedErrorPage( context = context, diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt index e3de6ecd43..b245030851 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt @@ -5,7 +5,6 @@ package org.mozilla.fenix.components.metrics import android.content.Context -import mozilla.components.browser.errorpages.ErrorType import mozilla.components.browser.state.search.SearchEngine import mozilla.components.feature.top.sites.TopSite import org.mozilla.fenix.GleanMetrics.Addons @@ -13,7 +12,6 @@ import org.mozilla.fenix.GleanMetrics.AppTheme import org.mozilla.fenix.GleanMetrics.Autoplay import org.mozilla.fenix.GleanMetrics.Collections import org.mozilla.fenix.GleanMetrics.ContextMenu -import org.mozilla.fenix.GleanMetrics.ErrorPage import org.mozilla.fenix.GleanMetrics.Events import org.mozilla.fenix.GleanMetrics.History import org.mozilla.fenix.GleanMetrics.Logins @@ -395,11 +393,6 @@ sealed class Event { ) } - data class ErrorPageVisited(val errorType: ErrorType) : Event() { - override val extras: Map? - get() = mapOf(ErrorPage.visitedErrorKeys.errorType to errorType.name) - } - data class SearchBarTapped(val source: Source) : Event() { enum class Source { HOME, BROWSER } diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt index e78cddabf4..ec3cb1b9bb 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt @@ -21,7 +21,6 @@ import org.mozilla.fenix.GleanMetrics.ContextualMenu import org.mozilla.fenix.GleanMetrics.CreditCards import org.mozilla.fenix.GleanMetrics.CustomTab import org.mozilla.fenix.GleanMetrics.CustomizeHome -import org.mozilla.fenix.GleanMetrics.ErrorPage import org.mozilla.fenix.GleanMetrics.Events import org.mozilla.fenix.GleanMetrics.ExperimentsDefaultBrowser import org.mozilla.fenix.GleanMetrics.History @@ -202,10 +201,6 @@ private val Event.wrapper: EventWrapper<*>? is Event.NormalAndPrivateUriOpened -> EventWrapper( { Events.normalAndPrivateUriCount.add(1) } ) - is Event.ErrorPageVisited -> EventWrapper( - { ErrorPage.visitedError.record(it) }, - { ErrorPage.visitedErrorKeys.valueOf(it) } - ) is Event.SyncAuthOpened -> EventWrapper( { SyncAuth.opened.record(it) } ) diff --git a/app/src/test/java/org/mozilla/fenix/AppRequestInterceptorTest.kt b/app/src/test/java/org/mozilla/fenix/AppRequestInterceptorTest.kt index 384f492ae3..a7eab7381c 100644 --- a/app/src/test/java/org/mozilla/fenix/AppRequestInterceptorTest.kt +++ b/app/src/test/java/org/mozilla/fenix/AppRequestInterceptorTest.kt @@ -15,13 +15,16 @@ import mozilla.components.browser.errorpages.ErrorPages import mozilla.components.browser.errorpages.ErrorType import mozilla.components.concept.engine.request.RequestInterceptor import mozilla.components.support.test.robolectric.testContext +import mozilla.telemetry.glean.testing.GleanTestRule import org.junit.Assert.assertEquals import org.junit.Assert.assertNull import org.junit.Before +import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.AppRequestInterceptor.Companion.HIGH_RISK_ERROR_PAGES import org.mozilla.fenix.AppRequestInterceptor.Companion.LOW_AND_MEDIUM_RISK_ERROR_PAGES +import org.mozilla.fenix.GleanMetrics.ErrorPage import org.mozilla.fenix.components.Services import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.isOnline @@ -30,6 +33,9 @@ import org.mozilla.fenix.helpers.FenixRobolectricTestRunner @RunWith(FenixRobolectricTestRunner::class) class AppRequestInterceptorTest { + @get:Rule + val gleanTestRule = GleanTestRule(testContext) + private lateinit var interceptor: RequestInterceptor private lateinit var navigationController: NavController @@ -156,7 +162,6 @@ class AppRequestInterceptorTest { @Test fun `onErrorRequest results in correct error page for low risk level error`() { - every { testContext.components.analytics } returns mockk(relaxed = true) setOf( ErrorType.UNKNOWN, ErrorType.ERROR_NET_INTERRUPT, @@ -185,12 +190,17 @@ class AppRequestInterceptorTest { ) assertEquals(expectedPage, actualPage) + // Check if the error metric was recorded + assertEquals(true, ErrorPage.visitedError.testHasValue()) + assertEquals( + error.name, + ErrorPage.visitedError.testGetValue().last().extra?.get("error_type") + ) } } @Test fun `onErrorRequest results in correct error page for medium risk level error`() { - every { testContext.components.analytics } returns mockk(relaxed = true) setOf( ErrorType.ERROR_SECURITY_BAD_CERT, ErrorType.ERROR_SECURITY_SSL, @@ -203,12 +213,17 @@ class AppRequestInterceptorTest { ) assertEquals(expectedPage, actualPage) + // Check if the error metric was recorded + assertEquals(true, ErrorPage.visitedError.testHasValue()) + assertEquals( + error.name, + ErrorPage.visitedError.testGetValue().last().extra?.get("error_type") + ) } } @Test fun `onErrorRequest results in correct error page for high risk level error`() { - every { testContext.components.analytics } returns mockk(relaxed = true) setOf( ErrorType.ERROR_SAFEBROWSING_HARMFUL_URI, ErrorType.ERROR_SAFEBROWSING_MALWARE_URI, @@ -222,6 +237,12 @@ class AppRequestInterceptorTest { ) assertEquals(expectedPage, actualPage) + // Check if the error metric was recorded + assertEquals(true, ErrorPage.visitedError.testHasValue()) + assertEquals( + error.name, + ErrorPage.visitedError.testGetValue().last().extra?.get("error_type") + ) } }