feat(android): [Unhandled Sessions 3] Add internal non-terminating envelope capture - #5921
feat(android): [Unhandled Sessions 3] Add internal non-terminating envelope capture#5921buenaflor wants to merge 26 commits into
Conversation
🚨 Detected changes in high risk code 🚨High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:
|
|
🚨 Detected changes in high risk code 🚨High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:
|
📲 Install BuildsAndroid
|
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| a416a65 | 333.78 ms | 410.37 ms | 76.59 ms |
| 2195398 | 319.02 ms | 342.38 ms | 23.36 ms |
| 62b579c | 349.26 ms | 426.26 ms | 77.00 ms |
| bbc35bb | 324.88 ms | 425.73 ms | 100.85 ms |
| e59e22a | 374.68 ms | 442.14 ms | 67.46 ms |
| 806307f | 357.85 ms | 424.64 ms | 66.79 ms |
| 62b579c | 312.88 ms | 361.57 ms | 48.70 ms |
| 8687935 | 332.52 ms | 362.23 ms | 29.71 ms |
| 1edbdfa | 364.77 ms | 450.29 ms | 85.52 ms |
| 2195398 | 322.52 ms | 361.91 ms | 39.39 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| a416a65 | 1.58 MiB | 2.12 MiB | 555.26 KiB |
| 2195398 | 0 B | 0 B | 0 B |
| 62b579c | 0 B | 0 B | 0 B |
| bbc35bb | 1.58 MiB | 2.12 MiB | 553.01 KiB |
| e59e22a | 1.58 MiB | 2.20 MiB | 635.34 KiB |
| 806307f | 1.58 MiB | 2.10 MiB | 533.42 KiB |
| 62b579c | 0 B | 0 B | 0 B |
| 8687935 | 1.58 MiB | 2.19 MiB | 619.17 KiB |
| 1edbdfa | 1.58 MiB | 2.20 MiB | 635.34 KiB |
| 2195398 | 0 B | 0 B | 0 B |
🚨 Detected changes in high risk code 🚨High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:
|
6 similar comments
🚨 Detected changes in high risk code 🚨High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:
|
🚨 Detected changes in high risk code 🚨High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:
|
🚨 Detected changes in high risk code 🚨High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:
|
🚨 Detected changes in high risk code 🚨High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:
|
🚨 Detected changes in high risk code 🚨High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:
|
🚨 Detected changes in high risk code 🚨High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:
|
56ab999 to
1906ec6
比较
🚨 Detected changes in high risk code 🚨High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:
|
2 similar comments
🚨 Detected changes in high risk code 🚨High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:
|
🚨 Detected changes in high risk code 🚨High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:
|
06f5823 to
ae23921
比较
🚨 Detected changes in high risk code 🚨High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:
|
ae23921 to
b2ff9d9
比较
🚨 Detected changes in high risk code 🚨High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:
|
b2ff9d9 to
945602b
比较
🚨 Detected changes in high risk code 🚨High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:
|
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…thods Both methods scanned the envelope's events to derive the same pair of booleans, but wrote it differently - one via isCrashed(), the other via getUnhandledException() != null, which is the same predicate. Extract a single scanEvents returning NONE/ERRORED/UNHANDLED so the two agree by construction and an unhandled-but-not-errored state is unrepresentable. Co-authored-by: Cursor <cursoragent@cursor.com>
The annotation had no mechanical effect: apiValidation configures only ignored包/ignored项目 and no nonPublicMarkers, so the type is tracked in sentry.api either way. Regenerating the dump after removing it produces no diff. The interface still has to be public, since the lambda in InternalSentrySdk.captureEnvelopeNonTerminating targets it from io.sentry.android.core. Co-authored-by: Cursor <cursoragent@cursor.com>
Both the method and the enum were plural nouns that read as if they returned the envelope's events, when they return a single summary value. That made "events != EnvelopeEvents.NONE" look like an emptiness check rather than "nothing worth recording happened". EnvelopeEventState also lines up with the Session.State vocabulary already used here. Co-authored-by: Cursor <cursoragent@cursor.com>
Same reasoning as the cache change: this catch predates the feature, and narrowing it changed how an Error during capture behaves for every existing caller while leaving the file's three other catch (Throwable) blocks untouched. captureEnvelopeNonTerminating and readEnvelope are new code and keep catch (Exception). Co-authored-by: Cursor <cursoragent@cursor.com>
Matches IWithTransaction and IWithPropagationContext. The interface has to be public for InternalSentrySdk to use it, but it is not supported API. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
captureEnvelopeNonTerminating called persistCurrentSession from inside the withSession callback, so synchronous session-file I/O ran while holding the scope sessionLock, on the platform thread for Flutter. Mutate under the lock and write the clone withSession returns, the way SentryClient.updateSessionData uses its snapshot. Co-authored-by: Cursor <cursoragent@cursor.com>
…lock" This reverts commit da288f8.
Three reviewers in a row read the persist as accidentally holding the scope lock, so say why it is deliberate. Also name abnormal alongside crashed as a terminal status that wins over the unhandled marker, following the Session javadoc. Co-authored-by: Cursor <cursoragent@cursor.com>
The envelope reader declares IOException and rejects malformed payloads with an unchecked IllegalArgumentException, so name both instead of catching Exception. Co-authored-by: Cursor <cursoragent@cursor.com>
Pull the session mutation out of captureEnvelopeNonTerminating so the dropped-event API can reuse it without duplicating the persist path. Co-authored-by: Cursor <cursoragent@cursor.com>
Every writer to the live session now holds Scope.sessionLock, and only clones leave it, so the lazy serialization on the transport thread and the session file write can no longer race a later mutation. Persisting moves to the executor service instead of running on the calling thread, an already terminated session is no longer written back to the session file, and the broad catch is narrowed to the one call that forces it. Co-authored-by: Cursor <cursoragent@cursor.com>
end() already gives a terminal status precedence via its status == Ok guard, so the clearing is about keeping the flag honest: without it a crashed session serializes a marker claiming it did not terminate, and the public accessor reports true for it. Co-authored-by: Cursor <cursoragent@cursor.com>
Deferring the write to the executor service meant the unhandled marker could be lost if the process died before the task drained, which is the guarantee this path exists for. It also let a queued snapshot land after the session was ended or replaced, rewriting an already-sent session or overwriting a newer current session, both of which surface as a duplicated session on the next launch. Writing under the scope's session lock closes that window, since every scope-side rotation takes the same lock. Also narrow captureEnvelope's blanket catch (Throwable) to the two checked exceptions actually thrown there: Exception from eventStateOf, whose getEvent reads through a Callable, and IOException from SentryEnvelopeItem.fromSession. Everything else in that method already has its own boundary catch in Scopes. Co-authored-by: Cursor <cursoragent@cursor.com>
The caveat that hybrid SDKs call this off the main thread used to live on the persist helper and was lost when that helper was inlined. It matters more now that the write is synchronous, so state it where callers read it. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
fe140b4 to
ad43885
比较
🚨 Detected changes in high risk code 🚨High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ad43885. Configure here.
…ions-internal-api
Co-authored-by: Cursor <cursoragent@cursor.com>
PR Stack (Unhandled Sessions)
📜 Description
Adds
InternalSentrySdk.captureEnvelopeNonTerminating(byte[])for hybrid runtimes where an unhandled exception does not terminate the process.Unlike
captureEnvelope(byte[], boolean), it does not treathandled=falseas a crash. Instead it:Okwith the same session id on the scope,The session is finalized later by normal lifecycle (
endSession, background, or previous-session recovery) asunhandled, unless a terminal status takes over first, such ascrashedfor a native crash orabnormalfor an ANR.captureEnvelope(byte[], boolean)is unchanged.Also in this PR:
Scope.IWithSessionbecomes public soInternalSentrySdkcan mutate the session under the scope lock; it is the only consumer. It carries@ApiStatus.InternallikeIWithTransactionandIWithPropagationContext, so it is not supported API despite being public.eventStateOfreturningEnvelopeEventState, replacing two loops that computed the same booleans by different routes, plus a sharedreadEnvelope.Exceptionrather thanThrowable, soOutOfMemoryErrorand friends propagate instead of being swallowed. The existingcaptureEnvelopekeeps itscatch (Throwable).💡 Motivation and Context
Flutter forwards
handled=falseevents through the terminating hybrid capture path. That marks the sessioncrashedand may start a replacement session even though the Flutter process keeps running, incorrectly lowering crash-free session rates.💚 How did you test it?
新建
InternalSentrySdkTestcoverage: the session stayingOkwith the same id and the flag persisted to disk;endSessionafterwards finalizing asunhandled; and a later hard crash finalizing the old session ascrashedand starting a freshOksession. ExistingcaptureEnvelopetests confirm that path is unchanged.📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
Update the Flutter Android bridge to use
captureEnvelopeNonTerminatingfor non-terminating unhandled events.