fix(session): When capturing unhandled hybrid exception session should be ended #3480
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,9 @@ | ||
| package io.sentry.android.core; | ||
|
|
||
| import static io.sentry.SentryLevel.DEBUG; | ||
| import static io.sentry.SentryLevel.INFO; | ||
| import static io.sentry.SentryLevel.WARNING; | ||
|
|
||
| import android.content.Context; | ||
| import android.content.pm.PackageInfo; | ||
| import android.content.pm.PackageManager; | ||
|
|
@@ -19,12 +23,14 @@ | |
| import io.sentry.android.core.performance.ActivityLifecycleTimeSpan; | ||
| import io.sentry.android.core.performance.AppStartMetrics; | ||
| import io.sentry.android.core.performance.TimeSpan; | ||
| import io.sentry.cache.EnvelopeCache; | ||
| import io.sentry.protocol.App; | ||
| import io.sentry.protocol.Device; | ||
| import io.sentry.protocol.SentryId; | ||
| import io.sentry.protocol.User; | ||
| import io.sentry.util.MapObjectWriter; | ||
| import java.io.ByteArrayInputStream; | ||
| import java.io.File; | ||
| import java.io.InputStream; | ||
| import java.util.ArrayList; | ||
| import java.util.HashMap; | ||
|
|
@@ -148,7 +154,8 @@ public static Map<String, Object> serializeScope( | |
| * captured | ||
| */ | ||
| @Nullable | ||
| public static SentryId captureEnvelope( | ||
| final @NotNull byte[] envelopeData, final boolean maybeStartNewSession) { | ||
| final @NotNull IHub hub = HubAdapter.getInstance(); | ||
| final @NotNull SentryOptions options = hub.getOptions(); | ||
|
|
||
|
|
@@ -184,6 +191,13 @@ public static SentryId captureEnvelope(final @NotNull byte[] envelopeData) { | |
| if (session != null) { | ||
| final SentryEnvelopeItem sessionItem = SentryEnvelopeItem.fromSession(serializer, session); | ||
| envelopeItems.add(sessionItem); | ||
| deleteCurrentSessionFile( | ||
| options, | ||
| // should be sync if going to crash or already not a main thread | ||
| !maybeStartNewSession || !hub.getOptions().getMainThreadChecker().isMainThread()); | ||
| if (maybeStartNewSession) { | ||
| hub.startSession(); | ||
| } | ||
| } | ||
|
|
||
| final SentryEnvelope repackagedEnvelope = | ||
|
|
@@ -233,15 +247,15 @@ private static void addTimeSpanToSerializedSpans(TimeSpan span, List<Map<String, | |
| HubAdapter.getInstance() | ||
| .getOptions() | ||
| .getLogger() | ||
| .log(WARNING, "Can not convert not-started TimeSpan to Map for Hybrid SDKs."); | ||
| return; | ||
| } | ||
|
|
||
| if (span.hasNotStopped()) { | ||
| HubAdapter.getInstance() | ||
| .getOptions() | ||
| .getLogger() | ||
| .log(WARNING, "Can not convert not-stopped TimeSpan to Map for Hybrid SDKs."); | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -252,6 +266,46 @@ 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); | ||
| } | ||
| } else { | ||
| deleteCurrentSessionFile(options); | ||
| } | ||
| } | ||
|
|
||
| private static void deleteCurrentSessionFile(final @NotNull SentryOptions options) { | ||
|
There was a problem hiding this comment. Hmm why do you need to delete the file manually here? I see we anyway delete the session file on session end here. There was a problem hiding this comment. 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. There was a problem hiding this comment. 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. There was a problem hiding this comment. Just for this is the warning I get if not cleaning the session file |
||
| final String cacheDirPath = options.getCacheDirPath(); | ||
| if (cacheDirPath == null) { | ||
| options.getLogger().log(INFO, "Cache dir is not set, not deleting the current session."); | ||
| return; | ||
| } | ||
|
|
||
| if (!options.isEnableAutoSessionTracking()) { | ||
| options | ||
| .getLogger() | ||
| .log(DEBUG, "Session tracking is disabled, bailing from deleting current session file."); | ||
| return; | ||
| } | ||
|
|
||
| final File sessionFile = EnvelopeCache.getCurrentSessionFile(cacheDirPath); | ||
| if (!sessionFile.delete()) { | ||
krystofwoldrich marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| options.getLogger().log(WARNING, "Failed to delete the current session file."); | ||
| } | ||
| } | ||
|
|
||
| @Nullable | ||
| private static Session updateSession( | ||
| final @NotNull IHub hub, | ||
|
|
@@ -268,11 +322,14 @@ private static Session updateSession( | |
| if (updated) { | ||
| if (session.getStatus() == Session.State.Crashed) { | ||
| session.end(); | ||
| // Session needs to be removed from the scope, otherwise it will be send twice | ||
| // standalone and with the crash event | ||
| scope.clearSession(); | ||
| } | ||
| sessionRef.set(session); | ||
| } | ||
| } else { | ||
| options.getLogger().log(INFO, "Session is null on updateSession"); | ||
| } | ||
| }); | ||
| return sessionRef.get(); | ||
|
|
||
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
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if I get it right on the Flutter Android side I just need to check if the error is unhandled and set
maybeStartNewSessionto true in those casesThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, exactly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the envelope contain any extra information we could use instead of
maybeStartNewSession?I see that we have
event.isCrashed()maybe, having aevent.isUnhandled()would make sense as well.