empty event store callback (#2076)

* feat(ExitInfo): ExitInfo setOnEventStoreEmptyCallback * feat(ExitInfo): ExitInfo setOnEventStoreEmptyCallback * refactor(EventStore): reworked the EventStore callback to only be called once, and only when the store is completely empty (no files in queue or in storage) * feat(ExitInfo): Track exitinfokey with event * feat(ExitInfo): ExitInfo setOnEventStoreEmptyCallback --------- Co-authored-by: jason <[email protected]>
bugsnag · Oct 8, 2024 · ee15c62 · ee15c62
1 parent 394f7d8
commit ee15c62
Show file tree

Hide file tree

Showing 12 changed files with 224 additions and 10 deletions.
diff --git a/bugsnag-android-core/api/bugsnag-android-core.api b/bugsnag-android-core/api/bugsnag-android-core.api
@@ -484,6 +484,7 @@ public class com/bugsnag/android/NativeInterface {
 	public static fun addMetadata (Ljava/lang/String;Ljava/lang/String;Ljava/lang/Object;)V
 	public static fun addMetadata (Ljava/lang/String;Ljava/util/Map;)V
 	public static fun clearMetadata (Ljava/lang/String;Ljava/lang/String;)V
+	public static fun createEmptyEvent ()Lcom/bugsnag/android/Event;
 	public static fun createEvent (Ljava/lang/Throwable;Lcom/bugsnag/android/Client;Lcom/bugsnag/android/SeverityReason;)Lcom/bugsnag/android/Event;
 	public static fun deliverReport (Ljava/io/File;)V
 	public static fun deliverReport ([B[B[BLjava/lang/String;Z)V

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStore.kt
@@ -41,6 +41,9 @@ internal class EventStore(
     private val callbackState: CallbackState
     override val logger: Logger
 
+    var onEventStoreEmptyCallback: () -> Unit = {}
+    private var isEmptyEventCallbackCalled: Boolean = false
+
     /**
      * Flush startup crashes synchronously on the main thread. Startup crashes block the main thread
      * when being sent (subject to [Configuration.setSendLaunchCrashesSynchronously])
@@ -52,7 +55,10 @@ internal class EventStore(
         val future = try {
             bgTaskService.submitTask(
                 TaskType.ERROR_REQUEST,
-                Runnable { flushLaunchCrashReport() }
+                Runnable {
+                    flushLaunchCrashReport()
+                    notifyEventQueueEmpty()
+                }
             )
         } catch (exc: RejectedExecutionException) {
             logger.d("Failed to flush launch crash reports, continuing.", exc)
@@ -136,6 +142,7 @@ internal class EventStore(
                         logger.d("No regular events to flush to Bugsnag.")
                     }
                     flushReports(storedFiles)
+                    notifyEventQueueEmpty()
                 }
             )
         } catch (exception: RejectedExecutionException) {
@@ -177,6 +184,7 @@ internal class EventStore(
             }
 
             DeliveryStatus.UNDELIVERED -> undeliveredEventPayload(eventFile)
+
             DeliveryStatus.FAILURE -> {
                 val exc: Exception = RuntimeException("Failed to deliver event payload")
                 handleEventFlushFailure(exc, eventFile)
@@ -259,6 +267,13 @@ internal class EventStore(
         return Date(findTimestampInFilename(file))
     }
 
+    private fun notifyEventQueueEmpty() {
+        if (isEmpty() && !isEmptyEventCallbackCalled) {
+            onEventStoreEmptyCallback()
+            isEmptyEventCallbackCalled = true
+        }
+    }
+
     companion object {
         private const val LAUNCH_CRASH_TIMEOUT_MS: Long = 2000
         val EVENT_COMPARATOR: Comparator<in File?> = Comparator { lhs, rhs ->

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt
@@ -8,7 +8,6 @@ import java.io.FileOutputStream
 import java.io.OutputStreamWriter
 import java.io.Writer
 import java.util.Collections
-import java.util.Comparator
 import java.util.concurrent.ConcurrentSkipListSet
 import java.util.concurrent.locks.Lock
 import java.util.concurrent.locks.ReentrantLock
@@ -54,6 +53,11 @@ internal abstract class FileStore(
         return true
     }
 
+    /**
+     * Test whether this `FileStore` is definitely empty
+     */
+    fun isEmpty(): Boolean = queuedFiles.isEmpty() && storageDir.list().isNullOrEmpty()
+
     fun enqueueContentForDelivery(content: String?, filename: String) {
         if (!isStorageDirValid(storageDir)) {
             return

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/NativeInterface.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/NativeInterface.java
@@ -51,7 +51,8 @@ private static Client getClient() {
      * will populate the Error and then pass the Event object to
      * {@link Client#populateAndNotifyAndroidEvent(Event, OnErrorCallback)}.
      */
-    private static Event createEmptyEvent() {
+    @NonNull
+    public static Event createEmptyEvent() {
         Client client = getClient();
 
         return new Event(

diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/EmptyEventCallbackTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/EmptyEventCallbackTest.kt
@@ -0,0 +1,156 @@
+package com.bugsnag.android
+
+import com.bugsnag.android.BugsnagTestUtils.generateConfiguration
+import com.bugsnag.android.BugsnagTestUtils.generateEvent
+import com.bugsnag.android.FileStore.Delegate
+import com.bugsnag.android.internal.BackgroundTaskService
+import com.bugsnag.android.internal.ImmutableConfig
+import com.bugsnag.android.internal.convertToImmutableConfig
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+import org.mockito.ArgumentMatchers.any
+import org.mockito.Mockito.mock
+import org.mockito.Mockito.`when`
+import java.io.File
+import java.nio.file.Files
+import java.util.concurrent.CountDownLatch
+
+class EmptyEventCallbackTest {
+
+    private lateinit var storageDir: File
+    private lateinit var errorDir: File
+    private lateinit var backgroundTaskService: BackgroundTaskService
+
+    @Before
+    fun setUp() {
+        storageDir = Files.createTempDirectory("tmp").toFile()
+        storageDir.deleteRecursively()
+        errorDir = File(storageDir, "bugsnag/errors")
+        backgroundTaskService = BackgroundTaskService()
+    }
+
+    @After
+    fun tearDown() {
+        storageDir.deleteRecursively()
+        backgroundTaskService.shutdown()
+    }
+
+    @Test
+    fun emptyQueuedEventTriggerEventStoreEmptyCallback() {
+        val config = generateConfiguration().apply {
+            maxPersistedEvents = 0
+            persistenceDirectory = storageDir
+        }
+        val eventStore = createEventStore(convertToImmutableConfig(config))
+        eventStore.write(generateEvent())
+
+        val callbackLatch = CountDownLatch(1)
+        eventStore.onEventStoreEmptyCallback = { callbackLatch.countDown() }
+        eventStore.flushAsync()
+        callbackLatch.await()
+
+        assertTrue(eventStore.isEmpty())
+    }
+
+    @Test
+    fun testFailedDeliveryEvents() {
+        val mockDelivery = mock(Delivery::class.java)
+        `when`(mockDelivery.deliver(any<EventPayload>(), any<DeliveryParams>()))
+            .thenReturn(
+                DeliveryStatus.DELIVERED,
+                DeliveryStatus.FAILURE
+            )
+
+        val config = generateConfiguration().apply {
+            maxPersistedEvents = 3
+            persistenceDirectory = storageDir
+            delivery = mockDelivery
+        }
+        val eventStore = createEventStore(convertToImmutableConfig(config))
+        repeat(3) {
+            eventStore.write(generateEvent())
+        }
+
+        // the EventStore should not be considered empty with 3 events in it
+        assertFalse(eventStore.isEmpty())
+
+        var eventStoreEmptyCount = 0
+        eventStore.onEventStoreEmptyCallback = { eventStoreEmptyCount++ }
+        eventStore.flushAsync()
+        backgroundTaskService.shutdown()
+
+        assertTrue(
+            "there should be no undelivered payloads in the EventStore",
+            eventStore.isEmpty()
+        )
+
+        assertEquals(
+            "onEventStoreEmptyCallback have been called even with a failed (deleted) payload",
+            1,
+            eventStoreEmptyCount
+        )
+    }
+
+    @Test
+    fun testUndeliveredEvents() {
+        val mockDelivery = mock(Delivery::class.java)
+        `when`(mockDelivery.deliver(any<EventPayload>(), any<DeliveryParams>()))
+            .thenReturn(
+                DeliveryStatus.DELIVERED,
+                DeliveryStatus.FAILURE,
+                DeliveryStatus.UNDELIVERED
+            )
+
+        val config = generateConfiguration().apply {
+            maxPersistedEvents = 3
+            persistenceDirectory = storageDir
+            delivery = mockDelivery
+        }
+        val eventStore = createEventStore(convertToImmutableConfig(config))
+        repeat(3) {
+            eventStore.write(generateEvent())
+        }
+
+        // the EventStore should not be considered empty with 3 events in it
+        assertFalse(eventStore.isEmpty())
+
+        var eventStoreEmptyCount = 0
+        eventStore.onEventStoreEmptyCallback = { eventStoreEmptyCount++ }
+        eventStore.flushAsync()
+        backgroundTaskService.shutdown()
+
+        // the last payload should not have been delivered
+        assertFalse(
+            "there should be one undelivered payload in the EventStore",
+            eventStore.isEmpty()
+        )
+
+        assertEquals(
+            "onEventStoreEmptyCallback should not be called when there are undelivered payloads",
+            0,
+            eventStoreEmptyCount
+        )
+    }
+
+    private fun createEventStore(config: ImmutableConfig): EventStore {
+        return EventStore(
+            config,
+            NoopLogger,
+            Notifier(),
+            backgroundTaskService,
+            object : Delegate {
+                override fun onErrorIOFailure(
+                    exception: Exception?,
+                    errorFile: File?,
+                    context: String?
+                ) {
+                }
+            },
+            CallbackState()
+        )
+    }
+}
diff --git a/bugsnag-plugin-android-exitinfo/detekt-baseline.xml b/bugsnag-plugin-android-exitinfo/detekt-baseline.xml
@@ -7,6 +7,7 @@
     <ID>LongParameterList:TombstoneParser.kt$TombstoneParser$( exitInfo: ApplicationExitInfo, listOpenFds: Boolean, includeLogcat: Boolean, threadConsumer: (BugsnagThread) -> Unit, fileDescriptorConsumer: (Int, String, String) -> Unit, logcatConsumer: (String) -> Unit )</ID>
     <ID>MagicNumber:TraceParser.kt$TraceParser$16</ID>
     <ID>MagicNumber:TraceParser.kt$TraceParser$3</ID>
+    <ID>MaxLineLength:ExitInfoCallback.kt$ExitInfoCallback$val allExitInfo: List&lt;ApplicationExitInfo> = am.getHistoricalProcessExitReasons(context.packageName, 0, MAX_EXIT_INFO)</ID>
     <ID>MaxLineLength:TraceParserInvalidStackframesTest.kt$TraceParserInvalidStackframesTest.Companion$"#01 pc 0000000000000c5c /data/app/~~sKQbJGqVJA5glcnvEjZCMg==/com.example.bugsnag.android-fVuoJh5GpAL7sRAeI3vjSw==/lib/arm64/libentrypoint.so "</ID>
     <ID>MaxLineLength:TraceParserInvalidStackframesTest.kt$TraceParserInvalidStackframesTest.Companion$"#01 pc 0000000000000c5c /data/app/~~sKQbJGqVJA5glcnvEjZCMg==/com.example.bugsnag.android-fVuoJh5GpAL7sRAeI3vjSw==/lib/arm64/libentrypoint.so (Java_com_example_bugsnag_android_BaseCrashyActivity_anrFromCXX+20"</ID>
     <ID>MaxLineLength:TraceParserInvalidStackframesTest.kt$TraceParserInvalidStackframesTest.Companion$"#01 pc 0000000000000c5c /data/app/~~sKQbJGqVJA5glcnvEjZCMg==/com.example.bugsnag.android-fVuoJh5GpAL7sRAeI3vjSw==/lib/arm64/libentrypoint.so (Java_com_example_bugsnag_android_BaseCrashyActivity_anrFromCXX+20) ("</ID>

diff --git a/...droid-exitinfo/src/androidTest/java/com/bugsnag/android/BugsnagExitInfoPluginStoreTest.kt b/...droid-exitinfo/src/androidTest/java/com/bugsnag/android/BugsnagExitInfoPluginStoreTest.kt
@@ -94,6 +94,20 @@ internal class BugsnagExitInfoPluginStoreTest {
         assertEquals(expectedExitInfoKeys, storageExitInfoKeys2)
     }
 
+    @Test
+    fun addExitInfoKeyToFileContents() {
+        file.writeText("12345")
+        exitInfoPluginStore = ExitInfoPluginStore(immutableConfig)
+        val expectedPid1 = requireNotNull(exitInfoPluginStore.load())
+        assertEquals(12345, expectedPid1.first)
+
+        val expectedExitInfoKey = ExitInfoKey(111, 100L)
+        exitInfoPluginStore.addExitInfoKey(expectedExitInfoKey)
+        val (expectedPid2, storageExitInfoKeys2) = requireNotNull(exitInfoPluginStore.load())
+        assertEquals(expectedPid1.first, expectedPid2)
+        assertEquals(setOf(expectedExitInfoKey), storageExitInfoKeys2)
+    }
+
     private fun generateConfiguration(): Configuration {
         val configuration = Configuration("5d1ec5bd39a74caa1267142706a7fb21")
         configuration.delivery = generateDelivery()

diff --git a/bugsnag-plugin-android-exitinfo/src/main/java/com/bugsnag/android/BugsnagExitInfoPlugin.kt b/bugsnag-plugin-android-exitinfo/src/main/java/com/bugsnag/android/BugsnagExitInfoPlugin.kt
@@ -39,7 +39,8 @@ class BugsnagExitInfoPlugin @JvmOverloads constructor(
             TraceEventEnhancer(
                 client.logger,
                 client.immutableConfig.projectPackages
-            )
+            ),
+            exitInfoPluginStore
         )
 
         client.addOnSend(exitInfoCallback)

diff --git a/bugsnag-plugin-android-exitinfo/src/main/java/com/bugsnag/android/ExitInfoCallback.kt b/bugsnag-plugin-android-exitinfo/src/main/java/com/bugsnag/android/ExitInfoCallback.kt
@@ -26,15 +26,17 @@ internal class ExitInfoCallback(
     private val context: Context,
     private val pid: Int?,
     private val nativeEnhancer: (Event, ApplicationExitInfo) -> Unit,
-    private val anrEventEnhancer: (Event, ApplicationExitInfo) -> Unit
+    private val anrEventEnhancer: (Event, ApplicationExitInfo) -> Unit,
+    private val exitInfoPluginStore: ExitInfoPluginStore?
 ) : OnSendCallback {
 
     override fun onSend(event: Event): Boolean {
-        val am = context.getSystemService(Context.ACTIVITY_SERVICE) as ActivityManager
-        val allExitInfo = am.getHistoricalProcessExitReasons(context.packageName, 0, MAX_EXIT_INFO)
-        val sessionIdBytes = event.session?.id?.toByteArray() ?: return true
-        val exitInfo = findExitInfoBySessionId(allExitInfo, sessionIdBytes)
+        val am: ActivityManager = context.getSystemService(Context.ACTIVITY_SERVICE) as ActivityManager
+        val allExitInfo: List<ApplicationExitInfo> = am.getHistoricalProcessExitReasons(context.packageName, 0, MAX_EXIT_INFO)
+        val sessionIdBytes: ByteArray = event.session?.id?.toByteArray() ?: return true
+        val exitInfo: ApplicationExitInfo = findExitInfoBySessionId(allExitInfo, sessionIdBytes)
             ?: findExitInfoByPid(allExitInfo) ?: return true
+        exitInfoPluginStore?.addExitInfoKey(ExitInfoKey(exitInfo.pid, exitInfo.timestamp))
 
         try {
             val reason = exitReasonOf(exitInfo)

diff --git a/bugsnag-plugin-android-exitinfo/src/main/java/com/bugsnag/android/ExitInfoPluginStore.kt b/bugsnag-plugin-android-exitinfo/src/main/java/com/bugsnag/android/ExitInfoPluginStore.kt
@@ -72,4 +72,10 @@ internal class ExitInfoPluginStore(config: ImmutableConfig) {
             return null
         }
     }
+
+    fun addExitInfoKey(exitInfoKey: ExitInfoKey) {
+        val (oldPid, exitInfoKeys) = load()
+        val newExitInfoKeys = exitInfoKeys.toMutableSet().plus(exitInfoKey)
+        oldPid?.let { persist(it, newExitInfoKeys) }
+    }
 }
diff --git a/bugsnag-plugin-android-exitinfo/src/main/java/com/bugsnag/android/InternalHooks.java b/bugsnag-plugin-android-exitinfo/src/main/java/com/bugsnag/android/InternalHooks.java
@@ -0,0 +1,13 @@
+package com.bugsnag.android;
+
+import kotlin.Unit;
+import kotlin.jvm.functions.Function0;
+
+class InternalHooks {
+
+    private InternalHooks() {}
+
+    public static void setEventStoreEmptyCallback(Client client, Function0<Unit> callback) {
+        client.eventStore.setOnEventStoreEmptyCallback(callback);
+    }
+}
diff --git a/bugsnag-plugin-android-exitinfo/src/test/java/com/bugsnag/android/ExitInfoCallbackTest.kt b/bugsnag-plugin-android-exitinfo/src/test/java/com/bugsnag/android/ExitInfoCallbackTest.kt
@@ -45,7 +45,7 @@ internal class ExitInfoCallbackTest {
 
     @Before
     fun setUp() {
-        exitInfoCallback = ExitInfoCallback(context, 100, nativeEnhancer, anrEventEnhancer)
+        exitInfoCallback = ExitInfoCallback(context, 100, nativeEnhancer, anrEventEnhancer, null)
         exitInfos = listOf(exitInfo1)
         `when`(context.getSystemService(any())).thenReturn(am)
         `when`(am.getHistoricalProcessExitReasons(any(), anyInt(), anyInt()))