make PreviousSessionFinalizer aware of tombstone handled sessions and add crashedLastRun

Author: supervacuus

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

@@ -86,16 +86,30 @@ public void run() {
                   "Stream from path %s resulted in a null envelope.",
                   previousSessionFile.getAbsolutePath());
         } else {
-          Date timestamp = null;
           final File crashMarkerFile =
               new File(options.getCacheDirPath(), NATIVE_CRASH_MARKER_FILE);
-          if (crashMarkerFile.exists()) {
+
+          if (session.getStatus() == Session.State.Crashed) {
+            // this means the session was already updated from tombstone data and there is no need
+            // to consider the crash-marker file from the Native SDK. However, we can use this to
+            // set crashedLastRun.
+            SentryCrashLastRunState.getInstance().setCrashedLastRun(true);
+          } else if (crashMarkerFile.exists()) {
+            // this means that the Native SDK is solely responsible for Native crashes and we must
+            // update the session state and timestamp.
             options
                 .getLogger()
                 .log(INFO, "Crash marker file exists, last Session is gonna be Crashed.");
 
-            timestamp = getTimestampFromCrashMarkerFile(crashMarkerFile);
+            @Nullable Date timestamp = getTimestampFromCrashMarkerFile(crashMarkerFile);
+            session.update(Session.State.Crashed, null, true);
+            session.end(timestamp);
+          }
 
+          // Independent of whether the TombstoneIntegration is active or the Native SDK operates
+          // alone, we must finalize the Native SDK crash marker life-cycle, otherwise it would
+          // report crashes forever.
+          if (crashMarkerFile.exists()) {
             if (!crashMarkerFile.delete()) {
               options
                   .getLogger()
@@ -104,12 +118,6 @@ public void run() {
                       "Failed to delete the crash marker file. %s.",
                       crashMarkerFile.getAbsolutePath());
             }
-            session.update(Session.State.Crashed, null, true);
-          }
-          // if the session has abnormal mechanism, we do not overwrite its end timestamp, because
-          // it's already set
-          if (session.getAbnormalMechanism() == null) {
-            session.end(timestamp);
           }
 
           // if the App. has been upgraded and there's a new version of the SDK running,

sentry/src/main/java/io/sentry/cache/EnvelopeCache.java

@@ -131,9 +131,6 @@ private boolean storeInternal(final @NotNull SentryEnvelope envelope, final @Not
 
       boolean crashedLastRun = false;
       final File crashMarkerFile = new File(options.getCacheDirPath(), NATIVE_CRASH_MARKER_FILE);
-      // TODO: this should probably check whether the Native SDK integration is currently enabled or
-      // remove the marker file if it isn't. Otherwise, application that disable the Native SDK,
-      // will report a crash for the last run forever.
       if (crashMarkerFile.exists()) {
         crashedLastRun = true;
       }
@@ -158,7 +155,6 @@ private boolean storeInternal(final @NotNull SentryEnvelope envelope, final @Not
         }
       }
 
-      // TODO: maybe set crashLastRun for tombstone too?
       SentryCrashLastRunState.getInstance().setCrashedLastRun(crashedLastRun);
 
       flushPreviousSession();

sentry/src/test/java/io/sentry/PreviousSessionFinalizerTest.kt

@@ -4,7 +4,11 @@ import io.sentry.Session.State.Crashed
 import io.sentry.cache.EnvelopeCache
 import java.io.File
 import java.util.Date
+import kotlin.test.AfterTest
+import kotlin.test.BeforeTest
 import kotlin.test.assertFalse
+import kotlin.test.assertNull
+import kotlin.test.assertTrue
 import org.junit.Rule
 import org.junit.Test
 import org.junit.rules.TemporaryFolder
@@ -17,6 +21,16 @@ import org.mockito.kotlin.verify
 class PreviousSessionFinalizerTest {
   @get:Rule val tmpDir = TemporaryFolder()
 
+  @BeforeTest
+  fun `set up`() {
+    SentryCrashLastRunState.getInstance().reset()
+  }
+
+  @AfterTest
+  fun `tear down`() {
+    SentryCrashLastRunState.getInstance().reset()
+  }
+
   class Fixture {
     val options = SentryOptions()
     val scopes = mock<IScopes>()
@@ -205,4 +219,62 @@ class PreviousSessionFinalizerTest {
       )
     verify(fixture.scopes, never()).captureEnvelope(any())
   }
+
+  @Test
+  fun `when previous session is already crashed, sets crashedLastRun to true`() {
+    // Create a session that is already in Crashed state (simulating tombstone integration)
+    val crashedSession =
+      Session(null, null, null, "io.sentry.sample@1.0").apply { update(Crashed, null, true) }
+
+    val finalizer = fixture.getSut(tmpDir, session = crashedSession)
+
+    // crashedLastRun should not be set before running the finalizer
+    assertNull(SentryCrashLastRunState.getInstance().isCrashedLastRun(null, false))
+
+    finalizer.run()
+
+    // crashedLastRun should be set to true after running the finalizer
+    assertTrue(SentryCrashLastRunState.getInstance().isCrashedLastRun(null, false)!!)
+  }
+
+  @Test
+  fun `when native crash marker exists but session is not crashed, does not set crashedLastRun`() {
+    // Session is not crashed, but native crash marker exists
+    val finalizer =
+      fixture.getSut(
+        tmpDir,
+        session = Session(null, null, null, "io.sentry.sample@1.0"),
+        nativeCrashTimestamp = Date(2023, 10, 1),
+      )
+
+    // crashedLastRun should not be set before running the finalizer
+    assertNull(SentryCrashLastRunState.getInstance().isCrashedLastRun(null, false))
+
+    finalizer.run()
+
+    // crashedLastRun should NOT be set by PreviousSessionFinalizer for native crash marker case
+    // (it's handled by EnvelopeCache at session start instead)
+    assertNull(SentryCrashLastRunState.getInstance().isCrashedLastRun(null, false))
+  }
+
+  @Test
+  fun `when previous session is already crashed and native crash marker exists, sets crashedLastRun and deletes marker`() {
+    // Session is already crashed (tombstone case) AND native crash marker exists
+    val crashedSession =
+      Session(null, null, null, "io.sentry.sample@1.0").apply { update(Crashed, null, true) }
+
+    val finalizer =
+      fixture.getSut(tmpDir, session = crashedSession, nativeCrashTimestamp = Date(2023, 10, 1))
+
+    val nativeCrashMarker =
+      File(fixture.options.cacheDirPath!!, EnvelopeCache.NATIVE_CRASH_MARKER_FILE)
+    assertTrue(nativeCrashMarker.exists())
+
+    finalizer.run()
+
+    // crashedLastRun should be set to true
+    assertTrue(SentryCrashLastRunState.getInstance().isCrashedLastRun(null, false)!!)
+    // Native crash marker should be deleted
+    assertFalse(nativeCrashMarker.exists())
+  }
 }