Skip to content

Commit 44633fa

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 44633fa

5 files changed

Lines changed: 218 additions & 62 deletions

File tree

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

Lines changed: 80 additions & 33 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,36 @@ 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(
81+
name = UNKNOWN_ENTRY_NAME,
82+
argumentDropReason = ArgumentDropReason.MAPPING_UNAVAILABLE,
83+
)
7784
}
7885

79-
val arguments = info.arguments?.let(sanitizer::sanitizeEntry) ?: emptyMap()
86+
val sanitizedArguments =
87+
info.arguments?.let(sanitizer::sanitizeEntry) ?: SanitizedArguments(emptyMap())
8088
val formattedName = NormalizedSentryBackStackEntry.formatName(info.name)
8189

8290
return if (formattedName.isBlank()) {
8391
warningState.logInvalidNameWarning(logger)
84-
NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME, arguments)
92+
NormalizedSentryBackStackEntry(
93+
name = UNKNOWN_ENTRY_NAME,
94+
arguments = sanitizedArguments.values,
95+
argumentDropReason = sanitizedArguments.dropReason,
96+
)
8597
} else {
86-
NormalizedSentryBackStackEntry(formattedName, arguments)
98+
NormalizedSentryBackStackEntry(
99+
name = formattedName,
100+
arguments = sanitizedArguments.values,
101+
argumentDropReason = sanitizedArguments.dropReason,
102+
)
87103
}
88104
}
89105

@@ -109,6 +125,11 @@ internal class BackStackConverter<T : Any>(
109125
KEEP_LAST,
110126
}
111127

128+
internal data class SanitizedArguments(
129+
val values: Map<String, Any?>,
130+
val dropReason: ArgumentDropReason? = null,
131+
)
132+
112133
/**
113134
* Sanitizes a back stack entry's arguments and writes them in a serializable form. It bounds
114135
* depth and total value count, and it rejects cyclic structures.
@@ -125,32 +146,32 @@ internal class BackStackConverter<T : Any>(
125146
private val activeContainers = IdentityHashMap<Any, Unit>()
126147
private var remainingValues = MAX_ARGUMENT_COUNT
127148
private var remainingCharacters = MAX_ARGUMENT_CHARACTERS
128-
private var budgetExhausted = false
149+
private var dropReason: ArgumentDropReason? = null
129150

130151
/**
131152
* Sanitizes one entry's arguments, or returns an empty map to drop them, either because the
132153
* structure is cyclic or too deeply nested (this entry only), or because the shared per-update
133154
* value budget is spent (this entry and every older one).
134155
*/
135156
@Suppress("TooGenericExceptionCaught")
136-
fun sanitizeEntry(raw: Map<String, Any?>): Map<String, Any?> {
137-
if (budgetExhausted) {
138-
return emptyMap()
157+
fun sanitizeEntry(raw: Map<String, Any?>): SanitizedArguments {
158+
dropReason?.let { reason ->
159+
return SanitizedArguments(emptyMap(), reason)
139160
}
140161

141162
return try {
142-
sanitizeMap(raw, depth = 0)
163+
SanitizedArguments(sanitizeMap(raw, depth = 0))
143164
} catch (drop: DropSubtree) {
144-
if (drop.exhaustsBudget) {
145-
budgetExhausted = true
165+
if (drop.reason.exhaustsUpdateBudget) {
166+
dropReason = drop.reason
146167
}
147168
logger.log(WARNING, drop.warning)
148-
emptyMap()
169+
SanitizedArguments(emptyMap(), drop.reason)
149170
} catch (t: Throwable) {
150171
// Extracted maps may invoke host app code while iterating or stringifying values.
151172
ExceptionUtils.rethrowIfFatal(t)
152173
logger.log(WARNING, STRUCTURE_WARNING, t)
153-
emptyMap()
174+
SanitizedArguments(emptyMap(), ArgumentDropReason.SANITIZATION_FAILED)
154175
}
155176
}
156177

@@ -196,8 +217,9 @@ internal class BackStackConverter<T : Any>(
196217
value is Map<*, *> -> sanitizeMap(value, depth)
197218
collection != null -> sanitizeCollection(collection, depth)
198219
else -> {
199-
warningState.logUnsupportedValueWarning(value::class.simpleName, logger)
200-
value.toString().also { consumeCharacters(it.length) }
220+
val unsupportedValue = checkNotNull(value)
221+
warningState.logUnsupportedValueWarning(unsupportedValue::class.simpleName, logger)
222+
unsupportedValue.toString().also { consumeCharacters(it.length) }
201223
}
202224
}
203225
}
@@ -223,23 +245,23 @@ internal class BackStackConverter<T : Any>(
223245
*/
224246
private fun visit(depth: Int) {
225247
if (depth > MAX_ARGUMENT_DEPTH) {
226-
throw DropSubtree(STRUCTURE_WARNING, exhaustsBudget = false)
248+
throw DropSubtree(STRUCTURE_WARNING, ArgumentDropReason.INVALID_STRUCTURE)
227249
}
228250
if (--remainingValues < 0) {
229-
throw DropSubtree(BUDGET_WARNING, exhaustsBudget = true)
251+
throw DropSubtree(MAX_COUNT_WARNING, ArgumentDropReason.MAX_COUNT)
230252
}
231253
}
232254

233255
private fun consumeCharacters(count: Int) {
234256
if (count > remainingCharacters) {
235-
throw DropSubtree(CHARACTER_BUDGET_WARNING, exhaustsBudget = true)
257+
throw DropSubtree(MAX_CHARACTER_WARNING, ArgumentDropReason.CHARACTER_LIMIT)
236258
}
237259
remainingCharacters -= count
238260
}
239261

240262
private fun enter(container: Any) {
241263
if (activeContainers.put(container, Unit) != null) {
242-
throw DropSubtree(STRUCTURE_WARNING, exhaustsBudget = false)
264+
throw DropSubtree(STRUCTURE_WARNING, ArgumentDropReason.INVALID_STRUCTURE)
243265
}
244266
}
245267

@@ -251,11 +273,10 @@ internal class BackStackConverter<T : Any>(
251273
* Control-flow signal to abort sanitization of the current subtree. Internal to
252274
* [ArgumentSanitizer].
253275
*
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.
276+
* [reason] distinguishes entry-local drops from update-wide budget exhaustion. Overrides
277+
* [fillInStackTrace] to skip stack-trace capture.
257278
*/
258-
private class DropSubtree(val warning: String, val exhaustsBudget: Boolean) :
279+
private class DropSubtree(val warning: String, val reason: ArgumentDropReason) :
259280
RuntimeException() {
260281
override fun fillInStackTrace(): Throwable = this
261282
}
@@ -293,22 +314,21 @@ internal class BackStackConverter<T : Any>(
293314
*/
294315
private const val MAX_ARGUMENT_COUNT = 200
295316

296-
// TODO ADAM: Add visual indicator when we truncate args.
297317
/**
298318
* Max number of characters visited while sanitizing all entries in a given back stack update.
299319
*
300320
* Caps payload size in the presence of large individual arguments.
301321
*/
302322
private const val MAX_ARGUMENT_CHARACTERS = 4_096
303323

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 =
324+
private const val MAX_CHARACTER_WARNING =
309325
"Nav3 arguments exceeded the maximum total character count for one backstack update. Skipping " +
310326
"arguments for this and older captured entries."
311327

328+
private const val MAX_COUNT_WARNING =
329+
"Nav3 arguments exceeded the maximum total count for one backstack update. Skipping arguments for " +
330+
"this and older captured entries."
331+
312332
private const val STRUCTURE_WARNING =
313333
"Nav3 argument sanitization failed (possibly a cyclic or deeply nested structure). Skipping arguments."
314334
}
@@ -372,6 +392,8 @@ internal data class NormalizedSentryBackStackEntry(
372392
val name: String,
373393
/** Sanitized [SentryBackStackEntry.arguments] (i.e., bounded in size and depth). */
374394
val arguments: Map<String, Any?> = emptyMap(),
395+
/** The reason why the SDK dropped host-provided arguments. */
396+
val argumentDropReason: ArgumentDropReason? = null,
375397
) {
376398

377399
companion object {
@@ -393,6 +415,15 @@ internal data class NormalizedSentryBackStackEntry(
393415
}
394416
}
395417

418+
/** Returns sanitized host arguments together with SDK-owned argument metadata. */
419+
fun argumentsWithMetadata(): Map<String, Any?> {
420+
val reason = argumentDropReason ?: return arguments
421+
return buildMap {
422+
putAll(arguments)
423+
put(ARGUMENT_DROP_REASON_KEY, reason.serializedValue)
424+
}
425+
}
426+
396427
/**
397428
* Returns this entry in serialized form. E.g.:
398429
* ```
@@ -407,11 +438,27 @@ internal data class NormalizedSentryBackStackEntry(
407438
*/
408439
fun serialize(): Map<String, Any?> = buildMap {
409440
put("entry", name)
410-
if (arguments.isNotEmpty()) {
411-
put("arguments", arguments)
412-
}
441+
argumentsWithMetadata().takeIf { it.isNotEmpty() }?.let { put("arguments", it) }
413442
}
414443
}
415444

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

‎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)