Skip to content

Commit f636ca7

Browse files
romtsnromtsn
committed
fix(sessions): Move and flush unfinished previous session on init
1 parent 72b3637 commit f636ca7

File tree

7 files changed

+383
-28
lines changed

7 files changed

+383
-28
lines changed

‎sentry/api/sentry.api‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4346,13 +4346,15 @@ public class io/sentry/cache/EnvelopeCache : io/sentry/cache/IEnvelopeCache {
43464346
public static final field SUFFIX_ENVELOPE_FILE Ljava/lang/String;
43474347
protected static final field UTF_8 Ljava/nio/charset/Charset;
43484348
protected final field cacheLock Lio/sentry/util/AutoClosableReentrantLock;
4349+
protected final field sessionLock Lio/sentry/util/AutoClosableReentrantLock;
43494350
public fun <init> (Lio/sentry/SentryOptions;Ljava/lang/String;I)V
43504351
public static fun create (Lio/sentry/SentryOptions;)Lio/sentry/cache/IEnvelopeCache;
43514352
public fun discard (Lio/sentry/SentryEnvelope;)V
43524353
public fun flushPreviousSession ()V
43534354
public static fun getCurrentSessionFile (Ljava/lang/String;)Ljava/io/File;
43544355
public static fun getPreviousSessionFile (Ljava/lang/String;)Ljava/io/File;
43554356
public fun iterator ()Ljava/util/Iterator;
4357+
public fun movePreviousSession (Ljava/io/File;Ljava/io/File;)V
43564358
public fun store (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)V
43574359
public fun waitPreviousSessionFlush ()Z
43584360
}

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
package io.sentry;
2+
3+
import static io.sentry.SentryLevel.DEBUG;
4+
import static io.sentry.SentryLevel.INFO;
5+
6+
import io.sentry.cache.EnvelopeCache;
7+
import io.sentry.cache.IEnvelopeCache;
8+
import java.io.File;
9+
import org.jetbrains.annotations.NotNull;
10+
11+
final class MovePreviousSession implements Runnable {
12+
13+
private final @NotNull SentryOptions options;
14+
15+
MovePreviousSession(final @NotNull SentryOptions options) {
16+
this.options = options;
17+
}
18+
19+
@Override
20+
public void run() {
21+
final String cacheDirPath = options.getCacheDirPath();
22+
if (cacheDirPath == null) {
23+
options.getLogger().log(INFO, "Cache dir is not set, not moving the previous session.");
24+
return;
25+
}
26+
27+
if (!options.isEnableAutoSessionTracking()) {
28+
options
29+
.getLogger()
30+
.log(DEBUG, "Session tracking is disabled, bailing from previous session mover.");
31+
return;
32+
}
33+
34+
final IEnvelopeCache cache = options.getEnvelopeDiskCache();
35+
if (cache instanceof EnvelopeCache) {
36+
final File currentSessionFile = EnvelopeCache.getCurrentSessionFile(cacheDirPath);
37+
final File previousSessionFile = EnvelopeCache.getPreviousSessionFile(cacheDirPath);
38+
39+
((EnvelopeCache) cache).movePreviousSession(currentSessionFile, previousSessionFile);
40+
41+
((EnvelopeCache) cache).flushPreviousSession();
42+
}
43+
}
44+
}

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,8 @@ private static void init(final @NotNull SentryOptions options, final boolean glo
346346
options.setExecutorService(new SentryExecutorService(options));
347347
options.getExecutorService().prewarm();
348348
}
349+
350+
movePreviousSession(options);
349351
// when integrations are registered on Scopes ctor and async integrations are fired,
350352
// it might and actually happened that integrations called captureSomething
351353
// and Scopes was still NoOp.
@@ -497,6 +499,16 @@ private static void handleAppStartProfilingConfig(
497499
return options.getInternalTracesSampler().sample(appStartSamplingContext);
498500
}
499501

502+
@SuppressWarnings("FutureReturnValueIgnored")
503+
private static void movePreviousSession(final @NotNull SentryOptions options) {
504+
// enqueue a task to move previous unfinished session to its own file
505+
try {
506+
options.getExecutorService().submit(new MovePreviousSession(options));
507+
} catch (Throwable e) {
508+
options.getLogger().log(SentryLevel.DEBUG, "Failed to move previous session.", e);
509+
}
510+
}
511+
500512
@SuppressWarnings("FutureReturnValueIgnored")
501513
private static void finalizePreviousSession(
502514
final @NotNull SentryOptions options, final @NotNull IScopes scopes) {

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

Lines changed: 29 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ public class EnvelopeCache extends CacheStrategy implements IEnvelopeCache {
7373

7474
private final @NotNull Map<SentryEnvelope, String> fileNameMap = new WeakHashMap<>();
7575
protected final @NotNull AutoClosableReentrantLock cacheLock = new AutoClosableReentrantLock();
76+
protected final @NotNull AutoClosableReentrantLock sessionLock = new AutoClosableReentrantLock();
7677

7778
public static @NotNull IEnvelopeCache create(final @NotNull SentryOptions options) {
7879
final String cacheDirPath = options.getCacheDirPath();
@@ -113,20 +114,7 @@ public void store(final @NotNull SentryEnvelope envelope, final @NotNull Hint hi
113114
}
114115

115116
if (HintUtils.hasType(hint, SessionStart.class)) {
116-
if (currentSessionFile.exists()) {
117-
options.getLogger().log(WARNING, "Current session is not ended, we'd need to end it.");
118-
119-
try (final Reader reader =
120-
new BufferedReader(
121-
new InputStreamReader(new FileInputStream(currentSessionFile), UTF_8))) {
122-
final Session session = serializer.getValue().deserialize(reader, Session.class);
123-
if (session != null) {
124-
writeSessionToDisk(previousSessionFile, session);
125-
}
126-
} catch (Throwable e) {
127-
options.getLogger().log(SentryLevel.ERROR, "Error processing session.", e);
128-
}
129-
}
117+
movePreviousSession(currentSessionFile, previousSessionFile);
130118
updateCurrentSession(currentSessionFile, envelope);
131119

132120
boolean crashedLastRun = false;
@@ -316,17 +304,12 @@ private void writeEnvelopeToDisk(
316304
}
317305

318306
private void writeSessionToDisk(final @NotNull File file, final @NotNull Session session) {
319-
if (file.exists()) {
307+
try (final OutputStream outputStream = new FileOutputStream(file);
308+
final Writer writer = new BufferedWriter(new OutputStreamWriter(outputStream, UTF_8))) {
320309
options
321310
.getLogger()
322311
.log(DEBUG, "Overwriting session to offline storage: %s", session.getSessionId());
323-
if (!file.delete()) {
324-
options.getLogger().log(SentryLevel.ERROR, "Failed to delete: %s", file.getAbsolutePath());
325-
}
326-
}
327312

328-
try (final OutputStream outputStream = new FileOutputStream(file);
329-
final Writer writer = new BufferedWriter(new OutputStreamWriter(outputStream, UTF_8))) {
330313
serializer.getValue().serialize(session, writer);
331314
} catch (Throwable e) {
332315
options
@@ -441,4 +424,29 @@ public boolean waitPreviousSessionFlush() {
441424
public void flushPreviousSession() {
442425
previousSessionLatch.countDown();
443426
}
427+
428+
public void movePreviousSession(
429+
final @NotNull File currentSessionFile, final @NotNull File previousSessionFile) {
430+
try (final @NotNull ISentryLifecycleToken ignored = sessionLock.acquire()) {
431+
if (previousSessionFile.exists()) {
432+
options.getLogger().log(DEBUG, "Previous session file already exists.");
433+
return;
434+
}
435+
436+
if (currentSessionFile.exists()) {
437+
options.getLogger().log(INFO, "Moving current session to previous session.");
438+
439+
try {
440+
final boolean renamed = currentSessionFile.renameTo(previousSessionFile);
441+
if (!renamed) {
442+
options.getLogger().log(WARNING, "Unable to move current session to previous session.");
443+
}
444+
} catch (Throwable e) {
445+
options
446+
.getLogger()
447+
.log(SentryLevel.ERROR, "Error moving current session to previous session.", e);
448+
}
449+
}
450+
}
451+
}
444452
}

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

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
package io.sentry
2+
3+
import io.sentry.cache.EnvelopeCache
4+
import io.sentry.cache.IEnvelopeCache
5+
import io.sentry.transport.NoOpEnvelopeCache
6+
import java.nio.file.Files
7+
import java.nio.file.Path
8+
import kotlin.test.AfterTest
9+
import kotlin.test.BeforeTest
10+
import kotlin.test.Test
11+
import kotlin.test.assertFalse
12+
import kotlin.test.assertTrue
13+
import org.mockito.kotlin.any
14+
import org.mockito.kotlin.mock
15+
import org.mockito.kotlin.never
16+
import org.mockito.kotlin.verify
17+
18+
class MovePreviousSessionTest {
19+
20+
private class Fixture {
21+
val tempDir: Path = Files.createTempDirectory("sentry-move-session-test")
22+
val options =
23+
SentryOptions().apply {
24+
isDebug = true
25+
setLogger(SystemOutLogger())
26+
}
27+
val cache = mock<EnvelopeCache>()
28+
29+
fun getSUT(
30+
cacheDirPath: String? = tempDir.toAbsolutePath().toFile().absolutePath,
31+
isEnableSessionTracking: Boolean = true,
32+
envelopeCache: IEnvelopeCache? = null,
33+
): MovePreviousSession {
34+
options.cacheDirPath = cacheDirPath
35+
options.isEnableAutoSessionTracking = isEnableSessionTracking
36+
options.setEnvelopeDiskCache(envelopeCache ?: EnvelopeCache.create(options))
37+
return MovePreviousSession(options)
38+
}
39+
40+
fun cleanup() {
41+
tempDir.toFile().deleteRecursively()
42+
}
43+
}
44+
45+
private lateinit var fixture: Fixture
46+
47+
@BeforeTest
48+
fun setup() {
49+
fixture = Fixture()
50+
}
51+
52+
@AfterTest
53+
fun teardown() {
54+
fixture.cleanup()
55+
}
56+
57+
@Test
58+
fun `when cache dir is null, logs and returns early`() {
59+
val sut = fixture.getSUT(cacheDirPath = null, envelopeCache = fixture.cache)
60+
61+
sut.run()
62+
63+
verify(fixture.cache, never()).movePreviousSession(any(), any())
64+
verify(fixture.cache, never()).flushPreviousSession()
65+
}
66+
67+
@Test
68+
fun `when session tracking is disabled, logs and returns early`() {
69+
val sut = fixture.getSUT(isEnableSessionTracking = false, envelopeCache = fixture.cache)
70+
71+
sut.run()
72+
73+
verify(fixture.cache, never()).movePreviousSession(any(), any())
74+
verify(fixture.cache, never()).flushPreviousSession()
75+
}
76+
77+
@Test
78+
fun `when envelope cache is not EnvelopeCache instance, does nothing`() {
79+
val sut = fixture.getSUT(envelopeCache = NoOpEnvelopeCache.getInstance())
80+
81+
sut.run()
82+
83+
verify(fixture.cache, never()).movePreviousSession(any(), any())
84+
verify(fixture.cache, never()).flushPreviousSession()
85+
}
86+
87+
@Test
88+
fun `integration test with real EnvelopeCache`() {
89+
val sut = fixture.getSUT()
90+
91+
// Create a current session file
92+
val currentSessionFile = EnvelopeCache.getCurrentSessionFile(fixture.options.cacheDirPath!!)
93+
val previousSessionFile = EnvelopeCache.getPreviousSessionFile(fixture.options.cacheDirPath!!)
94+
95+
currentSessionFile.createNewFile()
96+
currentSessionFile.writeText("session content")
97+
98+
assertTrue(currentSessionFile.exists())
99+
assertFalse(previousSessionFile.exists())
100+
101+
sut.run()
102+
103+
// Wait for flush to complete
104+
(fixture.options.envelopeDiskCache as EnvelopeCache).waitPreviousSessionFlush()
105+
106+
// Current session file should have been moved to previous
107+
assertFalse(currentSessionFile.exists())
108+
assertTrue(previousSessionFile.exists())
109+
assert(previousSessionFile.readText() == "session content")
110+
111+
fixture.cleanup()
112+
}
113+
114+
@Test
115+
fun `integration test when current session file does not exist`() {
116+
val sut = fixture.getSUT()
117+
118+
val currentSessionFile = EnvelopeCache.getCurrentSessionFile(fixture.options.cacheDirPath!!)
119+
val previousSessionFile = EnvelopeCache.getPreviousSessionFile(fixture.options.cacheDirPath!!)
120+
121+
assertFalse(currentSessionFile.exists())
122+
assertFalse(previousSessionFile.exists())
123+
124+
sut.run()
125+
126+
(fixture.options.envelopeDiskCache as EnvelopeCache).waitPreviousSessionFlush()
127+
128+
assertFalse(currentSessionFile.exists())
129+
assertFalse(previousSessionFile.exists())
130+
131+
fixture.cleanup()
132+
}
133+
134+
@Test
135+
fun `integration test when previous session file already exists`() {
136+
val sut = fixture.getSUT()
137+
138+
val currentSessionFile = EnvelopeCache.getCurrentSessionFile(fixture.options.cacheDirPath!!)
139+
val previousSessionFile = EnvelopeCache.getPreviousSessionFile(fixture.options.cacheDirPath!!)
140+
141+
currentSessionFile.createNewFile()
142+
currentSessionFile.writeText("current session")
143+
previousSessionFile.createNewFile()
144+
previousSessionFile.writeText("previous session")
145+
146+
assertTrue(currentSessionFile.exists())
147+
assertTrue(previousSessionFile.exists())
148+
149+
sut.run()
150+
151+
(fixture.options.envelopeDiskCache as EnvelopeCache).waitPreviousSessionFlush()
152+
153+
// Files should remain unchanged when previous already exists
154+
assertTrue(currentSessionFile.exists())
155+
assertTrue(previousSessionFile.exists())
156+
assert(currentSessionFile.readText() == "current session")
157+
assert(previousSessionFile.readText() == "previous session")
158+
159+
fixture.cleanup()
160+
}
161+
}

0 commit comments

Comments
 (0)