fix: log image load errors and show placeholder on error

gotify · Jun 22, 2024 · 337af76 · 337af76
1 parent 7d6399b
commit 337af76
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 16 deletions.
diff --git a/app/src/main/kotlin/com/github/gotify/CoilInstance.kt b/app/src/main/kotlin/com/github/gotify/CoilInstance.kt
@@ -3,13 +3,17 @@ package com.github.gotify
 import android.content.Context
 import android.graphics.Bitmap
 import android.graphics.BitmapFactory
-import android.graphics.drawable.BitmapDrawable
+import androidx.annotation.DrawableRes
+import androidx.appcompat.content.res.AppCompatResources
+import androidx.core.graphics.drawable.toBitmap
 import coil.ImageLoader
 import coil.annotation.ExperimentalCoilApi
 import coil.decode.SvgDecoder
 import coil.disk.DiskCache
 import coil.executeBlocking
+import coil.request.ErrorResult
 import coil.request.ImageRequest
+import coil.request.SuccessResult
 import com.github.gotify.api.CertUtils
 import com.github.gotify.client.model.Application
 import java.io.IOException
@@ -23,27 +27,34 @@ object CoilInstance {
     private var holder: Pair<SSLSettings, ImageLoader>? = null
 
     @Throws(IOException::class)
-    fun getImageFromUrl(context: Context, url: String?): Bitmap {
-        val request = ImageRequest.Builder(context)
-            .data(url)
-            .build()
-        return (get(context).executeBlocking(request).drawable as BitmapDrawable).bitmap
+    fun getImageFromUrl(
+        context: Context,
+        url: String?,
+        @DrawableRes placeholder: Int = R.drawable.ic_placeholder
+    ): Bitmap {
+        val request = ImageRequest.Builder(context).data(url).build()
+
+        return when (val result = get(context).executeBlocking(request)) {
+            is SuccessResult -> result.drawable.toBitmap()
+            is ErrorResult -> {
+                Logger.error(
+                    result.throwable
+                ) { "Could not load image ${Utils.redactPassword(url)}" }
+                AppCompatResources.getDrawable(context, placeholder)!!.toBitmap()
+            }
+        }
     }
 
     fun getIcon(context: Context, app: Application?): Bitmap {
         if (app == null) {
             return BitmapFactory.decodeResource(context.resources, R.drawable.gotify)
         }
         val baseUrl = Settings(context).url
-        try {
-            return getImageFromUrl(
-                context,
-                Utils.resolveAbsoluteUrl("$baseUrl/", app.image)
-            )
-        } catch (e: IOException) {
-            Logger.error(e, "Could not load image for notification")
-        }
-        return BitmapFactory.decodeResource(context.resources, R.drawable.gotify)
+        return getImageFromUrl(
+            context,
+            Utils.resolveAbsoluteUrl("$baseUrl/", app.image),
+            R.drawable.gotify
+        )
     }
 
     @OptIn(ExperimentalCoilApi::class)

diff --git a/app/src/main/kotlin/com/github/gotify/MarkwonFactory.kt b/app/src/main/kotlin/com/github/gotify/MarkwonFactory.kt
@@ -11,6 +11,8 @@ import android.text.style.StyleSpan
 import android.text.style.TypefaceSpan
 import androidx.core.content.ContextCompat
 import coil.ImageLoader
+import coil.request.Disposable
+import coil.request.ImageRequest
 import io.noties.markwon.AbstractMarkwonPlugin
 import io.noties.markwon.Markwon
 import io.noties.markwon.MarkwonSpansFactory
@@ -22,6 +24,7 @@ import io.noties.markwon.core.MarkwonTheme
 import io.noties.markwon.ext.strikethrough.StrikethroughPlugin
 import io.noties.markwon.ext.tables.TableAwareMovementMethod
 import io.noties.markwon.ext.tables.TablePlugin
+import io.noties.markwon.image.AsyncDrawable
 import io.noties.markwon.image.coil.CoilImagesPlugin
 import io.noties.markwon.movement.MovementMethodPlugin
 import org.commonmark.ext.gfm.tables.TableCell
@@ -34,13 +37,37 @@ import org.commonmark.node.Link
 import org.commonmark.node.ListItem
 import org.commonmark.node.StrongEmphasis
 import org.commonmark.parser.Parser
+import org.tinylog.kotlin.Logger
 
 internal object MarkwonFactory {
     fun createForMessage(context: Context, imageLoader: ImageLoader): Markwon {
         return Markwon.builder(context)
             .usePlugin(CorePlugin.create())
             .usePlugin(MovementMethodPlugin.create(TableAwareMovementMethod.create()))
-            .usePlugin(CoilImagesPlugin.create(context, imageLoader))
+            .usePlugin(
+                CoilImagesPlugin.create(
+                    object : CoilImagesPlugin.CoilStore {
+                        override fun load(drawable: AsyncDrawable): ImageRequest {
+                            return ImageRequest.Builder(context)
+                                .data(drawable.destination)
+                                .placeholder(R.drawable.ic_placeholder)
+                                .listener(onError = { _, err ->
+                                    Logger.error(err.throwable) {
+                                        "Could not load markdown image: ${Utils.redactPassword(
+                                            drawable.destination
+                                        )}"
+                                    }
+                                })
+                                .build()
+                        }
+
+                        override fun cancel(disposable: Disposable) {
+                            disposable.dispose()
+                        }
+                    },
+                    imageLoader
+                )
+            )
             .usePlugin(StrikethroughPlugin.create())
             .usePlugin(TablePlugin.create(context))
             .usePlugin(object : AbstractMarkwonPlugin() {

diff --git a/app/src/main/kotlin/com/github/gotify/Utils.kt b/app/src/main/kotlin/com/github/gotify/Utils.kt
@@ -20,6 +20,7 @@ import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.launch
+import okhttp3.HttpUrl.Companion.toHttpUrlOrNull
 import org.threeten.bp.OffsetDateTime
 import org.tinylog.kotlin.Logger
 
@@ -92,4 +93,13 @@ internal object Utils {
         context.getSystemService(ActivityManager::class.java).appTasks?.getOrNull(0)
             ?.setExcludeFromRecents(excludeFromRecent)
     }
+
+    fun redactPassword(stringUrl: String?): String {
+        val url = stringUrl?.toHttpUrlOrNull()
+        return when {
+            url == null -> "unknown"
+            url.password.isEmpty() -> url.toString()
+            else -> url.newBuilder().password("REDACTED").toString()
+        }
+    }
 }