fix(push): prevent lost notifications and missing images

README.md

@@ -7,7 +7,7 @@
 ![GitHub tag (latest)](https://img.shields.io/github/v/tag/ppgco/android-sdk?style=flat-square)
 
 > [!IMPORTANT]
-> **Version 3.2.1**
+> **Version 3.2.3**
 >
 > Introducing **Live Activities (Android Live Updates)** — real-time, continuously
 > updated notifications (football match tracking template) on Android 16+.

library/src/main/java/com/pushpushgo/sdk/PushPushGo.kt

@@ -48,7 +48,7 @@ class PushPushGo private constructor(
   private val customBaseUrl: String?,
 ) {
   companion object {
-    const val VERSION = "3.2.2"
+    const val VERSION = "3.2.3"
 
     internal const val TAG = "PPGo"
 

library/src/main/java/com/pushpushgo/sdk/push/PushNotificationDelegate.kt

@@ -7,6 +7,7 @@ import android.content.Context
 import android.content.Intent
 import android.graphics.Bitmap
 import android.net.Uri
+import android.os.Looper
 import android.os.SystemClock
 import androidx.core.app.NotificationCompat
 import androidx.core.app.NotificationManagerCompat
@@ -21,6 +22,7 @@ import com.pushpushgo.sdk.push.liveactivity.data.LiveActivityPayloadParser
 import com.pushpushgo.sdk.utils.*
 import com.pushpushgo.sdk.work.UploadDelegate
 import kotlinx.coroutines.*
+import java.io.IOException
 import kotlin.random.Random
 
 internal class PushNotificationDelegate(
@@ -80,32 +82,54 @@ internal class PushNotificationDelegate(
       return
     }
 
-    delegateScope.launch(errorHandler) {
-      val notificationId = getNotificationId(pushMessage.data["nId"] ?: "default")
-      logDebug("Notification ID: $notificationId")
+    if (Looper.myLooper() == Looper.getMainLooper()) {
+      // Blocking the main thread would risk an ANR, so integrations that forward
+      // messages from the main thread keep the asynchronous path.
+      delegateScope.launch(errorHandler) { showNotification(pushMessage, context, notificationManager) }
+      return
+    }
 
-      val notification =
-        when {
-          pushMessage.data.isNotEmpty() ->
-            getDataNotification(
-              context = context,
-              remoteMessage = pushMessage,
-              notificationId = notificationId,
-            )
+    // Messaging services deliver onMessageReceived on a worker thread and stop the
+    // service as soon as it returns. Returning before the notification was posted
+    // let the service be destroyed mid image download, which dropped the
+    // notification (or its image), so wait for it here.
+    try {
+      runBlocking { showNotification(pushMessage, context, notificationManager) }
+    } catch (e: Throwable) {
+      logError(e)
+    }
+  }
 
-          pushMessage.notification != null ->
-            getSimpleNotification(
-              context = context,
-              remoteMessage = pushMessage,
-              notificationId = notificationId,
-            )
+  @SuppressLint("MissingPermission")
+  private suspend fun showNotification(
+    pushMessage: PushMessage,
+    context: Context,
+    notificationManager: NotificationManagerCompat,
+  ) {
+    val notificationId = getNotificationId(pushMessage.data["nId"] ?: "default")
+    logDebug("Notification ID: $notificationId")
 
-          else -> throw IllegalStateException("Unknown notification type")
-        }
+    val notification =
+      when {
+        pushMessage.data.isNotEmpty() ->
+          getDataNotification(
+            context = context,
+            remoteMessage = pushMessage,
+            notificationId = notificationId,
+          )
 
-      notificationManager.notify(notificationId, notification)
-      logDebug("Notification sent: $notificationId => $notification")
-    }
+        pushMessage.notification != null ->
+          getSimpleNotification(
+            context = context,
+            remoteMessage = pushMessage,
+            notificationId = notificationId,
+          )
+
+        else -> throw IllegalStateException("Unknown notification type")
+      }
+
+    notificationManager.notify(notificationId, notification)
+    logDebug("Notification sent: $notificationId => $notification")
   }
 
   fun onNewToken(token: String) {
@@ -117,7 +141,9 @@ internal class PushNotificationDelegate(
   }
 
   fun onDestroy() {
-    job.cancelChildren()
+    // Intentionally does not cancel in-flight work: the service is destroyed right
+    // after onMessageReceived returns, and cancelling here dropped notifications
+    // mid image download. Pending work is bounded by IMAGE_DOWNLOAD_BUDGET_MS.
   }
 
   private fun getUniqueNotificationId() = Random.nextInt(0, Int.MAX_VALUE)
@@ -211,39 +237,48 @@ internal class PushNotificationDelegate(
     if (url.isNullOrBlank()) return null
 
     val startedAt = SystemClock.elapsedRealtime()
-    try {
-      val bitmap =
-        withTimeoutOrNull(BITMAP_TIMEOUT_MS) {
-          withContext(Dispatchers.IO) {
-            PushPushGo.getInstance().getNetwork().getBitmapFromUrl(url)
-          }
-        }
-      val elapsedMs = SystemClock.elapsedRealtime() - startedAt
-
-      // Diagnostics for missing notification images: withTimeoutOrNull() swallows
-      // the timeout and returns null, so without this the notification is posted
-      // without a picture and nothing at all shows up in the logs.
-      return when {
-        bitmap != null -> {
-          logDebug("Bitmap downloaded in ${elapsedMs}ms (${bitmap.width}x${bitmap.height}): $url")
-          bitmap
-        }
+    val bitmap = withTimeoutOrNull(IMAGE_DOWNLOAD_BUDGET_MS) { downloadBitmapWithRetry(url, startedAt) }
 
-        elapsedMs >= BITMAP_TIMEOUT_MS -> {
-          logWarning("Bitmap download timed out after ${elapsedMs}ms (limit ${BITMAP_TIMEOUT_MS}ms): $url")
-          null
-        }
+    // withTimeoutOrNull() swallows the timeout and returns null, so log it here;
+    // otherwise the notification is posted without a picture and nothing shows up in the logs.
+    val elapsedMs = SystemClock.elapsedRealtime() - startedAt
+    if (bitmap == null && elapsedMs >= IMAGE_DOWNLOAD_BUDGET_MS) {
+      logWarning("Bitmap download timed out after ${elapsedMs}ms (limit ${IMAGE_DOWNLOAD_BUDGET_MS}ms): $url")
+    }
+    return bitmap
+  }
 
-        else -> {
+  private suspend fun downloadBitmapWithRetry(
+    url: String,
+    startedAt: Long,
+  ): Bitmap? {
+    var attempt = 0
+    while (true) {
+      attempt++
+      try {
+        val bitmap = withContext(Dispatchers.IO) { PushPushGo.getInstance().getNetwork().getBitmapFromUrl(url) }
+        val elapsedMs = SystemClock.elapsedRealtime() - startedAt
+        if (bitmap == null) {
           logWarning("Bitmap decoded to null after ${elapsedMs}ms: $url")
-          null
+        } else {
+          logDebug("Bitmap downloaded in ${elapsedMs}ms (${bitmap.width}x${bitmap.height}, attempt $attempt): $url")
         }
+        return bitmap
+      } catch (e: CancellationException) {
+        throw e
+      } catch (e: IOException) {
+        // Right after a cold start the app's network can still be blocked (DNS fails
+        // until the FCM network allowlist is applied), so network errors are retried
+        // until the download budget runs out.
+        val retryInMs = RETRY_DELAYS_MS.getOrElse(attempt - 1) { RETRY_DELAYS_MS.last() }
+        val elapsedMs = SystemClock.elapsedRealtime() - startedAt
+        logWarning("Bitmap download attempt $attempt failed after ${elapsedMs}ms, retrying in ${retryInMs}ms: $url ($e)")
+        delay(retryInMs)
+      } catch (e: Throwable) {
+        logError("Failed to download bitmap picture after ${SystemClock.elapsedRealtime() - startedAt}ms: $url", e)
+        return null
       }
-    } catch (e: Throwable) {
-      logError("Failed to download bitmap picture after ${SystemClock.elapsedRealtime() - startedAt}ms: $url", e)
     }
-
-    return null
   }
 
   private fun createNotification(
@@ -401,7 +436,11 @@ internal class PushNotificationDelegate(
   )
 
   companion object {
-    private const val BITMAP_TIMEOUT_MS = 10_000L
+    // Shared by the image and icon downloads (they run concurrently). Kept well
+    // below the time Firebase allows for handling a message, since
+    // onMessageReceived now waits for the notification to be posted.
+    private const val IMAGE_DOWNLOAD_BUDGET_MS = 8_000L
+    private val RETRY_DELAYS_MS = listOf(250L, 500L, 1_000L, 2_000L)
 
     const val NOTIFICATION_ID_EXTRA = "notification_id"
     const val CAMPAIGN_ID_EXTRA = "campaign"

library/src/test/java/com/pushpushgo/sdk/push/PushNotificationDelegateTest.kt

@@ -0,0 +1,148 @@
+package com.pushpushgo.sdk.push
+
+import android.app.Notification
+import android.app.NotificationManager
+import android.content.Context
+import android.graphics.Bitmap
+import android.os.Looper
+import androidx.test.core.app.ApplicationProvider.getApplicationContext
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import com.pushpushgo.sdk.PushPushGo
+import com.pushpushgo.sdk.network.ApiRepository
+import io.mockk.coEvery
+import io.mockk.coVerify
+import io.mockk.every
+import io.mockk.mockk
+import io.mockk.mockkObject
+import io.mockk.unmockkAll
+import kotlinx.coroutines.CompletableDeferred
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.Shadows.shadowOf
+import org.robolectric.annotation.Config
+import java.net.UnknownHostException
+import java.util.concurrent.CountDownLatch
+import kotlin.concurrent.thread
+
+@RunWith(AndroidJUnit4::class)
+@Config(sdk = [33])
+class PushNotificationDelegateTest {
+  private val context: Context = getApplicationContext()
+  private val api = mockk<ApiRepository>(relaxed = true)
+  private val bitmap = Bitmap.createBitmap(10, 5, Bitmap.Config.ARGB_8888)
+
+  @Before
+  fun setUp() {
+    val pushPushGo = mockk<PushPushGo>(relaxed = true)
+    every { pushPushGo.isPPGoPush(any<Map<String, String>>()) } returns true
+    every { pushPushGo.getProjectId() } returns PROJECT_ID
+    every { pushPushGo.getNetwork() } returns api
+    coEvery { api.getBitmapFromUrl(ICON_URL) } returns null
+
+    mockkObject(PushPushGo.Companion)
+    every { PushPushGo.getInstance() } returns pushPushGo
+    every { PushPushGo.isInitialized() } returns true
+  }
+
+  @After
+  fun tearDown() = unmockkAll()
+
+  @Test
+  fun `notification is posted with image when service is destroyed mid download`() {
+    val downloadStarted = CountDownLatch(1)
+    val finishDownload = CompletableDeferred<Unit>()
+    coEvery { api.getBitmapFromUrl(IMAGE_URL) } coAnswers {
+      downloadStarted.countDown()
+      finishDownload.await()
+      bitmap
+    }
+    val delegate = PushNotificationDelegate(context)
+
+    // Messaging services call onMessageReceived on a worker thread.
+    val worker = thread { delegate.onMessageReceived(pushMessage(), context) }
+    pumpMainLooperUntil { downloadStarted.count == 0L }
+    assertEquals("image download did not start", 0L, downloadStarted.count)
+    // The service gets destroyed while the image is still downloading.
+    delegate.onDestroy()
+    finishDownload.complete(Unit)
+    pumpMainLooperUntil { !worker.isAlive && postedNotifications().isNotEmpty() }
+
+    assertEquals(BIG_PICTURE_STYLE, postedNotification().extras.getString(Notification.EXTRA_TEMPLATE))
+  }
+
+  @Test
+  fun `image download is retried after transient network error`() {
+    var attempts = 0
+    coEvery { api.getBitmapFromUrl(IMAGE_URL) } coAnswers {
+      if (attempts++ == 0) throw UnknownHostException("Unable to resolve host")
+      bitmap
+    }
+
+    deliver(PushNotificationDelegate(context))
+
+    assertEquals(BIG_PICTURE_STYLE, postedNotification().extras.getString(Notification.EXTRA_TEMPLATE))
+    coVerify(exactly = 2) { api.getBitmapFromUrl(IMAGE_URL) }
+  }
+
+  @Test
+  fun `notification is posted without image when image cannot be downloaded`() {
+    coEvery { api.getBitmapFromUrl(IMAGE_URL) } throws IllegalStateException("HTTP 404")
+
+    deliver(PushNotificationDelegate(context))
+
+    assertEquals(BIG_TEXT_STYLE, postedNotification().extras.getString(Notification.EXTRA_TEMPLATE))
+    coVerify(exactly = 1) { api.getBitmapFromUrl(IMAGE_URL) }
+  }
+
+  /** Delivers the message the way a messaging service does (on a worker thread) and waits for the notification. */
+  private fun deliver(delegate: PushNotificationDelegate) {
+    val worker = thread { delegate.onMessageReceived(pushMessage(), context) }
+    pumpMainLooperUntil { !worker.isAlive && postedNotifications().isNotEmpty() }
+  }
+
+  /** Also runs work dispatched to the main thread, which Robolectric only executes when its looper is idled. */
+  private fun pumpMainLooperUntil(condition: () -> Boolean) {
+    val deadline = System.currentTimeMillis() + 10_000
+    while (!condition() && System.currentTimeMillis() < deadline) {
+      shadowOf(Looper.getMainLooper()).idle()
+      Thread.sleep(20)
+    }
+  }
+
+  private fun postedNotifications() = shadowOf(context.getSystemService(NotificationManager::class.java)).allNotifications
+
+  private fun postedNotification(): Notification {
+    val notifications = postedNotifications()
+    assertEquals(1, notifications.size)
+    return notifications.single()
+  }
+
+  private fun pushMessage() =
+    PushMessage(
+      from = "702491788352",
+      data =
+        mapOf(
+          "project" to PROJECT_ID,
+          "subscriber" to "subscriber-id",
+          "campaign" to "campaign-id",
+          "nId" to "notification-id",
+          "image" to IMAGE_URL,
+          "icon" to ICON_URL,
+          "redirectLink" to "https://www.polsatnews.pl/",
+          "notification" to
+            """{"title":"Title","body":"Body","sound":"default","vibrate":"true","priority":0,"badge":1,"click_action":"APP_PUSH_CLICK"}""",
+        ),
+      notification = null,
+    )
+
+  private companion object {
+    const val PROJECT_ID = "648bf40e32ca635c992e0186"
+    const val IMAGE_URL = "https://static-f.pushpushgo.com/image_medium.webp"
+    const val ICON_URL = "https://static-a.pushpushgo.com/icon_original.webp"
+    const val BIG_PICTURE_STYLE = "android.app.Notification\$BigPictureStyle"
+    const val BIG_TEXT_STYLE = "android.app.Notification\$BigTextStyle"
+  }
+}

settings.gradle

@@ -11,7 +11,7 @@ if (!System.env.JITPACK) include ':sample', ':samplehms', ':samplejava', ':sampl
 dependencyResolutionManagement {
     versionCatalogs {
         libs {
-            version('sdk', '3.2.2')
+            version('sdk', '3.2.3')
             version('kotlin', '2.2.0')
             version('ksp', '2.2.0-2.0.2')
             version('coroutines', '1.10.1')