Skip to content

Commit 0111382

Browse files
0xadam-brown0xadam-brown
committed
ref(android-nav3): Refine SentryNavEffect behavior
1 parent 879e2b4 commit 0111382

3 files changed

Lines changed: 142 additions & 47 deletions

File tree

‎sentry-android-navigation3/build.gradle.kts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ dependencies {
6565

6666
compileOnly(libs.androidx.compose.runtime)
6767

68-
testImplementation(libs.androidx.compose.runtime)
6968
testImplementation(libs.androidx.compose.ui.test.junit4)
7069
testImplementation(libs.androidx.test.core)
7170
testImplementation(libs.androidx.test.ext.junit)

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

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import androidx.compose.runtime.rememberUpdatedState
77
import io.sentry.IScopes
88
import io.sentry.ScopesAdapter
99
import io.sentry.SentryOptions
10+
import org.jetbrains.annotations.ApiStatus
1011

1112
/**
1213
* An effect for generating Sentry data from your Nav3 backstack. Configure it via [options] and
@@ -23,9 +24,9 @@ import io.sentry.SentryOptions
2324
* // get attributed to the appropriate nav transaction.
2425
* SentryNavEffect(
2526
* backStack = navBackStack,
26-
* options = SentryNavOptions(maxCapturedBackStackEntries = 10),
2727
* nameExtractor = { route -> route.extractName() },
2828
* argumentsExtractor = { route -> route.extractArgument() },
29+
* options = SentryNavOptions(),
2930
* )
3031
*
3132
* // Configure your NavDisplay like usual.
@@ -63,33 +64,38 @@ import io.sentry.SentryOptions
6364
* gestures. That means, for instance, that spans produced by predictively rendered composables can
6465
* show up under the current destination's transaction.
6566
*
66-
* **Privacy / PII**
67-
*
68-
* Values returned from [nameExtractor] and [argumentsExtractor] are ***not*** scrubbed by the
69-
* Sentry SDK before being sent to Sentry. Only return route names and arguments that are known to
70-
* be safe or have been pre-scrubbed.
71-
*
7267
* @param backStack The navigation backstack to observe.
73-
* @param scopes A scopes instance used to track generated Sentry data.
68+
* @param nameExtractor Extracts a human-readable route name from each entry of the [backStack].
69+
* @param argumentsExtractor Optional extractor for a map of argument name -> argument values from
70+
* each entry of the [backStack]. If not provided, no arguments are attached.
7471
* @param options The kinds of navigation info this effect should record.
75-
* @param nameExtractor Optional lambda to extract a human-readable route name from the top entry of
76-
* the [backStack]. If not provided, defaults to the simple name of the entry's class.
77-
* @param argumentsExtractor Optional lambda to extract a map of argument name -> argument values
78-
* from the top entry of the [backStack]. If not provided, no arguments are attached. The
79-
* following scalar values are supported: [String], [CharSequence], [Char], [Boolean], any
80-
* [Number], enums (via [Enum.name]), and `null`. Supported container values are: [Array]s,
81-
* primitive arrays, [Map]s, and [Collection]s of supported values, including nested containers.
82-
* All other types are stringified via `toString()`. Cyclic or deeply nested containers are
83-
* skipped. Return only the arguments needed for diagnostics and avoid large structures.
8472
*/
73+
@ApiStatus.Experimental
74+
@Composable
75+
@Suppress("FunctionNaming")
76+
internal fun <T : Any> SentryNavEffect(
77+
backStack: List<T>,
78+
nameExtractor: RouteNameExtractor<T>,
79+
argumentsExtractor: RouteArgumentsExtractor<T>? = null,
80+
options: SentryNavOptions = SentryNavOptions(),
81+
) {
82+
SentryNavEffect(
83+
backStack = backStack,
84+
nameExtractor = nameExtractor,
85+
argumentsExtractor = argumentsExtractor,
86+
options = options,
87+
scopes = ScopesAdapter.getInstance(),
88+
)
89+
}
90+
8591
@Composable
8692
@Suppress("FunctionNaming")
8793
internal fun <T : Any> SentryNavEffect(
8894
backStack: List<T>,
89-
scopes: IScopes = ScopesAdapter.getInstance(),
95+
nameExtractor: RouteNameExtractor<T>,
96+
argumentsExtractor: RouteArgumentsExtractor<T>? = null,
9097
options: SentryNavOptions = SentryNavOptions(),
91-
nameExtractor: ((T) -> String)? = null,
92-
argumentsExtractor: ((T) -> Map<String, Any?>)? = null,
98+
scopes: IScopes,
9399
) {
94100
val routeResolvers = rememberUpdatedState(RouteResolvers(nameExtractor, argumentsExtractor))
95101

@@ -102,10 +108,12 @@ internal fun <T : Any> SentryNavEffect(
102108
)
103109
}
104110

105-
val capturedBackStack = backStack.toList()
111+
// The incoming back stack is mutable and shared with the host app; copy it so that BackStackKey
112+
// and BackStackObserver are guaranteed to have the same (stable) view.
113+
val copy = backStack.toList()
106114

107-
DisposableEffect(observer, BackStackKey(capturedBackStack)) {
108-
observer.onBackStackChanged(backStack = capturedBackStack)
115+
DisposableEffect(observer, BackStackKey(copy)) {
116+
observer.onBackStackChanged(backStack = copy)
109117
onDispose {}
110118
}
111119

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

Lines changed: 111 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ import org.robolectric.annotation.Config
3939
@Config(sdk = [30])
4040
class SentryNavEffectTest {
4141

42+
private val defaultNameExtractor =
43+
RouteNameExtractor<Any> { entry -> entry::class.simpleName ?: "unknown" }
44+
4245
@get:Rule(order = 1)
4346
val addActivityToRobolectricRule =
4447
object : TestWatcher() {
@@ -104,7 +107,12 @@ class SentryNavEffectTest {
104107
val backStack = mutableStateListOf<Any>(HomeRoute())
105108

106109
composeRule.setContent {
107-
SentryNavEffect(backStack = backStack, scopes = fixture.scopes)
110+
SentryNavEffect(
111+
backStack = backStack,
112+
nameExtractor = defaultNameExtractor,
113+
options = SentryNavOptions(),
114+
scopes = fixture.scopes,
115+
)
108116
}
109117

110118
composeRule.waitForIdle()
@@ -122,7 +130,12 @@ class SentryNavEffectTest {
122130
val backStack = mutableStateListOf<Any>(HomeRoute())
123131

124132
composeRule.setContent {
125-
SentryNavEffect(backStack = backStack, scopes = fixture.scopes)
133+
SentryNavEffect(
134+
backStack = backStack,
135+
nameExtractor = defaultNameExtractor,
136+
options = SentryNavOptions(),
137+
scopes = fixture.scopes,
138+
)
126139
}
127140
composeRule.waitForIdle()
128141

@@ -144,7 +157,12 @@ class SentryNavEffectTest {
144157
val backStack = mutableStateListOf(HomeRoute(), ProfileRoute("123"))
145158

146159
composeRule.setContent {
147-
SentryNavEffect(backStack = backStack, scopes = fixture.scopes)
160+
SentryNavEffect(
161+
backStack = backStack,
162+
nameExtractor = defaultNameExtractor,
163+
options = SentryNavOptions(),
164+
scopes = fixture.scopes,
165+
)
148166
}
149167
composeRule.waitForIdle()
150168

@@ -166,7 +184,12 @@ class SentryNavEffectTest {
166184
val backStack = mutableStateListOf(HomeRoute(), ProfileRoute("123"))
167185

168186
composeRule.setContent {
169-
SentryNavEffect(backStack = backStack, scopes = fixture.scopes)
187+
SentryNavEffect(
188+
backStack = backStack,
189+
nameExtractor = defaultNameExtractor,
190+
options = SentryNavOptions(),
191+
scopes = fixture.scopes,
192+
)
170193
}
171194
composeRule.waitForIdle()
172195

@@ -194,7 +217,12 @@ class SentryNavEffectTest {
194217
val backStack = mutableStateListOf<Any>(home, profile)
195218

196219
composeRule.setContent {
197-
SentryNavEffect(backStack = backStack, scopes = fixture.scopes)
220+
SentryNavEffect(
221+
backStack = backStack,
222+
nameExtractor = defaultNameExtractor,
223+
options = SentryNavOptions(),
224+
scopes = fixture.scopes,
225+
)
198226
}
199227
composeRule.waitForIdle()
200228

@@ -216,6 +244,27 @@ class SentryNavEffectTest {
216244
)
217245
}
218246

247+
@Test
248+
fun `changing back stack with capture limit of 0 still emits top-entry Sentry data but not back stack copy`() {
249+
val fixture = Fixture()
250+
val backStack = mutableStateListOf(HomeRoute(), ProfileRoute("123"))
251+
252+
composeRule.setContent {
253+
SentryNavEffect(
254+
backStack = backStack,
255+
nameExtractor = defaultNameExtractor,
256+
options = SentryNavOptions { maxCapturedBackStackEntries = 0 },
257+
scopes = fixture.scopes,
258+
)
259+
}
260+
composeRule.waitForIdle()
261+
262+
assertThat(fixture.breadcrumbs.single().data["to"]).isEqualTo("/ProfileRoute")
263+
assertThat(fixture.transactions.single().name).isEqualTo("/ProfileRoute")
264+
assertThat(fixture.scope.screen).isEqualTo("/ProfileRoute")
265+
assertThat(fixture.scope.contexts.containsKey(NAVIGATION_CONTEXT_KEY)).isFalse()
266+
}
267+
219268
@Test
220269
fun `unrelated recomposition does not re-emit Sentry data`() {
221270
val fixture = Fixture()
@@ -224,7 +273,12 @@ class SentryNavEffectTest {
224273

225274
composeRule.setContent {
226275
recomposeTick.intValue
227-
SentryNavEffect(backStack = backStack, scopes = fixture.scopes)
276+
SentryNavEffect(
277+
backStack = backStack,
278+
nameExtractor = defaultNameExtractor,
279+
options = SentryNavOptions(),
280+
scopes = fixture.scopes,
281+
)
228282
}
229283
composeRule.waitForIdle()
230284

@@ -255,7 +309,12 @@ class SentryNavEffectTest {
255309
val observedTransactionNames = mutableListOf<String>()
256310

257311
composeRule.setContent {
258-
SentryNavEffect(backStack = backStack, scopes = fixture.scopes)
312+
SentryNavEffect(
313+
backStack = backStack,
314+
nameExtractor = defaultNameExtractor,
315+
options = SentryNavOptions(),
316+
scopes = fixture.scopes,
317+
)
259318

260319
val currentTop = backStack.last()
261320
LaunchedEffect(currentTop) {
@@ -275,19 +334,23 @@ class SentryNavEffectTest {
275334
fun `updated name extractor is used for later navigation changes`() {
276335
val fixture = Fixture()
277336
val backStack = mutableStateListOf<Any>(HomeRoute())
278-
val nameExtractor = mutableStateOf<((Any) -> String)?>(null)
337+
val nameExtractor =
338+
mutableStateOf<RouteNameExtractor<Any>>(
339+
RouteNameExtractor { entry -> entry::class.simpleName ?: "unknown" }
340+
)
279341

280342
composeRule.setContent {
281343
SentryNavEffect(
282344
backStack = backStack,
283-
scopes = fixture.scopes,
284345
nameExtractor = nameExtractor.value,
346+
options = SentryNavOptions(),
347+
scopes = fixture.scopes,
285348
)
286349
}
287350
composeRule.waitForIdle()
288351

289352
composeRule.runOnIdle {
290-
nameExtractor.value = { entry ->
353+
nameExtractor.value = RouteNameExtractor { entry ->
291354
if (entry is ProfileRoute) "profile-updated" else "home-updated"
292355
}
293356
}
@@ -306,19 +369,23 @@ class SentryNavEffectTest {
306369
fun `changing the name extractor alone does not re-emit Sentry data for the current top entry`() {
307370
val fixture = Fixture()
308371
val backStack = mutableStateListOf<Any>(HomeRoute(), ProfileRoute("123"))
309-
val nameExtractor = mutableStateOf<((Any) -> String)?>(null)
372+
val nameExtractor =
373+
mutableStateOf<RouteNameExtractor<Any>>(
374+
RouteNameExtractor { entry -> entry::class.simpleName ?: "unknown" }
375+
)
310376

311377
composeRule.setContent {
312378
SentryNavEffect(
313379
backStack = backStack,
314-
scopes = fixture.scopes,
315380
nameExtractor = nameExtractor.value,
381+
options = SentryNavOptions(),
382+
scopes = fixture.scopes,
316383
)
317384
}
318385
composeRule.waitForIdle()
319386

320387
composeRule.runOnIdle {
321-
nameExtractor.value = { entry ->
388+
nameExtractor.value = RouteNameExtractor { entry ->
322389
if (entry is ProfileRoute) "profile-updated" else "home-updated"
323390
}
324391
}
@@ -337,19 +404,21 @@ class SentryNavEffectTest {
337404
fun `updated arguments extractor is used for later navigation changes`() {
338405
val fixture = Fixture()
339406
val backStack = mutableStateListOf<Any>(HomeRoute())
340-
val argumentsExtractor = mutableStateOf<((Any) -> Map<String, Any?>)?>(null)
407+
val argumentsExtractor = mutableStateOf<RouteArgumentsExtractor<Any>?>(null)
341408

342409
composeRule.setContent {
343410
SentryNavEffect(
344411
backStack = backStack,
345-
scopes = fixture.scopes,
412+
nameExtractor = defaultNameExtractor,
346413
argumentsExtractor = argumentsExtractor.value,
414+
options = SentryNavOptions(),
415+
scopes = fixture.scopes,
347416
)
348417
}
349418
composeRule.waitForIdle()
350419

351420
composeRule.runOnIdle {
352-
argumentsExtractor.value = { entry ->
421+
argumentsExtractor.value = RouteArgumentsExtractor { entry ->
353422
if (entry is ProfileRoute) mapOf("userId" to entry.userId) else emptyMap()
354423
}
355424
}
@@ -375,19 +444,21 @@ class SentryNavEffectTest {
375444
fun `changing the arguments extractor alone does not re-emit Sentry data for the current top entry`() {
376445
val fixture = Fixture()
377446
val backStack = mutableStateListOf<Any>(HomeRoute(), ProfileRoute("123"))
378-
val argumentsExtractor = mutableStateOf<((Any) -> Map<String, Any?>)?>(null)
447+
val argumentsExtractor = mutableStateOf<RouteArgumentsExtractor<Any>?>(null)
379448

380449
composeRule.setContent {
381450
SentryNavEffect(
382451
backStack = backStack,
383-
scopes = fixture.scopes,
452+
nameExtractor = defaultNameExtractor,
384453
argumentsExtractor = argumentsExtractor.value,
454+
options = SentryNavOptions(),
455+
scopes = fixture.scopes,
385456
)
386457
}
387458
composeRule.waitForIdle()
388459

389460
composeRule.runOnIdle {
390-
argumentsExtractor.value = { entry ->
461+
argumentsExtractor.value = RouteArgumentsExtractor { entry ->
391462
if (entry is ProfileRoute) mapOf("userId" to entry.userId) else emptyMap()
392463
}
393464
}
@@ -408,16 +479,29 @@ class SentryNavEffectTest {
408479
fun `changing options applies the new observer configuration`() {
409480
val fixture = Fixture()
410481
val backStack = mutableStateListOf<Any>(HomeRoute())
411-
val options = mutableStateOf(SentryNavOptions(captureBackStack = true))
482+
val options = mutableStateOf(SentryNavOptions { captureBackStack = true })
412483

413484
composeRule.setContent {
414-
SentryNavEffect(backStack = backStack, scopes = fixture.scopes, options = options.value)
485+
SentryNavEffect(
486+
backStack = backStack,
487+
options = options.value,
488+
nameExtractor = defaultNameExtractor,
489+
scopes = fixture.scopes,
490+
)
415491
}
416492
composeRule.waitForIdle()
417493

418-
composeRule.runOnIdle { options.value = SentryNavOptions(captureBackStack = false) }
494+
val originalTransaction = composeRule.runOnIdle { fixture.transactions.single() }
495+
496+
composeRule.runOnIdle { options.value = SentryNavOptions { captureBackStack = false } }
419497
composeRule.waitForIdle()
420498

499+
assertThat(originalTransaction.isFinished).isTrue()
500+
assertThat(fixture.transactions).hasSize(2)
501+
assertThat(fixture.transactions.last().name).isEqualTo("/HomeRoute")
502+
assertThat(fixture.transactions.last().isFinished).isFalse()
503+
assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.transactions.last())
504+
assertThat(fixture.scope.screen).isEqualTo("/HomeRoute")
421505
assertThat(fixture.scope.contexts.containsKey("navigation")).isFalse()
422506
}
423507

@@ -429,7 +513,11 @@ class SentryNavEffectTest {
429513

430514
composeRule.setContent {
431515
if (isShown.value) {
432-
SentryNavEffect(backStack = backStack, scopes = fixture.scopes)
516+
SentryNavEffect(
517+
backStack = backStack,
518+
nameExtractor = defaultNameExtractor,
519+
scopes = fixture.scopes,
520+
)
433521
}
434522
}
435523
composeRule.waitForIdle()

0 commit comments

Comments
 (0)