Address Max's comments

Author: 0xadam-brown

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

@@ -72,14 +72,19 @@ internal class BackStackConverter<T : Any>(
         return NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME)
       }
 
+    if (info == null) {
+      return NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME)
+    }
+
     val arguments = info.arguments?.let(sanitizer::sanitizeEntry) ?: emptyMap()
     val formattedName = NormalizedSentryBackStackEntry.formatName(info.name)
-    if (formattedName.isBlank()) {
+
+    return if (formattedName.isBlank()) {
       warningState.logInvalidNameWarning(logger)
-      return NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME, arguments)
+      NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME, arguments)
+    } else {
+      NormalizedSentryBackStackEntry(formattedName, arguments)
     }
-
-    return NormalizedSentryBackStackEntry(formattedName, arguments)
   }
 
   /**

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

@@ -77,9 +77,9 @@ public class SentryBackStackEntry(
  *
  * **Names fall back to "/unknown"**
  *
- * If [map] throws or returns a blank [name][SentryBackStackEntry.name], Sentry records the
- * destination as "/unknown". Doing so signals that name extraction needs to be fixed while avoiding
- * misleading gaps in navigation data.
+ * If [map] throws, returns `null`, or returns a blank [name][SentryBackStackEntry.name], Sentry
+ * records the destination as "/unknown". Doing so signals that name extraction needs to be fixed
+ * while avoiding misleading gaps in navigation data.
  *
  * For instance, if a user navigates from `/home -> /detail -> /settings`, but the mapper for
  * `/detail` throws, the back stack record will be `/home -> /unknown -> /settings` rather than
@@ -109,8 +109,8 @@ public class SentryBackStackEntry(
  *
  * **Arguments fall back to `toString()` or nothing**
  *
- * All non-supported argument types are stringified via `toString()`. If [map] throws, no arguments
- * are recorded for that back stack entry.
+ * All non-supported argument types are stringified via `toString()`. If [map] throws or returns
+ * `null`, no arguments are recorded for that back stack entry.
  *
  * **Using kotlinx.serialization**
  *
@@ -123,14 +123,15 @@ public class SentryBackStackEntry(
  * ```kotlin
  * @Serializable
  * @SerialName("Home")
- * data object Home(userName: String) : NavKey
+ * data class Home(userName: String) : NavKey
  *
  * @Serializable
  * @SerialName("ProductDetail")
  * data class ProductDetail(userName: String, productId: String, tab: Tab) : NavKey
- * ```
- * ```kotlin
- * val backStackItemMapper = BackStackEntryMapper<Any> { entry ->
+ *
+ * ...
+ *
+ * val backStackItemMapper = BackStackEntryMapper<NavKey> { entry ->
  *   when (entry) {
  *     is Home -> SentryBackStackEntry(Home.serializer().descriptor.serialName)
  *     is ProductDetail -> SentryBackStackEntry(
@@ -139,14 +140,15 @@ public class SentryBackStackEntry(
  *       // or non-performant.
  *       arguments = mapOf("product_id" to entry.productId, "tab" to entry.tab)
  *     )
+ *     ...
  *   }
  * }
  * ```
  */
 @ApiStatus.Experimental
 @ApiStatus.Internal
 public fun interface BackStackEntryMapper<T : Any> {
-  public fun map(backStackEntry: T): SentryBackStackEntry
+  public fun map(backStackEntry: T): SentryBackStackEntry?
 }
 
 /**
@@ -165,7 +167,7 @@ public fun interface BackStackEntryMapper<T : Any> {
 internal class ForwardingBackStackEntryMapper<T : Any>(
   private val currentMapper: () -> BackStackEntryMapper<T>
 ) {
-  fun map(backStackEntry: T): SentryBackStackEntry = Snapshot.withoutReadObservation {
+  fun map(backStackEntry: T): SentryBackStackEntry? = Snapshot.withoutReadObservation {
     currentMapper().map(backStackEntry)
   }
 }

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

@@ -21,8 +21,8 @@ import org.jetbrains.annotations.ApiStatus
  *
  *    // Place SentryNavEffect in the same composable as your NavDisplay and call
  *    // the effect first. Doing so ensures the effect's lifecycle matches your
- *    // NavDisplay, and that any Sentry data produced by your nav destinations
- *    // get attributed to the appropriate nav transaction.
+ *    // NavDisplay, and that any Sentry data produced by your initial nav
+ *    // destination get attributed to the appropriate nav transaction.
  *    SentryNavEffect(
  *      backStack = navBackStack,
  *      backStackEntryMapper = { entry ->
@@ -65,6 +65,10 @@ import org.jetbrains.annotations.ApiStatus
  * gestures. That means that spans produced by predictively rendered composables can show up under
  * the current destination's transaction.
  *
+ * Multiple simultaneously active `SentryNavEffect` instances writing to the same Sentry scope are
+ * not supported. Violating this restriction can result in interleaved breadcrumbs, clobbered screen
+ * names and back stacks, and transactions that interfere with one another.
+ *
  * @param backStack The navigation backstack to observe.
  * @param backStackEntryMapper Maps each entry of the [backStack] to a name and optional arguments
  *   for display in Sentry. See the [BackStackEntryMapper] KDoc for best practices.

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

@@ -13,7 +13,7 @@ private const val DEFAULT_MAX_CAPTURED_BACK_STACK_ENTRIES = 10
  * Instances are immutable; create one with the SentryNavOptions DSL:
  * ```kotlin
  * val options = SentryNavOptions {
- *   captureBackStack = false
+ *   enableNavigationBreadcrumbs = false
  *   maxCapturedBackStackEntries = 5
  * }
  * ```
@@ -110,7 +110,7 @@ private constructor(
  * Creates [SentryNavOptions]. Optionally configure it via [configure]. E.g.:
  * ```kotlin
  * val options = SentryNavOptions {
- *   captureBackStack = false
+ *   enableNavigationBreadcrumbs = false
  *   maxCapturedBackStackEntries = 5
  * }
  * ```

sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackConverterTest.kt

@@ -32,7 +32,7 @@ class BackStackConverterTest {
 
   private val defaultEntryMapper =
     BackStackEntryMapper<Any> { entry ->
-      SentryBackStackEntry(entry::class.simpleName ?: "unknown")
+      SentryBackStackEntry(entry::class.simpleName ?: "<unknown>")
     }
 
   private fun getSut(
@@ -44,7 +44,7 @@ class BackStackConverterTest {
     )
 
   private fun entryInfo(entry: Any, arguments: Map<String, Any?>? = null): SentryBackStackEntry =
-    SentryBackStackEntry(entry::class.simpleName ?: "unknown", arguments)
+    SentryBackStackEntry(entry::class.simpleName ?: "<unknown>", arguments)
 
   private fun BackStackConverter<Any>.convert(entry: Any): NormalizedSentryBackStackEntry =
     convert(listOf(entry), RetentionPolicy.KEEP_FIRST).single()
@@ -300,6 +300,14 @@ class BackStackConverterTest {
       )
   }
 
+  @Test
+  fun `convert returns unknown name without arguments if mapper returns null`() {
+    val sut = getSut(entryMapper = { null })
+
+    assertThat(sut.convert(HomeScreen()))
+      .isEqualTo(NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME))
+  }
+
   @Test
   fun `convert returns unknown name without arguments if mapper throws`() {
     val sut = getSut(entryMapper = { error("boom") })

sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt

@@ -46,7 +46,7 @@ class BackStackObserverTest {
   private class Fixture {
     private val defaultEntryMapper =
       BackStackEntryMapper<Any> { entry ->
-        SentryBackStackEntry(entry::class.simpleName ?: "unknown")
+        SentryBackStackEntry(entry::class.simpleName ?: "<unknown>")
       }
 
     val logger = mock<ILogger>()

sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/ForwardingBackStackEntryMapperTest.kt

@@ -31,8 +31,15 @@ class ForwardingBackStackEntryMapperTest {
       BackStackEntryMapper<HomeScreen> { SentryBackStackEntry(it.id) }
     }
 
-    assertThat(sut.map(HomeScreen())).isEqualTo(SentryBackStackEntry("home"))
-    assertThat(sut.map(HomeScreen()).arguments).isNull()
+    val mapped = sut.map(HomeScreen())
+    assertThat(mapped).isEqualTo(SentryBackStackEntry("home"))
+    assertThat(mapped?.arguments).isNull()
+  }
+
+  @Test
+  fun `mapper forwards null results`() {
+    val sut = ForwardingBackStackEntryMapper<HomeScreen> { BackStackEntryMapper { null } }
+    assertThat(sut.map(HomeScreen())).isNull()
   }
 
   @Test

sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectTest.kt

@@ -337,7 +337,7 @@ class SentryNavEffectTest {
     val entryMapper =
       mutableStateOf<BackStackEntryMapper<Any>>(
         BackStackEntryMapper { entry ->
-          SentryBackStackEntry(entry::class.simpleName ?: "unknown")
+          SentryBackStackEntry(entry::class.simpleName ?: "<unknown>")
         }
       )