feat: enhance GeoIP DB update process and improve error handling

aanorbel · aanorbel · commit eef3dbc0416f · 2025-10-28T19:11:21.000+03:00
diff --git a/composeApp/src/androidMain/kotlin/org/ooni/probe/net/Http.android.kt b/composeApp/src/androidMain/kotlin/org/ooni/probe/net/Http.android.kt
@@ -11,7 +11,13 @@ import java.net.URL
 
 actual suspend fun httpGetBytes(url: String): Result<ByteArray, GetBytesException> =
     withContext(Dispatchers.IO) {
-        val connection = (URL(url).openConnection() as HttpURLConnection)
+        val connection: HttpURLConnection
+        try {
+            connection = URL(url).openConnection() as HttpURLConnection
+        } catch (e: Throwable) {
+            return@withContext Failure(GetBytesException(e))
+        }
+
         connection.requestMethod = "GET"
         connection.instanceFollowRedirects = true
         connection.connectTimeout = 15000
diff --git a/composeApp/src/commonMain/kotlin/org/ooni/engine/Engine.kt b/composeApp/src/commonMain/kotlin/org/ooni/engine/Engine.kt
@@ -197,7 +197,7 @@ class Engine(
         MAX_RUNTIME_DISABLED
     }
 
-    private suspend fun buildSessionConfig(
+    private fun buildSessionConfig(
         taskOrigin: TaskOrigin,
         preferences: EnginePreferences,
     ) = OonimkallBridge.SessionConfig(
diff --git a/composeApp/src/commonMain/kotlin/org/ooni/engine/TaskEventMapper.kt b/composeApp/src/commonMain/kotlin/org/ooni/engine/TaskEventMapper.kt
@@ -101,7 +101,7 @@ class TaskEventMapper(
                 )
 
             "status.resolver_lookup" -> value?.geoIpdb?.let {
-                println(it)
+                Logger.d("GeoIP DB info in resolver lookup: $it")
                 null
             }
 
diff --git a/composeApp/src/commonMain/kotlin/org/ooni/probe/di/Dependencies.kt b/composeApp/src/commonMain/kotlin/org/ooni/probe/di/Dependencies.kt
@@ -210,7 +210,7 @@ class Dependencies(
     private val downloader by lazy {
         DownloadFile(
             fileSystem = FileSystem.SYSTEM,
-            fetchBytes = { url -> httpGetBytes(url) },
+            fetchBytes = ::httpGetBytes,
         )
     }
 
diff --git a/composeApp/src/commonMain/kotlin/org/ooni/probe/domain/FetchGeoIpDbUpdates.kt b/composeApp/src/commonMain/kotlin/org/ooni/probe/domain/FetchGeoIpDbUpdates.kt
@@ -1,14 +1,17 @@
 package org.ooni.probe.domain
 
+import co.touchlab.kermit.Logger
 import kotlinx.coroutines.flow.first
 import kotlinx.serialization.SerialName
 import kotlinx.serialization.Serializable
+import kotlinx.serialization.SerializationException
 import kotlinx.serialization.json.Json
 import okio.Path
 import ooniprobe.composeapp.generated.resources.Res
 import ooniprobe.composeapp.generated.resources.engine_mmdb_version
 import org.jetbrains.compose.resources.getString
 import org.ooni.engine.Engine
+import org.ooni.engine.Engine.MkException
 import org.ooni.engine.models.Failure
 import org.ooni.engine.models.Result
 import org.ooni.engine.models.Success
@@ -38,8 +41,14 @@ class FetchGeoIpDbUpdates(
 
                     downloadFile(url, target)
                         .onSuccess { downloadedPath ->
-                            preferencesRepository.setValueByKey(SettingsKey.MMDB_VERSION, versionName)
-                            preferencesRepository.setValueByKey(SettingsKey.MMDB_LAST_CHECK, Clock.System.now().toEpochMilliseconds())
+                            preferencesRepository.setValueByKey(
+                                SettingsKey.MMDB_VERSION,
+                                versionName,
+                            )
+                            preferencesRepository.setValueByKey(
+                                SettingsKey.MMDB_LAST_CHECK,
+                                Clock.System.now().toEpochMilliseconds(),
+                            )
                             return Success(downloadedPath)
                         }.onFailure { downloadError ->
                             return Failure(Engine.MkException(downloadError))
@@ -56,17 +65,33 @@ class FetchGeoIpDbUpdates(
      */
     private suspend fun isGeoIpDbLatest(latestVersion: String): Triple<Boolean, String, String> {
         val currentGeoIpDbVersion: String =
-            (preferencesRepository.getValueByKey(SettingsKey.MMDB_VERSION).first() ?: getString(Res.string.engine_mmdb_version)) as String
+            (
+                preferencesRepository.getValueByKey(SettingsKey.MMDB_VERSION).first()
+                    ?: getString(Res.string.engine_mmdb_version)
+            ) as String
 
-        return Triple(normalize(currentGeoIpDbVersion) >= normalize(latestVersion), currentGeoIpDbVersion, latestVersion)
+        return Triple(
+            normalize(currentGeoIpDbVersion) >= normalize(latestVersion),
+            currentGeoIpDbVersion,
+            latestVersion,
+        )
     }
 
-    private suspend fun getLatestEngineVersion(): Result<String, Engine.MkException> {
+    private suspend fun getLatestEngineVersion(): Result<String, MkException> {
         val url = "https://api.github.com/repos/aanorbel/oomplt-mmdb/releases/latest"
 
         return engineHttpDo("GET", url, TaskOrigin.OoniRun).map { payload ->
-            val jsonStr = payload ?: throw Engine.MkException(Throwable("Empty body"))
-            json.decodeFromString(GhRelease.serializer(), jsonStr).tag
+            payload?.let {
+                try {
+                    json.decodeFromString(GhRelease.serializer(), payload).tag
+                } catch (e: SerializationException) {
+                    Logger.e(e) { "Failed to decode release info" }
+                    null
+                } catch (e: IllegalArgumentException) {
+                    Logger.e(e) { "Failed to decode  release info" }
+                    null
+                }
+            } ?: throw MkException(Throwable("Failed to fetch latest version"))
         }
     }
 
diff --git a/composeApp/src/iosMain/kotlin/org/ooni/probe/net/Http.ios.kt b/composeApp/src/iosMain/kotlin/org/ooni/probe/net/Http.ios.kt
@@ -16,31 +16,42 @@ import kotlin.coroutines.resume
 
 actual suspend fun httpGetBytes(url: String): Result<ByteArray, GetBytesException> =
     suspendCancellableCoroutine { cont ->
-        val nsurl = NSURL.URLWithString(url)!!
+        val nsurl = NSURL.URLWithString(url) ?: run {
+            cont.resume(Failure(GetBytesException(RuntimeException("Invalid URL: $url"))))
+            return@suspendCancellableCoroutine
+        }
         val task = NSURLSession.sharedSession.dataTaskWithURL(nsurl) { data, response, error ->
-            when {
-                error != null -> cont.resume(Failure(GetBytesException(RuntimeException(error.localizedDescription))))
-                data != null -> {
-                    // If we have an HTTP response, check status code
-                    val http = response as? platform.Foundation.NSHTTPURLResponse
-                    val status = http?.statusCode?.toInt() ?: 200
-                    if (status in 200..299) {
-                        cont.resume(Success((data as NSData).toByteArray()))
+            if (error != null) {
+                cont.resume(Failure(GetBytesException(RuntimeException(error.toString()))))
+                return@dataTaskWithURL
+            }
+
+            when (val r = response) {
+                is platform.Foundation.NSHTTPURLResponse -> {
+                    val statusCode = r.statusCode
+                    if (statusCode in 200..299) {
+                        cont.resume(Success(data?.toByteArray() ?: ByteArray(0)))
                     } else {
-                        cont.resume(Failure(GetBytesException(RuntimeException("HTTP $status while GET $url"))))
+                        cont.resume(Failure(GetBytesException(RuntimeException("HTTP $statusCode while GET $url"))))
+                    }
+                }
+                else -> {
+                    // This could be for non-HTTP responses (e.g. file://) or an invalid state
+                    if (data != null) {
+                        cont.resume(Success(data.toByteArray()))
+                    } else {
+                        cont.resume(Failure(GetBytesException(RuntimeException("Request to $url returned no data and no error"))))
                     }
                 }
-                else -> cont.resume(Success(ByteArray(0)))
             }
         }
         cont.invokeOnCancellation { task.cancel() }
         task.resume()
     }
 
-private fun NSData.toByteArray(): ByteArray {
-    val result = ByteArray(length.toInt())
-    result.usePinned {
-        memcpy(it.addressOf(0), this.bytes, this.length)
+private fun NSData.toByteArray(): ByteArray =
+    ByteArray(length.toInt()).apply {
+        usePinned {
+            memcpy(it.addressOf(0), bytes, length)
+        }
     }
-    return result
-}

Original file line number	Diff line number	Diff line change
`@@ -197,7 +197,7 @@ class Engine(`
`197`	`197`	`MAX_RUNTIME_DISABLED`
`198`	`198`	`}`
`199`	`199`
`200`		`- private suspend fun buildSessionConfig(`
	`200`	`+ private fun buildSessionConfig(`
`201`	`201`	`taskOrigin: TaskOrigin,`
`202`	`202`	`preferences: EnginePreferences,`
`203`	`203`	`) = OonimkallBridge.SessionConfig(`
Original file line number	Diff line number	Diff line change
`@@ -101,7 +101,7 @@ class TaskEventMapper(`
`101`	`101`	`)`
`102`	`102`
`103`	`103`	`"status.resolver_lookup" -> value?.geoIpdb?.let {`
`104`		`- println(it)`
	`104`	`+ Logger.d("GeoIP DB info in resolver lookup: $it")`
`105`	`105`	`null`
`106`	`106`	`}`
`107`	`107`
Original file line number	Diff line number	Diff line change
`@@ -210,7 +210,7 @@ class Dependencies(`
`210`	`210`	`private val downloader by lazy {`
`211`	`211`	`DownloadFile(`
`212`	`212`	`fileSystem = FileSystem.SYSTEM,`
`213`		`- fetchBytes = { url -> httpGetBytes(url) },`
	`213`	`+ fetchBytes = ::httpGetBytes,`
`214`	`214`	`)`
`215`	`215`	`}`
`216`	`216`