From 9958d9f35d01056c16453262d95d41f6bcc2f06b Mon Sep 17 00:00:00 2001 From: jason Date: Wed, 11 Jun 2025 10:18:27 +0100 Subject: [PATCH 1/3] fix(startup): more startup processing to complete in the background --- bugsnag-android-core/detekt-baseline.xml | 2 +- .../android/AppDataCollectorForegroundTest.kt | 3 +- .../bugsnag/android/AppDataCollectorTest.kt | 15 +++--- .../java/com/bugsnag/android/FileStoreTest.kt | 3 +- .../com/bugsnag/android/AppDataCollector.kt | 11 +++-- .../bugsnag/android/DataCollectionModule.kt | 2 +- .../com/bugsnag/android/EventStorageModule.kt | 6 +-- .../java/com/bugsnag/android/EventStore.kt | 3 +- .../java/com/bugsnag/android/FileStore.kt | 7 +-- .../android/InternalReportDelegate.java | 10 ++-- .../java/com/bugsnag/android/SessionStore.kt | 3 +- .../AppDataCollectorSerializationTest.kt | 3 +- .../android/AppMetadataSerializationTest.kt | 3 +- .../bugsnag/android/EmptyEventCallbackTest.kt | 17 ++++--- .../com/bugsnag/android/EventFilenameTest.kt | 49 +++++++++++-------- .../bugsnag/android/EventStoreMaxLimitTest.kt | 17 ++++--- .../InternalEventPayloadDelegateTest.kt | 4 +- .../android/LaunchCrashDeliveryTest.kt | 17 ++++--- .../android/SessionStoreMaxLimitTest.kt | 17 ++++--- 19 files changed, 110 insertions(+), 82 deletions(-) diff --git a/bugsnag-android-core/detekt-baseline.xml b/bugsnag-android-core/detekt-baseline.xml index 843019fbd7..a44dcfc279 100644 --- a/bugsnag-android-core/detekt-baseline.xml +++ b/bugsnag-android-core/detekt-baseline.xml @@ -6,7 +6,7 @@ CyclomaticComplexMethod:ConfigInternal.kt$ConfigInternal$fun getConfigDifferences(): Map<String, Any> ImplicitDefaultLocale:Deliverable.kt$Deliverable$String.format("%02x", byte) LongParameterList:App.kt$App$( /** * The architecture of the running application binary */ var binaryArch: String?, /** * The package name of the application */ var id: String?, /** * The release stage set in [Configuration.releaseStage] */ var releaseStage: String?, /** * The version of the application set in [Configuration.version] */ var version: String?, /** The revision ID from the manifest (React Native apps only) */ var codeBundleId: String?, /** * The unique identifier for the build of the application set in [Configuration.buildUuid] */ buildUuid: Provider<String?>?, /** * The application type set in [Configuration#version] */ var type: String?, /** * The version code of the application set in [Configuration.versionCode] */ var versionCode: Number? ) - LongParameterList:AppDataCollector.kt$AppDataCollector$( appContext: Context, private val packageManager: PackageManager?, private val config: ImmutableConfig, private val sessionTracker: SessionTracker, private val activityManager: ActivityManager?, private val launchCrashTracker: LaunchCrashTracker, private val memoryTrimState: MemoryTrimState ) + LongParameterList:AppDataCollector.kt$AppDataCollector$( appContext: Context, private val packageManager: PackageManager?, private val config: ImmutableConfig, private val sessionTracker: Provider<SessionTracker>, private val activityManager: ActivityManager?, private val launchCrashTracker: LaunchCrashTracker, private val memoryTrimState: MemoryTrimState ) LongParameterList:AppWithState.kt$AppWithState$( binaryArch: String?, id: String?, releaseStage: String?, version: String?, codeBundleId: String?, buildUuid: Provider<String?>?, type: String?, versionCode: Number?, /** * The number of milliseconds the application was running before the event occurred */ var duration: Number?, /** * The number of milliseconds the application was running in the foreground before the * event occurred */ var durationInForeground: Number?, /** * Whether the application was in the foreground when the event occurred */ var inForeground: Boolean?, /** * Whether the application was launching when the event occurred */ var isLaunching: Boolean? ) LongParameterList:AppWithState.kt$AppWithState$( binaryArch: String?, id: String?, releaseStage: String?, version: String?, codeBundleId: String?, buildUuid: String?, type: String?, versionCode: Number?, /** * The number of milliseconds the application was running before the event occurred */ duration: Number?, /** * The number of milliseconds the application was running in the foreground before the * event occurred */ durationInForeground: Number?, /** * Whether the application was in the foreground when the event occurred */ inForeground: Boolean?, /** * Whether the application was launching when the event occurred */ isLaunching: Boolean? ) LongParameterList:AppWithState.kt$AppWithState$( config: ImmutableConfig, binaryArch: String?, id: String?, releaseStage: String?, version: String?, codeBundleId: String?, duration: Number?, durationInForeground: Number?, inForeground: Boolean?, isLaunching: Boolean? ) diff --git a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/AppDataCollectorForegroundTest.kt b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/AppDataCollectorForegroundTest.kt index d57317c881..18f052ea8f 100644 --- a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/AppDataCollectorForegroundTest.kt +++ b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/AppDataCollectorForegroundTest.kt @@ -2,6 +2,7 @@ package com.bugsnag.android import android.content.Context import android.os.SystemClock +import com.bugsnag.android.internal.dag.ValueProvider import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue import org.junit.Before @@ -41,7 +42,7 @@ class AppDataCollectorForegroundTest { appContext, null, config, - sessionTracker, + ValueProvider(sessionTracker), null, launchCrashTracker, memoryTrimState diff --git a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/AppDataCollectorTest.kt b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/AppDataCollectorTest.kt index 09da647a6a..4207d15ed9 100644 --- a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/AppDataCollectorTest.kt +++ b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/AppDataCollectorTest.kt @@ -6,6 +6,7 @@ import android.content.pm.PackageManager import android.os.Build import android.os.Process import androidx.test.core.app.ApplicationProvider +import com.bugsnag.android.internal.dag.ValueProvider import org.junit.Assert.assertEquals import org.junit.Assert.assertNull import org.junit.Assert.assertTrue @@ -49,7 +50,7 @@ class AppDataCollectorTest { context, context.packageManager, client.immutableConfig, - client.sessionTracker, + ValueProvider(client.sessionTracker), am, client.launchCrashTracker, client.memoryTrimState @@ -70,7 +71,7 @@ class AppDataCollectorTest { context, context.packageManager, client.immutableConfig, - client.sessionTracker, + ValueProvider(client.sessionTracker), am, client.launchCrashTracker, client.memoryTrimState @@ -93,7 +94,7 @@ class AppDataCollectorTest { context, context.packageManager, client.immutableConfig, - client.sessionTracker, + ValueProvider(client.sessionTracker), am, client.launchCrashTracker, client.memoryTrimState @@ -115,7 +116,7 @@ class AppDataCollectorTest { context, packageManager, client.immutableConfig, - client.sessionTracker, + ValueProvider(client.sessionTracker), am, client.launchCrashTracker, client.memoryTrimState @@ -144,7 +145,7 @@ class AppDataCollectorTest { context, packageManager, client.immutableConfig, - client.sessionTracker, + ValueProvider(client.sessionTracker), am, client.launchCrashTracker, client.memoryTrimState @@ -171,7 +172,7 @@ class AppDataCollectorTest { context, packageManager, client.immutableConfig, - client.sessionTracker, + ValueProvider(client.sessionTracker), am, client.launchCrashTracker, client.memoryTrimState @@ -198,7 +199,7 @@ class AppDataCollectorTest { context, packageManager, client.immutableConfig, - client.sessionTracker, + ValueProvider(client.sessionTracker), am, client.launchCrashTracker, client.memoryTrimState diff --git a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/FileStoreTest.kt b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/FileStoreTest.kt index 6c4733ac54..3363cf5b21 100644 --- a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/FileStoreTest.kt +++ b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/FileStoreTest.kt @@ -2,6 +2,7 @@ package com.bugsnag.android import android.app.Application import androidx.test.core.app.ApplicationProvider +import com.bugsnag.android.internal.dag.ValueProvider import org.junit.Assert.assertEquals import org.junit.Test import org.junit.runner.RunWith @@ -48,6 +49,6 @@ internal class CustomFileStore( folder: File, maxStoreCount: Int, delegate: Delegate? -) : FileStore(folder, maxStoreCount, NoopLogger, delegate) { +) : FileStore(folder, maxStoreCount, NoopLogger, ValueProvider(delegate)) { override fun getFilename(obj: Any?) = "foo.json" } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/AppDataCollector.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/AppDataCollector.kt index c1eb409bc8..0347f2a428 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/AppDataCollector.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/AppDataCollector.kt @@ -24,6 +24,7 @@ import android.os.Build.VERSION_CODES import android.os.Process import android.os.SystemClock import com.bugsnag.android.internal.ImmutableConfig +import com.bugsnag.android.internal.dag.Provider /** * Collects various data on the application state @@ -32,7 +33,7 @@ internal class AppDataCollector( appContext: Context, private val packageManager: PackageManager?, private val config: ImmutableConfig, - private val sessionTracker: SessionTracker, + private val sessionTracker: Provider, private val activityManager: ActivityManager?, private val launchCrashTracker: LaunchCrashTracker, private val memoryTrimState: MemoryTrimState @@ -54,7 +55,7 @@ internal class AppDataCollector( App(config, binaryArch, packageName, releaseStage, versionName, codeBundleId) fun generateAppWithState(): AppWithState { - val inForeground = sessionTracker.isInForeground + val inForeground = sessionTracker.get().isInForeground val durationInForeground = calculateDurationInForeground(inForeground) return AppWithState( @@ -118,7 +119,7 @@ internal class AppDataCollector( fun getAppDataMetadata(): MutableMap { val map = HashMap() map["name"] = appName - map["activeScreen"] = sessionTracker.contextActivity + map["activeScreen"] = sessionTracker.get().contextActivity map["lowMemory"] = memoryTrimState.isLowMemory map["memoryTrimLevel"] = memoryTrimState.trimLevelDescription map["processImportance"] = getProcessImportance() @@ -168,7 +169,7 @@ internal class AppDataCollector( * * @return the duration in ms */ - internal fun calculateDurationInForeground(inForeground: Boolean? = sessionTracker.isInForeground): Long? { + internal fun calculateDurationInForeground(inForeground: Boolean? = sessionTracker.get().isInForeground): Long? { if (inForeground == null) { return null } @@ -176,7 +177,7 @@ internal class AppDataCollector( val nowMs = SystemClock.elapsedRealtime() var durationMs: Long = 0 - val sessionStartTimeMs: Long = sessionTracker.lastEnteredForegroundMs + val sessionStartTimeMs: Long = sessionTracker.get().lastEnteredForegroundMs if (inForeground && sessionStartTimeMs != 0L) { durationMs = nowMs - sessionStartTimeMs diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/DataCollectionModule.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/DataCollectionModule.kt index e8967de359..4b9edee875 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/DataCollectionModule.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/DataCollectionModule.kt @@ -34,7 +34,7 @@ internal class DataCollectionModule( ctx, ctx.packageManager, cfg, - trackerModule.sessionTracker.get(), + trackerModule.sessionTracker, systemServiceModule.activityManager, trackerModule.launchCrashTracker, memoryTrimState diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStorageModule.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStorageModule.kt index 2d519eb5b1..09996e232a 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStorageModule.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStorageModule.kt @@ -29,9 +29,9 @@ internal class EventStorageModule( cfg.logger, cfg, systemServiceModule.storageManager, - dataCollectionModule.appDataCollector.get(), + dataCollectionModule.appDataCollector, dataCollectionModule.deviceDataCollector, - trackerModule.sessionTracker.get(), + trackerModule.sessionTracker, notifier, bgTaskService ) else null @@ -43,7 +43,7 @@ internal class EventStorageModule( cfg.logger, notifier, bgTaskService, - delegate.getOrNull(), + delegate, callbackState ) } 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 index 5d68837b05..e11fe96ba6 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStore.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStore.kt @@ -9,6 +9,7 @@ import com.bugsnag.android.internal.BackgroundTaskService import com.bugsnag.android.internal.ForegroundDetector import com.bugsnag.android.internal.ImmutableConfig import com.bugsnag.android.internal.TaskType +import com.bugsnag.android.internal.dag.Provider import java.io.File import java.util.Calendar import java.util.Date @@ -27,7 +28,7 @@ internal class EventStore( logger: Logger, notifier: Notifier, bgTaskService: BackgroundTaskService, - delegate: Delegate?, + delegate: Provider?, callbackState: CallbackState ) : FileStore( File(config.persistenceDirectory.value, "bugsnag/errors"), 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 index f7ede678cb..aa49df6d54 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt @@ -1,6 +1,7 @@ package com.bugsnag.android import com.bugsnag.android.JsonStream.Streamable +import com.bugsnag.android.internal.dag.Provider import java.io.BufferedWriter import java.io.File import java.io.FileNotFoundException @@ -15,7 +16,7 @@ internal abstract class FileStore( val storageDir: File, private val maxStoreCount: Int, protected open val logger: Logger, - protected val delegate: Delegate? + protected val delegate: Provider? ) { internal fun interface Delegate { /** @@ -66,7 +67,7 @@ internal abstract class FileStore( out.write(content) } catch (exc: Exception) { val eventFile = File(filePath) - delegate?.onErrorIOFailure(exc, eventFile, "NDK Crash report copy") + delegate?.getOrNull()?.onErrorIOFailure(exc, eventFile, "NDK Crash report copy") IOUtils.deleteFile(eventFile, logger) } finally { try { @@ -100,7 +101,7 @@ internal abstract class FileStore( logger.w("Ignoring FileNotFoundException - unable to create file", exc) } catch (exc: Exception) { val eventFile = File(filename) - delegate?.onErrorIOFailure(exc, eventFile, "Crash report serialization") + delegate?.getOrNull()?.onErrorIOFailure(exc, eventFile, "Crash report serialization") IOUtils.deleteFile(eventFile, logger) } finally { IOUtils.closeQuietly(stream) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/InternalReportDelegate.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/InternalReportDelegate.java index ad1d791bdf..f45c764a92 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/InternalReportDelegate.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/InternalReportDelegate.java @@ -32,10 +32,10 @@ class InternalReportDelegate implements EventStore.Delegate { @Nullable final StorageManager storageManager; - final AppDataCollector appDataCollector; + final Provider appDataCollector; final Provider deviceDataCollector; final Context appContext; - final SessionTracker sessionTracker; + final Provider sessionTracker; final Notifier notifier; final BackgroundTaskService backgroundTaskService; @@ -43,9 +43,9 @@ class InternalReportDelegate implements EventStore.Delegate { Logger logger, ImmutableConfig immutableConfig, @Nullable StorageManager storageManager, - AppDataCollector appDataCollector, + Provider appDataCollector, Provider deviceDataCollector, - SessionTracker sessionTracker, + Provider sessionTracker, Notifier notifier, BackgroundTaskService backgroundTaskService) { this.logger = logger; @@ -101,7 +101,7 @@ void recordStorageCacheBehavior(Event event) { * This is intended for internal use only, and reports will not be visible to end-users. */ void reportInternalBugsnagError(@NonNull Event event) { - event.setApp(appDataCollector.generateAppWithState()); + event.setApp(appDataCollector.get().generateAppWithState()); event.setDevice(deviceDataCollector.get().generateDeviceWithState(new Date().getTime())); event.addMetadata(INTERNAL_DIAGNOSTICS_TAB, "notifierName", notifier.getName()); diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionStore.kt index e6e0ce6aa8..82241ca949 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionStore.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionStore.kt @@ -2,6 +2,7 @@ package com.bugsnag.android import com.bugsnag.android.SessionFilenameInfo.Companion.defaultFilename import com.bugsnag.android.SessionFilenameInfo.Companion.findTimestampInFilename +import com.bugsnag.android.internal.dag.Provider import java.io.File import java.util.Calendar import java.util.Comparator @@ -16,7 +17,7 @@ internal class SessionStore( maxPersistedSessions: Int, private val apiKey: String, logger: Logger, - delegate: Delegate? + delegate: Provider? ) : FileStore( File(bugsnagDir, "sessions"), maxPersistedSessions, diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/AppDataCollectorSerializationTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/AppDataCollectorSerializationTest.kt index 93f33991a7..4f58d2b6fe 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/AppDataCollectorSerializationTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/AppDataCollectorSerializationTest.kt @@ -5,6 +5,7 @@ import android.content.Context import android.content.pm.ApplicationInfo import android.content.pm.PackageManager import com.bugsnag.android.BugsnagTestUtils.convert +import com.bugsnag.android.internal.dag.ValueProvider import org.junit.Test import org.junit.runner.RunWith import org.junit.runners.Parameterized @@ -49,7 +50,7 @@ internal class AppDataCollectorSerializationTest { context, pm, convert(config), - sessionTracker, + ValueProvider(sessionTracker), am, launchCrashTracker, memoryTrimState diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/AppMetadataSerializationTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/AppMetadataSerializationTest.kt index 70cf84c87b..829468dcd8 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/AppMetadataSerializationTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/AppMetadataSerializationTest.kt @@ -5,6 +5,7 @@ import android.content.Context import android.content.pm.ApplicationInfo import android.content.pm.PackageManager import com.bugsnag.android.internal.convertToImmutableConfig +import com.bugsnag.android.internal.dag.ValueProvider import org.junit.Assert.assertNotNull import org.junit.Test import org.junit.runner.RunWith @@ -50,7 +51,7 @@ internal class AppMetadataSerializationTest { context, pm, convertToImmutableConfig(config, null, null, ApplicationInfo()), - sessionTracker, + ValueProvider(sessionTracker), am, launchCrashTracker, memoryTrimState 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 index 6c802250a3..13da2d8cd4 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/EmptyEventCallbackTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/EmptyEventCallbackTest.kt @@ -6,6 +6,7 @@ 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 com.bugsnag.android.internal.dag.ValueProvider import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse @@ -142,14 +143,16 @@ class EmptyEventCallbackTest { NoopLogger, Notifier(), backgroundTaskService, - object : Delegate { - override fun onErrorIOFailure( - exception: Exception?, - errorFile: File?, - context: String? - ) { + ValueProvider( + object : Delegate { + override fun onErrorIOFailure( + exception: Exception?, + errorFile: File?, + context: String? + ) { + } } - }, + ), CallbackState() ) } diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/EventFilenameTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/EventFilenameTest.kt index 53742e044a..d042eeafb0 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/EventFilenameTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/EventFilenameTest.kt @@ -3,6 +3,7 @@ package com.bugsnag.android import com.bugsnag.android.EventStore.Companion.EVENT_COMPARATOR import com.bugsnag.android.FileStore.Delegate import com.bugsnag.android.internal.BackgroundTaskService +import com.bugsnag.android.internal.dag.ValueProvider import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNull @@ -58,14 +59,16 @@ internal class EventFilenameTest { NoopLogger, Notifier(), BackgroundTaskService(), - object : Delegate { - override fun onErrorIOFailure( - exception: Exception?, - errorFile: File?, - context: String? - ) { + ValueProvider( + object : Delegate { + override fun onErrorIOFailure( + exception: Exception?, + errorFile: File?, + context: String? + ) { + } } - }, + ), CallbackState() ) @@ -87,14 +90,16 @@ internal class EventFilenameTest { NoopLogger, Notifier(), BackgroundTaskService(), - object : Delegate { - override fun onErrorIOFailure( - exception: Exception?, - errorFile: File?, - context: String? - ) { + ValueProvider( + object : Delegate { + override fun onErrorIOFailure( + exception: Exception?, + errorFile: File?, + context: String? + ) { + } } - }, + ), CallbackState() ) @@ -110,14 +115,16 @@ internal class EventFilenameTest { NoopLogger, Notifier(), BackgroundTaskService(), - object : Delegate { - override fun onErrorIOFailure( - exception: Exception?, - errorFile: File?, - context: String? - ) { + ValueProvider( + object : Delegate { + override fun onErrorIOFailure( + exception: Exception?, + errorFile: File?, + context: String? + ) { + } } - }, + ), CallbackState() ) diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/EventStoreMaxLimitTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/EventStoreMaxLimitTest.kt index 4bcf5f964f..5e21302fc2 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/EventStoreMaxLimitTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/EventStoreMaxLimitTest.kt @@ -6,6 +6,7 @@ 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 com.bugsnag.android.internal.dag.ValueProvider import org.junit.After import org.junit.Assert.assertEquals import org.junit.Before @@ -84,14 +85,16 @@ class EventStoreMaxLimitTest { NoopLogger, Notifier(), BackgroundTaskService(), - object : Delegate { - override fun onErrorIOFailure( - exception: Exception?, - errorFile: File?, - context: String? - ) { + ValueProvider( + object : Delegate { + override fun onErrorIOFailure( + exception: Exception?, + errorFile: File?, + context: String? + ) { + } } - }, + ), CallbackState() ) } diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/InternalEventPayloadDelegateTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/InternalEventPayloadDelegateTest.kt index d17dac578f..666e412131 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/InternalEventPayloadDelegateTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/InternalEventPayloadDelegateTest.kt @@ -53,9 +53,9 @@ internal class InternalEventPayloadDelegateTest { NoopLogger, config, storageManager, - appDataCollector, + ValueProvider(appDataCollector), ValueProvider(deviceDataCollector), - sessionTracker, + ValueProvider(sessionTracker), Notifier(), BackgroundTaskService() ) diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/LaunchCrashDeliveryTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/LaunchCrashDeliveryTest.kt index 401e8fd1fa..1a6fe613a8 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/LaunchCrashDeliveryTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/LaunchCrashDeliveryTest.kt @@ -4,6 +4,7 @@ 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.dag.ValueProvider import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue @@ -161,14 +162,16 @@ class LaunchCrashDeliveryTest { NoopLogger, Notifier(), backgroundTaskService, - object : Delegate { - override fun onErrorIOFailure( - exception: Exception?, - errorFile: File?, - context: String? - ) { + ValueProvider( + object : Delegate { + override fun onErrorIOFailure( + exception: Exception?, + errorFile: File?, + context: String? + ) { + } } - }, + ), CallbackState() ) } diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/SessionStoreMaxLimitTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/SessionStoreMaxLimitTest.kt index 1a3e4c6f24..b9a17648b3 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/SessionStoreMaxLimitTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/SessionStoreMaxLimitTest.kt @@ -5,6 +5,7 @@ import com.bugsnag.android.BugsnagTestUtils.generateSession import com.bugsnag.android.FileStore.Delegate import com.bugsnag.android.internal.ImmutableConfig import com.bugsnag.android.internal.convertToImmutableConfig +import com.bugsnag.android.internal.dag.ValueProvider import org.junit.After import org.junit.Assert.assertEquals import org.junit.Before @@ -83,14 +84,16 @@ class SessionStoreMaxLimitTest { config.maxPersistedSessions, config.apiKey, NoopLogger, - object : Delegate { - override fun onErrorIOFailure( - exception: Exception?, - errorFile: File?, - context: String? - ) { + ValueProvider( + object : Delegate { + override fun onErrorIOFailure( + exception: Exception?, + errorFile: File?, + context: String? + ) { + } } - } + ) ) } } From 75693c825ef443a44e8635b07154a6126ab9e7f0 Mon Sep 17 00:00:00 2001 From: jason Date: Wed, 11 Jun 2025 14:32:05 +0100 Subject: [PATCH 2/3] fix(startup): allow the SessionTracker startup to complete later without blocking --- .../com/bugsnag/android/SessionTracker.java | 26 ++++++++++--------- .../java/com/bugsnag/android/TrackerModule.kt | 2 +- .../android/SessionTrackerPauseResumeTest.kt | 3 ++- .../bugsnag/android/SessionTrackerTest.java | 9 ++++--- 4 files changed, 22 insertions(+), 18 deletions(-) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionTracker.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionTracker.java index 25fb7058c4..82d1c80575 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionTracker.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionTracker.java @@ -5,6 +5,7 @@ import com.bugsnag.android.internal.ForegroundDetector; import com.bugsnag.android.internal.ImmutableConfig; import com.bugsnag.android.internal.TaskType; +import com.bugsnag.android.internal.dag.Provider; import android.app.Activity; @@ -32,7 +33,7 @@ class SessionTracker extends BaseObservable implements ForegroundDetector.OnActi private final ImmutableConfig configuration; private final CallbackState callbackState; private final Client client; - final SessionStore sessionStore; + final Provider sessionStore; private volatile Session currentSession = null; final BackgroundTaskService backgroundTaskService; final Logger logger; @@ -41,7 +42,7 @@ class SessionTracker extends BaseObservable implements ForegroundDetector.OnActi SessionTracker(ImmutableConfig configuration, CallbackState callbackState, Client client, - SessionStore sessionStore, + Provider sessionStore, Logger logger, BackgroundTaskService backgroundTaskService) { this(configuration, callbackState, client, DEFAULT_TIMEOUT_MS, @@ -52,7 +53,7 @@ class SessionTracker extends BaseObservable implements ForegroundDetector.OnActi CallbackState callbackState, Client client, long timeoutMs, - SessionStore sessionStore, + Provider sessionStore, Logger logger, BackgroundTaskService backgroundTaskService) { this.configuration = configuration; @@ -263,7 +264,7 @@ public void run() { * Attempts to flush session payloads stored on disk */ void flushStoredSessions() { - List storedFiles = sessionStore.findStoredFiles(); + List storedFiles = sessionStore.get().findStoredFiles(); for (File storedFile : storedFiles) { flushStoredSession(storedFile); @@ -282,27 +283,28 @@ void flushStoredSession(File storedFile) { } DeliveryStatus deliveryStatus = deliverSessionPayload(payload); + SessionStore store = sessionStore.get(); switch (deliveryStatus) { case DELIVERED: - sessionStore.deleteStoredFiles(Collections.singletonList(storedFile)); + store.deleteStoredFiles(Collections.singletonList(storedFile)); logger.d("Sent 1 new session to Bugsnag"); break; case UNDELIVERED: - if (sessionStore.isTooOld(storedFile)) { + if (store.isTooOld(storedFile)) { logger.w("Discarding historical session (from {" - + sessionStore.getCreationDate(storedFile) + + store.getCreationDate(storedFile) + "}) after failed delivery"); - sessionStore.deleteStoredFiles(Collections.singletonList(storedFile)); + store.deleteStoredFiles(Collections.singletonList(storedFile)); } else { - sessionStore.cancelQueuedFiles(Collections.singletonList(storedFile)); + store.cancelQueuedFiles(Collections.singletonList(storedFile)); logger.w("Leaving session payload for future delivery"); } break; case FAILURE: // drop bad data logger.w("Deleting invalid session tracking payload"); - sessionStore.deleteStoredFiles(Collections.singletonList(storedFile)); + store.deleteStoredFiles(Collections.singletonList(storedFile)); break; default: break; @@ -319,7 +321,7 @@ public void run() { }); } catch (RejectedExecutionException exception) { // This is on the current thread but there isn't much else we can do - sessionStore.write(session); + sessionStore.get().write(session); } } @@ -331,7 +333,7 @@ void deliverInMemorySession(Session session) { switch (deliveryStatus) { case UNDELIVERED: logger.w("Storing session payload for future delivery"); - sessionStore.write(session); + sessionStore.get().write(session); break; case FAILURE: logger.w("Dropping invalid session tracking payload"); diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/TrackerModule.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/TrackerModule.kt index 8eb6c47607..afc2be9779 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/TrackerModule.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/TrackerModule.kt @@ -26,7 +26,7 @@ internal class TrackerModule( config, callbackState, client, - storageModule.sessionStore.get(), + storageModule.sessionStore, config.logger, bgTaskService ) diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/SessionTrackerPauseResumeTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/SessionTrackerPauseResumeTest.kt index b2339cf096..1d82e5e354 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/SessionTrackerPauseResumeTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/SessionTrackerPauseResumeTest.kt @@ -5,6 +5,7 @@ import com.bugsnag.android.BugsnagTestUtils.generateConfiguration import com.bugsnag.android.BugsnagTestUtils.generateDevice import com.bugsnag.android.internal.BackgroundTaskService import com.bugsnag.android.internal.ImmutableConfig +import com.bugsnag.android.internal.dag.ValueProvider import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNotEquals @@ -60,7 +61,7 @@ internal class SessionTrackerPauseResumeTest { BugsnagTestUtils.generateImmutableConfig(), configuration.impl.callbackState, client, - sessionStore, + ValueProvider(sessionStore), NoopLogger, BackgroundTaskService() ) diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/SessionTrackerTest.java b/bugsnag-android-core/src/test/java/com/bugsnag/android/SessionTrackerTest.java index 172ee3a22b..7dfe12a158 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/SessionTrackerTest.java +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/SessionTrackerTest.java @@ -10,6 +10,7 @@ import com.bugsnag.android.internal.BackgroundTaskService; import com.bugsnag.android.internal.ForegroundDetector; import com.bugsnag.android.internal.ImmutableConfig; +import com.bugsnag.android.internal.dag.ValueProvider; import androidx.annotation.NonNull; @@ -71,8 +72,8 @@ public void setUp() { configuration.setDelivery(BugsnagTestUtils.generateDelivery()); immutableConfig = BugsnagTestUtils.generateImmutableConfig(); sessionTracker = new SessionTracker(immutableConfig, - configuration.impl.callbackState, client, sessionStore, NoopLogger.INSTANCE, - bgTaskService); + configuration.impl.callbackState, client, new ValueProvider<>(sessionStore), + NoopLogger.INSTANCE, bgTaskService); configuration.setAutoTrackSessions(true); user = new User(null, null, null); @@ -179,7 +180,7 @@ public void testBasicInForeground() { public void testZeroSessionTimeout() { CallbackState callbackState = configuration.impl.callbackState; sessionTracker = new SessionTracker(immutableConfig, callbackState, client, - 0, sessionStore, NoopLogger.INSTANCE, bgTaskService); + 0, new ValueProvider<>(sessionStore), NoopLogger.INSTANCE, bgTaskService); long now = System.currentTimeMillis(); sessionTracker.onForegroundStatus(true, now); @@ -199,7 +200,7 @@ public void testZeroSessionTimeout() { public void testSessionTimeout() { CallbackState callbackState = configuration.impl.callbackState; sessionTracker = new SessionTracker(immutableConfig, callbackState, client, - 100, sessionStore, NoopLogger.INSTANCE, bgTaskService); + 100, new ValueProvider<>(sessionStore), NoopLogger.INSTANCE, bgTaskService); long now = System.currentTimeMillis(); sessionTracker.onForegroundStatus(true, now); From 31b6fcc78857c453b9220e2cf565352c7a4f09ab Mon Sep 17 00:00:00 2001 From: jason Date: Fri, 20 Jun 2025 10:07:56 +0100 Subject: [PATCH 3/3] fix(root): add a process timeout to RootDetector --- CHANGELOG.md | 2 ++ bugsnag-android-core/detekt-baseline.xml | 1 + .../java/com/bugsnag/android/RootDetector.kt | 36 +++++++++++++++++++ 3 files changed, 39 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99ffd467fe..ba89ea7623 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ [#2197](https://github.com/bugsnag/bugsnag-android/pull/2197) * Improve the scoping of the build-id capturing in `bugsnag-plugin-android-ndk` to more reliably capture the build-id from the correct `.so` file [#2203](https://github.com/bugsnag/bugsnag-android/pull/2203) +* Fixed a background ANR that could occur during startup if processes do not launch or run quickly enough + [#2202](https://github.com/bugsnag/bugsnag-android/pull/2202) ## 6.14.0 (2025-06-04) diff --git a/bugsnag-android-core/detekt-baseline.xml b/bugsnag-android-core/detekt-baseline.xml index a44dcfc279..ac410321aa 100644 --- a/bugsnag-android-core/detekt-baseline.xml +++ b/bugsnag-android-core/detekt-baseline.xml @@ -61,6 +61,7 @@ SwallowedException:ForegroundDetector.kt$ForegroundDetector$e: Exception SwallowedException:JsonHelperTest.kt$JsonHelperTest$e: IllegalArgumentException SwallowedException:PluginClient.kt$PluginClient$exc: ClassNotFoundException + SwallowedException:RootDetector.kt$RootDetector$ex: IllegalThreadStateException SwallowedException:SharedPrefMigrator.kt$SharedPrefMigrator$e: RuntimeException ThrowsCount:JsonHelper.kt$JsonHelper$fun jsonToLong(value: Any?): Long? TooManyFunctions:ConfigInternal.kt$ConfigInternal : CallbackAwareMetadataAwareUserAwareFeatureFlagAware diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/RootDetector.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/RootDetector.kt index c187121d5e..7be09c0b0a 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/RootDetector.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/RootDetector.kt @@ -1,9 +1,14 @@ package com.bugsnag.android +import android.os.Build +import android.os.SystemClock import androidx.annotation.VisibleForTesting import java.io.File import java.io.IOException import java.io.Reader +import java.lang.Thread +import java.util.concurrent.TimeUnit +import kotlin.math.min /** * Attempts to detect whether the device is rooted. Root detection errs on the side of false @@ -21,6 +26,9 @@ internal class RootDetector @JvmOverloads constructor( ) { companion object { + private const val PROCESS_TIMEOUT = 250L + private const val PROCESS_POLL_DELAY = 50L + private val BUILD_PROP_FILE = File("/system/build.prop") private val ROOT_INDICATORS = listOf( @@ -120,7 +128,20 @@ internal class RootDetector @JvmOverloads constructor( var process: Process? = null return try { process = processBuilder.start() + val processComplete = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { + process.waitFor(PROCESS_TIMEOUT, TimeUnit.MILLISECONDS) + } else { + process.fallbackWaitFor(PROCESS_TIMEOUT) + } + + if (!processComplete) { + return false + } + process.inputStream.bufferedReader().use { it.isNotBlank() } + } catch (ignored: InterruptedException) { + Thread.currentThread().interrupt() // restore the interrupted status + false } catch (ignored: IOException) { false } finally { @@ -147,4 +168,19 @@ internal class RootDetector @JvmOverloads constructor( libraryLoaded -> performNativeRootChecks() else -> false } + + private fun Process.fallbackWaitFor(timeout: Long): Boolean { + val endTime = SystemClock.elapsedRealtime() + timeout + while (SystemClock.elapsedRealtime() < endTime) { + try { + exitValue() + return true + } catch (ex: IllegalThreadStateException) { + // Process is still running, wait a bit before checking again + Thread.sleep(min(PROCESS_POLL_DELAY, endTime - SystemClock.elapsedRealtime())) + } + } + + return false + } }