improve Stats failure handling

binarynoise · binarynoise · commit 220fb1fe593a · 2025-08-22T01:57:01.000+02:00
diff --git a/api/client/build.gradle.kts b/api/client/build.gradle.kts
@@ -4,7 +4,7 @@ plugins {
 
 dependencies {
     api(projects.api)
-    implementation(projects.util.okhttpKts)
+    api(projects.util.okhttpKts)
     
     implementation(libs.kotlinx.serialization.json)
     
diff --git a/app/src/main/kotlin/de/binarynoise/captiveportalautologin/Stats.kt b/app/src/main/kotlin/de/binarynoise/captiveportalautologin/Stats.kt
@@ -1,5 +1,12 @@
+@file:OptIn(ExperimentalTime::class)
+
 package de.binarynoise.captiveportalautologin
 
+import java.io.FileNotFoundException
+import kotlin.time.Clock
+import kotlin.time.Duration.Companion.minutes
+import kotlin.time.Duration.Companion.seconds
+import kotlinx.coroutines.delay
 import android.content.Context
 import androidx.work.Constraints
 import androidx.work.CoroutineWorker
@@ -14,7 +21,10 @@ import de.binarynoise.captiveportalautologin.preferences.SharedPreferences
 import de.binarynoise.captiveportalautologin.util.applicationContext
 import de.binarynoise.filedb.JsonDB
 import de.binarynoise.logger.Logger.log
+import de.binarynoise.util.okhttp.HttpStatusCodeException
 import okhttp3.HttpUrl.Companion.toHttpUrl
+import kotlin.time.ExperimentalTime
+import kotlin.time.Instant
 
 const val API_BASE = "https://captiveportalautologin.binarynoise.de/api/"
 
@@ -24,50 +34,83 @@ private val jsonDB = JsonDB(localCacheRoot)
 // Worker class to handle the upload
 class StatsWorker(appContext: Context, workerParams: WorkerParameters) : CoroutineWorker(appContext, workerParams) {
     
+    var lastRetryTime by SharedPreferences.stats_last_retry_time
+    private val retryCooldown = 5.minutes
+    
+    private fun hasRecentRetry(): Boolean {
+        return (Clock.System.now() - Instant.fromEpochMilliseconds(lastRetryTime)) < retryCooldown
+    }
+    
+    private fun recordRetry() {
+        lastRetryTime = Clock.System.now().toEpochMilliseconds()
+    }
+    
     override suspend fun doWork(): Result {
         log("StatsWorker started")
-        val apiBaseFromPreference by SharedPreferences.api_base
-        val apiClient = ApiClient((apiBaseFromPreference.takeUnless { it == "" } ?: API_BASE).toHttpUrl())
         
-        val harFiles = jsonDB.loadAll<HAR>("har")
-        harFiles.forEach { (key, har) ->
-            try {
-                apiClient.har.submitHar(key, har)
-                jsonDB.delete<HAR>(key, "har")
-                log("Uploaded HAR $key")
-            } catch (e: Exception) {
-                log("Failed to upload har", e)
-                // retry on the next worker run
-                return Result.retry()
-            }
+        if (hasRecentRetry()) {
+            log("Recent retry detected, delaying this attempt to avoid spamming")
+            delay(retryCooldown)
         }
         
-        val errorFiles = jsonDB.loadAll<Api.Liberator.Error>()
-        errorFiles.forEach { (key, error) ->
-            try {
-                apiClient.liberator.reportError(error)
-                jsonDB.delete<Api.Liberator.Error>(key)
-                log("Uploaded Api.Liberator.Error $key")
-            } catch (e: Exception) {
-                log("Failed to upload error", e)
-                return Result.retry()
-            }
-        }
+        val type = inputData.getString("type") ?: return Result.failure()
+        val key = inputData.getString("key") ?: return Result.failure()
+        
+        val apiBaseFromPreference by SharedPreferences.api_base
+        val apiClient = ApiClient((apiBaseFromPreference.takeUnless { it == "" } ?: API_BASE).toHttpUrl())
         
-        val successFiles = jsonDB.loadAll<Api.Liberator.Success>()
-        successFiles.forEach { (key, success) ->
-            try {
-                apiClient.liberator.reportSuccess(success)
-                jsonDB.delete<Api.Liberator.Success>(key)
-                log("Uploaded Api.Liberator.Success $key")
-            } catch (e: Exception) {
-                log("Failed to upload success", e)
-                return Result.retry()
+        try {
+            when (type) {
+                "har" -> {
+                    val har = jsonDB.load<HAR>(key, "har")
+                    apiClient.har.submitHar(key, har)
+                    jsonDB.delete<HAR>(key, "har")
+                    log("Uploaded HAR $key")
+                }
+                "error" -> {
+                    val error = jsonDB.load<Api.Liberator.Error>(key)
+                    apiClient.liberator.reportError(error)
+                    jsonDB.delete<Api.Liberator.Error>(key)
+                    log("Uploaded Api.Liberator.Error $key")
+                }
+                "success" -> {
+                    val success = jsonDB.load<Api.Liberator.Success>(key)
+                    apiClient.liberator.reportSuccess(success)
+                    jsonDB.delete<Api.Liberator.Success>(key)
+                    log("Uploaded Api.Liberator.Success $key")
+                }
+            }
+            
+            return Result.success()
+        } catch (e: FileNotFoundException) {
+            log("Failed to upload $type $key", e)
+            return Result.failure()
+        } catch (e: HttpStatusCodeException) {
+            when (e.code) {
+                429 -> {
+                    val timeout = e.response.header("Retry-After")?.toLongOrNull() ?: 0
+                    log("Failed to upload $type $key: HTTP 429 - timeout: $timeout")
+                    recordRetry()
+                    delay(timeout.seconds)
+                    return Result.retry()
+                }
+                
+                500, 501, 502, 503, 504, 507 -> {
+                    log("Server error trying to upload $type $key: HTTP ${e.code}, trying again later", e)
+                    recordRetry()
+                    return Result.retry()
+                }
+                
+                else -> {
+                    log("Failed to upload $type $key: HTTP ${e.code}", e)
+                    return Result.failure()
+                }
             }
+        } catch (e: Exception) {
+            log("Failed to upload $type $key", e)
+            recordRetry()
+            return Result.retry()
         }
-        
-        log("StatsWorker finished")
-        return Result.success()
     }
 }
 
@@ -80,7 +123,7 @@ object Stats : Api {
         override fun submitHar(name: String, har: HAR) {
             val key = name
             jsonDB.store(key, har, "har")
-            triggerUpload()
+            scheduleUpload("har", key)
         }
     }
     
@@ -96,21 +139,23 @@ object Stats : Api {
         override fun reportError(error: Api.Liberator.Error) {
             val key = "${System.currentTimeMillis()}_${error.hashCode()}"
             jsonDB.store(key, error)
-            triggerUpload()
+            scheduleUpload("error", key)
         }
         
         override fun reportSuccess(success: Api.Liberator.Success) {
             val key = "${System.currentTimeMillis()}_${success.hashCode()}"
             jsonDB.store(key, success)
-            triggerUpload()
+            scheduleUpload("success", key)
         }
     }
     
-    // Schedule the WorkRequest
-    fun triggerUpload() {
-        val constraints = Constraints.Builder().setRequiredNetworkType(NetworkType.CONNECTED).build()
-        val uploadRequest = OneTimeWorkRequestBuilder<StatsWorker>().setConstraints(constraints).build()
+    private fun scheduleUpload(type: String, key: String) {
+        val constraints = Constraints.Builder().setRequiredNetworkType(NetworkType.UNMETERED).build()
+        val inputData = androidx.work.workDataOf("type" to type, "key" to key)
+        val uploadRequest =
+            OneTimeWorkRequestBuilder<StatsWorker>().setConstraints(constraints).setInputData(inputData).build()
+        
         WorkManager.getInstance(applicationContext).enqueue(uploadRequest)
-        log("Scheduled upload")
+        log("Scheduled upload for $type: $key")
     }
 }
diff --git a/app/src/main/kotlin/de/binarynoise/captiveportalautologin/preferences/MainFragment.kt b/app/src/main/kotlin/de/binarynoise/captiveportalautologin/preferences/MainFragment.kt
@@ -10,19 +10,15 @@ import androidx.preference.CheckBoxPreference
 import androidx.preference.DropDownPreference
 import androidx.preference.ListPreference
 import androidx.preference.Preference
-import androidx.preference.PreferenceManager
 import androidx.preference.SwitchPreference
 import de.binarynoise.captiveportalautologin.BuildConfig
 import de.binarynoise.captiveportalautologin.ConnectivityChangeListenerService
 import de.binarynoise.captiveportalautologin.ConnectivityChangeListenerService.NetworkState
 import de.binarynoise.captiveportalautologin.ConnectivityChangeListenerService.ServiceState
 import de.binarynoise.captiveportalautologin.GeckoViewActivity
 import de.binarynoise.captiveportalautologin.Permissions
-import de.binarynoise.captiveportalautologin.Stats
-import de.binarynoise.captiveportalautologin.util.applicationContext
 import de.binarynoise.captiveportalautologin.xposed.Xposed
 import de.binarynoise.liberator.PortalDetection
-import de.binarynoise.logger.Logger.log
 import okhttp3.HttpUrl.Companion.toHttpUrl
 import org.mozilla.gecko.util.ThreadUtils.runOnUiThread
 
@@ -188,15 +184,6 @@ class MainFragment : AutoCleanupPreferenceFragment() {
                 isEnabled = false
             }
             
-            addPreference(Preference(ctx)) {
-                title = "Send Statistics now"
-                
-                setOnPreferenceClickListener {
-                    Stats.triggerUpload()
-                    true
-                }
-            }
-            
             if (BuildConfig.DEBUG) {
                 addPreference(EditTextPreference(ctx, SharedPreferences.api_base.get()) { editText, s ->
                     if (s.isBlank()) {
diff --git a/app/src/main/kotlin/de/binarynoise/captiveportalautologin/preferences/SharedPreferences.kt b/app/src/main/kotlin/de/binarynoise/captiveportalautologin/preferences/SharedPreferences.kt
@@ -14,6 +14,8 @@ object SharedPreferences {
     val liberator_send_stats by PreferenceProperty(true)
     val api_base by PreferenceProperty("")
     
+    val stats_last_retry_time by PreferenceProperty(0L)
+    
     private class PreferenceProperty<T>(private val defaultValue: T) {
         operator fun getValue(parent: Any, property: KProperty<*>): PreferencePropertyDelegate<T> {
             return PreferencePropertyDelegate(property, defaultValue)
@@ -31,16 +33,14 @@ class PreferencePropertyDelegate<T>(val parent: KProperty<*>, val defaultValue:
     }
     
     @Suppress("UNCHECKED_CAST")
-    operator fun getValue(parent: Nothing?, property: KProperty<*>?): T {
+    operator fun getValue(parent: Any?, property: KProperty<*>?): T {
         with(PreferenceManager.getDefaultSharedPreferences(applicationContext)) {
             return if (contains(key)) all[key] as T
             else defaultValue
         }
     }
     
-    operator fun getValue(parent: Any, property: KProperty<*>?): T = getValue(null, null)
-    
-    operator fun setValue(parent: Nothing?, property: KProperty<*>?, newValue: T) {
+    operator fun setValue(parent: Any?, property: KProperty<*>?, newValue: T) {
         PreferenceManager.getDefaultSharedPreferences(applicationContext).edit {
             when (newValue) {
                 null -> remove(key)
