feat(core): [Unhandled Sessions 1] Add Unhandled session state and non-terminating error flag - #5919
feat(core): [Unhandled Sessions 1] Add Unhandled session state and non-terminating error flag#5919buenaflor wants to merge 14 commits into
Conversation
Adds Session.State.Unhandled from the session protocol, plus a pending-unhandled marker that survives serialization. A session carrying the marker finalizes as Unhandled instead of Exited on end(), while Crashed and Abnormal keep taking precedence. Co-authored-by: Cursor <cursoragent@cursor.com>
"Unhandled" alone is ambiguous: a native crash is also an unhandled error, it just terminates the process and so ends the session as crashed rather than unhandled. Name the flag after the property that actually distinguishes the two and match the vocabulary of captureEnvelopeNonTerminating. Also clarify that the setter only restores the flag when rebuilding a session and must not be used to mutate a live one. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 6b019b7 | 343.31 ms | 417.23 ms | 73.91 ms |
| d15471f | 286.65 ms | 314.68 ms | 28.03 ms |
| d217708 | 409.83 ms | 474.72 ms | 64.89 ms |
| d500866 | 326.13 ms | 378.70 ms | 52.58 ms |
| fcec2f2 | 314.96 ms | 373.66 ms | 58.70 ms |
| d501a7e | 314.55 ms | 343.34 ms | 28.79 ms |
| 7414e9b | 322.49 ms | 378.88 ms | 56.39 ms |
| fcec2f2 | 357.47 ms | 447.32 ms | 89.85 ms |
| a416a65 | 316.52 ms | 359.67 ms | 43.15 ms |
| 983e0f0 | 350.64 ms | 386.44 ms | 35.79 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 6b019b7 | 0 B | 0 B | 0 B |
| d15471f | 1.58 MiB | 2.13 MiB | 559.54 KiB |
| d217708 | 1.58 MiB | 2.10 MiB | 532.97 KiB |
| d500866 | 0 B | 0 B | 0 B |
| fcec2f2 | 1.58 MiB | 2.12 MiB | 551.50 KiB |
| d501a7e | 0 B | 0 B | 0 B |
| 7414e9b | 0 B | 0 B | 0 B |
| fcec2f2 | 1.58 MiB | 2.12 MiB | 551.50 KiB |
| a416a65 | 1.58 MiB | 2.12 MiB | 555.26 KiB |
| 983e0f0 | 0 B | 0 B | 0 B |
Previous results on branch: feat/unhandled-sessions-protocol
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 0c0646f | 388.59 ms | 466.14 ms | 77.55 ms |
| 97f3c00 | 309.04 ms | 359.62 ms | 50.58 ms |
| fa28575 | 382.08 ms | 460.50 ms | 78.42 ms |
| 4a7952f | 324.85 ms | 360.54 ms | 35.69 ms |
| 02a6680 | 341.96 ms | 461.54 ms | 119.58 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 0c0646f | 0 B | 0 B | 0 B |
| 97f3c00 | 0 B | 0 B | 0 B |
| fa28575 | 0 B | 0 B | 0 B |
| 4a7952f | 0 B | 0 B | 0 B |
| 02a6680 | 0 B | 0 B | 0 B |
clone() and Session.Deserializer are both inside Session, so they can restore the field directly. Dropping the setter keeps it off the public API surface and makes it impossible to flip the flag on a live session without counting the error and advancing the sequence. Co-authored-by: Cursor <cursoragent@cursor.com>
📲 Install BuildsAndroid
|
Every other field is set at construction; the flag was the odd one out, assigned afterwards. A private canonical constructor keeps construction complete without putting the flag on the public API, which a 15-arg public overload would do. Co-authored-by: Cursor <cursoragent@cursor.com>
As a bare noun phrase the field read like it held the error rather than a boolean, most visibly where it is passed as a constructor argument. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
The round-trip case duplicated one already added to SessionSerializationTest. Keep JSON concerns in the serialization test and leave SessionTest to state transitions. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
It was the only field in Session without the one-line comment the surrounding declarations all carry. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
hasNonTerminatingUnhandledError is not final - recordNonTerminating UnhandledError and update() both write it - so setting it through a constructor established no invariant that a plain assignment does not. Both call sites are inside Session, so clone() and the deserializer can assign the field directly, which is what the deserializer already does for unknown. Removes the 15-parameter overload and the javadoc that existed to justify it. The public constructor is unchanged, so sentry.api is too. Co-authored-by: Cursor <cursoragent@cursor.com>
Also swaps assertFalse(serialize(...).contains(...)) for Truth's doesNotContain, which reports the offending json on failure instead of just "expected false". The two new PreviousSessionFinalizerTest cases are left on Mockito argThat, which needs a Boolean predicate rather than an assertion. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
Adds support in the core Session model for the sessions protocol’s unhandled terminal status (for unhandled errors that do not terminate the process) and introduces an internal flag to persist this condition across serialization and recovery.
Changes:
- Add
Session.State.Unhandledand internalhasNonTerminatingUnhandledErrortracking withrecordNonTerminatingUnhandledError(). - Update
Session.end()to finalize flagged sessions asunhandled(while keepingcrashed/abnormalprecedence). - Add/extend tests to cover JSON round-tripping, omission when unset, and previous-session finalization behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
sentry/src/main/java/io/sentry/Session.java |
Adds Unhandled session state, internal non-terminating-unhandled flag, serialization/deserialization support, and end/finalization behavior. |
sentry/src/test/java/io/sentry/protocol/SessionSerializationTest.kt |
Adds coverage for round-tripping unhandled + flag and ensuring the flag is omitted when unset. |
sentry/src/test/java/io/sentry/PreviousSessionFinalizerTest.kt |
Verifies previous-session finalization results in unhandled when flagged, and crashed when a native crash marker exists. |
sentry/api/sentry.api |
Updates API dump to reflect the new enum constant and internal methods/keys. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
runningcode
left a comment
There was a problem hiding this comment.
Nice! Thanks for breaking this up
| } | ||
|
|
||
| @Test | ||
| fun `serialize and deserialize round-trips Unhandled status and non-terminating flag`() { |
There was a problem hiding this comment.
i wonder if we can just have a single json / json test with all the different valid options instead of one test per json option
…recovery paths Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
| * <p>Never sent as a status while the session is alive; it is only persisted with the session. | ||
| */ | ||
| @ApiStatus.Internal | ||
| public boolean hasNonTerminatingUnhandledError() { |
There was a problem hiding this comment.
do we need to grab the sessionLock in order to read this?
otherwise if this could be set to true before errorCount is incremented. would that be a valid state?
| Crashed, | ||
| Abnormal | ||
| Abnormal, | ||
| Unhandled |
There was a problem hiding this comment.
should we javadoc this? i know the other ones don't but this feels like it is worht documenting given the surprising behavior
| } | ||
|
|
||
| /** | ||
| * Records that an active session experienced an unhandled error which did not terminate the |
There was a problem hiding this comment.
should we document that this is only meant to be called from hybrid sdks and never from java/kotlin/android?
PR Stack (Unhandled Sessions)
📜 Description
Adds the session protocol's
unhandledstatus to theSessionmodel.Session.State.Unhandled— an unhandled error the process survived.hasNonTerminatingUnhandledErrorflag (non_terminating_unhandled_errorin JSON), set byrecordNonTerminatingUnhandledError(), which counts the error without ending the session or changing its status while it is alive.end()finalizes a flagged session asUnhandledrather thanExited.CrashedandAbnormalstill take precedence, andupdate(Crashed, ...)clears the flag.clone()and the (de)serializer carry the flag; it is omitted from JSON when unset.Nothing sets the flag yet, so this PR is inert on its own.
The flag is named for what separates this case from a native crash, which is also unhandled but terminates the process and ends the session as
crashed.💡 Motivation and Context
Hybrid runtimes such as Flutter report
handled=falseexceptions that do not kill the process. Those currently go through the terminating capture path, which marks the sessioncrashedand starts a replacement even though the app keeps running, incorrectly lowering crash-free session rates.The protocol has had
unhandledfor exactly this case since 1.6.0, and Relay accepts it.💚 How did you test it?
SessionSerializationTestgets asession_unhandled.jsonfixture covering the full unhandled document, round-tripped through the same serialize/deserialize pair assession.json. Omission when the flag is unset needs no test of its own: the existingsession.jsonhas nonon_terminating_unhandled_errorkey and those tests compare exact strings, so a leaked key fails them.PreviousSessionFinalizerTestcovers recovering a flagged previous session, including a native crash escalating it tocrashed.EnvelopeCacheTestcovers the same precedence through theAbnormalExitandNativeCrashExitrecovery paths.📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
Persistence across process death in #5920, the capture API in #5921.
#skip-changelog