From f94cbe230f64c5ae9364654f4b463090a56438b2 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Fri, 14 Jun 2024 16:57:48 +0200 Subject: [PATCH 1/6] fix(session): When capturing unhandled hybrid exception session should be ended --- .../sentry/android/core/InternalSentrySdk.java | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java b/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java index a4d1db09df3..4f52f67c9f1 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java @@ -10,6 +10,7 @@ import io.sentry.IScope; import io.sentry.ISerializer; import io.sentry.ObjectWriter; +import io.sentry.Sentry; import io.sentry.SentryEnvelope; import io.sentry.SentryEnvelopeItem; import io.sentry.SentryEvent; @@ -164,7 +165,8 @@ public static SentryId captureEnvelope(final @NotNull byte[] envelopeData) { // determine session state based on events inside envelope @Nullable Session.State status = null; - boolean crashedOrErrored = false; + boolean crashed = false; + boolean errored = false; for (SentryEnvelopeItem item : envelope.getItems()) { envelopeItems.add(item); @@ -173,17 +175,24 @@ public static SentryId captureEnvelope(final @NotNull byte[] envelopeData) { if (event.isCrashed()) { status = Session.State.Crashed; } - if (event.isCrashed() || event.isErrored()) { - crashedOrErrored = true; + if (event.isCrashed()) { + crashed = true; + } + if (event.isErrored()) { + errored = true; } } } // update session and add it to envelope if necessary - final @Nullable Session session = updateSession(hub, options, status, crashedOrErrored); + final @Nullable Session session = updateSession(hub, options, status, crashed || errored); if (session != null) { final SentryEnvelopeItem sessionItem = SentryEnvelopeItem.fromSession(serializer, session); envelopeItems.add(sessionItem); + if (crashed) { + session.end(); + Sentry.startSession(); + } } final SentryEnvelope repackagedEnvelope = From f9538c87fc0e4a167c1ac7f01e04b5d0560cf8d8 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Mon, 17 Jun 2024 18:22:21 +0200 Subject: [PATCH 2/6] add clear session, add test, only capture session once, start new session if enabled --- .../android/core/InternalSentrySdk.java | 22 ++--- .../android/core/InternalSentrySdkTest.kt | 88 ++++++++++++++----- sentry/src/main/java/io/sentry/IScope.java | 3 + sentry/src/main/java/io/sentry/NoOpScope.java | 4 + sentry/src/main/java/io/sentry/Scope.java | 6 ++ 5 files changed, 92 insertions(+), 31 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java b/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java index 4f52f67c9f1..6b4b55b39d8 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java @@ -149,7 +149,9 @@ public static Map serializeScope( * captured */ @Nullable - public static SentryId captureEnvelope(final @NotNull byte[] envelopeData) { + public static SentryId captureEnvelope( + final @NotNull byte[] envelopeData, + final boolean maybeStartNewSession) { final @NotNull IHub hub = HubAdapter.getInstance(); final @NotNull SentryOptions options = hub.getOptions(); @@ -165,8 +167,7 @@ public static SentryId captureEnvelope(final @NotNull byte[] envelopeData) { // determine session state based on events inside envelope @Nullable Session.State status = null; - boolean crashed = false; - boolean errored = false; + boolean crashedOrErrored = false; for (SentryEnvelopeItem item : envelope.getItems()) { envelopeItems.add(item); @@ -175,22 +176,18 @@ public static SentryId captureEnvelope(final @NotNull byte[] envelopeData) { if (event.isCrashed()) { status = Session.State.Crashed; } - if (event.isCrashed()) { - crashed = true; - } - if (event.isErrored()) { - errored = true; + if (event.isCrashed() || event.isErrored()) { + crashedOrErrored = true; } } } // update session and add it to envelope if necessary - final @Nullable Session session = updateSession(hub, options, status, crashed || errored); + final @Nullable Session session = updateSession(hub, options, status, crashedOrErrored); if (session != null) { final SentryEnvelopeItem sessionItem = SentryEnvelopeItem.fromSession(serializer, session); envelopeItems.add(sessionItem); - if (crashed) { - session.end(); + if (maybeStartNewSession) { Sentry.startSession(); } } @@ -277,6 +274,9 @@ 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); } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/InternalSentrySdkTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/InternalSentrySdkTest.kt index 65ecdb3b8c1..339f8314f05 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/InternalSentrySdkTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/InternalSentrySdkTest.kt @@ -39,6 +39,7 @@ import java.util.concurrent.atomic.AtomicReference import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertNotEquals import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue @@ -84,7 +85,7 @@ class InternalSentrySdkTest { capturedEnvelopes.clear() } - fun captureEnvelopeWithEvent(event: SentryEvent = SentryEvent()) { + fun captureEnvelopeWithEvent(event: SentryEvent = SentryEvent(), maybeStartNewSession: Boolean = false) { // create an envelope with session data val options = Sentry.getCurrentHub().options val eventId = SentryId() @@ -103,7 +104,24 @@ class InternalSentrySdkTest { options.serializer.serialize(envelope, outputStream) val data = outputStream.toByteArray() - InternalSentrySdk.captureEnvelope(data) + InternalSentrySdk.captureEnvelope(data, maybeStartNewSession) + } + + fun createSentryEventWithUnhandledException(): SentryEvent { + return SentryEvent(RuntimeException()).apply { + val mechanism = Mechanism() + mechanism.isHandled = false + + val factory = SentryExceptionFactory(mock()) + val sentryExceptions = factory.getSentryExceptions( + ExceptionMechanismException( + mechanism, + Throwable(), + Thread() + ) + ) + exceptions = sentryExceptions + } } fun mockFinishedAppStart() { @@ -313,7 +331,7 @@ class InternalSentrySdkTest { @Test fun `captureEnvelope fails if payload is invalid`() { - assertNull(InternalSentrySdk.captureEnvelope(ByteArray(8))) + assertNull(InternalSentrySdk.captureEnvelope(ByteArray(8), false)) } @Test @@ -337,27 +355,12 @@ class InternalSentrySdkTest { } @Test - fun `captureEnvelope correctly enriches the envelope with session data`() { + fun `captureEnvelope correctly enriches the envelope with session data and does not start new session`() { val fixture = Fixture() fixture.init(context) // when capture envelope is called with an crashed event - fixture.captureEnvelopeWithEvent( - SentryEvent(RuntimeException()).apply { - val mechanism = Mechanism() - mechanism.isHandled = false - - val factory = SentryExceptionFactory(mock()) - val sentryExceptions = factory.getSentryExceptions( - ExceptionMechanismException( - mechanism, - Throwable(), - Thread() - ) - ) - exceptions = sentryExceptions - } - ) + fixture.captureEnvelopeWithEvent(fixture.createSentryEventWithUnhandledException()) val capturedEnvelope = fixture.capturedEnvelopes.first() val capturedEnvelopeItems = capturedEnvelope.items.toList() @@ -380,6 +383,51 @@ class InternalSentrySdkTest { scopeRef.set(scope) } assertEquals(Session.State.Crashed, scopeRef.get().session!!.status) + assertEquals(capturedSession.sessionId, scopeRef.get().session!!.sessionId) + } + + @Test + fun `captureEnvelope starts new session when enabled`() { + val fixture = Fixture() + fixture.init(context) + + // when capture envelope is called with an crashed event + fixture.captureEnvelopeWithEvent(fixture.createSentryEventWithUnhandledException(), true) + + val scopeRef = AtomicReference() + Sentry.configureScope { scope -> + scopeRef.set(scope) + } + + // first envelope is the new session start + val capturedStartSessionEnvelope = fixture.capturedEnvelopes.first() + val capturedNewSessionStart = fixture.options.serializer.deserialize( + InputStreamReader(ByteArrayInputStream(capturedStartSessionEnvelope.items.toList()[0].data)), + Session::class.java + )!! + assertEquals(capturedNewSessionStart.sessionId, scopeRef.get().session!!.sessionId) + assertEquals(Session.State.Ok, capturedNewSessionStart.status) + + val capturedEnvelope = fixture.capturedEnvelopes.last() + val capturedEnvelopeItems = capturedEnvelope.items.toList() + + // there should be two envelopes session start and captured crash + assertEquals(2, fixture.capturedEnvelopes.size) + + // then it should contain the original event + session + assertEquals(2, capturedEnvelopeItems.size) + assertEquals(SentryItemType.Event, capturedEnvelopeItems[0].header.type) + assertEquals(SentryItemType.Session, capturedEnvelopeItems[1].header.type) + + // and then the sent session should be marked as crashed + val capturedSession = fixture.options.serializer.deserialize( + InputStreamReader(ByteArrayInputStream(capturedEnvelopeItems[1].data)), + Session::class.java + )!! + assertEquals(Session.State.Crashed, capturedSession.status) + + // and the local session should be a new session + assertNotEquals(capturedSession.sessionId, scopeRef.get().session!!.sessionId) } @Test diff --git a/sentry/src/main/java/io/sentry/IScope.java b/sentry/src/main/java/io/sentry/IScope.java index 3842fb2c3a8..3064df8f79a 100644 --- a/sentry/src/main/java/io/sentry/IScope.java +++ b/sentry/src/main/java/io/sentry/IScope.java @@ -352,6 +352,9 @@ public interface IScope { @Nullable Session getSession(); + @ApiStatus.Internal + void clearSession(); + @ApiStatus.Internal void setPropagationContext(final @NotNull PropagationContext propagationContext); diff --git a/sentry/src/main/java/io/sentry/NoOpScope.java b/sentry/src/main/java/io/sentry/NoOpScope.java index c756fb49a39..ed787b00290 100644 --- a/sentry/src/main/java/io/sentry/NoOpScope.java +++ b/sentry/src/main/java/io/sentry/NoOpScope.java @@ -219,6 +219,10 @@ public void withTransaction(Scope.@NotNull IWithTransaction callback) {} return null; } + @ApiStatus.Internal + @Override + public void clearSession() {} + @ApiStatus.Internal @Override public void setPropagationContext(@NotNull PropagationContext propagationContext) {} diff --git a/sentry/src/main/java/io/sentry/Scope.java b/sentry/src/main/java/io/sentry/Scope.java index 91c9fcd8cfe..356ee2b57c5 100644 --- a/sentry/src/main/java/io/sentry/Scope.java +++ b/sentry/src/main/java/io/sentry/Scope.java @@ -913,6 +913,12 @@ public SentryOptions getOptions() { return session; } + @ApiStatus.Internal + @Override + public void clearSession() { + session = null; + } + @ApiStatus.Internal @Override public void setPropagationContext(final @NotNull PropagationContext propagationContext) { From 7ce635b0df36f67d46a78ad0fad8cd60c7152308 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Mon, 17 Jun 2024 16:24:28 +0000 Subject: [PATCH 3/6] Format code --- .../main/java/io/sentry/android/core/InternalSentrySdk.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java b/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java index 6b4b55b39d8..34bef56eb97 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java @@ -150,8 +150,7 @@ public static Map serializeScope( */ @Nullable public static SentryId captureEnvelope( - final @NotNull byte[] envelopeData, - final boolean maybeStartNewSession) { + final @NotNull byte[] envelopeData, final boolean maybeStartNewSession) { final @NotNull IHub hub = HubAdapter.getInstance(); final @NotNull SentryOptions options = hub.getOptions(); From 0181755ee8c22c758b8f92e8830fe9e7dec23b23 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 18 Jun 2024 13:58:26 +0200 Subject: [PATCH 4/6] remove processed session file --- .../android/core/InternalSentrySdk.java | 33 +++++++++++++++++-- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java b/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java index 34bef56eb97..e67b52baa9e 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java @@ -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; @@ -20,12 +24,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; @@ -186,6 +192,7 @@ public static SentryId captureEnvelope( if (session != null) { final SentryEnvelopeItem sessionItem = SentryEnvelopeItem.fromSession(serializer, session); envelopeItems.add(sessionItem); + deleteCurrentSessionFile(options); if (maybeStartNewSession) { Sentry.startSession(); } @@ -238,7 +245,7 @@ private static void addTimeSpanToSerializedSpans(TimeSpan span, List Date: Tue, 18 Jun 2024 12:00:27 +0000 Subject: [PATCH 5/6] Format code --- .../main/java/io/sentry/android/core/InternalSentrySdk.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java b/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java index e67b52baa9e..7a8fe33a47f 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java @@ -273,8 +273,8 @@ private static void deleteCurrentSessionFile(final @NotNull SentryOptions option if (!options.isEnableAutoSessionTracking()) { options - .getLogger() - .log(DEBUG, "Session tracking is disabled, bailing from deleting current session file."); + .getLogger() + .log(DEBUG, "Session tracking is disabled, bailing from deleting current session file."); return; } From 7ec01cc8a1c0df2f864a3f3077bf1205b6f54d08 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 18 Jun 2024 18:23:09 +0200 Subject: [PATCH 6/6] update api --- sentry-android-core/api/sentry-android-core.api | 2 +- sentry/api/sentry.api | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 97f7cddb7c0..adcc6ea87d7 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -204,7 +204,7 @@ public abstract interface class io/sentry/android/core/IDebugImagesLoader { public final class io/sentry/android/core/InternalSentrySdk { public fun ()V - public static fun captureEnvelope ([B)Lio/sentry/protocol/SentryId; + public static fun captureEnvelope ([BZ)Lio/sentry/protocol/SentryId; public static fun getAppStartMeasurement ()Ljava/util/Map; public static fun getCurrentScope ()Lio/sentry/IScope; public static fun serializeScope (Landroid/content/Context;Lio/sentry/android/core/SentryAndroidOptions;Lio/sentry/IScope;)Ljava/util/Map; diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index d29b04dd7d7..567e9d15110 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -667,6 +667,7 @@ public abstract interface class io/sentry/IScope { public abstract fun clear ()V public abstract fun clearAttachments ()V public abstract fun clearBreadcrumbs ()V + public abstract fun clearSession ()V public abstract fun clearTransaction ()V public abstract fun clone ()Lio/sentry/IScope; public abstract fun endSession ()Lio/sentry/Session; @@ -1221,6 +1222,7 @@ public final class io/sentry/NoOpScope : io/sentry/IScope { public fun clear ()V public fun clearAttachments ()V public fun clearBreadcrumbs ()V + public fun clearSession ()V public fun clearTransaction ()V public fun clone ()Lio/sentry/IScope; public synthetic fun clone ()Ljava/lang/Object; @@ -1592,6 +1594,7 @@ public final class io/sentry/Scope : io/sentry/IScope { public fun clear ()V public fun clearAttachments ()V public fun clearBreadcrumbs ()V + public fun clearSession ()V public fun clearTransaction ()V public fun clone ()Lio/sentry/IScope; public synthetic fun clone ()Ljava/lang/Object;