fix(core): Clear the persisted replay id when resetting the scope cache - #6033
Draft
runningcode wants to merge 2 commits into
Draft
fix(core): Clear the persisted replay id when resetting the scope cache#6033runningcode wants to merge 2 commits into
runningcode wants to merge 2 commits into
Conversation
resetCache() clears every other persisted scope value on init but leaves replay.json in place, so a replay id written by a previous process can still be attached to events from the current one. The reset already runs after the integrations that consume those values, so deleting it here is safe. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
📲 Install BuildsAndroid
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📜 Description
PersistingScopeObserver.resetCache()runs on SDK init and clears the persisted scope so values from the previous process don't leak into the new one. It deletes user, level, request, fingerprint, contexts, extras, tags, trace and transaction, and clears the breadcrumb queue — but it has never touchedreplay.json.This adds
delete(REPLAY_FILENAME)alongside the others, plus a test inPersistingScopeObserverBatchingTest.💡 Motivation and Context
A replay id points at a replay recorded by the process that wrote it. Leaving it on disk across an init means
ApplicationExitInfoEventProcessorcan read a stale id and attach it to an ANR event from the current process. The impact is bounded — the processor falls back to scanning for the newestreplay_*folder when the id's folder is gone, and the replay integration overwrites the file once it starts recording — but the id should not survive the reset in the first place.The intent was there from the start: the comment at the
resetCache()call site inSentry.notifyOptionsObserversalready givesreplayIdas its example of a value that must not reach new events. The implementation just never included it.Split out of #6031, which is a test-only flakiness fix. Joining a previously discarded
Futurethere revived an assertion that had been silently swallowed since #4181 and turned out never to have been satisfiable, which is how this gap surfaced.Why it's safe to delete
replay.jsonis used during init to finalize the replay from the previous process. That handoff is already finished by the time the reset runs, because all three steps are queued on the same single-threaded executor, in this order:AnrV2Integration.register()— ANR enrichment reads the replay id and writes back the one it resolvedReplayIntegration.register()—finalizePreviousReplay()reads it and finalizes that replaynotifyOptionsObservers()—resetCache()deletes itSteps 1 and 2 are deliberately ordered:
AndroidOptionsInitializerhas a comment that Anr must be installed before Replay "as ReplayIntegration relies on it to set the replayId in case of an ANR". Step 3 has always come last, which is why AnrV2 events still get the previous process's replay id today.Nothing reads the file after step 3, so deleting there only affects the next init — which is the point.
Deleting also doesn't introduce a new state to handle.
replay.jsoncan already holdEMPTY_IDtoday, written by the replay integration when a replay stops, and both readers treat a missing file exactly likeEMPTY_ID:finalizePreviousReplay()callscleanupReplays()either way, andApplicationExitInfoEventProcessorlooks for areplay_<id>folder that doesn't exist either way and takes the same fallback.💚 How did you test it?
New test
resetCache clears the replay id left behind by the previous processinPersistingScopeObserverBatchingTest. Verified it fails without the one-line change (expected: null/but was: <afcb46b1140ade5187c4bbb5daa804df>) and passes with it.Ran
:sentry:test,:sentry-android-core:testReleaseUnitTestand:sentry-android-replay:testReleaseUnitTest— all pass.📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
None.