From 9ac7a9741c1e6491af26a4cb7104455596563125 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Sat, 12 Sep 2026 07:01:51 +0200 Subject: [PATCH 01/13] feat(android-nav3): Track navigation state transitions Add the observer layer that turns Navigation 3 back stack changes into breadcrumbs, scope state, and navigation transactions. This is also the point where the module starts carrying generated BuildConfig version metadata and its first non-empty API snapshot. --- .../api/sentry-android-navigation3.api | 8 + sentry-android-navigation3/build.gradle.kts | 9 + .../compose/navigation3/BackStackObserver.kt | 440 ++++++++++++ .../compose/navigation3/SentryNavOptions.kt | 105 +++ .../navigation3/BackStackObserverTest.kt | 664 ++++++++++++++++++ .../navigation3/SentryNavOptionsTest.kt | 107 +++ sentry/api/sentry.api | 1 + .../main/java/io/sentry/TypeCheckHint.java | 3 + 8 files changed, 1337 insertions(+) create mode 100644 sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt create mode 100644 sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt create mode 100644 sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt create mode 100644 sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavOptionsTest.kt diff --git a/sentry-android-navigation3/api/sentry-android-navigation3.api b/sentry-android-navigation3/api/sentry-android-navigation3.api index e69de29bb2..be90eab5cf 100644 --- a/sentry-android-navigation3/api/sentry-android-navigation3.api +++ b/sentry-android-navigation3/api/sentry-android-navigation3.api @@ -0,0 +1,8 @@ +public final class io/sentry/compose/navigation3/BuildConfig { + public static final field BUILD_TYPE Ljava/lang/String; + public static final field DEBUG Z + public static final field LIBRARY_PACKAGE_NAME Ljava/lang/String; + public static final field VERSION_NAME Ljava/lang/String; + public fun ()V +} + diff --git a/sentry-android-navigation3/build.gradle.kts b/sentry-android-navigation3/build.gradle.kts index 3e35211b22..8eb45db293 100644 --- a/sentry-android-navigation3/build.gradle.kts +++ b/sentry-android-navigation3/build.gradle.kts @@ -16,6 +16,9 @@ android { defaultConfig { minSdk = libs.versions.minSdk.get().toInt() + + // for AGP 4.1 + buildConfigField("String", "VERSION_NAME", "\"${project.version}\"") } buildTypes { @@ -32,6 +35,10 @@ android { compilerOptions.apiVersion = KotlinVersion.KOTLIN_1_9 } + testOptions { + unitTests.isReturnDefaultValues = true + } + lint { warningsAsErrors = true checkDependencies = true @@ -40,6 +47,8 @@ android { checkReleaseBuilds = false } + buildFeatures { buildConfig = true } + androidComponents.beforeVariants { it.enable = !Config.Android.shouldSkipDebugVariant(it.buildType) } diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt new file mode 100644 index 0000000000..6638562015 --- /dev/null +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt @@ -0,0 +1,440 @@ +package io.sentry.compose.navigation3 + +import io.sentry.Breadcrumb +import io.sentry.Hint +import io.sentry.IScope +import io.sentry.IScopes +import io.sentry.ITransaction +import io.sentry.PropagationContext +import io.sentry.SentryIntegrationPackageStorage +import io.sentry.SentryLevel.DEBUG +import io.sentry.SentryLevel.ERROR +import io.sentry.SentryLevel.INFO +import io.sentry.SpanStatus +import io.sentry.TransactionContext +import io.sentry.TransactionOptions +import io.sentry.TypeCheckHint +import io.sentry.protocol.App +import io.sentry.protocol.TransactionNameSource +import io.sentry.util.ExceptionUtils +import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion +import java.lang.ref.WeakReference + +/** + * Observes the back stack managed by a single [SentryNavEffect] and records Sentry state as the + * back stack is updated. + * + * **Top of the stack == the current screen** + * + * This class treats top of the back stack as the current navigation destination and visible screen. + * It knows nothing about composite Scenes or multipane navigation scenarios. + * + * **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: + * + * - matches the typical Nav3 SnapshotStateList, where an entry instance has a stable identity for + * its lifetime in the stack; + * - mirrors [BackStackKey]'s policy; + * - doesn't depend on host-provided `equals()` / `hashCode()`, which can be absent, incorrect, or + * expensive; and + * - ensures we don't miss reporting a genuine top-of-stack change. + * + * **Thread safety** + * + * This class is ***not*** thread-safe. Clients should serialize calls to [onBackStackChanged] and + * [cleanup] (e.g., via invocation from an `*Effect` or another form of thread confinement). + */ +@Suppress("TooManyFunctions") +internal class BackStackObserver( + private val scopes: IScopes, + private val options: SentryNavOptions, + private val resolvers: () -> RouteResolvers, +) { + + private val navTransactions = NavTransactionManager(scopes, NAVIGATION_OP, TRANSACTION_ORIGIN) + private val screenTracker = ScreenTracker() + private val routeTranslator = RouteTranslator(resolvers, scopes.options.logger) + + private var previousTopEntry: WeakReference? = null + private var previousTopRoute: Route? = null + + private val areNavigationTransactionsEnabled: Boolean + get() = scopes.options.isTracingEnabled && options.enableNavigationTransactions + + init { + addIntegrationToSdkVersion("ComposeNavigation3") + } + + internal companion object { + + private const val BACKSTACK_KEY = "backstack" + private const val NAVIGATION_CONTEXT_KEY = "navigation" + private const val NAVIGATION_OP: String = "navigation" + private const val TRANSACTION_ORIGIN = "auto.navigation.nav3" + + init { + SentryIntegrationPackageStorage.getInstance() + .addPackage("maven:io.sentry:sentry-android-navigation3", BuildConfig.VERSION_NAME) + } + } + + /** + * Updates recorded Sentry data based on the provided [backStack]. + * + * Note: This method is ***not*** idempotent. Callers should protect against repeat invocations + * with the same back stack. + */ + internal fun onBackStackChanged(backStack: List) { + guard("onBackStackChanged") { + val topEntry = backStack.lastOrNull() + + val status = + when { + topEntry == null -> BackStackStatus.EMPTY + topEntry === previousTopEntry?.get() -> BackStackStatus.SAME_TOP + else -> BackStackStatus.NEW_TOP + } + + scopes.configureScope { scope -> + applyBackStackChange(scope, status, backStack) + } + } + } + + internal fun cleanup() { + guard("cleanup") { + previousTopEntry = null + previousTopRoute = null + + scopes.configureScope { scope -> + navTransactions.stop(scope) + screenTracker.clear(scope) + + if (options.captureBackStack) { + // This observer owns the nav context while it's in the composition, and cleanup removes + // it to avoid leaking stale back stack data after observation stops. If the host app + // replaces one observer with another, there may be a brief gap where events lack nav + // context. Apps should keep the observer at the nav root so cleanup only runs when the + // navigation session is ending, not during normal destination changes. + scope.removeNavigationContext() + } + } + } + } + + private fun applyBackStackChange( + scope: IScope, + status: BackStackStatus, + backStack: List, + ) { + when (status) { + BackStackStatus.EMPTY -> handleEmptyBackStack(scope) + + BackStackStatus.NEW_TOP -> { + val data = backStack.extractData() + handleNewTop(scope, previousTopRoute, data) + storeAsPreviousTop(data.topEntry, data.topRoute) + } + + BackStackStatus.SAME_TOP -> { + val data = backStack.extractData() + handleSameTop(scope, data) + storeAsPreviousTop(data.topEntry, data.topRoute) + } + } + } + + /** + * Extracts Sentry data from the receiver (i.e., a list of host app back stack entries) in the + * form of a [BackStackData]. + * + * Throws if the receiver is empty. + */ + private fun List.extractData(): BackStackData { + check(this.isNotEmpty()) + + val topEntry = this.last() + val shouldCaptureBackStack = options.captureBackStack && options.maxCapturedBackStackEntries > 0 + + val entriesToTranslate = + when { + shouldCaptureBackStack -> + // Reverse entries so they're displayed with the newest entry on top in the Sentry UI. + this.takeLast(options.maxCapturedBackStackEntries).asReversed() + + // We always need to translate the top entry for use with breadcrumbs, etc., even if we're + // not capturing the back stack. + else -> listOf(topEntry) + } + + val routes = routeTranslator.translate(entriesToTranslate) + + return BackStackData( + topEntry = topEntry, + topRoute = routes.first(), + capturedRoutes = if (shouldCaptureBackStack) routes else emptyList(), + ) + } + + private fun handleNewTop( + scope: IScope, + previousTop: Route?, + currentBackStack: BackStackData, + ) { + val currentTopRoute = currentBackStack.topRoute + + scope.updateNavigationContext(currentBackStack.capturedRoutes) + + if (scopes.options.isEnableScreenTracking) { + screenTracker.track(scope, currentTopRoute.name) + } + + if (options.enableNavigationBreadcrumbs) { + scopes.addNav3Breadcrumb( + from = previousTop, + toEntry = currentBackStack.topEntry, + toRoute = currentBackStack.topRoute, + ) + } + + navTransactions.stop(scope) + + if (areNavigationTransactionsEnabled) { + navTransactions + .start( + scope, + currentTopRoute.name, + currentTopRoute.arguments, + ) + ?.updateNavigationContext(currentBackStack) + } else { + // Rotate the propagation context. + scope.withPropagationContext { scope.setPropagationContext(PropagationContext()) } + } + } + + private fun handleSameTop(scope: IScope, backStack: BackStackData) { + scope.updateNavigationContext(backStack.capturedRoutes) + } + + private fun handleEmptyBackStack(scope: IScope) { + scope.updateNavigationContext(emptyList()) + navTransactions.stop(scope) + screenTracker.clear(scope) + previousTopEntry = null + previousTopRoute = null + } + + private fun storeAsPreviousTop(topEntry: T, topRoute: Route) { + previousTopEntry = WeakReference(topEntry) + previousTopRoute = topRoute + } + + private fun IScope.updateNavigationContext(capturedRoutes: List) { + if (capturedRoutes.isEmpty()) { + this.removeNavigationContext() + } else { + this.setContexts(NAVIGATION_CONTEXT_KEY, capturedRoutes.toNavigationContext()) + } + } + + private fun IScope.removeNavigationContext() { + // We purposefully don't call IScope.removeContexts(), as it doesn't notify IScopeObserver and + // therefore doesn't write its updates to disk ¯\_ (ツ)_/¯. + this.setContexts(NAVIGATION_CONTEXT_KEY, null as Any?) + } + + /** + * Updates the receiver's context with the provided navigation info. + * + * 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. + */ + private fun ITransaction.updateNavigationContext(backStack: BackStackData) { + val appContext = contexts.app ?: App().also { contexts.setApp(it) } + appContext.viewNames = listOf(backStack.topRoute.name) + + if (options.captureBackStack && backStack.capturedRoutes.isNotEmpty()) { + setContext(NAVIGATION_CONTEXT_KEY, backStack.capturedRoutes.toNavigationContext()) + } + } + + /** Builds the `{"backstack": [...]}` map bound under [NAVIGATION_CONTEXT_KEY]. */ + private fun List.toNavigationContext(): Map = + mapOf(BACKSTACK_KEY to serialize()) + + private fun IScopes.addNav3Breadcrumb( + from: Route?, + toEntry: T, + toRoute: Route, + ) { + val breadcrumb = + Breadcrumb().apply { + type = NAVIGATION_OP + category = NAVIGATION_OP + + from?.let { + data["from"] = it.name + if (it.arguments.isNotEmpty()) { + data["from_arguments"] = it.arguments + } + } + + data["to"] = toRoute.name + if (toRoute.arguments.isNotEmpty()) { + data["to_arguments"] = toRoute.arguments + } + + level = INFO + } + + val hint = Hint() + hint.set(TypeCheckHint.NAV3_DESTINATION, toEntry) + this.addBreadcrumb(breadcrumb, hint) + } + + @Suppress("TooGenericExceptionCaught") + private inline fun guard(operation: String, body: () -> Unit) { + try { + body() + } catch (t: Throwable) { + ExceptionUtils.rethrowIfFatal(t) + scopes.options.logger.log( + ERROR, + t, + "Nav3 instrumentation failed during %s. Skipping this navigation update.", + operation, + ) + } + } +} + +private enum class BackStackStatus { + EMPTY, + /** + * The top of the back stack has changed, and one or more entries below it may have been updated + * as well. + */ + NEW_TOP, + /** The top of the back stack is unchanged, but one or more entries below it have been updated. */ + SAME_TOP, +} + +/** Info extracted from the host app's back stack in a form suitable for Sentry data. */ +private data class BackStackData( + val topEntry: T, + val topRoute: Route, + /** + * [Route]s representing the newest [SentryNavOption.maxCapturedBackStackEntries] entries from the + * host app's back stack. Possibly empty. + */ + val capturedRoutes: List, +) + +/** Tracks a provided name as the current visible screen. */ +private class ScreenTracker { + + private var lastScreenName: String? = null + + fun track(scope: IScope, screenName: String) { + scope.screen = screenName + lastScreenName = screenName + } + + fun clear(scope: IScope) { + val routeName = lastScreenName ?: return + if (scope.screen == routeName) { + scope.screen = null + } + lastScreenName = null + } +} + +private class NavTransactionManager( + private val scopes: IScopes, + private val navigationOp: String, + private val transactionOrigin: String, +) { + + private var activeNavTransaction: ITransaction? = null + + /** Starts an idle navigation transaction, or no-ops if another transaction is already active. */ + fun start( + scope: IScope, + routeName: String, + arguments: Map, + ): ITransaction? { + clearFinishedScopeTransaction(scope) + + if (scope.transaction != null) { + scopes.options.logger.log( + DEBUG, + "Nav3 transaction for route %s won't be created because another transaction is active.", + routeName, + ) + + return null + } + + 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 + } + + val transaction = + scopes.startTransaction( + TransactionContext(routeName, TransactionNameSource.ROUTE, navigationOp), + transactionOptions, + ) + + activeNavTransaction = transaction + + transaction.apply { + spanContext.origin = transactionOrigin + if (arguments.isNotEmpty()) { + setData("arguments", arguments) + } + } + + scope.withTransaction { tx -> + if (tx == null) { + scope.transaction = transaction + } + } + + return transaction + } + + /** Finishes and unsets the active navigation transaction, if one exists. */ + fun stop(scope: IScope) { + val transaction = activeNavTransaction ?: return + val status = transaction.status ?: SpanStatus.OK + transaction.finish(status) + + scope.withTransaction { tx -> + if (tx == transaction) { + scope.clearTransaction() + } + } + + activeNavTransaction = null + } + + /** Clears a stale finished transaction that's still bound to the default scope. */ + private fun clearFinishedScopeTransaction(scope: IScope) { + scope.withTransaction { tx -> + if (tx?.isFinished == true) { + scope.clearTransaction() + } + } + } +} diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt new file mode 100644 index 0000000000..d1598fedb8 --- /dev/null +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt @@ -0,0 +1,105 @@ +package io.sentry.compose.navigation3 + +import androidx.compose.runtime.Immutable +import org.jetbrains.annotations.ApiStatus + +// Keep the default low: every captured entry may require route-name extraction, argument +// extraction, and recursive argument sanitization when navigation changes are observed. +private const val DEFAULT_MAX_CAPTURED_BACK_STACK_ENTRIES = 10 + +/** + * Configuration info for a [SentryNavEffect]. + * + * Instances are immutable; create one with the [SentryNavOptions] DSL: + * ```kotlin + * val options = SentryNavOptions { + * captureBackStack = false + * maxCapturedBackStackEntries = 5 + * } + * ``` + */ +@ApiStatus.Experimental +@Immutable +internal class SentryNavOptions +private constructor( + val enableNavigationBreadcrumbs: Boolean, + val enableNavigationTransactions: Boolean, + val captureBackStack: Boolean, + val maxCapturedBackStackEntries: Int, +) { + + init { + require(maxCapturedBackStackEntries >= 0) { + "maxCapturedBackStackEntries must be non-negative, was $maxCapturedBackStackEntries" + } + } + + /** + * Mutable builder for [SentryNavOptions]. Prefer the [SentryNavOptions] DSL to using this + * directly. + * + * Lets us keep the resulting instance [Immutable] while preserving binary compatibility, should + * new properties be added in the future. + */ + class Builder { + + /** + * Whether navigation should produce Sentry breadcrumbs. If `true`, a new nav destination + * generates a breadcrumb like `from=/Home` and `to=/Profile`. + */ + var enableNavigationBreadcrumbs: Boolean = true + + /** + * Whether navigation should start a Sentry transaction. If `true`, navigating from `/Home` to + * `/Profile` starts a `/Profile` transaction and finishes the current `/Home` transaction. + */ + var enableNavigationTransactions: Boolean = true + + /** + * Whether Sentry should record back stack information for inclusion with crashes, errors, and + * other captured events. If `true`, a stack like `/Home -> /Profile` is recorded alongside the + * event, ordered with the current/top entry first. + */ + var captureBackStack: Boolean = true + + /** + * Maximum number of entries Sentry should record per captured back stack (starting with the + * most recent). Set to `0` to capture no back stack entries. + * + * Note: Sentry resolves and sanitizes up to [maxCapturedBackStackEntries] names + argument maps + * whenever your back stack changes. Keep name and argument extractors lightweight, and reduce + * the max captured count if extractor work is unusually expensive. + */ + var maxCapturedBackStackEntries: Int = DEFAULT_MAX_CAPTURED_BACK_STACK_ENTRIES + + fun build(): SentryNavOptions = + SentryNavOptions( + enableNavigationBreadcrumbs = enableNavigationBreadcrumbs, + enableNavigationTransactions = enableNavigationTransactions, + captureBackStack = captureBackStack, + maxCapturedBackStackEntries = maxCapturedBackStackEntries, + ) + } + + override fun equals(other: Any?): Boolean = + this === other || + (other is SentryNavOptions && + enableNavigationBreadcrumbs == other.enableNavigationBreadcrumbs && + enableNavigationTransactions == other.enableNavigationTransactions && + captureBackStack == other.captureBackStack && + maxCapturedBackStackEntries == other.maxCapturedBackStackEntries) + + override fun hashCode(): Int { + var result = enableNavigationBreadcrumbs.hashCode() + result = 31 * result + enableNavigationTransactions.hashCode() + result = 31 * result + captureBackStack.hashCode() + result = 31 * result + maxCapturedBackStackEntries + return result + } +} + +/** Creates [SentryNavOptions]. Optionally configure it via [configure]. */ +@ApiStatus.Experimental +internal fun SentryNavOptions( + configure: SentryNavOptions.Builder.() -> Unit = {} +): SentryNavOptions = SentryNavOptions.Builder().apply(configure).build() diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt new file mode 100644 index 0000000000..ad5c934010 --- /dev/null +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt @@ -0,0 +1,664 @@ +package io.sentry.compose.navigation3 + +import com.google.common.truth.Truth.assertThat +import io.sentry.Breadcrumb +import io.sentry.Hint +import io.sentry.ILogger +import io.sentry.IScope +import io.sentry.IScopes +import io.sentry.ISpan +import io.sentry.ITransaction +import io.sentry.Scope +import io.sentry.ScopeCallback +import io.sentry.SentryOptions +import io.sentry.SentryTracer +import io.sentry.TransactionContext +import io.sentry.TransactionOptions +import io.sentry.TypeCheckHint +import io.sentry.protocol.TransactionNameSource +import kotlin.test.Test +import kotlin.test.assertNull +import org.mockito.kotlin.any +import org.mockito.kotlin.doAnswer +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever + +class BackStackObserverTest { + + private data class HomeRoute(val id: String = "home") + + private data class ProfileRoute(val userId: String) + + private data class CartRoute(val productId: String) + + private data class SettingsRoute(val section: String) + + private data class ObserverConfig( + val enableNavigationBreadcrumbs: Boolean = true, + val enableNavigationTransactions: Boolean = true, + val captureBackStack: Boolean = true, + val maxCapturedBackStackEntries: Int = 10, + val enableScreenTracking: Boolean = true, + ) + + private class Fixture { + private val defaultNameExtractor = + RouteNameExtractor { entry -> entry::class.simpleName ?: "unknown" } + + val logger = mock() + val scope = Scope(createOptions(logger)) + val scopes = mock() + val breadcrumbs = mutableListOf() + val breadcrumbHints = mutableListOf() + val startedTransactions = mutableListOf() + + init { + whenever(scopes.options).thenReturn(scope.options) + whenever(scopes.getSpan()).thenAnswer { scope.span } + whenever(scopes.getTransaction()).thenAnswer { scope.transaction } + doAnswer { + (it.arguments[0] as ScopeCallback).run(scope) + null + } + .whenever(scopes) + .configureScope(any()) + doAnswer { + val transactionContext = it.arguments[0] as TransactionContext + val transactionOptions = it.arguments[1] as TransactionOptions + SentryTracer(transactionContext, scopes, transactionOptions) + .also(startedTransactions::add) + } + .whenever(scopes) + .startTransaction(any(), any()) + doAnswer { + breadcrumbs += it.arguments[0] as Breadcrumb + breadcrumbHints += it.arguments[1] as Hint + null + } + .whenever(scopes) + .addBreadcrumb(any(), any()) + } + + fun getSut( + config: ObserverConfig = ObserverConfig(), + nameExtractor: RouteNameExtractor = defaultNameExtractor, + argumentsExtractor: RouteArgumentsExtractor? = null, + ): BackStackObserver { + scope.options.isEnableScreenTracking = config.enableScreenTracking + + return BackStackObserver( + scopes = scopes, + options = + SentryNavOptions { + enableNavigationBreadcrumbs = config.enableNavigationBreadcrumbs + enableNavigationTransactions = config.enableNavigationTransactions + captureBackStack = config.captureBackStack + maxCapturedBackStackEntries = config.maxCapturedBackStackEntries + }, + resolvers = { RouteResolvers(nameExtractor, argumentsExtractor) }, + ) + } + + private companion object { + fun createOptions(logger: ILogger): SentryOptions = + SentryOptions().apply { + dsn = "http://key@localhost/proj" + setTracesSampleRate(1.0) + isEnableScreenTracking = true + isDebug = true + setLogger(logger) + idleTimeout = null + deadlineTimeout = 0 + } + } + } + + @Test + fun `onBackStackChanged emits a breadcrumb for the top back stack entry when breadcrumbs are enabled`() { + val fixture = Fixture() + val sut = + fixture.getSut( + config = ObserverConfig(enableNavigationBreadcrumbs = true), + argumentsExtractor = + RouteArgumentsExtractor { entry -> + when (entry) { + is HomeRoute -> mapOf("tab" to entry.id) + is ProfileRoute -> mapOf("userId" to entry.userId) + else -> emptyMap() + } + }, + ) + val home = HomeRoute() + val profile = ProfileRoute("123") + + sut.onBackStackChanged(listOf(home)) + sut.onBackStackChanged(listOf(home, profile)) + + val breadcrumb = fixture.breadcrumbs.last() + assertThat(breadcrumb.type).isEqualTo("navigation") + assertThat(breadcrumb.category).isEqualTo("navigation") + assertThat(breadcrumb.data) + .containsExactly( + "from", + "/HomeRoute", + "from_arguments", + mapOf("tab" to "home"), + "to", + "/ProfileRoute", + "to_arguments", + mapOf("userId" to "123"), + ) + assertThat(fixture.breadcrumbHints.last().get(TypeCheckHint.NAV3_DESTINATION)) + .isSameInstanceAs(profile) + } + + @Test + fun `onBackStackChanged reuses the previous top snapshot for breadcrumb from payload`() { + val fixture = Fixture() + val previousProfile = ProfileRoute("123") + val replacementProfile = ProfileRoute("123") + var profileName = "profile" + var profileArguments = mapOf("userId" to "123") + val sut = + fixture.getSut( + nameExtractor = + RouteNameExtractor { entry -> + when (entry) { + is HomeRoute -> "home" + is ProfileRoute -> profileName + is SettingsRoute -> "settings" + else -> error("unknown route: $entry") + } + }, + argumentsExtractor = + RouteArgumentsExtractor { entry -> + when (entry) { + is HomeRoute -> mapOf("tab" to entry.id) + is ProfileRoute -> profileArguments + is SettingsRoute -> mapOf("section" to entry.section) + else -> emptyMap() + } + }, + ) + + sut.onBackStackChanged(listOf(HomeRoute(), previousProfile)) + profileName = "mutated-profile" + profileArguments = mapOf("userId" to "999") + + sut.onBackStackChanged(listOf(HomeRoute(), replacementProfile, SettingsRoute("privacy"))) + + assertThat(fixture.breadcrumbs.last().data) + .containsExactly( + "from", + "/profile", + "from_arguments", + mapOf("userId" to "123"), + "to", + "/settings", + "to_arguments", + mapOf("section" to "privacy"), + ) + } + + @Test + fun `onBackStackChanged does not emit a breadcrumb when breadcrumbs are disabled`() { + val fixture = Fixture() + val sut = fixture.getSut(config = ObserverConfig(enableNavigationBreadcrumbs = false)) + + sut.onBackStackChanged(listOf(HomeRoute())) + + assertThat(fixture.breadcrumbs).isEmpty() + } + + @Test + fun `onBackStackChanged emits a screen name for the top back stack entry when screen tracking is enabled`() { + val fixture = Fixture() + val sut = fixture.getSut(config = ObserverConfig(enableScreenTracking = true)) + + sut.onBackStackChanged(listOf(HomeRoute(), ProfileRoute("123"))) + + assertThat(fixture.scope.screen).isEqualTo("/ProfileRoute") + assertThat(fixture.scope.contexts.app?.viewNames).isEqualTo(listOf("/ProfileRoute")) + } + + @Test + fun `onBackStackChanged does not emit a screen name when screen tracking is disabled`() { + val fixture = Fixture() + val sut = fixture.getSut(config = ObserverConfig(enableScreenTracking = false)) + + sut.onBackStackChanged(listOf(HomeRoute())) + + assertThat(fixture.scope.screen).isNull() + assertThat(fixture.scope.contexts.app?.viewNames).isNull() + } + + @Test + fun `onBackStackChanged emits a copy of the back stack up to max captured entries when enabled`() { + val fixture = Fixture() + val sut = + fixture.getSut( + config = ObserverConfig(captureBackStack = true, maxCapturedBackStackEntries = 2) + ) + + sut.onBackStackChanged(listOf(HomeRoute(), ProfileRoute("123"), SettingsRoute("privacy"))) + + assertThat(fixture.scope.navigationBackStack()) + .isEqualTo(listOf(mapOf("route" to "/SettingsRoute"), mapOf("route" to "/ProfileRoute"))) + } + + @Test + fun `onBackStackChanged emits an updated copy of the back stack even when the top entry is unchanged`() { + val fixture = Fixture() + val sut = fixture.getSut(config = ObserverConfig(captureBackStack = true)) + val home = HomeRoute() + val profile = ProfileRoute("123") + + sut.onBackStackChanged(listOf(home, profile)) + sut.onBackStackChanged(listOf(home, SettingsRoute("privacy"), profile)) + + assertThat(fixture.breadcrumbs).hasSize(1) + assertThat(fixture.startedTransactions).hasSize(1) + assertThat(fixture.scope.screen).isEqualTo("/ProfileRoute") + assertThat(fixture.scope.navigationBackStack()) + .isEqualTo( + listOf( + mapOf("route" to "/ProfileRoute"), + mapOf("route" to "/SettingsRoute"), + mapOf("route" to "/HomeRoute"), + ) + ) + } + + @Test + fun `onBackStackChanged emits new top-entry data when the top entry is replaced by an equal new instance`() { + val fixture = Fixture() + val sut = fixture.getSut(config = ObserverConfig(captureBackStack = true)) + val home = HomeRoute() + val firstProfile = ProfileRoute("123") + val replacementProfile = ProfileRoute("123") + + sut.onBackStackChanged(listOf(home, firstProfile)) + sut.onBackStackChanged(listOf(home, replacementProfile)) + + assertThat(fixture.breadcrumbs).hasSize(2) + assertThat(fixture.breadcrumbs.last().data["from"]).isEqualTo("/ProfileRoute") + assertThat(fixture.breadcrumbs.last().data["to"]).isEqualTo("/ProfileRoute") + assertThat(fixture.breadcrumbHints.last().get(TypeCheckHint.NAV3_DESTINATION)) + .isSameInstanceAs(replacementProfile) + assertThat(fixture.startedTransactions).hasSize(2) + assertThat(fixture.startedTransactions.last().name).isEqualTo("/ProfileRoute") + assertThat(fixture.startedTransactions.first().isFinished).isTrue() + assertThat(fixture.scope.screen).isEqualTo("/ProfileRoute") + assertThat(fixture.scope.navigationBackStack()) + .isEqualTo(listOf(mapOf("route" to "/ProfileRoute"), mapOf("route" to "/HomeRoute"))) + } + + @Test + fun `onBackStackChanged does not emit a back stack copy when max captured entries is 0`() { + val fixture = Fixture() + val sut = + fixture.getSut( + config = ObserverConfig(captureBackStack = true, maxCapturedBackStackEntries = 0) + ) + fixture.scope.setContexts( + "navigation", + mapOf("backstack" to listOf(mapOf("route" to "/Stale"))), + ) + + sut.onBackStackChanged(listOf(HomeRoute())) + + // Doesn't emit a back stack... + assertThat(fixture.scope.contexts.containsKey("navigation")).isFalse() + + // ...but continues to emit all other Sentry data. + assertThat(fixture.breadcrumbs.single().data["to"]).isEqualTo("/HomeRoute") + assertThat(fixture.startedTransactions).hasSize(1) + assertThat(fixture.startedTransactions.single().name).isEqualTo("/HomeRoute") + assertThat(fixture.scope.screen).isEqualTo("/HomeRoute") + } + + @Test + fun `onBackStackChanged does not emit a back stack copy when back stack capture is disabled`() { + val fixture = Fixture() + val sut = fixture.getSut(config = ObserverConfig(captureBackStack = false)) + fixture.scope.setContexts( + "navigation", + mapOf("backstack" to listOf(mapOf("route" to "/Stale"))), + ) + + sut.onBackStackChanged(listOf(HomeRoute())) + + // Doesn't emit a back stack... + assertThat(fixture.scope.contexts.containsKey("navigation")).isFalse() + + // ...but continues to emit all other Sentry data. + assertThat(fixture.breadcrumbs.single().data["to"]).isEqualTo("/HomeRoute") + assertThat(fixture.startedTransactions).hasSize(1) + assertThat(fixture.startedTransactions.single().name).isEqualTo("/HomeRoute") + assertThat(fixture.scope.screen).isEqualTo("/HomeRoute") + } + + // Like `onBackStackChanged does not emit a back stack copy when back stack capture is disabled`, + // but here we actually verify that no unnecessary work is done. + @Test + fun `onBackStackChanged skips lower back stack resolution when back stack capture is disabled`() { + val fixture = Fixture() + val home = HomeRoute() + val profile = ProfileRoute("123") + val nameCalls = mutableMapOf() + val argumentCalls = mutableMapOf() + val sut = + fixture.getSut( + config = ObserverConfig(captureBackStack = false), + nameExtractor = { entry -> + nameCalls[entry] = (nameCalls[entry] ?: 0) + 1 + entry::class.simpleName ?: "unknown" + }, + argumentsExtractor = { entry -> + argumentCalls[entry] = (argumentCalls[entry] ?: 0) + 1 + when (entry) { + is HomeRoute -> mapOf("tab" to entry.id) + is ProfileRoute -> mapOf("userId" to entry.userId) + else -> emptyMap() + } + }, + ) + + sut.onBackStackChanged(listOf(home, profile)) + + assertThat(nameCalls[profile]).isEqualTo(1) + assertThat(argumentCalls[profile]).isEqualTo(1) + assertThat(nameCalls).doesNotContainKey(home) + assertThat(argumentCalls).doesNotContainKey(home) + } + + @Test + fun `onBackStackChanged creates a nav transaction when enabled and no ambient transaction is active`() { + val fixture = Fixture() + val sut = + fixture.getSut( + config = ObserverConfig(enableNavigationTransactions = true), + argumentsExtractor = + RouteArgumentsExtractor { entry -> + when (entry) { + is ProfileRoute -> mapOf("userId" to entry.userId) + else -> emptyMap() + } + }, + ) + + sut.onBackStackChanged(listOf(HomeRoute(), ProfileRoute("123"))) + + val transaction = fixture.startedTransactions.single() + + assertThat(transaction.name).isEqualTo("/ProfileRoute") + assertThat(transaction.transactionNameSource).isEqualTo(TransactionNameSource.ROUTE) + assertThat(transaction.operation).isEqualTo("navigation") + assertThat(transaction.spanContext.origin).isEqualTo("auto.navigation.nav3") + assertThat(transaction.getData("arguments")).isEqualTo(mapOf("userId" to "123")) + assertThat(transaction.contexts.app?.viewNames).isEqualTo(listOf("/ProfileRoute")) + assertThat(transaction.navigationBackStack()) + .isEqualTo( + listOf( + mapOf("route" to "/ProfileRoute", "args" to mapOf("userId" to "123")), + mapOf("route" to "/HomeRoute"), + ) + ) + assertThat(fixture.scope.transaction).isSameInstanceAs(transaction) + } + + @Test + fun `onBackStackChanged resolves top entry arguments once per update`() { + val fixture = Fixture() + val home = HomeRoute() + val profile = ProfileRoute("123") + val argumentCalls = mutableMapOf() + val sut = + fixture.getSut( + argumentsExtractor = + RouteArgumentsExtractor { entry -> + argumentCalls[entry] = (argumentCalls[entry] ?: 0) + 1 + when (entry) { + is HomeRoute -> mapOf("tab" to entry.id) + is ProfileRoute -> mapOf("userId" to entry.userId) + else -> emptyMap() + } + } + ) + + sut.onBackStackChanged(listOf(home, profile)) + + assertThat(argumentCalls[profile]).isEqualTo(1) + assertThat(argumentCalls[home]).isEqualTo(1) + } + + @Test + fun `onBackStackChanged creates a nav transaction when only an ambient span is active`() { + val fixture = Fixture() + val sut = fixture.getSut(config = ObserverConfig(enableNavigationTransactions = true)) + + fixture.scope.setActiveSpan(mock()) + sut.onBackStackChanged(listOf(HomeRoute())) + + assertThat(fixture.startedTransactions).hasSize(1) + assertThat(fixture.startedTransactions.single().name).isEqualTo("/HomeRoute") + assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransactions.single()) + assertThat(fixture.scope.screen).isEqualTo("/HomeRoute") + } + + @Test + fun `onBackStackChanged does not create a nav transaction when an ambient transaction is active`() { + val fixture = Fixture() + val ambientTransaction = + SentryTracer( + TransactionContext("ambient", TransactionNameSource.CUSTOM, "ui.load"), + fixture.scopes, + ) + ambientTransaction.startChild("db.query") + fixture.scope.transaction = ambientTransaction + val sut = fixture.getSut(config = ObserverConfig(enableNavigationTransactions = true)) + + sut.onBackStackChanged(listOf(HomeRoute())) + + assertThat(fixture.startedTransactions).isEmpty() + assertThat(fixture.scope.transaction).isSameInstanceAs(ambientTransaction) + assertThat(fixture.scope.screen).isEqualTo("/HomeRoute") + } + + @Test + fun `onBackStackChanged does not create a nav transaction when navigation transactions are disabled`() { + val fixture = Fixture() + val sut = fixture.getSut(config = ObserverConfig(enableNavigationTransactions = false)) + val originalPropagationContext = fixture.scope.propagationContext + + sut.onBackStackChanged(listOf(HomeRoute())) + + assertThat(fixture.startedTransactions).isEmpty() + assertThat(fixture.scope.transaction).isNull() + assertThat(fixture.scope.propagationContext).isNotSameInstanceAs(originalPropagationContext) + } + + @Test + fun `onBackStackChanged clears a finished stale scope transaction before starting a fresh nav transaction`() { + val fixture = Fixture() + val staleTransaction = + SentryTracer( + TransactionContext("stale", TransactionNameSource.CUSTOM, "ui.load"), + fixture.scopes, + ) + staleTransaction.finish() + fixture.scope.transaction = staleTransaction + val sut = fixture.getSut(config = ObserverConfig(enableNavigationTransactions = true)) + + sut.onBackStackChanged(listOf(HomeRoute())) + + assertThat(fixture.startedTransactions).hasSize(1) + assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransactions.single()) + } + + @Test + fun `onBackStackChanged clears tracked scope state when the back stack becomes empty`() { + val fixture = Fixture() + val sut = fixture.getSut() + + sut.onBackStackChanged(listOf(HomeRoute())) + val transaction = fixture.startedTransactions.single() + + sut.onBackStackChanged(emptyList()) + + assertThat(transaction.isFinished).isTrue() + assertThat(fixture.scope.transaction).isNull() + assertThat(fixture.scope.screen).isNull() + assertNull(fixture.scope.contexts.app?.viewNames) + assertThat(fixture.scope.contexts.containsKey("navigation")).isFalse() + assertThat(fixture.breadcrumbs).hasSize(1) + } + + @Test + @Suppress("LongMethod") + fun `onBackStackChanged records unknown route names when destination route name can't be extracted`() { + val fixture = Fixture() + val home = HomeRoute() + val profile = ProfileRoute(userId = "123") + val cart = CartRoute(productId = "987") + val settings = SettingsRoute(section = "privacy") + val sut = + fixture.getSut( + nameExtractor = + RouteNameExtractor { entry -> + when (entry) { + is HomeRoute -> "home" + is ProfileRoute -> " " + is CartRoute -> error("throwing in order to simulate a buggy name extractor") + is SettingsRoute -> "settings" + else -> error("unknown route: $entry") + } + } + ) + + // Navigate to the home screen and verify that a transaction has started and related Sentry data + // have been generated (i.e., screen name, breadcrumb, and updated back stack context), as the + // host app's RouteNameExtractor returned a valid route name for the home screen entry. + sut.onBackStackChanged(listOf(home)) + val transaction = fixture.startedTransactions.single() + assertThat(transaction.isFinished).isFalse() + assertThat(fixture.scope.transaction).isNotNull() + assertThat(fixture.scope.screen).isEqualTo("/home") + assertThat(fixture.scope.contexts.app?.viewNames).isEqualTo(listOf("/home")) + assertThat(fixture.breadcrumbs).hasSize(1) + assertThat(fixture.scope.navigationBackStack()).isEqualTo(listOf(mapOf("route" to "/home"))) + + // Navigate to the profile screen and verify the invalid route name is recorded as /unknown so + // the transition history remains intact. + sut.onBackStackChanged(listOf(home, profile)) + assertThat(transaction.isFinished).isTrue() + assertThat(fixture.startedTransactions).hasSize(2) + val profileTransaction = fixture.startedTransactions.last() + assertThat(profileTransaction.isFinished).isFalse() + assertThat(profileTransaction.name).isEqualTo(RouteTranslator.UNKNOWN_ROUTE_NAME) + assertThat(fixture.scope.transaction).isSameInstanceAs(profileTransaction) + assertThat(fixture.scope.screen).isEqualTo(RouteTranslator.UNKNOWN_ROUTE_NAME) + assertThat(fixture.scope.contexts.app?.viewNames) + .isEqualTo(listOf(RouteTranslator.UNKNOWN_ROUTE_NAME)) + assertThat(fixture.breadcrumbs).hasSize(2) + assertThat(fixture.breadcrumbs.last().data) + .containsExactly("from", "/home", "to", RouteTranslator.UNKNOWN_ROUTE_NAME) + assertThat(fixture.scope.navigationBackStack()) + .isEqualTo( + listOf( + mapOf("route" to RouteTranslator.UNKNOWN_ROUTE_NAME), + mapOf("route" to "/home"), + ) + ) + + // Navigate to the cart screen and verify the later failure is also recorded as /unknown rather + // than collapsing the route history. + sut.onBackStackChanged(listOf(home, profile, cart)) + assertThat(profileTransaction.isFinished).isTrue() + assertThat(fixture.startedTransactions).hasSize(3) + val cartTransaction = fixture.startedTransactions.last() + assertThat(cartTransaction.isFinished).isFalse() + assertThat(cartTransaction.name).isEqualTo(RouteTranslator.UNKNOWN_ROUTE_NAME) + assertThat(fixture.scope.transaction).isSameInstanceAs(cartTransaction) + assertThat(fixture.scope.screen).isEqualTo(RouteTranslator.UNKNOWN_ROUTE_NAME) + assertThat(fixture.scope.contexts.app?.viewNames) + .isEqualTo(listOf(RouteTranslator.UNKNOWN_ROUTE_NAME)) + assertThat(fixture.breadcrumbs).hasSize(3) + assertThat(fixture.breadcrumbs.last().data) + .containsExactly( + "from", + RouteTranslator.UNKNOWN_ROUTE_NAME, + "to", + RouteTranslator.UNKNOWN_ROUTE_NAME, + ) + assertThat(fixture.scope.navigationBackStack()) + .isEqualTo( + listOf( + mapOf("route" to RouteTranslator.UNKNOWN_ROUTE_NAME), + mapOf("route" to RouteTranslator.UNKNOWN_ROUTE_NAME), + mapOf("route" to "/home"), + ) + ) + + // Navigate to the settings screen and verify a new /settings transaction is started and Sentry + // data are generated again, as we received a valid route name. + sut.onBackStackChanged(listOf(home, profile, cart, settings)) + + assertThat(cartTransaction.isFinished).isTrue() + assertThat(fixture.startedTransactions).hasSize(4) + val settingsTransaction = fixture.startedTransactions.last() + assertThat(settingsTransaction.isFinished).isFalse() + assertThat(settingsTransaction.name).isEqualTo("/settings") + assertThat(fixture.scope.transaction).isSameInstanceAs(settingsTransaction) + assertThat(fixture.scope.screen).isEqualTo("/settings") + assertThat(fixture.scope.contexts.app?.viewNames).isEqualTo(listOf("/settings")) + assertThat(fixture.breadcrumbs).hasSize(4) + assertThat(fixture.breadcrumbs.last().data) + .containsExactly("from", RouteTranslator.UNKNOWN_ROUTE_NAME, "to", "/settings") + assertThat(fixture.scope.navigationBackStack()) + .isEqualTo( + listOf( + mapOf("route" to "/settings"), + mapOf("route" to RouteTranslator.UNKNOWN_ROUTE_NAME), + mapOf("route" to RouteTranslator.UNKNOWN_ROUTE_NAME), + mapOf("route" to "/home"), + ) + ) + } + + @Test + fun `cleanup clears observer owned tracked state`() { + val fixture = Fixture() + val sut = fixture.getSut() + + sut.onBackStackChanged(listOf(HomeRoute())) + val transaction = fixture.startedTransactions.single() + + sut.cleanup() + + assertThat(transaction.isFinished).isTrue() + assertThat(fixture.scope.transaction).isNull() + assertThat(fixture.scope.screen).isNull() + assertNull(fixture.scope.contexts.app?.viewNames) + assertThat(fixture.scope.contexts.containsKey("navigation")).isFalse() + } + + private fun IScope.navigationBackStack(): List>? { + val navigationContext = contexts[NAVIGATION_CONTEXT_KEY] as? Map<*, *> ?: return null + + @Suppress("UNCHECKED_CAST") + return navigationContext[BACKSTACK_KEY] as? List> + } + + private fun ITransaction.navigationBackStack(): List>? { + val navigationContext = contexts[NAVIGATION_CONTEXT_KEY] as? Map<*, *> ?: return null + + @Suppress("UNCHECKED_CAST") + return navigationContext[BACKSTACK_KEY] as? List> + } + + private companion object { + const val NAVIGATION_CONTEXT_KEY = "navigation" + const val BACKSTACK_KEY = "backstack" + } +} diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavOptionsTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavOptionsTest.kt new file mode 100644 index 0000000000..0833e2cbe2 --- /dev/null +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavOptionsTest.kt @@ -0,0 +1,107 @@ +package io.sentry.compose.navigation3 + +import com.google.common.truth.Truth.assertThat +import java.lang.reflect.Modifier +import kotlin.test.Test +import kotlin.test.assertFailsWith + +class SentryNavOptionsTest { + + @Test + fun `accepts positive max captured backstack entries`() { + val options = SentryNavOptions { maxCapturedBackStackEntries = 1 } + + assertThat(options.maxCapturedBackStackEntries).isEqualTo(1) + } + + @Test + fun `accepts zero max captured backstack entries`() { + val options = SentryNavOptions { maxCapturedBackStackEntries = 0 } + + assertThat(options.maxCapturedBackStackEntries).isEqualTo(0) + } + + @Test + fun `rejects negative max captured backstack entries`() { + val exception = + assertFailsWith { + SentryNavOptions { maxCapturedBackStackEntries = -1 } + } + + assertThat(exception) + .hasMessageThat() + .isEqualTo("maxCapturedBackStackEntries must be non-negative, was -1") + } + + @Test + fun `equal instances share the same hash code`() { + val first = SentryNavOptions() + val second = SentryNavOptions() + + assertThat(first).isEqualTo(second) + assertThat(first.hashCode()).isEqualTo(second.hashCode()) + } + + @Test + fun `equals and hash code include every property`() { + val base = SentryNavOptions() + val instanceFields = + SentryNavOptions::class + .java + .declaredFields + .filterNot { Modifier.isStatic(it.modifiers) } + .map { it.name } + + assertThat(propertyMutators.keys).containsExactlyElementsIn(instanceFields) + + propertyMutators.forEach { (propertyName, mutate) -> + val changed = mutate(base) + + assertThat(changed).isNotEqualTo(base) + assertThat(changed.hashCode()).isNotEqualTo(base.hashCode()) + assertThat(propertyName).isIn(instanceFields) + } + } + + private companion object { + val propertyMutators = + mapOf SentryNavOptions>( + "enableNavigationBreadcrumbs" to + { options -> + SentryNavOptions { + enableNavigationBreadcrumbs = !options.enableNavigationBreadcrumbs + enableNavigationTransactions = options.enableNavigationTransactions + captureBackStack = options.captureBackStack + maxCapturedBackStackEntries = options.maxCapturedBackStackEntries + } + }, + "enableNavigationTransactions" to + { options -> + SentryNavOptions { + enableNavigationBreadcrumbs = options.enableNavigationBreadcrumbs + enableNavigationTransactions = !options.enableNavigationTransactions + captureBackStack = options.captureBackStack + maxCapturedBackStackEntries = options.maxCapturedBackStackEntries + } + }, + "captureBackStack" to + { options -> + SentryNavOptions { + enableNavigationBreadcrumbs = options.enableNavigationBreadcrumbs + enableNavigationTransactions = options.enableNavigationTransactions + captureBackStack = !options.captureBackStack + maxCapturedBackStackEntries = options.maxCapturedBackStackEntries + } + }, + "maxCapturedBackStackEntries" to + { options -> + SentryNavOptions { + enableNavigationBreadcrumbs = options.enableNavigationBreadcrumbs + enableNavigationTransactions = options.enableNavigationTransactions + captureBackStack = options.captureBackStack + maxCapturedBackStackEntries = options.maxCapturedBackStackEntries + 1 + } + }, + ) + } +} diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index f28fffd6b8..b070c48657 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -4743,6 +4743,7 @@ public final class io/sentry/TypeCheckHint { public static final field KTOR_CLIENT_RESPONSE Ljava/lang/String; public static final field LOG4J_LOG_EVENT Ljava/lang/String; public static final field LOGBACK_LOGGING_EVENT Ljava/lang/String; + public static final field NAV3_DESTINATION Ljava/lang/String; public static final field OKHTTP_REQUEST Ljava/lang/String; public static final field OKHTTP_RESPONSE Ljava/lang/String; public static final field OPEN_FEIGN_REQUEST Ljava/lang/String; diff --git a/sentry/src/main/java/io/sentry/TypeCheckHint.java b/sentry/src/main/java/io/sentry/TypeCheckHint.java index 3260b46f16..0f3b41e27b 100644 --- a/sentry/src/main/java/io/sentry/TypeCheckHint.java +++ b/sentry/src/main/java/io/sentry/TypeCheckHint.java @@ -51,6 +51,9 @@ public final class TypeCheckHint { /** Used for Navigation breadrcrumbs. */ public static final String ANDROID_NAV_DESTINATION = "android:navigationDestination"; + /** Used for Navigation 3 breadcrumbs. */ + @ApiStatus.Internal public static final String NAV3_DESTINATION = "navigation3:destination"; + /** Used for Network breadrcrumbs. */ public static final String ANDROID_NETWORK_CAPABILITIES = "android:networkCapabilities"; From 3e5858db1fd642f47c7512a32f996aacd48eb7eb Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Fri, 18 Sep 2026 13:43:11 +0200 Subject: [PATCH 02/13] fix(android-nav3): Preserve app context when updating transaction context --- .../compose/navigation3/BackStackObserver.kt | 13 ++-- .../navigation3/BackStackObserverTest.kt | 61 +++++++++++++------ 2 files changed, 51 insertions(+), 23 deletions(-) diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt index 6638562015..31d074d1e5 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt @@ -208,7 +208,7 @@ internal class BackStackObserver( currentTopRoute.name, currentTopRoute.arguments, ) - ?.updateNavigationContext(currentBackStack) + ?.updateNavigationContext(scope, currentBackStack) } else { // Rotate the propagation context. scope.withPropagationContext { scope.setPropagationContext(PropagationContext()) } @@ -255,9 +255,13 @@ internal class BackStackObserver( * past what's relevant to a given transaction. This method prevents misassociation by binding * proper values to the transaction context instead. */ - private fun ITransaction.updateNavigationContext(backStack: BackStackData) { - val appContext = contexts.app ?: App().also { contexts.setApp(it) } - appContext.viewNames = listOf(backStack.topRoute.name) + private fun ITransaction.updateNavigationContext(scope: IScope, backStack: BackStackData) { + if (scopes.options.isEnableScreenTracking) { + val appContext = contexts.app ?: io.sentry.protocol.Contexts(scope.contexts).app ?: App() + + appContext.viewNames = listOf(backStack.topRoute.name) + contexts.setApp(appContext) + } if (options.captureBackStack && backStack.capturedRoutes.isNotEmpty()) { setContext(NAVIGATION_CONTEXT_KEY, backStack.capturedRoutes.toNavigationContext()) @@ -303,6 +307,7 @@ internal class BackStackObserver( try { body() } catch (t: Throwable) { + // Nav instrumentation can invoke host code through route translation and scope mutation. ExceptionUtils.rethrowIfFatal(t) scopes.options.logger.log( ERROR, diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt index ad5c934010..d17ca7bda4 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt @@ -15,6 +15,7 @@ import io.sentry.SentryTracer import io.sentry.TransactionContext import io.sentry.TransactionOptions import io.sentry.TypeCheckHint +import io.sentry.protocol.App import io.sentry.protocol.TransactionNameSource import kotlin.test.Test import kotlin.test.assertNull @@ -230,6 +231,7 @@ class BackStackObserverTest { assertThat(fixture.scope.screen).isNull() assertThat(fixture.scope.contexts.app?.viewNames).isNull() + assertThat(fixture.startedTransactions.single().contexts.app?.viewNames).isNull() } @Test @@ -372,6 +374,31 @@ class BackStackObserverTest { assertThat(argumentCalls).doesNotContainKey(home) } + @Test + fun `onBackStackChanged resolves top entry arguments once per update`() { + val fixture = Fixture() + val home = HomeRoute() + val profile = ProfileRoute("123") + val argumentCalls = mutableMapOf() + val sut = + fixture.getSut( + argumentsExtractor = + RouteArgumentsExtractor { entry -> + argumentCalls[entry] = (argumentCalls[entry] ?: 0) + 1 + when (entry) { + is HomeRoute -> mapOf("tab" to entry.id) + is ProfileRoute -> mapOf("userId" to entry.userId) + else -> emptyMap() + } + } + ) + + sut.onBackStackChanged(listOf(home, profile)) + + assertThat(argumentCalls[profile]).isEqualTo(1) + assertThat(argumentCalls[home]).isEqualTo(1) + } + @Test fun `onBackStackChanged creates a nav transaction when enabled and no ambient transaction is active`() { val fixture = Fixture() @@ -408,28 +435,24 @@ class BackStackObserverTest { } @Test - fun `onBackStackChanged resolves top entry arguments once per update`() { + fun `onBackStackChanged preserves scope app fields on the nav transaction`() { val fixture = Fixture() - val home = HomeRoute() - val profile = ProfileRoute("123") - val argumentCalls = mutableMapOf() - val sut = - fixture.getSut( - argumentsExtractor = - RouteArgumentsExtractor { entry -> - argumentCalls[entry] = (argumentCalls[entry] ?: 0) + 1 - when (entry) { - is HomeRoute -> mapOf("tab" to entry.id) - is ProfileRoute -> mapOf("userId" to entry.userId) - else -> emptyMap() - } - } - ) + val scopeApp = + App().apply { + appName = "Demo App" + appIdentifier = "io.sentry.demo" + } + fixture.scope.contexts.setApp(scopeApp) + val sut = fixture.getSut(config = ObserverConfig(enableScreenTracking = true)) - sut.onBackStackChanged(listOf(home, profile)) + sut.onBackStackChanged(listOf(HomeRoute(), ProfileRoute("123"))) - assertThat(argumentCalls[profile]).isEqualTo(1) - assertThat(argumentCalls[home]).isEqualTo(1) + val transactionApp = fixture.startedTransactions.single().contexts.app + assertThat(transactionApp).isNotNull() + assertThat(transactionApp).isNotSameInstanceAs(scopeApp) + assertThat(transactionApp?.appName).isEqualTo("Demo App") + assertThat(transactionApp?.appIdentifier).isEqualTo("io.sentry.demo") + assertThat(transactionApp?.viewNames).isEqualTo(listOf("/ProfileRoute")) } @Test From c2518cea21dda59be2bb2c0170261ab43792956e Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Fri, 18 Sep 2026 13:59:59 +0200 Subject: [PATCH 03/13] ref(android-nav3): Prepare back stack changes --- .../compose/navigation3/BackStackObserver.kt | 78 +++++++++++-------- 1 file changed, 45 insertions(+), 33 deletions(-) diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt index 31d074d1e5..bc4cf20346 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt @@ -14,6 +14,9 @@ import io.sentry.SpanStatus import io.sentry.TransactionContext import io.sentry.TransactionOptions import io.sentry.TypeCheckHint +import io.sentry.compose.navigation3.PreparedChange.BackStackHasNewTop +import io.sentry.compose.navigation3.PreparedChange.BackStackHasSameTop +import io.sentry.compose.navigation3.PreparedChange.BackStackIsEmpty import io.sentry.protocol.App import io.sentry.protocol.TransactionNameSource import io.sentry.util.ExceptionUtils @@ -88,18 +91,8 @@ internal class BackStackObserver( */ internal fun onBackStackChanged(backStack: List) { guard("onBackStackChanged") { - val topEntry = backStack.lastOrNull() - - val status = - when { - topEntry == null -> BackStackStatus.EMPTY - topEntry === previousTopEntry?.get() -> BackStackStatus.SAME_TOP - else -> BackStackStatus.NEW_TOP - } - - scopes.configureScope { scope -> - applyBackStackChange(scope, status, backStack) - } + val change = prepareChange(backStack) + scopes.configureScope { scope -> applyPreparedChange(scope, change) } } } @@ -124,24 +117,29 @@ internal class BackStackObserver( } } - private fun applyBackStackChange( - scope: IScope, - status: BackStackStatus, - backStack: List, - ) { - when (status) { - BackStackStatus.EMPTY -> handleEmptyBackStack(scope) + private fun prepareChange(backStack: List): PreparedChange { + val topEntry = backStack.lastOrNull() ?: return BackStackIsEmpty + val data = backStack.extractData() + + return if (topEntry === previousTopEntry?.get()) { + BackStackHasSameTop(data) + } else { + BackStackHasNewTop(previousTopRoute, data) + } + } + + private fun applyPreparedChange(scope: IScope, change: PreparedChange) { + when (change) { + is BackStackIsEmpty -> handleEmptyBackStack(scope) - BackStackStatus.NEW_TOP -> { - val data = backStack.extractData() - handleNewTop(scope, previousTopRoute, data) - storeAsPreviousTop(data.topEntry, data.topRoute) + is BackStackHasNewTop -> { + handleNewTop(scope, change.previousTopRoute, change.data) + storeAsPreviousTop(change.data.topEntry, change.data.topRoute) } - BackStackStatus.SAME_TOP -> { - val data = backStack.extractData() - handleSameTop(scope, data) - storeAsPreviousTop(data.topEntry, data.topRoute) + is BackStackHasSameTop -> { + handleSameTop(scope, change.data) + storeAsPreviousTop(change.data.topEntry, change.data.topRoute) } } } @@ -319,15 +317,29 @@ internal class BackStackObserver( } } -private enum class BackStackStatus { - EMPTY, +/** + * A model for applying one back stack update. + * + * Lets us separate change preparation from its application so that the [IScopes.configureScope] + * callback in charge of application can use already-computed navigation state. Otherwise, any + * exceptions thrown during state computation would be swallowed by `configureScope`'s over-broad + * `catch` clause. + */ +private sealed interface PreparedChange { + + /** The incoming back stack is empty. */ + data object BackStackIsEmpty : PreparedChange + /** - * The top of the back stack has changed, and one or more entries below it may have been updated - * as well. + * The top of the back stack has changed, and one or more entries below it may have been updated. */ - NEW_TOP, + data class BackStackHasNewTop( + val previousTopRoute: Route?, + val data: BackStackData, + ) : PreparedChange + /** The top of the back stack is unchanged, but one or more entries below it have been updated. */ - SAME_TOP, + data class BackStackHasSameTop(val data: BackStackData) : PreparedChange } /** Info extracted from the host app's back stack in a form suitable for Sentry data. */ From 640bcab0d9579ab52741751e2d62bbb73f3a7cf5 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Fri, 18 Sep 2026 14:57:41 +0200 Subject: [PATCH 04/13] ref(android-nav3): Rename nav3 destination hint Keep the hint internal for the current stack, but mark it experimental ahead of the follow-on public surface work. Also rename the key and field to match the Android hint naming pattern. --- .../kotlin/io/sentry/compose/navigation3/BackStackObserver.kt | 2 +- .../io/sentry/compose/navigation3/BackStackObserverTest.kt | 4 ++-- sentry/api/sentry.api | 2 +- sentry/src/main/java/io/sentry/TypeCheckHint.java | 3 ++- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt index bc4cf20346..1adf8d7a03 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt @@ -296,7 +296,7 @@ internal class BackStackObserver( } val hint = Hint() - hint.set(TypeCheckHint.NAV3_DESTINATION, toEntry) + hint.set(TypeCheckHint.ANDROID_NAV3_DESTINATION, toEntry) this.addBreadcrumb(breadcrumb, hint) } diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt index d17ca7bda4..6efad1a918 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt @@ -149,7 +149,7 @@ class BackStackObserverTest { "to_arguments", mapOf("userId" to "123"), ) - assertThat(fixture.breadcrumbHints.last().get(TypeCheckHint.NAV3_DESTINATION)) + assertThat(fixture.breadcrumbHints.last().get(TypeCheckHint.ANDROID_NAV3_DESTINATION)) .isSameInstanceAs(profile) } @@ -285,7 +285,7 @@ class BackStackObserverTest { assertThat(fixture.breadcrumbs).hasSize(2) assertThat(fixture.breadcrumbs.last().data["from"]).isEqualTo("/ProfileRoute") assertThat(fixture.breadcrumbs.last().data["to"]).isEqualTo("/ProfileRoute") - assertThat(fixture.breadcrumbHints.last().get(TypeCheckHint.NAV3_DESTINATION)) + assertThat(fixture.breadcrumbHints.last().get(TypeCheckHint.ANDROID_NAV3_DESTINATION)) .isSameInstanceAs(replacementProfile) assertThat(fixture.startedTransactions).hasSize(2) assertThat(fixture.startedTransactions.last().name).isEqualTo("/ProfileRoute") diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index b070c48657..c77d94cc79 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -4730,6 +4730,7 @@ public final class io/sentry/TypeCheckHint { public static final field ANDROID_FRAGMENT Ljava/lang/String; public static final field ANDROID_INTENT Ljava/lang/String; public static final field ANDROID_MOTION_EVENT Ljava/lang/String; + public static final field ANDROID_NAV3_DESTINATION Ljava/lang/String; public static final field ANDROID_NAV_DESTINATION Ljava/lang/String; public static final field ANDROID_NETWORK_CAPABILITIES Ljava/lang/String; public static final field ANDROID_SENSOR_EVENT Ljava/lang/String; @@ -4743,7 +4744,6 @@ public final class io/sentry/TypeCheckHint { public static final field KTOR_CLIENT_RESPONSE Ljava/lang/String; public static final field LOG4J_LOG_EVENT Ljava/lang/String; public static final field LOGBACK_LOGGING_EVENT Ljava/lang/String; - public static final field NAV3_DESTINATION Ljava/lang/String; public static final field OKHTTP_REQUEST Ljava/lang/String; public static final field OKHTTP_RESPONSE Ljava/lang/String; public static final field OPEN_FEIGN_REQUEST Ljava/lang/String; diff --git a/sentry/src/main/java/io/sentry/TypeCheckHint.java b/sentry/src/main/java/io/sentry/TypeCheckHint.java index 0f3b41e27b..852f960192 100644 --- a/sentry/src/main/java/io/sentry/TypeCheckHint.java +++ b/sentry/src/main/java/io/sentry/TypeCheckHint.java @@ -52,7 +52,8 @@ public final class TypeCheckHint { public static final String ANDROID_NAV_DESTINATION = "android:navigationDestination"; /** Used for Navigation 3 breadcrumbs. */ - @ApiStatus.Internal public static final String NAV3_DESTINATION = "navigation3:destination"; + @ApiStatus.Experimental @ApiStatus.Internal + public static final String ANDROID_NAV3_DESTINATION = "android:nav3Destination"; /** Used for Network breadrcrumbs. */ public static final String ANDROID_NETWORK_CAPABILITIES = "android:networkCapabilities"; From 421190018dd3efbda7b357dcaa2a41868c96a267 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Fri, 25 Sep 2026 09:10:45 +0200 Subject: [PATCH 05/13] Specify RouteTransltor's retention policy --- .../compose/navigation3/BackStackObserver.kt | 11 +++++-- .../compose/navigation3/RouteTranslator.kt | 4 +-- .../navigation3/BackStackObserverTest.kt | 31 ++++++++++++++++++- 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt index 1adf8d7a03..5d0164f172 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt @@ -17,6 +17,7 @@ import io.sentry.TypeCheckHint 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.RouteTranslator.RetentionPolicy import io.sentry.protocol.App import io.sentry.protocol.TransactionNameSource import io.sentry.util.ExceptionUtils @@ -53,12 +54,12 @@ import java.lang.ref.WeakReference internal class BackStackObserver( private val scopes: IScopes, private val options: SentryNavOptions, - private val resolvers: () -> RouteResolvers, + private val extractors: () -> RouteExtractors, ) { private val navTransactions = NavTransactionManager(scopes, NAVIGATION_OP, TRANSACTION_ORIGIN) private val screenTracker = ScreenTracker() - private val routeTranslator = RouteTranslator(resolvers, scopes.options.logger) + private val routeTranslator = RouteTranslator(extractors, scopes.options.logger) private var previousTopEntry: WeakReference? = null private var previousTopRoute: Route? = null @@ -167,7 +168,11 @@ internal class BackStackObserver( else -> listOf(topEntry) } - val routes = routeTranslator.translate(entriesToTranslate) + val routes = + routeTranslator.translate( + backStackEntries = entriesToTranslate, + retentionPolicy = RetentionPolicy.KEEP_FIRST, + ) return BackStackData( topEntry = topEntry, diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/RouteTranslator.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/RouteTranslator.kt index d687a7d890..fb7a6c14df 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/RouteTranslator.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/RouteTranslator.kt @@ -25,13 +25,13 @@ internal class RouteTranslator( } /** Translates the provided [backStackEntries] into [Route]s and returns them in input order. */ - fun translate(backStackEntries: List, policy: RetentionPolicy): List { + fun translate(backStackEntries: List, retentionPolicy: RetentionPolicy): List { val warningState = WarningState() val sanitizer = ArgumentSanitizer(logger, warningState) val routes = MutableList(backStackEntries.size) { null } val indicesInPolicyOrder = - when (policy) { + when (retentionPolicy) { RetentionPolicy.KEEP_FIRST -> backStackEntries.indices RetentionPolicy.KEEP_LAST -> backStackEntries.indices.reversed() } diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt index 6efad1a918..0351621f84 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt @@ -96,7 +96,7 @@ class BackStackObserverTest { captureBackStack = config.captureBackStack maxCapturedBackStackEntries = config.maxCapturedBackStackEntries }, - resolvers = { RouteResolvers(nameExtractor, argumentsExtractor) }, + extractors = { RouteExtractors(nameExtractor, argumentsExtractor) }, ) } @@ -248,6 +248,35 @@ class BackStackObserverTest { .isEqualTo(listOf(mapOf("route" to "/SettingsRoute"), mapOf("route" to "/ProfileRoute"))) } + @Test + fun `onBackStackChanged preserves top entry arguments when lower entries exhaust the shared budget`() { + val fixture = Fixture() + val sut = + fixture.getSut( + config = ObserverConfig(captureBackStack = true), + argumentsExtractor = + RouteArgumentsExtractor { entry -> + when (entry) { + is HomeRoute -> mapOf("values" to List(999) { it }) + is ProfileRoute -> mapOf("userId" to entry.userId) + else -> emptyMap() + } + }, + ) + + sut.onBackStackChanged(listOf(HomeRoute(), ProfileRoute("123"))) + + assertThat(fixture.scope.navigationBackStack()) + .isEqualTo( + listOf( + mapOf("route" to "/ProfileRoute", "args" to mapOf("userId" to "123")), + mapOf("route" to "/HomeRoute"), + ) + ) + assertThat(fixture.startedTransactions.single().getData("arguments")) + .isEqualTo(mapOf("userId" to "123")) + } + @Test fun `onBackStackChanged emits an updated copy of the back stack even when the top entry is unchanged`() { val fixture = Fixture() From dd5b5cae88bc951485688aa855e72b8ae467fa8a Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Fri, 25 Sep 2026 09:18:31 +0200 Subject: [PATCH 06/13] fix(android-nav3): set nav tx origin before start --- .../compose/navigation3/BackStackObserver.kt | 2 +- .../navigation3/BackStackObserverTest.kt | 26 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt index 5d0164f172..dca3c924db 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt @@ -410,6 +410,7 @@ private class NavTransactionManager( val deadlineTimeoutMillis = scopes.options.deadlineTimeout it.deadlineTimeout = if (deadlineTimeoutMillis <= 0) null else deadlineTimeoutMillis it.isTrimEnd = true + it.origin = transactionOrigin } val transaction = @@ -421,7 +422,6 @@ private class NavTransactionManager( activeNavTransaction = transaction transaction.apply { - spanContext.origin = transactionOrigin if (arguments.isNotEmpty()) { setData("arguments", arguments) } diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt index 0351621f84..93f022e9b6 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt @@ -20,6 +20,7 @@ import io.sentry.protocol.TransactionNameSource import kotlin.test.Test import kotlin.test.assertNull import org.mockito.kotlin.any +import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.whenever @@ -463,6 +464,31 @@ class BackStackObserverTest { assertThat(fixture.scope.transaction).isSameInstanceAs(transaction) } + // Regression test: Setting origin after starting the transaction breaks the ignoredSpanOrigins + // check (see SentryOptions.getIgnoredSpanOrigins()). + @Test + fun `onBackStackChanged sets nav transaction origin before starting the transaction`() { + val fixture = Fixture() + val transactionOptionsCaptor = argumentCaptor() + whenever( + fixture.scopes.startTransaction( + any(), + transactionOptionsCaptor.capture(), + ) + ) + .thenAnswer { + val transactionContext = it.arguments[0] as TransactionContext + val transactionOptions = it.arguments[1] as TransactionOptions + SentryTracer(transactionContext, fixture.scopes, transactionOptions) + .also(fixture.startedTransactions::add) + } + val sut = fixture.getSut(config = ObserverConfig(enableNavigationTransactions = true)) + + sut.onBackStackChanged(listOf(HomeRoute())) + + assertThat(transactionOptionsCaptor.firstValue.origin).isEqualTo("auto.navigation.nav3") + } + @Test fun `onBackStackChanged preserves scope app fields on the nav transaction`() { val fixture = Fixture() From 2e53ba7088f9f1481b84a0337c6970ecfd6e3a61 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Fri, 25 Sep 2026 09:29:12 +0200 Subject: [PATCH 07/13] ref(android-nav3): clarify weak ref safety --- .../compose/navigation3/BackStackObserver.kt | 6 +++-- .../compose/navigation3/RouteExtractors.kt | 24 +++++++++++-------- .../compose/navigation3/RouteTranslator.kt | 8 +++++++ .../compose/navigation3/SentryNavOptions.kt | 12 ++++++++-- 4 files changed, 36 insertions(+), 14 deletions(-) diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt index dca3c924db..cc513dbe54 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt @@ -8,12 +8,13 @@ import io.sentry.ITransaction import io.sentry.PropagationContext import io.sentry.SentryIntegrationPackageStorage import io.sentry.SentryLevel.DEBUG -import io.sentry.SentryLevel.ERROR import io.sentry.SentryLevel.INFO +import io.sentry.SentryLevel.WARNING import io.sentry.SpanStatus import io.sentry.TransactionContext import io.sentry.TransactionOptions import io.sentry.TypeCheckHint +import io.sentry.compose.navigation3.BackStackObserver.Companion.NAVIGATION_CONTEXT_KEY import io.sentry.compose.navigation3.PreparedChange.BackStackHasNewTop import io.sentry.compose.navigation3.PreparedChange.BackStackHasSameTop import io.sentry.compose.navigation3.PreparedChange.BackStackIsEmpty @@ -61,6 +62,7 @@ internal class BackStackObserver( private val screenTracker = ScreenTracker() private val routeTranslator = RouteTranslator(extractors, scopes.options.logger) + // Safe because the host back stack retains the current top entry strongly between updates. private var previousTopEntry: WeakReference? = null private var previousTopRoute: Route? = null @@ -313,7 +315,7 @@ internal class BackStackObserver( // Nav instrumentation can invoke host code through route translation and scope mutation. ExceptionUtils.rethrowIfFatal(t) scopes.options.logger.log( - ERROR, + WARNING, t, "Nav3 instrumentation failed during %s. Skipping this navigation update.", operation, diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/RouteExtractors.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/RouteExtractors.kt index 92dd05407f..02ee056cba 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/RouteExtractors.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/RouteExtractors.kt @@ -11,7 +11,7 @@ import org.jetbrains.annotations.ApiStatus * Values returned from [extract] are ***not*** scrubbed by the Sentry SDK before being sent to * Sentry. Only return names that are known to be safe or have been pre-scrubbed. * - * **Choosing stable route names** + * **Choosing appropriate route names** * * Implementations should return stable, low-cardinality names that don't depend on object identity, * argument values, or runtime class-name preservation. E.g., `Home`, `DetailScreen`, etc. @@ -19,6 +19,9 @@ import org.jetbrains.annotations.ApiStatus * In particular, avoid `::class.simpleName` in release builds, as R8 obfuscates class names and may * map them to different symbols across builds. * + * Extractors are invoked synchronously from [SentryNavEffect] on the same apply thread that runs + * the effect. Avoid non-performant extraction logic. + * * **Falls back to "/unknown"** * * If [extract] throws or returns a blank route name, Sentry records the destination as "/unknown". @@ -31,8 +34,8 @@ import org.jetbrains.annotations.ApiStatus * * **Using kotlinx.serialization** * - * If your back stack contains `@Serializable` route types, you may want to consider mapping each - * route type to a stable serializer name. For instance: + * If your back stack contains `@Serializable` route types, consider mapping each route type to a + * stable serializer name. For instance: * ```kotlin * val nameExtractor = RouteNameExtractor { route -> * when (route) { @@ -43,7 +46,7 @@ import org.jetbrains.annotations.ApiStatus * } * ``` * - * Doing so prevents route names from being obfuscated while leaving per-route arguments to + * Doing so gives each route type a stable, non-obfuscated name while leaving per-route arguments to * [RouteArgumentsExtractor]. */ @ApiStatus.Experimental @@ -60,13 +63,14 @@ internal fun interface RouteNameExtractor { * Values returned from [extract] are ***not*** scrubbed by the Sentry SDK before being sent to * Sentry. Only return arguments that are known to be safe or have been pre-scrubbed. * - * **Choosing performant route arguments** + * **Choosing appropriate route arguments** * * Return only a small subset of route data useful for diagnostics. Data should be stable enough to * inspect in Sentry. * - * For performance reasons, implementations should avoid large structures. Cyclic or deeply nested - * containers will be skipped. (See `RouteTranslator` for more details.) + * Extractors are invoked synchronously from [SentryNavEffect] on the same apply thread that runs + * the effect. For performance reasons, implementations should avoid large structures. Cyclic or + * deeply nested containers will be skipped. (See `RouteTranslator` for more details.) * * **Accepted value types** * @@ -96,9 +100,9 @@ internal fun interface RouteNameExtractor { * * **Using kotlinx.serialization** * - * Even if your back stack contains `@Serializable` route types, consider mapping each route type to - * a small set of diagnostic arguments to avoid the cost of serializing and returning the entire - * route object. For instance: + * If your back stack contains `@Serializable` route types, avoid returning the entire route object + * when it may be large, nested, or privacy-sensitive. Prefer a small set of diagnostic arguments + * instead. For instance: * ```kotlin * val argumentsExtractor = RouteArgumentsExtractor { route -> * when (route) { diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/RouteTranslator.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/RouteTranslator.kt index fb7a6c14df..7007a8162f 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/RouteTranslator.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/RouteTranslator.kt @@ -9,6 +9,14 @@ import org.jetbrains.annotations.TestOnly /** * Translates app-defined back stack entries into input-ordered [Route]s. * + * **Exception handling policy** + * + * Invocations of host-provided [extractors] and sanitization of host-defined arguments are + * protected by broad `try-catch` clauses, as each may throw arbitrary exceptions. We avoid failing + * fast on the assumption that navigation telemetry is supplemental from host apps' perspective, and + * that falling back to an `/unknown` route name or losing an argument map is preferable to + * crashing. + * * **Threading policy** * * This class performs work synchronously on the calling thread. Host-provided [extractors] are diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt index d1598fedb8..5d7e378f6d 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt @@ -10,7 +10,7 @@ private const val DEFAULT_MAX_CAPTURED_BACK_STACK_ENTRIES = 10 /** * Configuration info for a [SentryNavEffect]. * - * Instances are immutable; create one with the [SentryNavOptions] DSL: + * Instances are immutable; create one with the SentryNavOptions DSL: * ```kotlin * val options = SentryNavOptions { * captureBackStack = false @@ -98,7 +98,15 @@ private constructor( } } -/** Creates [SentryNavOptions]. Optionally configure it via [configure]. */ +/** + * Creates [SentryNavOptions]. Optionally configure it via [configure]. E.g.: + * ```kotlin + * val options = SentryNavOptions { + * captureBackStack = false + * maxCapturedBackStackEntries = 5 + * } + * ``` + */ @ApiStatus.Experimental internal fun SentryNavOptions( configure: SentryNavOptions.Builder.() -> Unit = {} From a372c7bf77341bc4fdff7f64e455c0c99c88996c Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Fri, 25 Sep 2026 10:33:27 +0200 Subject: [PATCH 08/13] ref(android-nav3): Drop observer-wide exception guard Remove BackStackObserver's broad top-level try/catch around navigation updates and cleanup. Keep broad catching at the actual host-callback boundaries in RouteTranslator, where route extractors and later value sanitization can execute arbitrary application code. Those are the places where graceful degradation is justified. Do not keep a second observer-wide catch above that layer. It mostly hides SDK bugs in BackStackObserver itself, and configureScope already catches exceptions thrown from the scope-mutation callback. Dropping the extra wrapper leaves host callback failures isolated where they occur while making internal observer failures easier to surface during testing and dogfooding. Co-Authored-By: OpenAI GPT-5.4 --- .../compose/navigation3/BackStackObserver.kt | 58 ++++++------------- 1 file changed, 18 insertions(+), 40 deletions(-) diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt index cc513dbe54..88bbc4ea12 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt @@ -9,7 +9,6 @@ import io.sentry.PropagationContext import io.sentry.SentryIntegrationPackageStorage import io.sentry.SentryLevel.DEBUG import io.sentry.SentryLevel.INFO -import io.sentry.SentryLevel.WARNING import io.sentry.SpanStatus import io.sentry.TransactionContext import io.sentry.TransactionOptions @@ -21,7 +20,6 @@ import io.sentry.compose.navigation3.PreparedChange.BackStackIsEmpty import io.sentry.compose.navigation3.RouteTranslator.RetentionPolicy import io.sentry.protocol.App import io.sentry.protocol.TransactionNameSource -import io.sentry.util.ExceptionUtils import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion import java.lang.ref.WeakReference @@ -90,32 +88,28 @@ internal class BackStackObserver( * Updates recorded Sentry data based on the provided [backStack]. * * Note: This method is ***not*** idempotent. Callers should protect against repeat invocations - * with the same back stack. + * with the same back stack to avoid emitting duplicate Sentry data. */ internal fun onBackStackChanged(backStack: List) { - guard("onBackStackChanged") { - val change = prepareChange(backStack) - scopes.configureScope { scope -> applyPreparedChange(scope, change) } - } + val change = prepareChange(backStack) + scopes.configureScope { scope -> applyChange(scope, change) } } internal fun cleanup() { - guard("cleanup") { - previousTopEntry = null - previousTopRoute = null - - scopes.configureScope { scope -> - navTransactions.stop(scope) - screenTracker.clear(scope) - - if (options.captureBackStack) { - // This observer owns the nav context while it's in the composition, and cleanup removes - // it to avoid leaking stale back stack data after observation stops. If the host app - // replaces one observer with another, there may be a brief gap where events lack nav - // context. Apps should keep the observer at the nav root so cleanup only runs when the - // navigation session is ending, not during normal destination changes. - scope.removeNavigationContext() - } + previousTopEntry = null + previousTopRoute = null + + scopes.configureScope { scope -> + navTransactions.stop(scope) + screenTracker.clear(scope) + + if (options.captureBackStack) { + // This observer owns the nav context while it's in the composition, and cleanup removes + // it to avoid leaking stale back stack data after observation stops. If the host app + // replaces one observer with another, there may be a brief gap where events lack nav + // context. Apps should keep the observer at the nav root so cleanup only runs when the + // navigation session is ending, not during normal destination changes. + scope.removeNavigationContext() } } } @@ -131,7 +125,7 @@ internal class BackStackObserver( } } - private fun applyPreparedChange(scope: IScope, change: PreparedChange) { + private fun applyChange(scope: IScope, change: PreparedChange) { when (change) { is BackStackIsEmpty -> handleEmptyBackStack(scope) @@ -306,22 +300,6 @@ internal class BackStackObserver( hint.set(TypeCheckHint.ANDROID_NAV3_DESTINATION, toEntry) this.addBreadcrumb(breadcrumb, hint) } - - @Suppress("TooGenericExceptionCaught") - private inline fun guard(operation: String, body: () -> Unit) { - try { - body() - } catch (t: Throwable) { - // Nav instrumentation can invoke host code through route translation and scope mutation. - ExceptionUtils.rethrowIfFatal(t) - scopes.options.logger.log( - WARNING, - t, - "Nav3 instrumentation failed during %s. Skipping this navigation update.", - operation, - ) - } - } } /** From 8d713108f344109ce8c0255b59fe1419a1683cdc Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Fri, 25 Sep 2026 11:01:59 +0200 Subject: [PATCH 09/13] ref(android-nav3): Extract nav context binder Move scope and transaction navigation-context updates out of BackStackObserver into a dedicated private helper. Keep the behavior unchanged while separating the route-to-context binding concern from the observer's change-detection and transaction lifecycle logic. This reduces the observer's surface area and makes the remaining TooManyFunctions pressure easier to address with smaller follow-up extractions. Co-Authored-By: OpenAI GPT-5.4 --- .../compose/navigation3/BackStackObserver.kt | 119 ++++++++++-------- 1 file changed, 67 insertions(+), 52 deletions(-) diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt index 88bbc4ea12..aa7c7933de 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt @@ -13,7 +13,6 @@ import io.sentry.SpanStatus import io.sentry.TransactionContext import io.sentry.TransactionOptions import io.sentry.TypeCheckHint -import io.sentry.compose.navigation3.BackStackObserver.Companion.NAVIGATION_CONTEXT_KEY import io.sentry.compose.navigation3.PreparedChange.BackStackHasNewTop import io.sentry.compose.navigation3.PreparedChange.BackStackHasSameTop import io.sentry.compose.navigation3.PreparedChange.BackStackIsEmpty @@ -23,6 +22,11 @@ import io.sentry.protocol.TransactionNameSource import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion import java.lang.ref.WeakReference +private const val BACKSTACK_KEY = "backstack" +private const val NAVIGATION_CONTEXT_KEY = "navigation" +private const val NAVIGATION_OP: String = "navigation" +private const val TRANSACTION_ORIGIN = "auto.navigation.nav3" + /** * Observes the back stack managed by a single [SentryNavEffect] and records Sentry state as the * back stack is updated. @@ -72,12 +76,6 @@ internal class BackStackObserver( } internal companion object { - - private const val BACKSTACK_KEY = "backstack" - private const val NAVIGATION_CONTEXT_KEY = "navigation" - private const val NAVIGATION_OP: String = "navigation" - private const val TRANSACTION_ORIGIN = "auto.navigation.nav3" - init { SentryIntegrationPackageStorage.getInstance() .addPackage("maven:io.sentry:sentry-android-navigation3", BuildConfig.VERSION_NAME) @@ -109,7 +107,7 @@ internal class BackStackObserver( // replaces one observer with another, there may be a brief gap where events lack nav // context. Apps should keep the observer at the nav root so cleanup only runs when the // navigation session is ending, not during normal destination changes. - scope.removeNavigationContext() + NavigationContextBinder.updateScope(scope, emptyList()) } } } @@ -184,7 +182,7 @@ internal class BackStackObserver( ) { val currentTopRoute = currentBackStack.topRoute - scope.updateNavigationContext(currentBackStack.capturedRoutes) + NavigationContextBinder.updateScope(scope, currentBackStack.capturedRoutes) if (scopes.options.isEnableScreenTracking) { screenTracker.track(scope, currentTopRoute.name) @@ -207,7 +205,15 @@ internal class BackStackObserver( currentTopRoute.name, currentTopRoute.arguments, ) - ?.updateNavigationContext(scope, currentBackStack) + ?.let { transaction -> + NavigationContextBinder.updateTransaction( + transaction = transaction, + scope = scope, + backStack = currentBackStack, + isScreenTrackingEnabled = scopes.options.isEnableScreenTracking, + captureBackStack = options.captureBackStack, + ) + } } else { // Rotate the propagation context. scope.withPropagationContext { scope.setPropagationContext(PropagationContext()) } @@ -215,11 +221,11 @@ internal class BackStackObserver( } private fun handleSameTop(scope: IScope, backStack: BackStackData) { - scope.updateNavigationContext(backStack.capturedRoutes) + NavigationContextBinder.updateScope(scope, backStack.capturedRoutes) } private fun handleEmptyBackStack(scope: IScope) { - scope.updateNavigationContext(emptyList()) + NavigationContextBinder.updateScope(scope, emptyList()) navTransactions.stop(scope) screenTracker.clear(scope) previousTopEntry = null @@ -231,46 +237,6 @@ internal class BackStackObserver( previousTopRoute = topRoute } - private fun IScope.updateNavigationContext(capturedRoutes: List) { - if (capturedRoutes.isEmpty()) { - this.removeNavigationContext() - } else { - this.setContexts(NAVIGATION_CONTEXT_KEY, capturedRoutes.toNavigationContext()) - } - } - - private fun IScope.removeNavigationContext() { - // We purposefully don't call IScope.removeContexts(), as it doesn't notify IScopeObserver and - // therefore doesn't write its updates to disk ¯\_ (ツ)_/¯. - this.setContexts(NAVIGATION_CONTEXT_KEY, null as Any?) - } - - /** - * Updates the receiver's context with the provided navigation info. - * - * 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. - */ - private fun ITransaction.updateNavigationContext(scope: IScope, backStack: BackStackData) { - if (scopes.options.isEnableScreenTracking) { - val appContext = contexts.app ?: io.sentry.protocol.Contexts(scope.contexts).app ?: App() - - appContext.viewNames = listOf(backStack.topRoute.name) - contexts.setApp(appContext) - } - - if (options.captureBackStack && backStack.capturedRoutes.isNotEmpty()) { - setContext(NAVIGATION_CONTEXT_KEY, backStack.capturedRoutes.toNavigationContext()) - } - } - - /** Builds the `{"backstack": [...]}` map bound under [NAVIGATION_CONTEXT_KEY]. */ - private fun List.toNavigationContext(): Map = - mapOf(BACKSTACK_KEY to serialize()) - private fun IScopes.addNav3Breadcrumb( from: Route?, toEntry: T, @@ -338,6 +304,55 @@ private data class BackStackData( val capturedRoutes: List, ) +private object NavigationContextBinder { + + fun updateScope(scope: IScope, capturedRoutes: List) { + if (capturedRoutes.isEmpty()) { + removeScopeContext(scope) + } else { + scope.setContexts(NAVIGATION_CONTEXT_KEY, capturedRoutes.toNavigationContext()) + } + } + + /** + * Updates the transaction with the provided navigation info. + * + * 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. + */ + fun updateTransaction( + transaction: ITransaction, + scope: IScope, + backStack: BackStackData, + isScreenTrackingEnabled: Boolean, + captureBackStack: Boolean, + ) { + if (isScreenTrackingEnabled) { + val appContext = transaction.contexts.app ?: io.sentry.protocol.Contexts(scope.contexts).app ?: App() + + appContext.viewNames = listOf(backStack.topRoute.name) + transaction.contexts.setApp(appContext) + } + + if (captureBackStack && backStack.capturedRoutes.isNotEmpty()) { + transaction.setContext(NAVIGATION_CONTEXT_KEY, backStack.capturedRoutes.toNavigationContext()) + } + } + + private fun removeScopeContext(scope: IScope) { + // We purposefully don't call IScope.removeContexts(), as it doesn't notify IScopeObserver and + // therefore doesn't write its updates to disk ¯\_ (ツ)_/¯. + scope.setContexts(NAVIGATION_CONTEXT_KEY, null as Any?) + } + + /** Builds the `{"backstack": [...]}` map bound under [NAVIGATION_CONTEXT_KEY]. */ + private fun List.toNavigationContext(): Map = + mapOf(BACKSTACK_KEY to serialize()) +} + /** Tracks a provided name as the current visible screen. */ private class ScreenTracker { From 76a901395be6b31f7832faf48a46961936ecc09c Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Fri, 25 Sep 2026 11:15:05 +0200 Subject: [PATCH 10/13] ref(android-nav3): Rename observer helper classes Rename the local BackStackObserver collaborators to a consistent Nav* pattern. These types exist only to simplify the observer's implementation rather than to model stable domain abstractions. Using a shared helper-oriented naming scheme makes that status clearer and keeps the file's private implementation pieces on the same footing. Co-Authored-By: OpenAI GPT-5.4 --- .../compose/navigation3/BackStackObserver.kt | 289 +++++++++--------- 1 file changed, 153 insertions(+), 136 deletions(-) diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt index aa7c7933de..22a329ccc2 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt @@ -53,17 +53,19 @@ private const val TRANSACTION_ORIGIN = "auto.navigation.nav3" * This class is ***not*** thread-safe. Clients should serialize calls to [onBackStackChanged] and * [cleanup] (e.g., via invocation from an `*Effect` or another form of thread confinement). */ -@Suppress("TooManyFunctions") internal class BackStackObserver( private val scopes: IScopes, private val options: SentryNavOptions, - private val extractors: () -> RouteExtractors, + extractors: () -> RouteExtractors, ) { - private val navTransactions = NavTransactionManager(scopes, NAVIGATION_OP, TRANSACTION_ORIGIN) - private val screenTracker = ScreenTracker() private val routeTranslator = RouteTranslator(extractors, scopes.options.logger) + private val navTransaction = NavTransaction(scopes, NAVIGATION_OP, TRANSACTION_ORIGIN) + private val navContext = NavContext(scopes, options) + private val navScreen = NavScreen() + private val navBreadcrumbs = NavBreadcrumbs(scopes) + // Safe because the host back stack retains the current top entry strongly between updates. private var previousTopEntry: WeakReference? = null private var previousTopRoute: Route? = null @@ -83,10 +85,28 @@ internal class BackStackObserver( } /** - * Updates recorded Sentry data based on the provided [backStack]. + * Updates Sentry nav data based on the provided [backStack]. + * + * **Data generated** + * + * By default, the following happens every time the top of the back stack changes: + * + * - a breadcrumb is emitted + * - a screen name is recorded + * - a new nav transaction is started and the old nav transaction, if any, is stopped. + * + * Names and other info for all of the above are derived from the new back stack top. + * + * By default, a record of the current back stack is recorded for every call, irrespective of + * whether the top changes. * - * Note: This method is ***not*** idempotent. Callers should protect against repeat invocations - * with the same back stack to avoid emitting duplicate Sentry data. + * Defaults can be configured via the [SentryNavOptions] instance passed to this class's + * constructor. (Screen names can be disabled via [SentryOptions.setEnableScreenTracking].) + * + * **Not idempotent** + * + * This method is ***not*** idempotent. Callers should protect against repeat invocations with the + * same back stack to avoid emitting duplicate Sentry data. */ internal fun onBackStackChanged(backStack: List) { val change = prepareChange(backStack) @@ -98,8 +118,8 @@ internal class BackStackObserver( previousTopRoute = null scopes.configureScope { scope -> - navTransactions.stop(scope) - screenTracker.clear(scope) + navTransaction.stop(scope) + navScreen.clear(scope) if (options.captureBackStack) { // This observer owns the nav context while it's in the composition, and cleanup removes @@ -107,7 +127,7 @@ internal class BackStackObserver( // replaces one observer with another, there may be a brief gap where events lack nav // context. Apps should keep the observer at the nav root so cleanup only runs when the // navigation session is ending, not during normal destination changes. - NavigationContextBinder.updateScope(scope, emptyList()) + navContext.update(scope, emptyList()) } } } @@ -128,20 +148,20 @@ internal class BackStackObserver( is BackStackIsEmpty -> handleEmptyBackStack(scope) is BackStackHasNewTop -> { - handleNewTop(scope, change.previousTopRoute, change.data) - storeAsPreviousTop(change.data.topEntry, change.data.topRoute) + handleNewTop(scope, change.previousTop, change.backStack) + storeAsPreviousTop(change.backStack.topEntry, change.backStack.topRoute) } is BackStackHasSameTop -> { - handleSameTop(scope, change.data) - storeAsPreviousTop(change.data.topEntry, change.data.topRoute) + handleSameTop(scope, change.backStack) + storeAsPreviousTop(change.backStack.topEntry, change.backStack.topRoute) } } } /** - * Extracts Sentry data from the receiver (i.e., a list of host app back stack entries) in the - * form of a [BackStackData]. + * Extracts Sentry-compatible data from the receiver (i.e., a list of host app back stack entries) + * in the form of a [BackStackData]. * * Throws if the receiver is empty. */ @@ -182,38 +202,30 @@ internal class BackStackObserver( ) { val currentTopRoute = currentBackStack.topRoute - NavigationContextBinder.updateScope(scope, currentBackStack.capturedRoutes) + navContext.update(scope, currentBackStack.capturedRoutes) if (scopes.options.isEnableScreenTracking) { - screenTracker.track(scope, currentTopRoute.name) + navScreen.update(scope, currentTopRoute) } if (options.enableNavigationBreadcrumbs) { - scopes.addNav3Breadcrumb( + navBreadcrumbs.emit( from = previousTop, toEntry = currentBackStack.topEntry, toRoute = currentBackStack.topRoute, ) } - navTransactions.stop(scope) + navTransaction.stop(scope) if (areNavigationTransactionsEnabled) { - navTransactions + navTransaction .start( scope, currentTopRoute.name, currentTopRoute.arguments, ) - ?.let { transaction -> - NavigationContextBinder.updateTransaction( - transaction = transaction, - scope = scope, - backStack = currentBackStack, - isScreenTrackingEnabled = scopes.options.isEnableScreenTracking, - captureBackStack = options.captureBackStack, - ) - } + ?.let { transaction -> navContext.updateTransaction(transaction, scope, currentBackStack) } } else { // Rotate the propagation context. scope.withPropagationContext { scope.setPropagationContext(PropagationContext()) } @@ -221,13 +233,13 @@ internal class BackStackObserver( } private fun handleSameTop(scope: IScope, backStack: BackStackData) { - NavigationContextBinder.updateScope(scope, backStack.capturedRoutes) + navContext.update(scope, backStack.capturedRoutes) } private fun handleEmptyBackStack(scope: IScope) { - NavigationContextBinder.updateScope(scope, emptyList()) - navTransactions.stop(scope) - screenTracker.clear(scope) + navTransaction.stop(scope) + navScreen.clear(scope) + navContext.update(scope, emptyList()) previousTopEntry = null previousTopRoute = null } @@ -236,36 +248,6 @@ internal class BackStackObserver( previousTopEntry = WeakReference(topEntry) previousTopRoute = topRoute } - - private fun IScopes.addNav3Breadcrumb( - from: Route?, - toEntry: T, - toRoute: Route, - ) { - val breadcrumb = - Breadcrumb().apply { - type = NAVIGATION_OP - category = NAVIGATION_OP - - from?.let { - data["from"] = it.name - if (it.arguments.isNotEmpty()) { - data["from_arguments"] = it.arguments - } - } - - data["to"] = toRoute.name - if (toRoute.arguments.isNotEmpty()) { - data["to_arguments"] = toRoute.arguments - } - - level = INFO - } - - val hint = Hint() - hint.set(TypeCheckHint.ANDROID_NAV3_DESTINATION, toEntry) - this.addBreadcrumb(breadcrumb, hint) - } } /** @@ -285,12 +267,12 @@ private sealed interface PreparedChange { * The top of the back stack has changed, and one or more entries below it may have been updated. */ data class BackStackHasNewTop( - val previousTopRoute: Route?, - val data: BackStackData, + val previousTop: Route?, + val backStack: BackStackData, ) : PreparedChange /** The top of the back stack is unchanged, but one or more entries below it have been updated. */ - data class BackStackHasSameTop(val data: BackStackData) : PreparedChange + data class BackStackHasSameTop(val backStack: BackStackData) : PreparedChange } /** Info extracted from the host app's back stack in a form suitable for Sentry data. */ @@ -304,75 +286,8 @@ private data class BackStackData( val capturedRoutes: List, ) -private object NavigationContextBinder { - - fun updateScope(scope: IScope, capturedRoutes: List) { - if (capturedRoutes.isEmpty()) { - removeScopeContext(scope) - } else { - scope.setContexts(NAVIGATION_CONTEXT_KEY, capturedRoutes.toNavigationContext()) - } - } - - /** - * Updates the transaction with the provided navigation info. - * - * 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. - */ - fun updateTransaction( - transaction: ITransaction, - scope: IScope, - backStack: BackStackData, - isScreenTrackingEnabled: Boolean, - captureBackStack: Boolean, - ) { - if (isScreenTrackingEnabled) { - val appContext = transaction.contexts.app ?: io.sentry.protocol.Contexts(scope.contexts).app ?: App() - - appContext.viewNames = listOf(backStack.topRoute.name) - transaction.contexts.setApp(appContext) - } - - if (captureBackStack && backStack.capturedRoutes.isNotEmpty()) { - transaction.setContext(NAVIGATION_CONTEXT_KEY, backStack.capturedRoutes.toNavigationContext()) - } - } - - private fun removeScopeContext(scope: IScope) { - // We purposefully don't call IScope.removeContexts(), as it doesn't notify IScopeObserver and - // therefore doesn't write its updates to disk ¯\_ (ツ)_/¯. - scope.setContexts(NAVIGATION_CONTEXT_KEY, null as Any?) - } - - /** Builds the `{"backstack": [...]}` map bound under [NAVIGATION_CONTEXT_KEY]. */ - private fun List.toNavigationContext(): Map = - mapOf(BACKSTACK_KEY to serialize()) -} - -/** Tracks a provided name as the current visible screen. */ -private class ScreenTracker { - - private var lastScreenName: String? = null - - fun track(scope: IScope, screenName: String) { - scope.screen = screenName - lastScreenName = screenName - } - - fun clear(scope: IScope) { - val routeName = lastScreenName ?: return - if (scope.screen == routeName) { - scope.screen = null - } - lastScreenName = null - } -} - -private class NavTransactionManager( +/** A helper class for managing nav transactions. */ +private class NavTransaction( private val scopes: IScopes, private val navigationOp: String, private val transactionOrigin: String, @@ -455,3 +370,105 @@ private class NavTransactionManager( } } } + +/** A helper class for updating [nav context][NAVIGATION_CONTEXT_KEY]. */ +private class NavContext(private val scopes: IScopes, private val options: SentryNavOptions) { + + fun update(scope: IScope, backStackRoutes: List) { + if (backStackRoutes.isEmpty()) { + removeNavContext(scope) + } else { + scope.setContexts(NAVIGATION_CONTEXT_KEY, backStackRoutes.toNavigationContext()) + } + } + + /** + * Updates the transaction with the provided navigation info. + * + * 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. + */ + fun updateTransaction( + transaction: ITransaction, + scope: IScope, + backStack: BackStackData, + ) { + if (scopes.options.isEnableScreenTracking) { + val appContext = + transaction.contexts.app ?: io.sentry.protocol.Contexts(scope.contexts).app ?: App() + + appContext.viewNames = listOf(backStack.topRoute.name) + transaction.contexts.setApp(appContext) + } + + if (options.captureBackStack && backStack.capturedRoutes.isNotEmpty()) { + transaction.setContext(NAVIGATION_CONTEXT_KEY, backStack.capturedRoutes.toNavigationContext()) + } + } + + private fun removeNavContext(scope: IScope) { + // We purposefully don't call IScope.removeContexts(), as it doesn't notify IScopeObserver and + // therefore doesn't write its updates to disk ¯\_ (ツ)_/¯. + scope.setContexts(NAVIGATION_CONTEXT_KEY, null as Any?) + } + + /** Builds the `{"backstack": [...]}` map bound under [NAVIGATION_CONTEXT_KEY]. */ + private fun List.toNavigationContext(): Map = + mapOf(BACKSTACK_KEY to serialize()) +} + +/** A helper class for updating the tracked screen name. */ +private class NavScreen { + + private var lastScreenName: String? = null + + fun update(scope: IScope, currentRoute: Route) { + scope.screen = currentRoute.name + lastScreenName = currentRoute.name + } + + fun clear(scope: IScope) { + val routeName = lastScreenName ?: return + if (scope.screen == routeName) { + scope.screen = null + } + lastScreenName = null + } +} + +/** A helper class for generating nav breadcrumbs. */ +private class NavBreadcrumbs(private val scopes: IScopes) { + + fun emit( + from: Route?, + toEntry: T, + toRoute: Route, + ) { + val breadcrumb = + Breadcrumb().apply { + type = NAVIGATION_OP + category = NAVIGATION_OP + + from?.let { + data["from"] = it.name + if (it.arguments.isNotEmpty()) { + data["from_arguments"] = it.arguments + } + } + + data["to"] = toRoute.name + if (toRoute.arguments.isNotEmpty()) { + data["to_arguments"] = toRoute.arguments + } + + level = INFO + } + + val hint = Hint() + hint.set(TypeCheckHint.ANDROID_NAV3_DESTINATION, toEntry) + scopes.addBreadcrumb(breadcrumb, hint) + } +} From 67c8ed410ca0dfa9f4e7105ae47e6df148f41d28 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Fri, 25 Sep 2026 14:20:26 +0200 Subject: [PATCH 11/13] ref(android-nav3): clarify observer nav state helpers --- .../compose/navigation3/BackStackObserver.kt | 82 ++++++++++--------- 1 file changed, 42 insertions(+), 40 deletions(-) diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt index 22a329ccc2..fd9b40f95c 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt @@ -22,10 +22,7 @@ import io.sentry.protocol.TransactionNameSource import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion import java.lang.ref.WeakReference -private const val BACKSTACK_KEY = "backstack" -private const val NAVIGATION_CONTEXT_KEY = "navigation" private const val NAVIGATION_OP: String = "navigation" -private const val TRANSACTION_ORIGIN = "auto.navigation.nav3" /** * Observes the back stack managed by a single [SentryNavEffect] and records Sentry state as the @@ -61,7 +58,7 @@ internal class BackStackObserver( private val routeTranslator = RouteTranslator(extractors, scopes.options.logger) - private val navTransaction = NavTransaction(scopes, NAVIGATION_OP, TRANSACTION_ORIGIN) + private val navTransaction = NavTransaction(scopes) private val navContext = NavContext(scopes, options) private val navScreen = NavScreen() private val navBreadcrumbs = NavBreadcrumbs(scopes) @@ -127,7 +124,7 @@ internal class BackStackObserver( // replaces one observer with another, there may be a brief gap where events lack nav // context. Apps should keep the observer at the nav root so cleanup only runs when the // navigation session is ending, not during normal destination changes. - navContext.update(scope, emptyList()) + navContext.clear(scope) } } } @@ -147,15 +144,8 @@ internal class BackStackObserver( when (change) { is BackStackIsEmpty -> handleEmptyBackStack(scope) - is BackStackHasNewTop -> { - handleNewTop(scope, change.previousTop, change.backStack) - storeAsPreviousTop(change.backStack.topEntry, change.backStack.topRoute) - } - - is BackStackHasSameTop -> { - handleSameTop(scope, change.backStack) - storeAsPreviousTop(change.backStack.topEntry, change.backStack.topRoute) - } + is BackStackHasNewTop -> handleNewTop(scope, change.previousTop, change.backStack) + is BackStackHasSameTop -> handleSameTop(scope, change.backStack) } } @@ -230,24 +220,31 @@ internal class BackStackObserver( // Rotate the propagation context. scope.withPropagationContext { scope.setPropagationContext(PropagationContext()) } } + + storeAsPreviousTop(currentBackStack.topEntry, currentBackStack.topRoute) } private fun handleSameTop(scope: IScope, backStack: BackStackData) { navContext.update(scope, backStack.capturedRoutes) + storeAsPreviousTop(backStack.topEntry, backStack.topRoute) } private fun handleEmptyBackStack(scope: IScope) { navTransaction.stop(scope) + navContext.clear(scope) navScreen.clear(scope) - navContext.update(scope, emptyList()) - previousTopEntry = null - previousTopRoute = null + clearPreviousTop() } private fun storeAsPreviousTop(topEntry: T, topRoute: Route) { previousTopEntry = WeakReference(topEntry) previousTopRoute = topRoute } + + private fun clearPreviousTop() { + previousTopEntry = null + previousTopRoute = null + } } /** @@ -287,27 +284,27 @@ private data class BackStackData( ) /** A helper class for managing nav transactions. */ -private class NavTransaction( - private val scopes: IScopes, - private val navigationOp: String, - private val transactionOrigin: String, -) { +private class NavTransaction(private val scopes: IScopes) { + + private companion object { + private const val TRANSACTION_ORIGIN = "auto.navigation.nav3" + } private var activeNavTransaction: ITransaction? = null /** Starts an idle navigation transaction, or no-ops if another transaction is already active. */ fun start( scope: IScope, - routeName: String, + name: String, arguments: Map, ): ITransaction? { - clearFinishedScopeTransaction(scope) + clearTransactionIfFinished(scope) if (scope.transaction != null) { scopes.options.logger.log( DEBUG, "Nav3 transaction for route %s won't be created because another transaction is active.", - routeName, + name, ) return null @@ -320,12 +317,12 @@ private class NavTransaction( val deadlineTimeoutMillis = scopes.options.deadlineTimeout it.deadlineTimeout = if (deadlineTimeoutMillis <= 0) null else deadlineTimeoutMillis it.isTrimEnd = true - it.origin = transactionOrigin + it.origin = TRANSACTION_ORIGIN } val transaction = scopes.startTransaction( - TransactionContext(routeName, TransactionNameSource.ROUTE, navigationOp), + TransactionContext(name, TransactionNameSource.ROUTE, NAVIGATION_OP), transactionOptions, ) @@ -362,7 +359,7 @@ private class NavTransaction( } /** Clears a stale finished transaction that's still bound to the default scope. */ - private fun clearFinishedScopeTransaction(scope: IScope) { + private fun clearTransactionIfFinished(scope: IScope) { scope.withTransaction { tx -> if (tx?.isFinished == true) { scope.clearTransaction() @@ -374,12 +371,24 @@ private class NavTransaction( /** A helper class for updating [nav context][NAVIGATION_CONTEXT_KEY]. */ private class NavContext(private val scopes: IScopes, private val options: SentryNavOptions) { + private companion object { + private const val BACKSTACK_KEY = "backstack" + private const val NAVIGATION_CONTEXT_KEY = "navigation" + } + fun update(scope: IScope, backStackRoutes: List) { if (backStackRoutes.isEmpty()) { - removeNavContext(scope) - } else { - scope.setContexts(NAVIGATION_CONTEXT_KEY, backStackRoutes.toNavigationContext()) + clear(scope) + return } + + scope.setContexts(NAVIGATION_CONTEXT_KEY, backStackRoutes.toBackStackMap()) + } + + fun clear(scope: IScope) { + // We purposefully don't call IScope.removeContexts(), as it doesn't notify IScopeObserver and + // therefore doesn't write its updates to disk ¯\_ (ツ)_/¯. + scope.setContexts(NAVIGATION_CONTEXT_KEY, null as Any?) } /** @@ -405,19 +414,12 @@ private class NavContext(private val scopes: IScopes, private val options: Sentr } if (options.captureBackStack && backStack.capturedRoutes.isNotEmpty()) { - transaction.setContext(NAVIGATION_CONTEXT_KEY, backStack.capturedRoutes.toNavigationContext()) + transaction.setContext(NAVIGATION_CONTEXT_KEY, backStack.capturedRoutes.toBackStackMap()) } } - private fun removeNavContext(scope: IScope) { - // We purposefully don't call IScope.removeContexts(), as it doesn't notify IScopeObserver and - // therefore doesn't write its updates to disk ¯\_ (ツ)_/¯. - scope.setContexts(NAVIGATION_CONTEXT_KEY, null as Any?) - } - /** Builds the `{"backstack": [...]}` map bound under [NAVIGATION_CONTEXT_KEY]. */ - private fun List.toNavigationContext(): Map = - mapOf(BACKSTACK_KEY to serialize()) + private fun List.toBackStackMap(): Map = mapOf(BACKSTACK_KEY to serialize()) } /** A helper class for updating the tracked screen name. */ From ce6d964f8e98ff33bc8c275b33b315cf9b9b303a Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Fri, 25 Sep 2026 14:46:33 +0200 Subject: [PATCH 12/13] fix(android-nav3): Ignore no-op nav transactions Treat no-op transactions from scopes.startTransaction as 'not started' in BackStackObserver. This prevents ignored or incompatible nav transactions from being bound to the default scope, which could otherwise leave a finished no-op transaction attached and block later real transactions until the next top-of-stack change. Add a regression test proving the observer leaves scope.transaction unset when startTransaction returns NoOpTransaction. Co-Authored-By: OpenAI GPT-5.4 --- .../compose/navigation3/BackStackObserver.kt | 4 ++++ .../compose/navigation3/BackStackObserverTest.kt | 15 +++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt index fd9b40f95c..cd1e09d14d 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt @@ -326,6 +326,10 @@ private class NavTransaction(private val scopes: IScopes) { transactionOptions, ) + if (transaction.isNoOp) { + return null + } + activeNavTransaction = transaction transaction.apply { diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt index 93f022e9b6..6e26b608b7 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt @@ -8,6 +8,7 @@ import io.sentry.IScope import io.sentry.IScopes import io.sentry.ISpan import io.sentry.ITransaction +import io.sentry.NoOpTransaction import io.sentry.Scope import io.sentry.ScopeCallback import io.sentry.SentryOptions @@ -574,6 +575,20 @@ class BackStackObserverTest { assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransactions.single()) } + @Test + fun `onBackStackChanged does not bind a no-op nav transaction to the scope`() { + val fixture = Fixture() + whenever(fixture.scopes.startTransaction(any(), any())) + .thenReturn(NoOpTransaction.getInstance()) + val sut = fixture.getSut(config = ObserverConfig(enableNavigationTransactions = true)) + + sut.onBackStackChanged(listOf(HomeRoute())) + + assertThat(fixture.startedTransactions).isEmpty() + assertThat(fixture.scope.transaction).isNull() + assertThat(fixture.scope.screen).isEqualTo("/HomeRoute") + } + @Test fun `onBackStackChanged clears tracked scope state when the back stack becomes empty`() { val fixture = Fixture() From 9cf6d1a184ab6274238c7b5d8bfea6d109b4f15f Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Fri, 25 Sep 2026 16:54:03 +0200 Subject: [PATCH 13/13] Address runningcode's comments --- sentry-android-navigation3/build.gradle.kts | 2 -- .../compose/navigation3/SentryNavOptions.kt | 7 ++++ .../navigation3/BackStackObserverTest.kt | 7 ++-- .../navigation3/RouteExtractorsTest.kt | 2 +- .../navigation3/RouteTranslatorTest.kt | 2 +- .../navigation3/SentryNavOptionsTest.kt | 33 +++++++++++++++++-- 6 files changed, 42 insertions(+), 11 deletions(-) diff --git a/sentry-android-navigation3/build.gradle.kts b/sentry-android-navigation3/build.gradle.kts index 8eb45db293..973e071ad9 100644 --- a/sentry-android-navigation3/build.gradle.kts +++ b/sentry-android-navigation3/build.gradle.kts @@ -17,7 +17,6 @@ android { defaultConfig { minSdk = libs.versions.minSdk.get().toInt() - // for AGP 4.1 buildConfigField("String", "VERSION_NAME", "\"${project.version}\"") } @@ -63,7 +62,6 @@ dependencies { testImplementation(libs.androidx.compose.runtime) testImplementation(libs.google.truth) - testImplementation(libs.kotlin.test.junit) testImplementation(libs.mockito.inline) testImplementation(libs.mockito.kotlin) } diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt index 5d7e378f6d..add98ce3ef 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt @@ -96,6 +96,13 @@ private constructor( result = 31 * result + maxCapturedBackStackEntries return result } + + override fun toString(): String = + "SentryNavOptions(" + + "enableNavigationBreadcrumbs=$enableNavigationBreadcrumbs, " + + "enableNavigationTransactions=$enableNavigationTransactions, " + + "captureBackStack=$captureBackStack, " + + "maxCapturedBackStackEntries=$maxCapturedBackStackEntries)" } /** diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt index 6e26b608b7..5c510ac9fd 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt @@ -18,8 +18,7 @@ import io.sentry.TransactionOptions import io.sentry.TypeCheckHint import io.sentry.protocol.App import io.sentry.protocol.TransactionNameSource -import kotlin.test.Test -import kotlin.test.assertNull +import org.junit.Test import org.mockito.kotlin.any import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.doAnswer @@ -602,7 +601,7 @@ class BackStackObserverTest { assertThat(transaction.isFinished).isTrue() assertThat(fixture.scope.transaction).isNull() assertThat(fixture.scope.screen).isNull() - assertNull(fixture.scope.contexts.app?.viewNames) + assertThat(fixture.scope.contexts.app?.viewNames).isNull() assertThat(fixture.scope.contexts.containsKey("navigation")).isFalse() assertThat(fixture.breadcrumbs).hasSize(1) } @@ -732,7 +731,7 @@ class BackStackObserverTest { assertThat(transaction.isFinished).isTrue() assertThat(fixture.scope.transaction).isNull() assertThat(fixture.scope.screen).isNull() - assertNull(fixture.scope.contexts.app?.viewNames) + assertThat(fixture.scope.contexts.app?.viewNames).isNull() assertThat(fixture.scope.contexts.containsKey("navigation")).isFalse() } diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteExtractorsTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteExtractorsTest.kt index f97e5241ac..fd0d7ff6e9 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteExtractorsTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteExtractorsTest.kt @@ -3,7 +3,7 @@ package io.sentry.compose.navigation3 import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.snapshots.Snapshot import com.google.common.truth.Truth.assertThat -import kotlin.test.Test +import org.junit.Test class RouteExtractorsTest { diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteTranslatorTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteTranslatorTest.kt index ca2f8db318..b6301bce18 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteTranslatorTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteTranslatorTest.kt @@ -7,7 +7,7 @@ import io.sentry.compose.navigation3.RouteTranslator.ArgumentSanitizer import io.sentry.compose.navigation3.RouteTranslator.RetentionPolicy import io.sentry.compose.navigation3.RouteTranslator.WarningState import java.util.AbstractCollection -import kotlin.test.Test +import org.junit.Test import org.mockito.kotlin.clearInvocations import org.mockito.kotlin.eq import org.mockito.kotlin.mock diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavOptionsTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavOptionsTest.kt index 0833e2cbe2..742a7c0ed6 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavOptionsTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavOptionsTest.kt @@ -2,8 +2,8 @@ package io.sentry.compose.navigation3 import com.google.common.truth.Truth.assertThat import java.lang.reflect.Modifier -import kotlin.test.Test -import kotlin.test.assertFailsWith +import org.junit.Assert.assertThrows +import org.junit.Test class SentryNavOptionsTest { @@ -24,7 +24,7 @@ class SentryNavOptionsTest { @Test fun `rejects negative max captured backstack entries`() { val exception = - assertFailsWith { + assertThrows(IllegalArgumentException::class.java) { SentryNavOptions { maxCapturedBackStackEntries = -1 } } @@ -63,6 +63,33 @@ class SentryNavOptionsTest { } } + @Test + fun `toString includes every property`() { + val options = SentryNavOptions() + val instanceFields = + SentryNavOptions::class + .java + .declaredFields + .filterNot { Modifier.isStatic(it.modifiers) } + .associate { field -> + field.isAccessible = true + field.name to field.get(options) + } + + instanceFields.forEach { (name, value) -> + assertThat(options.toString()).contains("$name=$value") + } + } + + @Test + fun `toString changes when any property changes`() { + val base = SentryNavOptions() + + propertyMutators.forEach { (_, mutate) -> + assertThat(mutate(base).toString()).isNotEqualTo(base.toString()) + } + } + private companion object { val propertyMutators = mapOf SentryNavOptions>(