[Android] Increase in 5xx errors (#5847)

anikiki · web-flow · commit 5ca3dbd62a8c · 2025-04-02T12:01:56.000+01:00
Task/Issue URL: https://app.asana.com/0/1200581511062568/1209741376363526/f ### Description Added more params to the m_webview_received_http_error_5xx_daily pixel. ### Steps to test this PR _HTTP Error Tracking_ - [ ] Generate a 5xx error and verify the error count is incremented in SharedPreferences - [ ] Verify the error details (status code, PPRo VPN status, External VPN status, WebView version) are correctly captured - [ ] Trigger the daily reporting worker and confirm 5xx error pixels are fired with the correct parameters - [ ] Verify the counter is reset after firing the pixel - [ ] Confirm pixels aren't fired again until the time window (24 hours) has elapsed ### UI changes NO UI changes.
diff --git 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
@@ -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
@@ -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
@@ -19,31 +19,74 @@ 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
 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<String> 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
@@ -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
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -4945,10 +4945,10 @@ class BrowserTabViewModelTest {`
`4945`	`4945`	`}`
`4946`	`4946`
`4947`	`4947`	`@Test`
`4948`		`- fun whenPageIsChangedWithHttpError5XXThenUpdateCountPixelCalledForWebViewReceivedHttpError5XXDaily() = runTest {`
	`4948`	`+ fun whenPageIsChangedWithHttpError5XXThenUpdate5xxCountPixelCalledForWebViewReceivedHttpError5XXDaily() = runTest {`
`4949`	`4949`	`testee.recordHttpErrorCode(statusCode = 504, url = "example2.com")`
`4950`	`4950`
`4951`		`- verify(mockHttpErrorPixels).updateCountPixel(HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_5XX_DAILY)`
	`4951`	`+ verify(mockHttpErrorPixels).update5xxCountPixel(HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_5XX_DAILY, 504)`
`4952`	`4952`	`}`
`4953`	`4953`
`4954`	`4954`	`@Test`
Original file line number	Diff line number	Diff line change
`@@ -48,7 +48,7 @@ class HttpErrorDailyReportingWorker(context: Context, workerParameters: WorkerPa`
`48`	`48`	`return withContext(dispatchers.io()) {`
`49`	`49`	`httpErrorPixels.fireCountPixel(HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_400_DAILY)`
`50`	`50`	`httpErrorPixels.fireCountPixel(HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_4XX_DAILY)`
`51`		`- httpErrorPixels.fireCountPixel(HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_5XX_DAILY)`
	`51`	`+ httpErrorPixels.fire5xxCountPixels()`
`52`	`52`	`return@withContext Result.success()`
`53`	`53`	`}`
`54`	`54`	`}`
Original file line number	Diff line number	Diff line change
`@@ -53,7 +53,7 @@ internal class HttpErrorDailyReportingWorkerTest {`
`53`	`53`
`54`	`54`	`verify(mockHttpErrorPixels).fireCountPixel(HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_400_DAILY)`
`55`	`55`	`verify(mockHttpErrorPixels).fireCountPixel(HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_4XX_DAILY)`
`56`		`- verify(mockHttpErrorPixels).fireCountPixel(HttpErrorPixelName.WEBVIEW_RECEIVED_HTTP_ERROR_5XX_DAILY)`
	`56`	`+ verify(mockHttpErrorPixels).fire5xxCountPixels()`
`57`	`57`	`assertEquals(result, ListenableWorker.Result.success())`
`58`	`58`	`}`
`59`	`59`	`}`