Skip to content

Commit 1b00195

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 1b00195

5 files changed

Lines changed: 208 additions & 61 deletions

File tree

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

Lines changed: 76 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,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

@@ -196,8 +214,9 @@ internal class BackStackConverter<T : Any>(
196214
value is Map<*, *> -> sanitizeMap(value, depth)
197215
collection != null -> sanitizeCollection(collection, depth)
198216
else -> {
199-
warningState.logUnsupportedValueWarning(value::class.simpleName, logger)
200-
value.toString().also { consumeCharacters(it.length) }
217+
val unsupportedValue = checkNotNull(value)
218+
warningState.logUnsupportedValueWarning(unsupportedValue::class.simpleName, logger)
219+
unsupportedValue.toString().also { consumeCharacters(it.length) }
201220
}
202221
}
203222
}
@@ -223,23 +242,23 @@ internal class BackStackConverter<T : Any>(
223242
*/
224243
private fun visit(depth: Int) {
225244
if (depth > MAX_ARGUMENT_DEPTH) {
226-
throw DropSubtree(STRUCTURE_WARNING, exhaustsBudget = false)
245+
throw DropSubtree(STRUCTURE_WARNING, ArgumentDropReason.INVALID_STRUCTURE)
227246
}
228247
if (--remainingValues < 0) {
229-
throw DropSubtree(BUDGET_WARNING, exhaustsBudget = true)
248+
throw DropSubtree(MAX_COUNT_WARNING, ArgumentDropReason.MAX_COUNT)
230249
}
231250
}
232251

233252
private fun consumeCharacters(count: Int) {
234253
if (count > remainingCharacters) {
235-
throw DropSubtree(CHARACTER_BUDGET_WARNING, exhaustsBudget = true)
254+
throw DropSubtree(MAX_CHARACTER_WARNING, ArgumentDropReason.CHARACTER_LIMIT)
236255
}
237256
remainingCharacters -= count
238257
}
239258

240259
private fun enter(container: Any) {
241260
if (activeContainers.put(container, Unit) != null) {
242-
throw DropSubtree(STRUCTURE_WARNING, exhaustsBudget = false)
261+
throw DropSubtree(STRUCTURE_WARNING, ArgumentDropReason.INVALID_STRUCTURE)
243262
}
244263
}
245264

@@ -251,11 +270,10 @@ internal class BackStackConverter<T : Any>(
251270
* Control-flow signal to abort sanitization of the current subtree. Internal to
252271
* [ArgumentSanitizer].
253272
*
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.
273+
* [reason] distinguishes entry-local drops from update-wide budget exhaustion. Overrides
274+
* [fillInStackTrace] to skip stack-trace capture.
257275
*/
258-
private class DropSubtree(val warning: String, val exhaustsBudget: Boolean) :
276+
private class DropSubtree(val warning: String, val reason: ArgumentDropReason) :
259277
RuntimeException() {
260278
override fun fillInStackTrace(): Throwable = this
261279
}
@@ -293,22 +311,21 @@ internal class BackStackConverter<T : Any>(
293311
*/
294312
private const val MAX_ARGUMENT_COUNT = 200
295313

296-
// TODO ADAM: Add visual indicator when we truncate args.
297314
/**
298315
* Max number of characters visited while sanitizing all entries in a given back stack update.
299316
*
300317
* Caps payload size in the presence of large individual arguments.
301318
*/
302319
private const val MAX_ARGUMENT_CHARACTERS = 4_096
303320

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

325+
private const val MAX_COUNT_WARNING =
326+
"Nav3 arguments exceeded the maximum total count for one backstack update. Skipping arguments for " +
327+
"this and older captured entries."
328+
312329
private const val STRUCTURE_WARNING =
313330
"Nav3 argument sanitization failed (possibly a cyclic or deeply nested structure). Skipping arguments."
314331
}
@@ -372,6 +389,8 @@ internal data class NormalizedSentryBackStackEntry(
372389
val name: String,
373390
/** Sanitized [SentryBackStackEntry.arguments] (i.e., bounded in size and depth). */
374391
val arguments: Map<String, Any?> = emptyMap(),
392+
/** The reason why the SDK dropped host-provided arguments. */
393+
val argumentDropReason: ArgumentDropReason? = null,
375394
) {
376395

377396
companion object {
@@ -393,6 +412,15 @@ internal data class NormalizedSentryBackStackEntry(
393412
}
394413
}
395414

415+
/** Returns sanitized host arguments together with SDK-owned argument metadata. */
416+
fun argumentsWithMetadata(): Map<String, Any?> {
417+
val reason = argumentDropReason ?: return arguments
418+
return buildMap {
419+
putAll(arguments)
420+
put(ARGUMENT_DROP_REASON_KEY, reason.serializedValue)
421+
}
422+
}
423+
396424
/**
397425
* Returns this entry in serialized form. E.g.:
398426
* ```
@@ -407,11 +435,26 @@ internal data class NormalizedSentryBackStackEntry(
407435
*/
408436
fun serialize(): Map<String, Any?> = buildMap {
409437
put("entry", name)
410-
if (arguments.isNotEmpty()) {
411-
put("arguments", arguments)
412-
}
438+
argumentsWithMetadata().takeIf { it.isNotEmpty() }?.let { put("arguments", it) }
413439
}
414440
}
415441

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

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