feat(android): Add non-terminating envelope capture for Flutter#5795
feat(android): Add non-terminating envelope capture for Flutter#5795buenaflor wants to merge 7 commits into
Conversation
…-unhandled sessions Hybrid unhandled exceptions that don't terminate the process (e.g. Flutter) no longer end the session as crashed. The session is kept alive and marked pending-unhandled, finalized as unhandled on session end or escalated to crashed on a native crash. Existing captureEnvelope(byte[], boolean) behavior (React Native) is unchanged. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep the existing hybrid capture path isolated from non-terminating Flutter session handling while retaining pending-unhandled session persistence. Co-authored-by: Cursor <cursoragent@cursor.com>
Serialize pending session mutations with scope and cache lifecycle operations so delayed session updates cannot overwrite or delete newer session state. Co-authored-by: Cursor <cursoragent@cursor.com>
🚨 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:
|
Added unreleased section with recent fixes for ANR and crash events.
|
🚨 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:
|
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit c6f705c. Configure here.
📲 Install BuildsAndroid
|
|
@sentry review |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| try (final @NotNull ISentryLifecycleToken ignored = sessionLock.acquire()) { | ||
| final @Nullable Session endingSession = readSessionFromEnvelope(envelope); | ||
| final @Nullable Session currentSession = readSessionFromDisk(currentSessionFile); | ||
| if (endingSession != null | ||
| && currentSession != null | ||
| && hasSameSessionId(endingSession, currentSession) | ||
| && !currentSessionFile.delete()) { | ||
| options.getLogger().log(WARNING, "Current envelope doesn't exist."); | ||
| } |
There was a problem hiding this comment.
Bug: The new logic for SessionEnd no longer unconditionally deletes the current session file. If reading the session from the envelope fails, a stale session file will be left on disk.
Severity: MEDIUM
Suggested Fix
Revert to the previous behavior of unconditionally deleting the currentSessionFile when handling a SessionEnd event. Alternatively, ensure the file is deleted even if parsing the ending session from the envelope fails, for instance, by moving the deletion logic outside the conditional check.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: sentry/src/main/java/io/sentry/cache/EnvelopeCache.java#L118-L126
Potential issue: On `SessionEnd`, the current session file is only deleted if the ending
session can be successfully read from the envelope. Previously, this deletion was
unconditional. If `readSessionFromEnvelope()` returns `null` due to a parsing error, an
empty envelope, or an incorrect item type, the new logic will fail to delete the session
file. This leaves a stale session file on disk, which can be picked up by
`PreviousSessionFinalizer` on the next app start, leading to incorrect session state
recovery and potentially skewing crash-free session rate metrics.
Did we get this right? 👍 / 👎 to inform future reviews.
Check for an unhandled exception directly instead of using the crash-named predicate in a path where the process intentionally continues. Co-authored-by: Cursor <cursoragent@cursor.com>
🚨 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:
|
1 participant
📜 Description
Add
InternalSentrySdk.captureEnvelopeNonTerminatingfor hybrid runtimes such as Flutter where an unhandled exception does not terminate the process.The new path keeps the current session alive with the same SID, increments its error count, persists a pending-unhandled marker in the existing session cache, and finalizes it as
unhandledonly when the session naturally ends. ExistingAbnormalandCrashedterminal states remain authoritative, and the existingcaptureEnvelope(byte[], boolean)behavior used by other SDKs is unchanged.💡 Motivation and Context
Flutter currently forwards
handled=falseevents through the terminating hybrid capture path. This marks the session ascrashedand may start a replacement session even though the Flutter process continues running, which lowers crash-free session rates incorrectly.This follows the session protocol's
unhandledstatus for exceptions that are unhandled by the application but do not terminate the process.💚 How did you test it?
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
captureEnvelopeNonTerminatingfor non-terminating unhandled events.Made with Cursor