diff --git a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/startup/ForwardUserId.kt b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/startup/ForwardUserId.kt new file mode 100644 index 0000000000..d74409b99d --- /dev/null +++ b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/startup/ForwardUserId.kt @@ -0,0 +1,28 @@ +package com.flipcash.app.internal.startup + +import com.bugsnag.android.Bugsnag +import com.getcode.utils.TraceManager +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.Dispatchers + +/** + * Forwards every [TraceManager.userId] change to [setUser] on [dispatcher], never on the thread + * that changed it. + * + * `Bugsnag.setUser` takes the NDK plugin's state lock, and Bugsnag's own worker holds that lock in + * `NativeBridge.synchronizeState` while it waits on `DeviceDataCollector.calculateFreeDisk`. Account + * creation sets the user id on the main thread, so calling Bugsnag there froze the UI until the + * worker let go — the ANR in Bugsnag `6acec70adbd69358a81f2824`. + * + * [dispatcher] runs one change at a time, in order, so a sign-out followed by a sign-in leaves + * Bugsnag with the signed-in id. + */ +@OptIn(kotlinx.coroutines.ExperimentalCoroutinesApi::class) +internal fun forwardUserIdOffThread( + dispatcher: CoroutineDispatcher = Dispatchers.IO.limitedParallelism(1), + setUser: (String?) -> Unit = { id -> Bugsnag.setUser(id, null, null) }, +) { + TraceManager.setOnUserIdChanged { id -> + launchBestEffort(tag = "bugsnag-user", dispatcher = dispatcher) { setUser(id) } + } +} diff --git a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/startup/TraceInitializer.kt b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/startup/TraceInitializer.kt index 10165c2cb1..57831c3500 100644 --- a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/startup/TraceInitializer.kt +++ b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/startup/TraceInitializer.kt @@ -57,9 +57,7 @@ class TraceInitializer: Initializer { ErrorUtils.addReporter(BugsnagErrorReporter()) TraceManager.addSink(BugsnagBreadcrumbSink()) - TraceManager.setOnUserIdChanged { id -> - Bugsnag.setUser(id, null, null) - } + forwardUserIdOffThread() stageProvider.fetchAndCache(versionCode) if (stageProvider.resolvedStage == ReleaseStage.Internal) { diff --git a/apps/flipcash/app/src/test/kotlin/com/flipcash/app/internal/startup/ForwardUserIdTest.kt b/apps/flipcash/app/src/test/kotlin/com/flipcash/app/internal/startup/ForwardUserIdTest.kt new file mode 100644 index 0000000000..93b979d00c --- /dev/null +++ b/apps/flipcash/app/src/test/kotlin/com/flipcash/app/internal/startup/ForwardUserIdTest.kt @@ -0,0 +1,46 @@ +package com.flipcash.app.internal.startup + +import com.getcode.utils.TraceManager +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import kotlin.test.AfterTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +@OptIn(ExperimentalCoroutinesApi::class) +class ForwardUserIdTest { + + @AfterTest + fun tearDown() { + TraceManager.setOnUserIdChanged(null) + TraceManager.userId = null + } + + @Test + fun `does not call setUser on the thread that changed the id`() = runTest { + val forwarded = mutableListOf() + forwardUserIdOffThread(dispatcher = StandardTestDispatcher(testScheduler)) { forwarded += it } + + TraceManager.userId = "user-1" + + assertTrue(forwarded.isEmpty()) + advanceUntilIdle() + assertEquals(listOf("user-1"), forwarded) + } + + @Test + fun `forwards changes in the order they were made`() = runTest { + val forwarded = mutableListOf() + forwardUserIdOffThread(dispatcher = StandardTestDispatcher(testScheduler)) { forwarded += it } + + TraceManager.userId = "user-1" + TraceManager.userId = null + TraceManager.userId = "user-2" + advanceUntilIdle() + + assertEquals(listOf("user-1", null, "user-2"), forwarded) + } +}