fix(android): Nav3 multi-Activity support now plays nicely with nav roots that don't have a SentryNavEffect

CHANGELOG.md

@@ -15,8 +15,14 @@
 
 - Most SDK log calls no longer allocate when `debug` is off ([#6600](https://github.com/getsentry/sentry-java/pull/6600))
 
+### Behavioral Changes
+
+- Nav3: Returning to an Activity or Fragment with its own `SentryNavEffect` within the app (e.g., closing the Activity that covered it, swiping back to its ViewPager2 page, or dismissing a dialog over it) now starts a new navigation transaction for the current destination if the previous one finished, so spans produced upon return can now be consistently captured ([#6482](https://github.com/getsentry/sentry-java/pull/6482))
+- Nav3: `ui.load` transactions are bound to `IScope` in preference to Nav3 transactions when transitioning between Activities, matching Nav2 behavior (API 29+) ([#6484](https://github.com/getsentry/sentry-java/pull/6484))
+
 ### Fixes
 
+- Nav3: Stop attributing errors and spans from an Activity or ViewPager2 page without a `SentryNavEffect` to the most recently visited Nav3 root ([#6483](https://github.com/getsentry/sentry-java/pull/6483))
 - Android: Count deep sleep in breadcrumb, screenshot and view hierarchy debounce windows, so the first low-memory or battery breadcrumb after a long Doze is no longer dropped ([#6590](https://github.com/getsentry/sentry-java/pull/6590))
 
 ## 8.64.0

sentry-android-navigation3/build.gradle.kts

@@ -47,6 +47,10 @@ android {
     warningsAsErrors = true
     checkDependencies = true
 
+    // Lifecycle 2.8.3's lint checks are incompatible with AGP 9.5. Lifecycle 2.10.0 fixes them,
+    // but raises minSdk to 23 while this SDK still supports 21.
+    disable += "ObsoleteLintCustomCheck"
+
     // We run a full lint analysis as build part in CI, so skip vital checks for assemble tasks.
     checkReleaseBuilds = false
   }
@@ -68,7 +72,10 @@ dependencies {
   implementation(libs.androidx.lifecycle.runtime.compose)
 
   compileOnly(libs.androidx.compose.runtime)
+  compileOnly(libs.androidx.compose.ui)
 
+  testImplementation(projects.sentryAndroidCore)
+  testImplementation(projects.sentryTestSupport)
   testImplementation(libs.androidx.compose.ui.test.junit4)
   testImplementation(libs.androidx.fragment.ktx)
   testImplementation(libs.androidx.test.core)

sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt

@@ -5,23 +5,18 @@ import io.sentry.Hint
 import io.sentry.SentryIntegrationPackageStorage
 import io.sentry.SentryLevel.INFO
 import io.sentry.SentryOptions
-import io.sentry.TransactionContext
-import io.sentry.TransactionOptions
 import io.sentry.compose.navigation3.BackStackConverter.RetentionPolicy
 import io.sentry.compose.navigation3.PreparedChange.BackStackHasNewTop
 import io.sentry.compose.navigation3.PreparedChange.BackStackHasSameTop
 import io.sentry.compose.navigation3.PreparedChange.BackStackIsEmpty
 import io.sentry.compose.navigation3.lifecycle.INavScope
 import io.sentry.compose.navigation3.lifecycle.INavScopes
-import io.sentry.compose.navigation3.lifecycle.INavTransaction
 import io.sentry.compose.navigation3.lifecycle.NAVIGATION_CONTEXT_KEY
-import io.sentry.protocol.App
-import io.sentry.protocol.TransactionNameSource
+import io.sentry.compose.navigation3.lifecycle.NAVIGATION_OP
+import io.sentry.compose.navigation3.lifecycle.NavTransactionRequest
 import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion
 import java.lang.ref.WeakReference
 
-private const val NAVIGATION_OP: String = "navigation"
-
 /**
  * Observes the back stack managed by a single [SentryNavEffect] and records Sentry state as the
  * back stack is updated.
@@ -33,8 +28,8 @@ private const val NAVIGATION_OP: String = "navigation"
  *
  * **Entry identity determines whether the top has changed**
  *
- * Referential equality (===), not structural equality, is used to determine whether the top of the
- * incoming back stack has changed. That approach:
+ * Referential equality (===) is used to determine whether the top of the incoming back stack has
+ * changed. That approach:
  *
  * - matches the typical Nav3 SnapshotStateList, where an entry instance has a stable identity for
  *   its lifetime in the stack;
@@ -57,10 +52,9 @@ internal class BackStackObserver<T : Any>(
 
   private val backStackConverter = BackStackConverter(entryMapper, scopes.options.logger)
 
-  private val navTransactions = NavTransactions(scopes)
   private val navBreadcrumbs = NavBreadcrumbs(scopes)
   private val navScreen = NavScreen()
-  private val navContext = NavContext(scopes, options)
+  private val navContext = NavContext(options)
 
   // Safe because the host back stack retains the current top entry strongly between updates.
   private var previousTop: WeakReference<T>? = null
@@ -192,44 +186,57 @@ internal class BackStackObserver<T : Any>(
       navBreadcrumbs.emit(fromEntry = previousTop, toEntry = currentBackStack.topEntryNormalized)
     }
 
-    navTransactions.stop(scope)
+    scope.finishNavTransaction()
 
     if (areNavigationTransactionsEnabled) {
-      navTransactions
-        .start(
-          scope,
-          currentTop.name,
-          currentTop.argumentsWithMetadata(),
-        )
-        ?.let { transaction -> navContext.updateTransaction(transaction, scope, currentBackStack) }
+      scope.startNavTransaction(navTransactionRequestFor(currentBackStack))
     } else {
       scope.rotatePropagationContext()
     }
 
-    storeAsPreviousTop(currentBackStack.topEntry, currentBackStack.topEntryNormalized)
+    storePreviousTop(currentBackStack)
   }
 
   private fun handleSameTop(scope: INavScope, backStack: BackStackData<T>) {
     navContext.update(scope, backStack.capturedEntriesNormalized)
-    storeAsPreviousTop(backStack.topEntry, backStack.topEntryNormalized)
+
+    // Keeps the nav context of transactions started later for the same destination (e.g., when the
+    // user returns to the nav root) in step with the back stack.
+    if (areNavigationTransactionsEnabled) {
+      scope.updateNavTransactionRequest(navTransactionRequestFor(backStack))
+    }
+
+    storePreviousTop(backStack)
   }
 
   private fun handleEmptyBackStack(scope: INavScope) {
     tearDown(scope)
   }
 
-  private fun storeAsPreviousTop(topEntry: T, topEntryNormalized: NormalizedSentryBackStackEntry) {
-    previousTop = WeakReference(topEntry)
-    previousTopNormalized = topEntryNormalized
+  private fun storePreviousTop(backStack: BackStackData<T>) {
+    previousTop = WeakReference(backStack.topEntry)
+    previousTopNormalized = backStack.topEntryNormalized
   }
 
   private fun clearPreviousTop() {
     previousTop = null
     previousTopNormalized = null
   }
 
+  private fun navTransactionRequestFor(backStack: BackStackData<T>): NavTransactionRequest {
+    val top = backStack.topEntryNormalized
+
+    return NavTransactionRequest.forDestination(
+      name = top.name,
+      arguments = top.argumentsWithMetadata(),
+      contexts = navContext.forTransaction(backStack.capturedEntriesNormalized),
+      viewNames = if (scopes.options.isEnableScreenTracking) listOf(top.name) else null,
+      options = scopes.options,
+    )
+  }
+
   private fun tearDown(scope: INavScope) {
-    navTransactions.stop(scope)
+    scope.finishNavTransaction()
     navContext.clear(scope)
     navScreen.clear(scope)
     clearPreviousTop()
@@ -274,58 +281,8 @@ private data class BackStackData<T>(
   val capturedEntriesNormalized: List<NormalizedSentryBackStackEntry>,
 )
 
-/** A helper class for managing nav transactions. */
-private class NavTransactions(private val scopes: INavScopes) {
-
-  private companion object {
-    private const val TRANSACTION_ORIGIN = "auto.navigation.nav3"
-  }
-
-  /**
-   * Starts an idle navigation transaction, or no-ops if another transaction is already active.
-   *
-   * [Stop][stop] the current navigation transaction first. See [INavScope.startNavTransaction].
-   */
-  fun start(
-    scope: INavScope,
-    name: String,
-    arguments: Map<String, Any?>,
-  ): INavTransaction? {
-    val transactionOptions =
-      TransactionOptions().also {
-        it.isWaitForChildren = true
-        it.idleTimeout = scopes.options.idleTimeout
-        val deadlineTimeoutMillis = scopes.options.deadlineTimeout
-        it.deadlineTimeout = if (deadlineTimeoutMillis <= 0) null else deadlineTimeoutMillis
-        it.isTrimEnd = true
-        it.origin = TRANSACTION_ORIGIN
-      }
-
-    val transaction =
-      scope.startNavTransaction(
-        TransactionContext(name, TransactionNameSource.ROUTE, NAVIGATION_OP),
-        transactionOptions,
-      ) ?: return null
-
-    if (arguments.isNotEmpty()) {
-      transaction.setData("arguments", arguments)
-    }
-
-    return transaction
-  }
-
-  /**
-   * Finishes and unsets the active navigation transaction, if one exists.
-   *
-   * See [INavScope.finishNavTransaction].
-   */
-  fun stop(scope: INavScope) {
-    scope.finishNavTransaction()
-  }
-}
-
 /** A helper class for updating [nav context][NAVIGATION_CONTEXT_KEY]. */
-private class NavContext(private val scopes: INavScopes, private val options: SentryNavOptions) {
+private class NavContext(private val options: SentryNavOptions) {
 
   private companion object {
     private const val BACKSTACK_KEY = "Back Stack"
@@ -345,33 +302,21 @@ private class NavContext(private val scopes: INavScopes, private val options: Se
   }
 
   /**
-   * Updates the transaction with the provided navigation info.
+   * The nav context to set on the nav transaction for a destination whose back stack is
+   * [capturedEntries], if any.
    *
    * Needed because transactions inherit base scope context on a per-key basis unless transactions
-   * have their own values for those keys. In our case, we need to keep fresh back stack and route
-   * values in the base context for purposes of crash reporting. But those values will often advance
-   * past what's relevant to a given transaction. This method prevents misassociation by binding
-   * proper values to the transaction context instead.
+   * have their own values for those keys. In our case, we need to keep fresh back stack values in
+   * the base context for purposes of crash reporting. But those values will often advance past
+   * what's relevant to a given transaction. Setting the destination's own back stack on its
+   * transaction prevents misassociation.
    */
-  fun <T : Any> updateTransaction(
-    transaction: INavTransaction,
-    scope: INavScope,
-    backStack: BackStackData<T>,
-  ) {
-    if (scopes.options.isEnableScreenTracking) {
-      val appContext = scope.app ?: App()
-
-      appContext.viewNames = listOf(backStack.topEntryNormalized.name)
-      transaction.setApp(appContext)
-    }
-
-    if (options.captureBackStack && backStack.capturedEntriesNormalized.isNotEmpty()) {
-      transaction.setContext(
-        NAVIGATION_CONTEXT_KEY,
-        backStack.capturedEntriesNormalized.toBackStackMap(),
-      )
+  fun forTransaction(capturedEntries: List<NormalizedSentryBackStackEntry>): Map<String, Any> =
+    if (options.captureBackStack && capturedEntries.isNotEmpty()) {
+      mapOf(NAVIGATION_CONTEXT_KEY to capturedEntries.toBackStackMap())
+    } else {
+      emptyMap()
     }
-  }
 
   /** Builds the `{"backstack": [...]}` map bound under [NAVIGATION_CONTEXT_KEY]. */
   private fun List<NormalizedSentryBackStackEntry>.toBackStackMap(): Map<String, Any?> =

sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/CoordinatedBackStackObserver.kt

@@ -1,15 +1,15 @@
 package io.sentry.compose.navigation3
 
-import androidx.lifecycle.Lifecycle
 import androidx.lifecycle.LifecycleEventObserver
 import io.sentry.IScope
 import io.sentry.IScopes
 import io.sentry.compose.navigation3.lifecycle.NavLeaseCoordinator
+import io.sentry.compose.navigation3.lifecycle.NavRootInfo
 import io.sentry.compose.navigation3.lifecycle.NavScopes
 
 /**
- * A [BackStackObserver] whose updates to a shared mutable [IScope] are coordinated with other
- * `BackStackObserver`s.
+ * A [BackStackObserver] whose updates to a shared mutable [IScope] are coordinated with those of
+ * other `BackStackObserver`s.
  *
  * Coordination is achieved via [NavScopes].
  *
@@ -35,8 +35,8 @@ internal class CoordinatedBackStackObserver<T : Any>(
   private val navScopes = NavScopes(scopes, coordinator)
   private val observer = BackStackObserver(navScopes, options, entryMapper)
 
-  fun attach(lifecycle: Lifecycle) {
-    navScopes.attach(lifecycle)
+  fun attach(rootInfo: NavRootInfo) {
+    navScopes.attach(rootInfo)
   }
 
   fun onBackStackChanged(backStack: List<T>) {
@@ -52,5 +52,24 @@ internal class CoordinatedBackStackObserver<T : Any>(
   }
 }
 
-/** The [NavLeaseCoordinator] shared by default by all [SentryNavEffect]s. */
+/**
+ * The [NavLeaseCoordinator] shared by default by all [CoordinatedBackStackObserver]s.
+ *
+ * **Lifetime**
+ *
+ * The default coordinator lives as long as the process, since lease state follows nav root
+ * lifecycles rather than the SDK's. Closing and re-initializing the SDK doesn't require a reset
+ * because:
+ *
+ * - `SentryNavEffect` resolves the current scopes on every call;
+ *
+ * - while the SDK is closed, bookkeeping continues without a scope; and
+ *
+ * - unpublishing compares by reference, so values published to an earlier scope don't affect the
+ *   current one.
+ *
+ * Limitation: A nav transaction that's still running when the SDK is closed and re-initialized can
+ * be bound to the new scope the next time it's published, although its client has been closed.
+ * Spans then attach to it, and are lost, until it times out or the next navigation finishes it.
+ */
 internal val defaultNavLeaseCoordinator: NavLeaseCoordinator = NavLeaseCoordinator()

sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavEffect.kt

@@ -4,17 +4,19 @@ import androidx.compose.runtime.Composable
 import androidx.compose.runtime.DisposableEffect
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.rememberUpdatedState
+import androidx.compose.ui.platform.LocalContext
 import androidx.lifecycle.compose.LocalLifecycleOwner
 import io.sentry.IScopes
 import io.sentry.ScopesAdapter
 import io.sentry.SentryOptions
 import io.sentry.compose.navigation3.lifecycle.NavLeaseCoordinator
+import io.sentry.compose.navigation3.lifecycle.NavRootInfo
+import io.sentry.compose.navigation3.lifecycle.findActivity
 import org.jetbrains.annotations.ApiStatus
 
 /**
- * An effect for generating Sentry data from your Nav3 backstack. Configure it via [options] and
- * call it before you invoke your `NavDisplay`.
- *
+ * A composable effect for generating Sentry data from your Nav3 backstack. Configure it via
+ * [options] and call it before you invoke your `NavDisplay`:
  * ```kotlin
  *  @Composable
  *  fun AppNavigation() {
@@ -24,7 +26,7 @@ import org.jetbrains.annotations.ApiStatus
  *    // Place SentryNavEffect in the same composable as your NavDisplay and call
  *    // the effect first. Doing so ensures the effect's lifecycle matches your
  *    // NavDisplay, and that any Sentry data produced by your initial nav
- *    // destination get attributed to the appropriate nav transaction.
+ *    // destination are attributed to the appropriate nav transaction.
  *    SentryNavEffect(
  *      backStack = navBackStack,
  *      backStackEntryMapper = { entry ->
@@ -105,6 +107,7 @@ internal fun <T : Any> SentryNavEffect(
 ) {
   val currentBackStackEntryMapper = rememberUpdatedState(backStackEntryMapper)
   val lifecycle = LocalLifecycleOwner.current.lifecycle
+  val context = LocalContext.current
 
   val observer =
     remember(scopes, options, lifecycle, coordinator) {
@@ -116,9 +119,13 @@ internal fun <T : Any> SentryNavEffect(
       )
     }
 
-  // Register first so the initial prepared back stack update can acquire navigation ownership.
+  // Intentionally don't key on context. It's only used to find the Activity that displays the nav
+  // root, which can't change without the lifecycle changing too, and that replaces the observer.
+  // The context's identity can change more often (e.g., if the host app provides a new
+  // ContextThemeWrapper on every recomposition), and re-attaching for those changes would
+  // needlessly finish the nav transaction.
   DisposableEffect(observer) {
-    observer.attach(lifecycle)
+    observer.attach(NavRootInfo(lifecycle, context.findActivity()))
     onDispose { observer.dispose() }
   }