TRIAGE-608: Fix retry-on-failure throttle logic

Author: junias-rokt

android-core/src/main/java/com/mparticle/internal/UploadHandler.java

@@ -252,16 +252,13 @@ void maybePrunePersistedRecords(long nowMillis) {
         if (mParticleDBManager == null) {
             return;
         }
-        try {
-            Integer configured = mConfigManager == null ? null : mConfigManager.getPersistenceMaxAgeSeconds();
-            long maxAgeMillis = (configured == null)
-                    ? DEFAULT_PERSISTENCE_MAX_AGE_MILLIS
-                    : configured.longValue() * 1000L;
-            long cutoffMillis = nowMillis - maxAgeMillis;
-            mParticleDBManager.deleteRecordsOlderThan(cutoffMillis);
+        Integer configured = mConfigManager == null ? null : mConfigManager.getPersistenceMaxAgeSeconds();
+        long maxAgeMillis = (configured == null)
+                ? DEFAULT_PERSISTENCE_MAX_AGE_MILLIS
+                : configured.longValue() * 1000L;
+        long cutoffMillis = nowMillis - maxAgeMillis;
+        if (mParticleDBManager.deleteRecordsOlderThan(cutoffMillis)) {
             mLastPersistenceCleanupMillis = nowMillis;
-        } catch (Exception e) {
-            Logger.warning(e, "Failed to prune persisted records by age.");
         }
     }
 

android-core/src/main/java/com/mparticle/internal/database/services/MParticleDBManager.java

@@ -7,6 +7,7 @@
 import android.os.Handler;
 import android.os.Looper;
 
+import androidx.annotation.CheckResult;
 import androidx.annotation.Nullable;
 
 import com.mparticle.MParticle;
@@ -211,21 +212,27 @@ public void deleteMessagesAndSessions(String currentSessionId) {
      * <p>
      * Deletes any messages and uploads whose {@code CREATED_AT} is strictly less than
      * {@code cutoffMillis}, and any sessions whose {@code END_TIME} is strictly less than
-     * {@code cutoffMillis}. Mirrors the behaviour of the iOS SDK's
-     * {@code MPPersistenceController.deleteRecordsOlderThan:}.
+     * {@code cutoffMillis}.
+     * {@code MPPersistenceController.deleteRecordsOlderThan:}, but reports success so
+     * callers can decide whether to arm retry/throttle state.
      *
      * @param cutoffMillis the unix-epoch millisecond cutoff; rows older than this are removed
+     * @return {@code true} if the transaction committed successfully, {@code false} if any
+     *         exception was caught (and logged) during the sweep
      */
-    public void deleteRecordsOlderThan(long cutoffMillis) {
+    @CheckResult
+    public boolean deleteRecordsOlderThan(long cutoffMillis) {
         MPDatabase db = getDatabase();
         try {
             db.beginTransaction();
             MessageService.deleteMessagesOlderThan(db, cutoffMillis);
             UploadService.deleteUploadsOlderThan(db, cutoffMillis);
             SessionService.deleteSessionsOlderThan(db, cutoffMillis);
             db.setTransactionSuccessful();
+            return true;
         } catch (Exception e) {
             Logger.warning(e, "Error pruning persisted records older than " + cutoffMillis + " ms.");
+            return false;
         } finally {
             db.endTransaction();
         }

android-core/src/test/kotlin/com/mparticle/internal/UploadHandlerTest.kt

@@ -150,7 +150,7 @@ class UploadHandlerTest {
         val capturedCutoff = java.util.concurrent.atomic.AtomicLong(Long.MIN_VALUE)
         Mockito.doAnswer { invocation ->
             capturedCutoff.set(invocation.getArgument(0))
-            null
+            true
         }.`when`(db).deleteRecordsOlderThan(Mockito.anyLong())
 
         val now = 1_700_000_000_000L
@@ -182,7 +182,7 @@ class UploadHandlerTest {
         val capturedCutoff = java.util.concurrent.atomic.AtomicLong(Long.MIN_VALUE)
         Mockito.doAnswer { invocation ->
             capturedCutoff.set(invocation.getArgument(0))
-            null
+            true
         }.`when`(db).deleteRecordsOlderThan(Mockito.anyLong())
 
         val now = 1_700_000_000_000L
@@ -194,6 +194,8 @@ class UploadHandlerTest {
     @Test
     fun testMaybePrunePersistedRecordsHonorsTwentyFourHourThrottle() {
         val db = handler.mParticleDBManager
+        // The sweep must succeed so the throttle arms; otherwise every call retries.
+        Mockito.`when`(db.deleteRecordsOlderThan(Mockito.anyLong())).thenReturn(true)
         val t0 = 1_000_000_000_000L
 
         // First call always runs the sweep.
@@ -213,8 +215,10 @@ class UploadHandlerTest {
     fun testMaybePrunePersistedRecordsRetriesAfterFailure() {
         val db = handler.mParticleDBManager
 
-        // Phase 1 - deleteRecordsOlderThan throws; throttle must NOT be armed.
-        Mockito.doThrow(RuntimeException("boom")).`when`(db).deleteRecordsOlderThan(Mockito.anyLong())
+        // Phase 1 - deleteRecordsOlderThan reports failure (false); throttle must NOT be armed.
+        // This models the real contract: MParticleDBManager.deleteRecordsOlderThan catches
+        // SQL exceptions internally and signals failure via its return value.
+        Mockito.`when`(db.deleteRecordsOlderThan(Mockito.anyLong())).thenReturn(false)
         val t1 = 1_000_000_000_000L
         handler.maybePrunePersistedRecords(t1)
         Mockito.verify(db, Mockito.times(1)).deleteRecordsOlderThan(Mockito.anyLong())
@@ -224,7 +228,7 @@ class UploadHandlerTest {
         Mockito.verify(db, Mockito.times(2)).deleteRecordsOlderThan(Mockito.anyLong())
 
         // Phase 3 - successful sweep arms the throttle.
-        Mockito.doNothing().`when`(db).deleteRecordsOlderThan(Mockito.anyLong())
+        Mockito.`when`(db.deleteRecordsOlderThan(Mockito.anyLong())).thenReturn(true)
         handler.maybePrunePersistedRecords(t1 + 120_000L)
         Mockito.verify(db, Mockito.times(3)).deleteRecordsOlderThan(Mockito.anyLong())