Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(session): When capturing unhandled hybrid exception session should be ended #3480

Merged
merged 16 commits into from
Jun 28, 2024
Merged

fix(session): When capturing unhandled hybrid exception session should be ended #3480

Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
f94cbe2
fix(session): When capturing unhandled hybrid exception session shoul…
krystofwoldrich Jun 14, 2024
f9538c8
add clear session, add test, only capture session once, start new ses…
krystofwoldrich Jun 17, 2024
7ce635b
Format code
getsentry-bot Jun 17, 2024
f59fbfc
Merge branch 'main' into kw/fix-handling-hybrid-unhandled-exceptions
krystofwoldrich Jun 18, 2024
0181755
remove processed session file
krystofwoldrich Jun 18, 2024
36b1640
Merge remote-tracking branch 'origin/kw/fix-handling-hybrid-unhandled…
krystofwoldrich Jun 18, 2024
0193bd7
Format code
getsentry-bot Jun 18, 2024
7ec01cc
update api
krystofwoldrich Jun 18, 2024
7128222
Merge remote-tracking branch 'origin/kw/fix-handling-hybrid-unhandled…
krystofwoldrich Jun 18, 2024
23b7734
fix test, and review comments
krystofwoldrich Jun 21, 2024
6a62b9b
add changelog
krystofwoldrich Jun 21, 2024
92eec8e
Merge branch 'main' into kw/fix-handling-hybrid-unhandled-exceptions
krystofwoldrich Jun 21, 2024
42f4007
Merge branch 'main' into kw/fix-handling-hybrid-unhandled-exceptions
krystofwoldrich Jun 27, 2024
06683e6
fix file io on the main thread
krystofwoldrich Jun 27, 2024
e944901
fix format
krystofwoldrich Jun 27, 2024
6de823e
Merge remote-tracking branch 'origin/main' into kw/fix-handling-hybri…
krystofwoldrich Jun 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,10 @@ public static SentryId captureEnvelope(
if (session != null) {
final SentryEnvelopeItem sessionItem = SentryEnvelopeItem.fromSession(serializer, session);
envelopeItems.add(sessionItem);
deleteCurrentSessionFile(options);
deleteCurrentSessionFile(
options,
// should be sync if going to crash or already not a main thread
!maybeStartNewSession || !hub.getOptions().getMainThreadChecker().isMainThread());
if (maybeStartNewSession) {
hub.startSession();
}
Expand Down Expand Up @@ -263,6 +266,20 @@ private static void addTimeSpanToSerializedSpans(TimeSpan span, List<Map<String,
spans.add(spanMap);
}

private static void deleteCurrentSessionFile(final @NotNull SentryOptions options, boolean isSync) {
if (!isSync) {
try {
options.getExecutorService().submit(() -> {
deleteCurrentSessionFile(options);
});
} catch (Throwable e) {
options.getLogger().log(WARNING, "Submission of deletion of the current session file rejected.", e);
krystofwoldrich marked this conversation as resolved.
Show resolved Hide resolved
}
} else {
deleteCurrentSessionFile(options);
}
}

private static void deleteCurrentSessionFile(final @NotNull SentryOptions options) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm why do you need to delete the file manually here? I see we anyway delete the session file on session end here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, when I don't explicitly delete it, it's not removed and it's read on the next app start. But I didn't know it should be deleted automatically.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm seeing this right, the envelope is only deleted when the session end is send as standalone envelope, but in the case of hybrids the session end will be send with the crash event in the same envelope.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for this is the warning I get if not cleaning the session file

sentry-java/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java

Line 74 in 6de823e

options.getLogger().log(WARNING, "Current session is not ended, we'd need to end it.");

final String cacheDirPath = options.getCacheDirPath();
if (cacheDirPath == null) {
Expand Down