fix(sessions): Move and flush unfinished previous session on init

sentry/api/sentry.api

@@ -4346,13 +4346,15 @@ public class io/sentry/cache/EnvelopeCache : io/sentry/cache/IEnvelopeCache {
 	public static final field SUFFIX_ENVELOPE_FILE Ljava/lang/String;
 	protected static final field UTF_8 Ljava/nio/charset/Charset;
 	protected final field cacheLock Lio/sentry/util/AutoClosableReentrantLock;
+	protected final field sessionLock Lio/sentry/util/AutoClosableReentrantLock;
 	public fun <init> (Lio/sentry/SentryOptions;Ljava/lang/String;I)V
 	public static fun create (Lio/sentry/SentryOptions;)Lio/sentry/cache/IEnvelopeCache;
 	public fun discard (Lio/sentry/SentryEnvelope;)V
 	public fun flushPreviousSession ()V
 	public static fun getCurrentSessionFile (Ljava/lang/String;)Ljava/io/File;
 	public static fun getPreviousSessionFile (Ljava/lang/String;)Ljava/io/File;
 	public fun iterator ()Ljava/util/Iterator;
+	public fun movePreviousSession (Ljava/io/File;Ljava/io/File;)V
 	public fun store (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)V
 	public fun waitPreviousSessionFlush ()Z
 }

sentry/src/main/java/io/sentry/MovePreviousSession.java

@@ -0,0 +1,44 @@
+package io.sentry;
+
+import static io.sentry.SentryLevel.DEBUG;
+import static io.sentry.SentryLevel.INFO;
+
+import io.sentry.cache.EnvelopeCache;
+import io.sentry.cache.IEnvelopeCache;
+import java.io.File;
+import org.jetbrains.annotations.NotNull;
+
+final class MovePreviousSession implements Runnable {
+
+  private final @NotNull SentryOptions options;
+
+  MovePreviousSession(final @NotNull SentryOptions options) {
+    this.options = options;
+  }
+
+  @Override
+  public void run() {
+    final String cacheDirPath = options.getCacheDirPath();
+    if (cacheDirPath == null) {
+      options.getLogger().log(INFO, "Cache dir is not set, not moving the previous session.");
+      return;
+    }
+
+    if (!options.isEnableAutoSessionTracking()) {
+      options
+          .getLogger()
+          .log(DEBUG, "Session tracking is disabled, bailing from previous session mover.");
+      return;
+    }
+
+    final IEnvelopeCache cache = options.getEnvelopeDiskCache();
+    if (cache instanceof EnvelopeCache) {
+      final File currentSessionFile = EnvelopeCache.getCurrentSessionFile(cacheDirPath);
+      final File previousSessionFile = EnvelopeCache.getPreviousSessionFile(cacheDirPath);
+
+      ((EnvelopeCache) cache).movePreviousSession(currentSessionFile, previousSessionFile);
+
+      ((EnvelopeCache) cache).flushPreviousSession();
+    }
+  }
+}

sentry/src/main/java/io/sentry/Sentry.java

@@ -346,6 +346,8 @@ private static void init(final @NotNull SentryOptions options, final boolean glo
           options.setExecutorService(new SentryExecutorService(options));
           options.getExecutorService().prewarm();
         }
+
+        movePreviousSession(options);
         // when integrations are registered on Scopes ctor and async integrations are fired,
         // it might and actually happened that integrations called captureSomething
         // and Scopes was still NoOp.
@@ -497,6 +499,16 @@ private static void handleAppStartProfilingConfig(
     return options.getInternalTracesSampler().sample(appStartSamplingContext);
   }
 
+  @SuppressWarnings("FutureReturnValueIgnored")
+  private static void movePreviousSession(final @NotNull SentryOptions options) {
+    // enqueue a task to move previous unfinished session to its own file
+    try {
+      options.getExecutorService().submit(new MovePreviousSession(options));
+    } catch (Throwable e) {
+      options.getLogger().log(SentryLevel.DEBUG, "Failed to move previous session.", e);
+    }
+  }
+
   @SuppressWarnings("FutureReturnValueIgnored")
   private static void finalizePreviousSession(
       final @NotNull SentryOptions options, final @NotNull IScopes scopes) {

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

@@ -73,6 +73,7 @@ public class EnvelopeCache extends CacheStrategy implements IEnvelopeCache {
 
   private final @NotNull Map<SentryEnvelope, String> fileNameMap = new WeakHashMap<>();
   protected final @NotNull AutoClosableReentrantLock cacheLock = new AutoClosableReentrantLock();
+  protected final @NotNull AutoClosableReentrantLock sessionLock = new AutoClosableReentrantLock();
 
   public static @NotNull IEnvelopeCache create(final @NotNull SentryOptions options) {
     final String cacheDirPath = options.getCacheDirPath();
@@ -113,20 +114,7 @@ public void store(final @NotNull SentryEnvelope envelope, final @NotNull Hint hi
     }
 
     if (HintUtils.hasType(hint, SessionStart.class)) {
-      if (currentSessionFile.exists()) {
-        options.getLogger().log(WARNING, "Current session is not ended, we'd need to end it.");
-
-        try (final Reader reader =
-            new BufferedReader(
-                new InputStreamReader(new FileInputStream(currentSessionFile), UTF_8))) {
-          final Session session = serializer.getValue().deserialize(reader, Session.class);
-          if (session != null) {
-            writeSessionToDisk(previousSessionFile, session);
-          }
-        } catch (Throwable e) {
-          options.getLogger().log(SentryLevel.ERROR, "Error processing session.", e);
-        }
-      }
+      movePreviousSession(currentSessionFile, previousSessionFile);
       updateCurrentSession(currentSessionFile, envelope);
 
       boolean crashedLastRun = false;
@@ -316,17 +304,12 @@ private void writeEnvelopeToDisk(
   }
 
   private void writeSessionToDisk(final @NotNull File file, final @NotNull Session session) {
-    if (file.exists()) {
+    try (final OutputStream outputStream = new FileOutputStream(file);
+        final Writer writer = new BufferedWriter(new OutputStreamWriter(outputStream, UTF_8))) {
       options
           .getLogger()
           .log(DEBUG, "Overwriting session to offline storage: %s", session.getSessionId());
-      if (!file.delete()) {
-        options.getLogger().log(SentryLevel.ERROR, "Failed to delete: %s", file.getAbsolutePath());
-      }
-    }
 
-    try (final OutputStream outputStream = new FileOutputStream(file);
-        final Writer writer = new BufferedWriter(new OutputStreamWriter(outputStream, UTF_8))) {
       serializer.getValue().serialize(session, writer);
     } catch (Throwable e) {
       options
@@ -441,4 +424,29 @@ public boolean waitPreviousSessionFlush() {
   public void flushPreviousSession() {
     previousSessionLatch.countDown();
   }
+
+  public void movePreviousSession(
+      final @NotNull File currentSessionFile, final @NotNull File previousSessionFile) {
+    try (final @NotNull ISentryLifecycleToken ignored = sessionLock.acquire()) {
+      if (previousSessionFile.exists()) {
+        options.getLogger().log(DEBUG, "Previous session file already exists.");
+        return;
+      }
+
+      if (currentSessionFile.exists()) {
+        options.getLogger().log(INFO, "Moving current session to previous session.");
+
+        try {
+          final boolean renamed = currentSessionFile.renameTo(previousSessionFile);
+          if (!renamed) {
+            options.getLogger().log(WARNING, "Unable to move current session to previous session.");
+          }
+        } catch (Throwable e) {
+          options
+              .getLogger()
+              .log(SentryLevel.ERROR, "Error moving current session to previous session.", e);
+        }
+      }
+    }
+  }
 }

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

@@ -0,0 +1,161 @@
+package io.sentry
+
+import io.sentry.cache.EnvelopeCache
+import io.sentry.cache.IEnvelopeCache
+import io.sentry.transport.NoOpEnvelopeCache
+import java.nio.file.Files
+import java.nio.file.Path
+import kotlin.test.AfterTest
+import kotlin.test.BeforeTest
+import kotlin.test.Test
+import kotlin.test.assertFalse
+import kotlin.test.assertTrue
+import org.mockito.kotlin.any
+import org.mockito.kotlin.mock
+import org.mockito.kotlin.never
+import org.mockito.kotlin.verify
+
+class MovePreviousSessionTest {
+
+  private class Fixture {
+    val tempDir: Path = Files.createTempDirectory("sentry-move-session-test")
+    val options =
+      SentryOptions().apply {
+        isDebug = true
+        setLogger(SystemOutLogger())
+      }
+    val cache = mock<EnvelopeCache>()
+
+    fun getSUT(
+      cacheDirPath: String? = tempDir.toAbsolutePath().toFile().absolutePath,
+      isEnableSessionTracking: Boolean = true,
+      envelopeCache: IEnvelopeCache? = null,
+    ): MovePreviousSession {
+      options.cacheDirPath = cacheDirPath
+      options.isEnableAutoSessionTracking = isEnableSessionTracking
+      options.setEnvelopeDiskCache(envelopeCache ?: EnvelopeCache.create(options))
+      return MovePreviousSession(options)
+    }
+
+    fun cleanup() {
+      tempDir.toFile().deleteRecursively()
+    }
+  }
+
+  private lateinit var fixture: Fixture
+
+  @BeforeTest
+  fun setup() {
+    fixture = Fixture()
+  }
+
+  @AfterTest
+  fun teardown() {
+    fixture.cleanup()
+  }
+
+  @Test
+  fun `when cache dir is null, logs and returns early`() {
+    val sut = fixture.getSUT(cacheDirPath = null, envelopeCache = fixture.cache)
+
+    sut.run()
+
+    verify(fixture.cache, never()).movePreviousSession(any(), any())
+    verify(fixture.cache, never()).flushPreviousSession()
+  }
+
+  @Test
+  fun `when session tracking is disabled, logs and returns early`() {
+    val sut = fixture.getSUT(isEnableSessionTracking = false, envelopeCache = fixture.cache)
+
+    sut.run()
+
+    verify(fixture.cache, never()).movePreviousSession(any(), any())
+    verify(fixture.cache, never()).flushPreviousSession()
+  }
+
+  @Test
+  fun `when envelope cache is not EnvelopeCache instance, does nothing`() {
+    val sut = fixture.getSUT(envelopeCache = NoOpEnvelopeCache.getInstance())
+
+    sut.run()
+
+    verify(fixture.cache, never()).movePreviousSession(any(), any())
+    verify(fixture.cache, never()).flushPreviousSession()
+  }
+
+  @Test
+  fun `integration test with real EnvelopeCache`() {
+    val sut = fixture.getSUT()
+
+    // Create a current session file
+    val currentSessionFile = EnvelopeCache.getCurrentSessionFile(fixture.options.cacheDirPath!!)
+    val previousSessionFile = EnvelopeCache.getPreviousSessionFile(fixture.options.cacheDirPath!!)
+
+    currentSessionFile.createNewFile()
+    currentSessionFile.writeText("session content")
+
+    assertTrue(currentSessionFile.exists())
+    assertFalse(previousSessionFile.exists())
+
+    sut.run()
+
+    // Wait for flush to complete
+    (fixture.options.envelopeDiskCache as EnvelopeCache).waitPreviousSessionFlush()
+
+    // Current session file should have been moved to previous
+    assertFalse(currentSessionFile.exists())
+    assertTrue(previousSessionFile.exists())
+    assert(previousSessionFile.readText() == "session content")
+
+    fixture.cleanup()
+  }
+
+  @Test
+  fun `integration test when current session file does not exist`() {
+    val sut = fixture.getSUT()
+
+    val currentSessionFile = EnvelopeCache.getCurrentSessionFile(fixture.options.cacheDirPath!!)
+    val previousSessionFile = EnvelopeCache.getPreviousSessionFile(fixture.options.cacheDirPath!!)
+
+    assertFalse(currentSessionFile.exists())
+    assertFalse(previousSessionFile.exists())
+
+    sut.run()
+
+    (fixture.options.envelopeDiskCache as EnvelopeCache).waitPreviousSessionFlush()
+
+    assertFalse(currentSessionFile.exists())
+    assertFalse(previousSessionFile.exists())
+
+    fixture.cleanup()
+  }
+
+  @Test
+  fun `integration test when previous session file already exists`() {
+    val sut = fixture.getSUT()
+
+    val currentSessionFile = EnvelopeCache.getCurrentSessionFile(fixture.options.cacheDirPath!!)
+    val previousSessionFile = EnvelopeCache.getPreviousSessionFile(fixture.options.cacheDirPath!!)
+
+    currentSessionFile.createNewFile()
+    currentSessionFile.writeText("current session")
+    previousSessionFile.createNewFile()
+    previousSessionFile.writeText("previous session")
+
+    assertTrue(currentSessionFile.exists())
+    assertTrue(previousSessionFile.exists())
+
+    sut.run()
+
+    (fixture.options.envelopeDiskCache as EnvelopeCache).waitPreviousSessionFlush()
+
+    // Files should remain unchanged when previous already exists
+    assertTrue(currentSessionFile.exists())
+    assertTrue(previousSessionFile.exists())
+    assert(currentSessionFile.readText() == "current session")
+    assert(previousSessionFile.readText() == "previous session")
+
+    fixture.cleanup()
+  }
+}