Do not send session updates for terminated sessions (#2849)

getsentry · Jul 20, 2023 · 101f707 · 101f707
1 parent 0bff5c1
commit 101f707
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,7 @@
 - Deduplicate events happening in multiple threads simultaneously (e.g. `OutOfMemoryError`) ([#2845](https://github.com/getsentry/sentry-java/pull/2845))
   - This will improve Crash-Free Session Rate as we no longer will send multiple Session updates with `Crashed` status, but only the one that is relevant
 - Ensure no Java 8 method reference sugar is used for Android ([#2857](https://github.com/getsentry/sentry-java/pull/2857))
+- Do not send session updates for terminated sessions ([#2849](https://github.com/getsentry/sentry-java/pull/2849))
 
 ## 6.26.0
 

diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api
@@ -2063,6 +2063,7 @@ public final class io/sentry/Session : io/sentry/JsonSerializable, io/sentry/Jso
 	public fun getTimestamp ()Ljava/util/Date;
 	public fun getUnknown ()Ljava/util/Map;
 	public fun getUserAgent ()Ljava/lang/String;
+	public fun isTerminated ()Z
 	public fun serialize (Lio/sentry/ObjectWriter;Lio/sentry/ILogger;)V
 	public fun setInitAsTrue ()V
 	public fun setUnknown (Ljava/util/Map;)V

diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java
@@ -137,7 +137,10 @@ private boolean shouldApplyScopeData(
     @Nullable Session session = null;
 
     if (event != null) {
-      session = updateSessionData(event, hint, scope);
+      // https://develop.sentry.dev/sdk/sessions/#terminal-session-states
+      if (sessionBeforeUpdate == null || !sessionBeforeUpdate.isTerminated()) {
+        session = updateSessionData(event, hint, scope);
+      }
 
       if (!sample()) {
         options
@@ -490,9 +493,8 @@ Session updateSessionData(
                     }
 
                     if (session.update(status, userAgent, crashedOrErrored, abnormalMechanism)) {
-                      // if we have an uncaughtExceptionHint we can end the session.
-                      if (HintUtils.hasType(
-                          hint, UncaughtExceptionHandlerIntegration.UncaughtExceptionHint.class)) {
+                      // if session terminated we can end it.
+                      if (session.isTerminated()) {
                         session.end();
                       }
                     }

diff --git a/sentry/src/main/java/io/sentry/Session.java b/sentry/src/main/java/io/sentry/Session.java
@@ -125,6 +125,10 @@ public Session(
         null);
   }
 
+  public boolean isTerminated() {
+    return status != State.Ok;
+  }
+
   @SuppressWarnings({"JdkObsolete", "JavaUtilDate"})
   public @Nullable Date getStarted() {
     if (started == null) {

diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt
@@ -1,6 +1,7 @@
 package io.sentry
 
 import io.sentry.Scope.IWithPropagationContext
+import io.sentry.Session.State.Crashed
 import io.sentry.clientreport.ClientReportTestHelper.Companion.assertClientReport
 import io.sentry.clientreport.DiscardReason
 import io.sentry.clientreport.DiscardedEvent
@@ -953,6 +954,21 @@ class SentryClientTest {
         }
     }
 
+    @Test
+    fun `When event is non handled, end the session`() {
+        val scope = Scope(fixture.sentryOptions)
+        scope.startSession()
+
+        val event = SentryEvent().apply {
+            exceptions = createNonHandledException()
+        }
+        fixture.getSut().updateSessionData(event, Hint(), scope)
+        scope.withSession {
+            assertEquals(Session.State.Crashed, it!!.status)
+            assertNotNull(it.duration)
+        }
+    }
+
     @Test
     fun `When event is handled, keep level as it is`() {
         val scope = Scope(fixture.sentryOptions)
@@ -1137,6 +1153,28 @@ class SentryClientTest {
         }
     }
 
+    @Test
+    fun `when session is in terminal state, does not send session update`() {
+        val sut = fixture.getSut()
+
+        val event = SentryEvent().apply {
+            exceptions = createNonHandledException()
+        }
+        val scope = Scope(fixture.sentryOptions)
+        val sessionPair = scope.startSession()
+        scope.withSession { it!!.update(Crashed, null, false) }
+
+        assertNotNull(sessionPair) {
+            sut.captureEvent(event, scope, null)
+            verify(fixture.transport).send(
+                check {
+                    assertNull(it.items.find { item -> item.header.type == SentryItemType.Session })
+                },
+                anyOrNull()
+            )
+        }
+    }
+
     @Test
     fun `when contexts property is set on the event, property from scope contexts is not applied`() {
         val sut = fixture.getSut()