diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt index 8e6b6ad27938..abdca8709cc2 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt @@ -4945,10 +4945,10 @@ class BrowserTabViewModelTest { } @Test - fun whenPageIsChangedWithHttpError5XXThenUpdateCountPixelCalledForWebViewReceivedHttpError5XXDaily() = runTest { + fun whenPageIsChangedWithHttpError5XXThenUpdate5xxCountPixelCalledForWebViewReceivedHttpError5XXDaily() = runTest { testee.recordHttpErrorCode(statusCode = 504, url = "example2.com") - verify(mockHttpErrorPixels).updateCountPixel(HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_5XX_DAILY) + verify(mockHttpErrorPixels).update5xxCountPixel(HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_5XX_DAILY, 504) } @Test diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt index 48bc3924e3dc..c65ee77a0b0a 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -3438,19 +3438,22 @@ class BrowserTabViewModel @Inject constructor( } private fun updateHttpErrorCount(statusCode: Int) { - when { - // 400 errors - statusCode == HTTP_STATUS_CODE_BAD_REQUEST_ERROR -> httpErrorPixels.get().updateCountPixel( - HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_400_DAILY, - ) - // all 4xx errors apart from 400 - statusCode / 100 == HTTP_STATUS_CODE_CLIENT_ERROR_PREFIX -> httpErrorPixels.get().updateCountPixel( - HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_4XX_DAILY, - ) - // all 5xx errors - statusCode / 100 == HTTP_STATUS_CODE_SERVER_ERROR_PREFIX -> httpErrorPixels.get().updateCountPixel( - HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_5XX_DAILY, - ) + viewModelScope.launch(dispatchers.io()) { + when { + // 400 errors + statusCode == HTTP_STATUS_CODE_BAD_REQUEST_ERROR -> httpErrorPixels.get().updateCountPixel( + HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_400_DAILY, + ) + // all 4xx errors apart from 400 + statusCode / 100 == HTTP_STATUS_CODE_CLIENT_ERROR_PREFIX -> httpErrorPixels.get().updateCountPixel( + HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_4XX_DAILY, + ) + // all 5xx errors + statusCode / 100 == HTTP_STATUS_CODE_SERVER_ERROR_PREFIX -> httpErrorPixels.get().update5xxCountPixel( + HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_5XX_DAILY, + statusCode, + ) + } } } diff --git a/app/src/main/java/com/duckduckgo/app/browser/httperrors/HttpErrorDailyReportingWorker.kt b/app/src/main/java/com/duckduckgo/app/browser/httperrors/HttpErrorDailyReportingWorker.kt index 427e67427cc0..d57168e42ba7 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/httperrors/HttpErrorDailyReportingWorker.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/httperrors/HttpErrorDailyReportingWorker.kt @@ -48,7 +48,7 @@ class HttpErrorDailyReportingWorker(context: Context, workerParameters: WorkerPa return withContext(dispatchers.io()) { httpErrorPixels.fireCountPixel(HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_400_DAILY) httpErrorPixels.fireCountPixel(HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_4XX_DAILY) - httpErrorPixels.fireCountPixel(HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_5XX_DAILY) + httpErrorPixels.fire5xxCountPixels() return@withContext Result.success() } } diff --git a/app/src/main/java/com/duckduckgo/app/browser/httperrors/HttpErrorPixels.kt b/app/src/main/java/com/duckduckgo/app/browser/httperrors/HttpErrorPixels.kt index 3421970481ce..24bb86f701f2 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/httperrors/HttpErrorPixels.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/httperrors/HttpErrorPixels.kt @@ -19,8 +19,12 @@ package com.duckduckgo.app.browser.httperrors import android.content.Context import android.content.SharedPreferences import androidx.core.content.edit +import com.duckduckgo.app.pixels.remoteconfig.AndroidBrowserConfigFeature import com.duckduckgo.app.statistics.pixels.Pixel +import com.duckduckgo.browser.api.WebViewVersionProvider import com.duckduckgo.di.scopes.AppScope +import com.duckduckgo.mobile.android.vpn.network.ExternalVpnDetector +import com.duckduckgo.networkprotection.api.NetworkProtectionState import com.squareup.anvil.annotations.ContributesBinding import java.time.Instant import java.util.concurrent.TimeUnit @@ -28,22 +32,61 @@ import javax.inject.Inject interface HttpErrorPixels { fun updateCountPixel(httpErrorPixelName: HttpErrorPixelName) + suspend fun update5xxCountPixel(httpErrorPixelName: HttpErrorPixelName, statusCode: Int) fun fireCountPixel(httpErrorPixelName: HttpErrorPixelName) + fun fire5xxCountPixels() } @ContributesBinding(AppScope::class) class RealHttpErrorPixels @Inject constructor( private val pixel: Pixel, private val context: Context, + private val webViewVersionProvider: WebViewVersionProvider, + private val networkProtectionState: NetworkProtectionState, + private val externalVpnDetector: ExternalVpnDetector, + private val androidBrowserConfig: AndroidBrowserConfigFeature, ) : HttpErrorPixels { private val preferences: SharedPreferences by lazy { context.getSharedPreferences(FILENAME, Context.MODE_PRIVATE) } + private val pixel5xxKeys: MutableSet by lazy { + preferences.getStringSet(PIXEL_5XX_KEYS_SET, mutableSetOf()) ?: mutableSetOf() + } override fun updateCountPixel(httpErrorPixelName: HttpErrorPixelName) { val count = preferences.getInt(httpErrorPixelName.appendCountSuffix(), 0) preferences.edit { putInt(httpErrorPixelName.appendCountSuffix(), count + 1) } } + override suspend fun update5xxCountPixel( + httpErrorPixelName: HttpErrorPixelName, + statusCode: Int, + ) { + // Kill switch + if (!androidBrowserConfig.self().isEnabled() || !androidBrowserConfig.httpError5xxPixel().isEnabled()) { + return + } + + val pProVpnConnected = runCatching { + networkProtectionState.isRunning() + }.getOrDefault(false) + + val externalVpnConnected = runCatching { + externalVpnDetector.isExternalVpnDetected() + }.getOrDefault(false) + + val webViewFullVersion = webViewVersionProvider.getFullVersion() + + val pixelPrefKey = "${httpErrorPixelName.pixelName}|$statusCode|$pProVpnConnected|$externalVpnConnected|$webViewFullVersion|_count" + + val updatedSet = pixel5xxKeys + updatedSet.add(pixelPrefKey) + val count = preferences.getInt(pixelPrefKey, 0) + preferences.edit { + putInt(pixelPrefKey, count + 1) + putStringSet(PIXEL_5XX_KEYS_SET, updatedSet) + } + } + override fun fireCountPixel(httpErrorPixelName: HttpErrorPixelName) { val now = Instant.now().toEpochMilli() @@ -64,6 +107,49 @@ class RealHttpErrorPixels @Inject constructor( } } + override fun fire5xxCountPixels() { + // Kill switch + if (!androidBrowserConfig.self().isEnabled() || !androidBrowserConfig.httpError5xxPixel().isEnabled()) { + return + } + + val now = Instant.now().toEpochMilli() + val updatedSet = pixel5xxKeys + updatedSet.forEach { pixelKey -> + val count = preferences.getInt(pixelKey, 0) + if (count != 0) { + val timestamp = preferences.getLong("${pixelKey}_timestamp", 0L) + if (timestamp == 0L || now >= timestamp) { + pixelKey.split("|").let { split -> + if (split.size == 6) { + val httpErrorPixelName = HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_5XX_DAILY + val statusCode = split[1].toInt() + val pProVpnConnected = split[2].toBoolean() + val externalVpnConnected = split[3].toBoolean() + val webViewFullVersion = split[4] + pixel.fire( + httpErrorPixelName, + mapOf( + HttpErrorPixelParameters.HTTP_ERROR_CODE_COUNT to count.toString(), + "error_code" to statusCode.toString(), + "ppro_user" to pProVpnConnected.toString(), + "vpn_user" to externalVpnConnected.toString(), + "webview_version" to webViewFullVersion, + ), + ) + } + } + .also { + preferences.edit { + putLong("${pixelKey}_timestamp", now.plus(TimeUnit.HOURS.toMillis(WINDOW_INTERVAL_HOURS))) + putInt(pixelKey, 0) + } + } + } + } + } + } + private fun HttpErrorPixelName.appendTimestampSuffix(): String { return "${this.pixelName}_timestamp" } @@ -75,5 +161,6 @@ class RealHttpErrorPixels @Inject constructor( companion object { private const val FILENAME = "com.duckduckgo.app.browser.httperrors" private const val WINDOW_INTERVAL_HOURS = 24L + internal const val PIXEL_5XX_KEYS_SET = "pixel_5xx_keys_set" } } diff --git a/app/src/main/java/com/duckduckgo/app/pixels/remoteconfig/AndroidBrowserConfigFeature.kt b/app/src/main/java/com/duckduckgo/app/pixels/remoteconfig/AndroidBrowserConfigFeature.kt index 99a49055ad64..89b1eb56293f 100644 --- a/app/src/main/java/com/duckduckgo/app/pixels/remoteconfig/AndroidBrowserConfigFeature.kt +++ b/app/src/main/java/com/duckduckgo/app/pixels/remoteconfig/AndroidBrowserConfigFeature.kt @@ -99,4 +99,12 @@ interface AndroidBrowserConfigFeature { */ @Toggle.DefaultValue(false) fun fireproofedWebLocalStorage(): Toggle + + /** + * @return `true` when the remote config has the global "httpError5xxPixel" androidBrowserConfig + * sub-feature flag enabled + * If the remote feature is not present defaults to `false` + */ + @Toggle.DefaultValue(false) + fun httpError5xxPixel(): Toggle } diff --git a/app/src/test/java/com/duckduckgo/app/browser/httperrors/HttpErrorDailyReportingWorkerTest.kt b/app/src/test/java/com/duckduckgo/app/browser/httperrors/HttpErrorDailyReportingWorkerTest.kt index 5aecfaae81de..a2210f969acb 100644 --- a/app/src/test/java/com/duckduckgo/app/browser/httperrors/HttpErrorDailyReportingWorkerTest.kt +++ b/app/src/test/java/com/duckduckgo/app/browser/httperrors/HttpErrorDailyReportingWorkerTest.kt @@ -53,7 +53,7 @@ internal class HttpErrorDailyReportingWorkerTest { verify(mockHttpErrorPixels).fireCountPixel(HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_400_DAILY) verify(mockHttpErrorPixels).fireCountPixel(HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_4XX_DAILY) - verify(mockHttpErrorPixels).fireCountPixel(HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_5XX_DAILY) + verify(mockHttpErrorPixels).fire5xxCountPixels() assertEquals(result, ListenableWorker.Result.success()) } } diff --git a/app/src/test/java/com/duckduckgo/app/browser/httperrors/RealHttpErrorPixelsTest.kt b/app/src/test/java/com/duckduckgo/app/browser/httperrors/RealHttpErrorPixelsTest.kt index e7eb76b45f34..fce6daac52ef 100644 --- a/app/src/test/java/com/duckduckgo/app/browser/httperrors/RealHttpErrorPixelsTest.kt +++ b/app/src/test/java/com/duckduckgo/app/browser/httperrors/RealHttpErrorPixelsTest.kt @@ -20,13 +20,25 @@ import android.content.Context import android.content.SharedPreferences import androidx.core.content.edit import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.duckduckgo.app.browser.httperrors.RealHttpErrorPixels.Companion.PIXEL_5XX_KEYS_SET +import com.duckduckgo.app.pixels.remoteconfig.AndroidBrowserConfigFeature import com.duckduckgo.app.statistics.pixels.Pixel import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.Count +import com.duckduckgo.browser.api.WebViewVersionProvider +import com.duckduckgo.common.test.CoroutineTestRule import com.duckduckgo.common.test.api.InMemorySharedPreferences +import com.duckduckgo.feature.toggles.api.FakeFeatureToggleFactory +import com.duckduckgo.feature.toggles.api.Toggle.State +import com.duckduckgo.mobile.android.vpn.network.ExternalVpnDetector +import com.duckduckgo.networkprotection.api.NetworkProtectionState import java.time.Instant import java.util.concurrent.TimeUnit +import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue import org.junit.Before +import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.mockito.kotlin.any @@ -39,17 +51,31 @@ import org.mockito.kotlin.whenever @RunWith(AndroidJUnit4::class) class RealHttpErrorPixelsTest { + @get:Rule + val coroutineTestRule: CoroutineTestRule = CoroutineTestRule() + private lateinit var testee: HttpErrorPixels private lateinit var prefs: SharedPreferences private val mockPixel: Pixel = mock() private val mockContext: Context = mock() + private val mockWebViewVersionProvider: WebViewVersionProvider = mock() + private val mockNetworkProtectionState: NetworkProtectionState = mock() + private val mockExternalVpnDetector: ExternalVpnDetector = mock() + private var fakeAndroidConfigBrowserFeature = FakeFeatureToggleFactory.create(AndroidBrowserConfigFeature::class.java) @Before fun setup() { prefs = InMemorySharedPreferences() whenever(mockContext.getSharedPreferences("com.duckduckgo.app.browser.httperrors", 0)).thenReturn(prefs) - testee = RealHttpErrorPixels(mockPixel, mockContext) + testee = RealHttpErrorPixels( + mockPixel, + mockContext, + mockWebViewVersionProvider, + mockNetworkProtectionState, + mockExternalVpnDetector, + fakeAndroidConfigBrowserFeature, + ) } @Test @@ -131,4 +157,158 @@ class RealHttpErrorPixelsTest { type = eq(Count), ) } + + @Test + fun whenUpdate5xxCountPixelCalledThenSharedPrefUpdated() = runTest { + fakeAndroidConfigBrowserFeature.self().setRawStoredState(State(enable = true)) + fakeAndroidConfigBrowserFeature.httpError5xxPixel().setRawStoredState(State(enable = true)) + whenever(mockNetworkProtectionState.isRunning()).thenReturn(true) + whenever(mockExternalVpnDetector.isExternalVpnDetected()).thenReturn(false) + whenever(mockWebViewVersionProvider.getFullVersion()).thenReturn("123.45.67.89") + + // The pixelKey format is: pixelName|statusCode|pProVpnConnected|externalVpnConnected|webViewVersion|_count + val expectedKey = "${HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_5XX_DAILY.pixelName}|503|true|false|123.45.67.89|_count" + + testee.update5xxCountPixel(HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_5XX_DAILY, 503) + + assertEquals(1, prefs.getInt(expectedKey, 0)) + assertTrue(prefs.getStringSet(PIXEL_5XX_KEYS_SET, emptySet())!!.contains(expectedKey)) + } + + @Test + fun whenUpdate5xxCountPixelCalledMultipleTimesThenCounterIncremented() = runTest { + fakeAndroidConfigBrowserFeature.self().setRawStoredState(State(enable = true)) + fakeAndroidConfigBrowserFeature.httpError5xxPixel().setRawStoredState(State(enable = true)) + whenever(mockNetworkProtectionState.isRunning()).thenReturn(true) + whenever(mockExternalVpnDetector.isExternalVpnDetected()).thenReturn(false) + whenever(mockWebViewVersionProvider.getFullVersion()).thenReturn("123.45.67.89") + + // The pixelKey format is: pixelName|statusCode|pProVpnConnected|externalVpnConnected|webViewVersion|_count + val expectedKey = "${HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_5XX_DAILY.pixelName}|503|true|false|123.45.67.89|_count" + + testee.update5xxCountPixel(HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_5XX_DAILY, 503) + testee.update5xxCountPixel(HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_5XX_DAILY, 503) + + assertEquals(2, prefs.getInt(expectedKey, 0)) + } + + @Test + fun whenUpdate5xxCountPixelCalledWirhFeatureFlagDisabledThenSharedPrefNotUpdated() = runTest { + fakeAndroidConfigBrowserFeature.self().setRawStoredState(State(enable = true)) + fakeAndroidConfigBrowserFeature.httpError5xxPixel().setRawStoredState(State(enable = false)) + whenever(mockNetworkProtectionState.isRunning()).thenReturn(true) + whenever(mockExternalVpnDetector.isExternalVpnDetected()).thenReturn(false) + whenever(mockWebViewVersionProvider.getFullVersion()).thenReturn("123.45.67.89") + + // The pixelKey format is: pixelName|statusCode|pProVpnConnected|externalVpnConnected|webViewVersion|_count + val expectedKey = "${HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_5XX_DAILY.pixelName}|503|true|false|123.45.67.89|_count" + + testee.update5xxCountPixel(HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_5XX_DAILY, 503) + + assertEquals(0, prefs.getInt(expectedKey, 0)) + assertFalse(prefs.getStringSet(PIXEL_5XX_KEYS_SET, emptySet())!!.contains(expectedKey)) + } + + @Test + fun whenFire5xxCountPixelCalledWithNonZeroCountPixelSent() = runTest { + val pixelName = HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_5XX_DAILY + val pixelKey = "${pixelName.pixelName}|503|true|false|123.45.67.89|_count" + val pixelKeys = mutableSetOf(pixelKey) + + fakeAndroidConfigBrowserFeature.self().setRawStoredState(State(enable = true)) + fakeAndroidConfigBrowserFeature.httpError5xxPixel().setRawStoredState(State(enable = true)) + prefs.edit { + putStringSet(PIXEL_5XX_KEYS_SET, pixelKeys) + putInt(pixelKey, 5) + } + + testee.fire5xxCountPixels() + + verify(mockPixel).fire( + pixel = eq(pixelName), + parameters = eq( + mapOf( + HttpErrorPixelParameters.HTTP_ERROR_CODE_COUNT to "5", + "error_code" to "503", + "ppro_user" to "true", + "vpn_user" to "false", + "webview_version" to "123.45.67.89", + ), + ), + encodedParameters = any(), + type = any(), + ) + + assertEquals(0, prefs.getInt(pixelKey, -1)) + } + + @Test + fun whenFire5xxCountPixelCalledWithZeroCountPixelNotSent() { + val pixelName = HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_5XX_DAILY + val pixelKey = "${pixelName.pixelName}|503|true|false|123.45.67.89|_count" + val pixelKeys = mutableSetOf(pixelKey) + + fakeAndroidConfigBrowserFeature.self().setRawStoredState(State(enable = true)) + fakeAndroidConfigBrowserFeature.httpError5xxPixel().setRawStoredState(State(enable = true)) + prefs.edit { + putStringSet(PIXEL_5XX_KEYS_SET, pixelKeys) + putInt(pixelKey, 0) + } + + testee.fire5xxCountPixels() + + verify(mockPixel, never()).fire( + pixel = eq(pixelName), + parameters = any(), + encodedParameters = any(), + type = any(), + ) + } + + @Test + fun whenFire5xxCountPixelsCalledBeforeTimeWindowThenPixelNotSent() { + val pixelName = HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_5XX_DAILY + val pixelKey = "${pixelName.pixelName}|503|true|false|123.45.67.89|_count" + val pixelKeys = mutableSetOf(pixelKey) + val now = Instant.now().toEpochMilli() + + fakeAndroidConfigBrowserFeature.self().setRawStoredState(State(enable = true)) + fakeAndroidConfigBrowserFeature.httpError5xxPixel().setRawStoredState(State(enable = true)) + prefs.edit { + putStringSet(PIXEL_5XX_KEYS_SET, pixelKeys) + putInt(pixelKey, 5) + putLong("${pixelKey}_timestamp", now + TimeUnit.HOURS.toMillis(1)) // 1 hour in future + } + + testee.fire5xxCountPixels() + + verify(mockPixel, never()).fire( + pixel = eq(pixelName), + parameters = any(), + encodedParameters = any(), + type = any(), + ) + + assertEquals(5, prefs.getInt(pixelKey, -1)) + } + + @Test + fun whenFire5xxCountPixelsCalledWithTheFeatureFlagDisabledThenPixelNotSent() { + val pixelName = HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_5XX_DAILY + val pixelKey = "${pixelName.pixelName}|503|true|false|123.45.67.89|_count" + + fakeAndroidConfigBrowserFeature.self().setRawStoredState(State(enable = true)) + fakeAndroidConfigBrowserFeature.httpError5xxPixel().setRawStoredState(State(enable = false)) + + testee.fire5xxCountPixels() + + verify(mockPixel, never()).fire( + pixel = eq(pixelName), + parameters = any(), + encodedParameters = any(), + type = any(), + ) + + assertEquals(-1, prefs.getInt(pixelKey, -1)) + } }