Skip to content

Commit 867786b

Browse files
0xadam-brownclaude
andcommitted
fix(android): Defer Nav3 transactions requested without the nav lease
A SentryNavEffect that navigates without the nav lease got no nav transaction for that destination. That covers a preloaded ViewPager2 page that's swiped to, and an owner that navigates while covered by another Activity or Fragment (e.g., a delayed push or a result callback) and is then returned to. Such a request is now staged and starts when the owner next acquires the lease, with the arguments, view names and nav context recorded in the meantime. The acquisition is its only chance to start: if another transaction is bound or the scopes decline it then, it's discarded rather than kept for a later acquisition. Swiping back to a page whose transaction has finished still starts none. The sample tags transactions again on resume, as a page can now get its transaction without composing. Refs JAVA-785 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 509777e commit 867786b

15 files changed

Lines changed: 1217 additions & 67 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
### Fixes
66

7+
- Fix missing Navigation 3 transactions for preloaded ViewPager2 pages and destinations reached while another Activity or Fragment owns navigation tracking ([#6400](https://github.com/getsentry/sentry-java/pull/6400))
78
- Keep UI profiling running when `startProfiler` is called before a requested stop takes effect at the end of the current chunk ([#6291](https://github.com/getsentry/sentry-java/pull/6291))
89
- Before, the start call was ignored and the profiler still stopped at the end of the chunk, even in `TRACE` mode while sampled spans were active. On API 34 and lower, every later profile also stopped after one chunk.
910

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,17 @@ import io.sentry.IScopes
133133
* Finish txB if unfinished │
134134
* ```
135135
*
136+
* **Nav transactions follow navigation, not the lease**
137+
*
138+
* A candidate gets a nav transaction for a destination it navigates to, never merely for acquiring
139+
* the lease. A destination reached without the lease gets its transaction when the lease is next
140+
* acquired, if it can start then; otherwise it gets none. On reacquiring the lease without having
141+
* navigated in the meantime, a candidate only gets its unfinished transaction back, if any.
142+
*
143+
* Hence the asymmetry for, e.g., ViewPager2 pages: the first time the user swipes to a preloaded
144+
* page, it gets the transaction for the destination it reached while offscreen, but swiping back to
145+
* a page whose transaction has finished starts none.
146+
*
136147
* **Exception policy**
137148
*
138149
* Methods in this class with an `IScope` parameter can throw arbitrary exceptions†, which propagate

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

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,11 @@ internal const val NAVIGATION_CONTEXT_KEY: String = "Navigation"
4343
* and publish their navigation data to the shared `IScope`. (The lease policy is implemented by
4444
* [NavLeaseCoordinator].)
4545
*
46-
* `NavScopes` instances without a lease write to their own copy of [stagedNavData], and keep their
47-
* unfinished nav transaction in [stagedNavTransaction], which lets them preserve their latest
48-
* updates without overwriting another instance's transaction or published data. When an instance
49-
* acquires the lease, it writes its staged data to the shared `IScope` and binds its unfinished
50-
* transaction again (subject to any publishing conditions, e.g., the transaction stays unbound if
51-
* another transaction is already bound to the shared `IScope`).
46+
* `NavScopes` instances without a lease write to their own copy of [stagedNavTransaction] and
47+
* [stagedNavData], which lets them preserve their latest updates without overwriting another
48+
* instance's transaction or published data. When an instance acquires the lease, it writes its
49+
* staged transaction and data to the shared `IScope` (subject to any publishing conditions, e.g.,
50+
* staged transactions are dropped if another transaction is already bound to the shared `IScope`).
5251
*
5352
* **Usage**
5453
*
@@ -151,6 +150,8 @@ internal class NavScopes(
151150
private inner class NavStatePublisher : LeasedNavState {
152151

153152
override fun publishTo(scope: IScope) {
153+
// Publishing the data first ensures a staged transaction is sampled against the rotated
154+
// propagation context when it starts.
154155
stagedNavData.publishTo(scope)
155156
stagedNavTransaction.publishTo(scope)
156157
}
@@ -219,13 +220,17 @@ internal class NavScopes(
219220
}
220221

221222
/**
222-
* Starts and binds a nav transaction with the nav lease (see [StagedNavTransaction]). Callers
223-
* must [finish][finishNavTransaction] the previous nav transaction first.
223+
* Starts and binds a nav transaction with the nav lease, or defers it until the lease is
224+
* acquired (see [StagedNavTransaction]). Callers must [finish][finishNavTransaction] the
225+
* previous nav transaction first.
224226
*
225-
* Without the lease, starts none and defers the propagation context rotation a new transaction
226-
* would have implied until the lease is acquired. A transaction started without the lease
227-
* couldn't be bound to the scope, so its destination's spans would attach to the leaseholder's
228-
* bound transaction instead, and the empty transaction would be dropped.
227+
* **Deferred transactions may mis-parent spans generated before lease acquisition**
228+
*
229+
* A deferred transaction starts when the lease is acquired, which may be later than when its
230+
* destination starts doing work (e.g., ViewPager2 can precompose offscreen pages, or an
231+
* Activity that navigates from a result callback before it resumes). Spans created in the
232+
* meantime attach to whichever transaction is bound to the scope at that time, usually the
233+
* leaseholder's.
229234
*/
230235
override fun startNavTransaction(
231236
transactionContext: TransactionContext,
@@ -235,7 +240,7 @@ internal class NavScopes(
235240
stagedNavTransaction.start(delegate, transactionContext, transactionOptions)
236241
} else {
237242
stagedNavData.deferPropagationContextRotation()
238-
null
243+
stagedNavTransaction.stage(transactionContext, transactionOptions)
239244
}
240245
}
241246

@@ -303,19 +308,26 @@ internal sealed interface INavScope {
303308

304309
/**
305310
* Starts a nav transaction and binds it to the scope, unless another unfinished transaction is
306-
* already bound to it or the nav lease isn't held. Returns `null` if no transaction was started.
311+
* already bound to it. Returns `null` if no transaction was started.
307312
*
308313
* (Note: Callers must first [finish][finishNavTransaction] the nav transaction most recently
309-
* started, if any. Otherwise, it blocks the new one while it's bound.)
314+
* requested, if any. Otherwise, with the nav lease, it blocks the new one while it's bound;
315+
* without the lease, it's replaced without being finished.)
316+
*
317+
* The start may be deferred until the nav lease is acquired, in which case writes to the returned
318+
* transaction are staged in the meantime, and whether another transaction blocks it is decided
319+
* then (see [StagedNavTransaction.publishTo]).
310320
*/
311321
fun startNavTransaction(
312322
transactionContext: TransactionContext,
313323
transactionOptions: TransactionOptions,
314324
): INavTransaction?
315325

316326
/**
317-
* Finishes the nav transaction most recently started via [startNavTransaction] and unbinds it
327+
* Finishes the nav transaction most recently requested via [startNavTransaction] and unbinds it
318328
* from the shared [IScope].
329+
*
330+
* Alternatively, discards the most recently requested transaction if it's still deferred.
319331
*/
320332
fun finishNavTransaction()
321333
}

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

Lines changed: 90 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,51 +1,129 @@
11
package io.sentry.compose.navigation3.lifecycle
22

3+
import io.sentry.IScopes
34
import io.sentry.ITransaction
45
import io.sentry.SpanStatus
6+
import io.sentry.TransactionContext
7+
import io.sentry.TransactionOptions
58
import io.sentry.protocol.App
69

710
/**
8-
* An [INavTransaction] handle backed by a started [ITransaction].
11+
* An [INavTransaction] handle backed by an [ITransaction] that's either started right away or, when
12+
* the nav lease isn't held, [started later][start].
13+
*
14+
* A deferred handle records what's set on it and replays it onto the transaction once started.
15+
* [Finishing][finish] a deferred handle before it starts discards it, so its transaction never
16+
* starts.
917
*
1018
* **Thread safety**
1119
*
1220
* This class is ***not*** thread-safe. Apply the thread safety policy from [NavScopes].
1321
*/
1422
internal class NavTransaction
1523
private constructor(
16-
/** The backing transaction. */
17-
val transaction: ITransaction
24+
val name: String,
25+
private var state: State,
1826
) : INavTransaction {
1927

20-
val name: String
21-
get() = transaction.name
28+
/**
29+
* The backing transaction, once [started][State.Started]; `null` while [pending][State.Pending]
30+
* or once [discarded][State.Discarded].
31+
*/
32+
val transaction: ITransaction?
33+
get() = (state as? State.Started)?.transaction
2234

2335
override fun setData(key: String, value: Any) {
24-
transaction.setData(key, value)
36+
when (val state = state) {
37+
is State.Pending -> state.data[key] = value
38+
is State.Started -> state.transaction.setData(key, value)
39+
State.Discarded -> Unit
40+
}
2541
}
2642

2743
override fun setContext(key: String, value: Any) {
28-
transaction.setContext(key, value)
44+
when (val state = state) {
45+
is State.Pending -> state.contexts[key] = value
46+
is State.Started -> state.transaction.setContext(key, value)
47+
State.Discarded -> Unit
48+
}
2949
}
3050

3151
override fun setApp(app: App) {
32-
transaction.contexts.setApp(app)
52+
when (val state = state) {
53+
is State.Pending -> state.app = app
54+
is State.Started -> state.transaction.contexts.setApp(app)
55+
State.Discarded -> Unit
56+
}
57+
}
58+
59+
/**
60+
* Starts a deferred handle's transaction via [scopes] and replays what was set on the handle onto
61+
* it.
62+
*
63+
* Returns the started transaction, or `null` if the handle was started already or discarded, or
64+
* [scopes] declined to start one.
65+
*/
66+
fun start(scopes: IScopes): ITransaction? {
67+
val pending = state as? State.Pending ?: return null
68+
val transaction =
69+
scopes.startTransaction(pending.context, pending.options).takeUnless { it.isNoOp }
70+
?: return null
71+
72+
state = State.Started(transaction)
73+
pending.data.forEach { (key, value) -> transaction.setData(key, value) }
74+
pending.contexts.forEach { (key, value) -> transaction.setContext(key, value) }
75+
pending.app?.let { transaction.contexts.setApp(it) }
76+
return transaction
3377
}
3478

3579
/**
3680
* Finishes the backing transaction with its own status, or [OK][SpanStatus.OK] if it has none,
3781
* and then passes it to [onFinished].
82+
*
83+
* A deferred handle that hasn't started is discarded instead, so it never starts, and
84+
* [onFinished] isn't called.
3885
*/
3986
fun finish(onFinished: (ITransaction) -> Unit) {
40-
if (!transaction.isFinished) {
41-
transaction.finish(transaction.status ?: SpanStatus.OK)
87+
when (val state = state) {
88+
is State.Pending -> this.state = State.Discarded
89+
is State.Started -> {
90+
if (!state.transaction.isFinished) {
91+
state.transaction.finish(state.transaction.status ?: SpanStatus.OK)
92+
}
93+
onFinished(state.transaction)
94+
}
95+
State.Discarded -> Unit
4296
}
43-
onFinished(transaction)
97+
}
98+
99+
/** Lifecycle state for a [NavTransaction] handle. */
100+
private sealed interface State {
101+
102+
/**
103+
* Not started yet: the transaction will be started from [context] and [options], and what's set
104+
* on the handle until then is recorded for replay.
105+
*/
106+
class Pending(val context: TransactionContext, val options: TransactionOptions) : State {
107+
val data = linkedMapOf<String, Any>()
108+
val contexts = linkedMapOf<String, Any>()
109+
var app: App? = null
110+
}
111+
112+
/** The backing [transaction] has been started. A terminal [State]. */
113+
class Started(val transaction: ITransaction) : State
114+
115+
/** The handle was finished before its backing transaction was started. A terminal [State]. */
116+
data object Discarded : State
44117
}
45118

46119
companion object {
47120
/** A handle backed by the already-started [transaction]. */
48-
fun started(transaction: ITransaction): NavTransaction = NavTransaction(transaction)
121+
fun started(transaction: ITransaction): NavTransaction =
122+
NavTransaction(transaction.name, State.Started(transaction))
123+
124+
/** A handle whose transaction is started later, via [start]. */
125+
fun deferred(context: TransactionContext, options: TransactionOptions): NavTransaction =
126+
NavTransaction(context.name, State.Pending(context, options))
49127
}
50128
}
51129

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

Lines changed: 49 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,11 @@ import io.sentry.TransactionOptions
1010
/**
1111
* Holds a nav transaction staged for publication to a shared [IScope].
1212
*
13-
* A started transaction that's [unpublished][unpublishFrom] is unbound but keeps running, so that
14-
* it can be bound again if it's republished while unfinished. Once [finished][finish], a
15-
* transaction is forgotten and never bound again.
13+
* A transaction requested while publishing isn't permitted is [staged][stage] rather than started,
14+
* and starts when it's next [published][publishTo]. That publication is its only chance to start:
15+
* if it can't start then, it's discarded. A started transaction that's [unpublished][unpublishFrom]
16+
* is unbound but keeps running, so that it can be bound again if it's republished while unfinished.
17+
* Once [finished][finish], a transaction is forgotten and never bound again.
1618
*
1719
* Starting or publishing a nav transaction never displaces an unfinished transaction bound to the
1820
* scope by someone else (e.g., the host app).
@@ -23,7 +25,9 @@ import io.sentry.TransactionOptions
2325
*/
2426
internal class StagedNavTransaction(private val scopes: IScopes) {
2527

26-
/** The nav transaction most recently started, unless it has been finished since then. */
28+
/**
29+
* The nav transaction most recently started or staged, unless it has been finished since then.
30+
*/
2731
private var navTransaction: NavTransaction? = null
2832

2933
/**
@@ -57,20 +61,46 @@ internal class StagedNavTransaction(private val scopes: IScopes) {
5761
}
5862

5963
/**
60-
* Binds the nav transaction to [scope]. A transaction that has finished in the meantime is
61-
* forgotten and unbound instead.
64+
* Stages a nav transaction that's started from [context] and [options] when it's
65+
* [published][publishTo], and returns its handle, which records what's set on it in the meantime.
66+
*
67+
* **Warning**: [Finish][finish] the current nav transaction before calling this method, as this
68+
* method replaces the current transaction without finishing it.
69+
*/
70+
fun stage(context: TransactionContext, options: TransactionOptions): NavTransaction =
71+
NavTransaction.deferred(context, options).also { navTransaction = it }
72+
73+
/**
74+
* Binds the nav transaction to [scope], starting it first if it's staged. A started transaction
75+
* that has finished in the meantime is forgotten and unbound instead.
6276
*
63-
* If another unfinished transaction is bound to [scope], the transaction stays unbound, so it can
64-
* be bound the next time it's published.
77+
* If another unfinished transaction is bound to [scope], a started transaction stays unbound, so
78+
* it can be bound the next time it's published. A staged transaction is discarded instead, as it
79+
* is if [scopes] return a no-op transaction for it (see [start]).
6580
*/
6681
fun publishTo(scope: IScope) {
6782
val navTransaction = navTransaction ?: return
6883
val started = navTransaction.transaction
69-
if (started.isFinished) {
70-
this.navTransaction = null
71-
unbind(scope, started)
72-
} else if (scope.isFreeFor(navTransaction.name, started)) {
73-
bind(scope, started)
84+
when {
85+
started?.isFinished == true -> {
86+
this.navTransaction = null
87+
unbind(scope, started)
88+
}
89+
started != null -> if (scope.isFreeFor(navTransaction.name, started)) bind(scope, started)
90+
else -> {
91+
val startedNow =
92+
if (scope.isFreeFor(navTransaction.name, transaction = null)) {
93+
navTransaction.start(scopes)
94+
} else {
95+
null
96+
}
97+
if (startedNow != null) {
98+
bind(scope, startedNow)
99+
} else {
100+
this.navTransaction = null
101+
navTransaction.finish(onFinished = {})
102+
}
103+
}
74104
}
75105
}
76106

@@ -79,10 +109,13 @@ internal class StagedNavTransaction(private val scopes: IScopes) {
79109
* [published][publishTo] again.
80110
*/
81111
fun unpublishFrom(scope: IScope) {
82-
navTransaction?.let { unbind(scope, it.transaction) }
112+
navTransaction?.transaction?.let { unbind(scope, it) }
83113
}
84114

85-
/** Finishes the nav transaction and unbinds it from [scope], if it's bound. */
115+
/**
116+
* Finishes the nav transaction and unbinds it from [scope], if it's bound. A staged transaction
117+
* is discarded instead.
118+
*/
86119
fun finish(scope: IScope) {
87120
val navTransaction = navTransaction ?: return
88121
// Forget the transaction before unbinding it: unbinding notifies the host app's scope
@@ -93,7 +126,7 @@ internal class StagedNavTransaction(private val scopes: IScopes) {
93126

94127
/**
95128
* Forgets the nav transaction without finishing it, e.g., when there's no scope to unbind it
96-
* from. It will be left to time out.
129+
* from. If started, it will be left to time out.
97130
*/
98131
fun forget() {
99132
navTransaction = null

0 commit comments

Comments
 (0)