Skip to content

Commit a82ee22

Browse files
0xadam-browncodex
andcommitted
fix(navigation3): Report dropped back stack arguments
Attach a stable reason marker when mapper failures or sanitization limits prevent arguments from being captured. Include the metadata consistently in navigation contexts, breadcrumbs, and transactions. Refs GH-6130 Co-Authored-By: Codex <noreply@openai.com>
1 parent 60d89ff commit a82ee22

5 files changed

Lines changed: 218 additions & 65 deletions

File tree

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

Lines changed: 86 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package io.sentry.compose.navigation3
22

33
import io.sentry.ILogger
44
import io.sentry.SentryLevel.WARNING
5+
import io.sentry.compose.navigation3.ArgumentDropReason.Companion.ARGUMENT_DROP_REASON_KEY
56
import io.sentry.compose.navigation3.NormalizedSentryBackStackEntry.Companion.UNKNOWN_ENTRY_NAME
67
import io.sentry.util.ExceptionUtils
78
import java.util.IdentityHashMap
@@ -69,21 +70,33 @@ internal class BackStackConverter<T : Any>(
6970
// Back stack entry mappers are host app callbacks.
7071
ExceptionUtils.rethrowIfFatal(t)
7172
warningState.logMapperFailureWarning(logger, t)
72-
return NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME)
73+
return NormalizedSentryBackStackEntry(
74+
name = UNKNOWN_ENTRY_NAME,
75+
argumentDropReason = ArgumentDropReason.MAPPING_FAILED,
76+
)
7377
}
7478

7579
if (info == null) {
76-
return NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME)
80+
return NormalizedSentryBackStackEntry(name = UNKNOWN_ENTRY_NAME)
7781
}
7882

79-
val arguments = info.arguments?.let(sanitizer::sanitizeEntry) ?: emptyMap()
83+
val sanitizedArguments =
84+
info.arguments?.let(sanitizer::sanitizeEntry) ?: SanitizedArguments(emptyMap())
8085
val formattedName = NormalizedSentryBackStackEntry.formatName(info.name)
8186

8287
return if (formattedName.isBlank()) {
8388
warningState.logInvalidNameWarning(logger)
84-
NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME, arguments)
89+
NormalizedSentryBackStackEntry(
90+
name = UNKNOWN_ENTRY_NAME,
91+
arguments = sanitizedArguments.values,
92+
argumentDropReason = sanitizedArguments.dropReason,
93+
)
8594
} else {
86-
NormalizedSentryBackStackEntry(formattedName, arguments)
95+
NormalizedSentryBackStackEntry(
96+
name = formattedName,
97+
arguments = sanitizedArguments.values,
98+
argumentDropReason = sanitizedArguments.dropReason,
99+
)
87100
}
88101
}
89102

@@ -109,6 +122,11 @@ internal class BackStackConverter<T : Any>(
109122
KEEP_LAST,
110123
}
111124

125+
internal data class SanitizedArguments(
126+
val values: Map<String, Any?>,
127+
val dropReason: ArgumentDropReason? = null,
128+
)
129+
112130
/**
113131
* Sanitizes a back stack entry's arguments and writes them in a serializable form. It bounds
114132
* depth and total value count, and it rejects cyclic structures.
@@ -125,32 +143,32 @@ internal class BackStackConverter<T : Any>(
125143
private val activeContainers = IdentityHashMap<Any, Unit>()
126144
private var remainingValues = MAX_ARGUMENT_COUNT
127145
private var remainingCharacters = MAX_ARGUMENT_CHARACTERS
128-
private var budgetExhausted = false
146+
private var dropReason: ArgumentDropReason? = null
129147

130148
/**
131149
* Sanitizes one entry's arguments, or returns an empty map to drop them, either because the
132150
* structure is cyclic or too deeply nested (this entry only), or because the shared per-update
133151
* value budget is spent (this entry and every older one).
134152
*/
135153
@Suppress("TooGenericExceptionCaught")
136-
fun sanitizeEntry(raw: Map<String, Any?>): Map<String, Any?> {
137-
if (budgetExhausted) {
138-
return emptyMap()
154+
fun sanitizeEntry(raw: Map<String, Any?>): SanitizedArguments {
155+
dropReason?.let { reason ->
156+
return SanitizedArguments(emptyMap(), reason)
139157
}
140158

141159
return try {
142-
sanitizeMap(raw, depth = 0)
160+
SanitizedArguments(sanitizeMap(raw, depth = 0))
143161
} catch (drop: DropSubtree) {
144-
if (drop.exhaustsBudget) {
145-
budgetExhausted = true
162+
if (drop.reason.exhaustsUpdateBudget) {
163+
dropReason = drop.reason
146164
}
147165
logger.log(WARNING, drop.warning)
148-
emptyMap()
166+
SanitizedArguments(emptyMap(), drop.reason)
149167
} catch (t: Throwable) {
150168
// Extracted maps may invoke host app code while iterating or stringifying values.
151169
ExceptionUtils.rethrowIfFatal(t)
152170
logger.log(WARNING, STRUCTURE_WARNING, t)
153-
emptyMap()
171+
SanitizedArguments(emptyMap(), ArgumentDropReason.SANITIZATION_FAILED)
154172
}
155173
}
156174

@@ -188,19 +206,26 @@ internal class BackStackConverter<T : Any>(
188206
val collection = value?.asSanitizableCollectionOrNull()
189207

190208
return when {
191-
value == null || value is Number || value is Boolean -> value
192-
value is String -> value.also { consumeCharacters(it.length) }
193-
value is CharSequence || value is Char ->
194-
value.toString().also { consumeCharacters(it.length) }
195-
value is Enum<*> -> value.name.also { consumeCharacters(it.length) }
196209
value is Map<*, *> -> sanitizeMap(value, depth)
197210
collection != null -> sanitizeCollection(collection, depth)
211+
else -> sanitizeScalar(value)
212+
}
213+
}
214+
215+
private fun sanitizeScalar(value: Any?): Any? =
216+
when (value) {
217+
null,
218+
is Number,
219+
is Boolean -> value
220+
is String -> value.also { consumeCharacters(it.length) }
221+
is CharSequence,
222+
is Char -> value.toString().also { consumeCharacters(it.length) }
223+
is Enum<*> -> value.name.also { consumeCharacters(it.length) }
198224
else -> {
199225
warningState.logUnsupportedValueWarning(value::class.simpleName, logger)
200226
value.toString().also { consumeCharacters(it.length) }
201227
}
202228
}
203-
}
204229

205230
private fun Any.asSanitizableCollectionOrNull(): Collection<*>? =
206231
when (this) {
@@ -223,23 +248,23 @@ internal class BackStackConverter<T : Any>(
223248
*/
224249
private fun visit(depth: Int) {
225250
if (depth > MAX_ARGUMENT_DEPTH) {
226-
throw DropSubtree(STRUCTURE_WARNING, exhaustsBudget = false)
251+
throw DropSubtree(STRUCTURE_WARNING, ArgumentDropReason.INVALID_STRUCTURE)
227252
}
228253
if (--remainingValues < 0) {
229-
throw DropSubtree(BUDGET_WARNING, exhaustsBudget = true)
254+
throw DropSubtree(MAX_COUNT_WARNING, ArgumentDropReason.MAX_COUNT)
230255
}
231256
}
232257

233258
private fun consumeCharacters(count: Int) {
234259
if (count > remainingCharacters) {
235-
throw DropSubtree(CHARACTER_BUDGET_WARNING, exhaustsBudget = true)
260+
throw DropSubtree(MAX_CHARACTER_WARNING, ArgumentDropReason.CHARACTER_LIMIT)
236261
}
237262
remainingCharacters -= count
238263
}
239264

240265
private fun enter(container: Any) {
241266
if (activeContainers.put(container, Unit) != null) {
242-
throw DropSubtree(STRUCTURE_WARNING, exhaustsBudget = false)
267+
throw DropSubtree(STRUCTURE_WARNING, ArgumentDropReason.INVALID_STRUCTURE)
243268
}
244269
}
245270

@@ -251,11 +276,10 @@ internal class BackStackConverter<T : Any>(
251276
* Control-flow signal to abort sanitization of the current subtree. Internal to
252277
* [ArgumentSanitizer].
253278
*
254-
* [exhaustsBudget] distinguishes an entry-local drop (cycle or over-deep structure) from an
255-
* update-wide one (the shared value budget is spent). Overrides [fillInStackTrace] to skip
256-
* stack-trace capture.
279+
* [reason] distinguishes entry-local drops from update-wide budget exhaustion. Overrides
280+
* [fillInStackTrace] to skip stack-trace capture.
257281
*/
258-
private class DropSubtree(val warning: String, val exhaustsBudget: Boolean) :
282+
private class DropSubtree(val warning: String, val reason: ArgumentDropReason) :
259283
RuntimeException() {
260284
override fun fillInStackTrace(): Throwable = this
261285
}
@@ -293,22 +317,21 @@ internal class BackStackConverter<T : Any>(
293317
*/
294318
private const val MAX_ARGUMENT_COUNT = 200
295319

296-
// TODO ADAM: Add visual indicator when we truncate args.
297320
/**
298321
* Max number of characters visited while sanitizing all entries in a given back stack update.
299322
*
300323
* Caps payload size in the presence of large individual arguments.
301324
*/
302325
private const val MAX_ARGUMENT_CHARACTERS = 4_096
303326

304-
private const val BUDGET_WARNING =
305-
"Nav3 arguments exceeded the maximum total value count for one backstack update. Skipping arguments " +
306-
"for this and older captured entries."
307-
308-
private const val CHARACTER_BUDGET_WARNING =
327+
private const val MAX_CHARACTER_WARNING =
309328
"Nav3 arguments exceeded the maximum total character count for one backstack update. Skipping " +
310329
"arguments for this and older captured entries."
311330

331+
private const val MAX_COUNT_WARNING =
332+
"Nav3 arguments exceeded the maximum total count for one backstack update. Skipping arguments for " +
333+
"this and older captured entries."
334+
312335
private const val STRUCTURE_WARNING =
313336
"Nav3 argument sanitization failed (possibly a cyclic or deeply nested structure). Skipping arguments."
314337
}
@@ -372,6 +395,8 @@ internal data class NormalizedSentryBackStackEntry(
372395
val name: String,
373396
/** Sanitized [SentryBackStackEntry.arguments] (i.e., bounded in size and depth). */
374397
val arguments: Map<String, Any?> = emptyMap(),
398+
/** The reason why the SDK dropped host-provided arguments. */
399+
val argumentDropReason: ArgumentDropReason? = null,
375400
) {
376401

377402
companion object {
@@ -393,6 +418,15 @@ internal data class NormalizedSentryBackStackEntry(
393418
}
394419
}
395420

421+
/** Returns sanitized host arguments together with SDK-owned argument metadata. */
422+
fun argumentsWithMetadata(): Map<String, Any?> {
423+
val reason = argumentDropReason ?: return arguments
424+
return buildMap {
425+
putAll(arguments)
426+
put(ARGUMENT_DROP_REASON_KEY, reason.serializedValue)
427+
}
428+
}
429+
396430
/**
397431
* Returns this entry in serialized form. E.g.:
398432
* ```
@@ -407,11 +441,26 @@ internal data class NormalizedSentryBackStackEntry(
407441
*/
408442
fun serialize(): Map<String, Any?> = buildMap {
409443
put("entry", name)
410-
if (arguments.isNotEmpty()) {
411-
put("arguments", arguments)
412-
}
444+
argumentsWithMetadata().takeIf { it.isNotEmpty() }?.let { put("arguments", it) }
413445
}
414446
}
415447

416448
internal fun List<NormalizedSentryBackStackEntry>.serialize(): List<Map<String, Any?>> =
417449
map(NormalizedSentryBackStackEntry::serialize)
450+
451+
/** Why host-provided arguments were unavailable in emitted navigation data. */
452+
internal enum class ArgumentDropReason(
453+
val serializedValue: String,
454+
val exhaustsUpdateBudget: Boolean = false,
455+
) {
456+
457+
CHARACTER_LIMIT("max_character_limit_exceeded", exhaustsUpdateBudget = true),
458+
INVALID_STRUCTURE("invalid_structure"),
459+
MAPPING_FAILED("mapping_failed"),
460+
MAX_COUNT("max_argument_count_exceeded", exhaustsUpdateBudget = true),
461+
SANITIZATION_FAILED("sanitization_failed");
462+
463+
companion object {
464+
const val ARGUMENT_DROP_REASON_KEY = "arguments_dropped_by_sentry"
465+
}
466+
}

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

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ internal class BackStackObserver<T : Any>(
208208
.start(
209209
scope,
210210
currentTop.name,
211-
currentTop.arguments,
211+
currentTop.argumentsWithMetadata(),
212212
)
213213
?.let { transaction -> navContext.updateTransaction(transaction, scope, currentBackStack) }
214214
} else {
@@ -455,17 +455,23 @@ private class NavBreadcrumbs(private val scopes: IScopes) {
455455
type = NAVIGATION_OP
456456
category = NAVIGATION_OP
457457

458-
fromEntry?.let {
459-
data["from"] = it.name
460-
if (it.arguments.isNotEmpty()) {
461-
data["from_arguments"] = it.arguments
462-
}
458+
fromEntry?.let { entry ->
459+
data["from"] = entry.name
460+
entry
461+
.argumentsWithMetadata()
462+
.takeIf { it.isNotEmpty() }
463+
?.let { arguments ->
464+
data["from_arguments"] = arguments
465+
}
463466
}
464467

465468
data["to"] = toEntry.name
466-
if (toEntry.arguments.isNotEmpty()) {
467-
data["to_arguments"] = toEntry.arguments
468-
}
469+
toEntry
470+
.argumentsWithMetadata()
471+
.takeIf { it.isNotEmpty() }
472+
?.let { arguments ->
473+
data["to_arguments"] = arguments
474+
}
469475

470476
level = INFO
471477
}

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,12 @@ internal fun <T : Any> SentryNavEffect(
108108
)
109109
}
110110

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.
111+
// Intentionally don't remember this copy. Snapshot-backed lists mutate in place, so
112+
// remember(backStack) { backStack.toList() } would cache a stale copy. (The key reference
113+
// retained by remember() and the backStack reference passed to this effect would point to the
114+
// same instance, causing remember() to always return the originally copied list.) Making a fresh
115+
// copy ensures a stable snapshot for the duration of each update and lets BackStackKey compare
116+
// it with the previous one.
113117
val copy = backStack.toList()
114118

115119
DisposableEffect(observer, BackStackKey(copy)) {

0 commit comments

Comments
 (0)