diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 8a810918c0..e898e2cd59 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -4862,6 +4862,7 @@ public class io/sentry/cache/EnvelopeCache : io/sentry/cache/IEnvelopeCache { 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 persistCurrentSession (Lio/sentry/Session;)V public fun store (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)V public fun storeEnvelope (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)Z public fun waitPreviousSessionFlush ()Z diff --git a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java index 618de65547..3c991134e4 100644 --- a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java +++ b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java @@ -106,6 +106,7 @@ public boolean storeEnvelope(final @NotNull SentryEnvelope envelope, final @NotN return storeInternal(envelope, hint); } + @SuppressWarnings("JavaUtilDate") private boolean storeInternal(final @NotNull SentryEnvelope envelope, final @NotNull Hint hint) { Objects.requireNonNull(envelope, "Envelope is required."); @@ -118,8 +119,22 @@ private boolean storeInternal(final @NotNull SentryEnvelope envelope, final @Not final File previousSessionFile = getPreviousSessionFile(directoryPath); if (HintUtils.hasType(hint, SessionEnd.class)) { - if (!currentSessionFile.delete()) { - options.getLogger().log(WARNING, "Current envelope doesn't exist."); + try (final @NotNull ISentryLifecycleToken ignored = sessionLock.acquire()) { + final @Nullable Session endingSession = readSessionFromEnvelope(envelope); + final @Nullable Session currentSession = readSessionFromDisk(currentSessionFile); + final boolean preserveCurrentSession = + endingSession != null + && currentSession != null + && currentSession.hasNonTerminatingUnhandledError() + && endingSession.getSessionId() != null + && currentSession.getSessionId() != null + && !Objects.equals(endingSession.getSessionId(), currentSession.getSessionId()) + && endingSession.getStarted() != null + && currentSession.getStarted() != null + && currentSession.getStarted().after(endingSession.getStarted()); + if (!preserveCurrentSession && !currentSessionFile.delete()) { + options.getLogger().log(WARNING, "Current envelope doesn't exist."); + } } } @@ -129,8 +144,22 @@ private boolean storeInternal(final @NotNull SentryEnvelope envelope, final @Not } if (HintUtils.hasType(hint, SessionStart.class)) { - movePreviousSession(currentSessionFile, previousSessionFile); - updateCurrentSession(currentSessionFile, envelope); + try (final @NotNull ISentryLifecycleToken ignored = sessionLock.acquire()) { + final @Nullable Session startingSession = readSessionFromEnvelope(envelope); + if (startingSession != null) { + final @Nullable Session currentSession = readSessionFromDisk(currentSessionFile); + if (currentSession != null && hasSameSessionId(currentSession, startingSession)) { + if (!isNewerUnhandledOrErrorSnapshot(currentSession, startingSession)) { + writeSessionToDisk(currentSessionFile, startingSession); + } + } else { + movePreviousSession(currentSessionFile, previousSessionFile); + writeSessionToDisk(currentSessionFile, startingSession); + } + } else { + movePreviousSession(currentSessionFile, previousSessionFile); + } + } boolean crashedLastRun = false; final File crashMarkerFile = new File(options.getCacheDirPath(), NATIVE_CRASH_MARKER_FILE); @@ -274,8 +303,7 @@ private void writeCrashMarkerFile() { } } - private void updateCurrentSession( - final @NotNull File currentSessionFile, final @NotNull SentryEnvelope envelope) { + private @Nullable Session readSessionFromEnvelope(final @NotNull SentryEnvelope envelope) { final Iterable items = envelope.getItems(); // we know that an envelope with a SessionStart hint has a single item inside @@ -295,7 +323,7 @@ private void updateCurrentSession( "Item of type %s returned null by the parser.", item.getHeader().getType()); } else { - writeSessionToDisk(currentSessionFile, session); + return session; } } catch (Throwable e) { options.getLogger().log(ERROR, "Item failed to process.", e); @@ -309,10 +337,36 @@ private void updateCurrentSession( item.getHeader().getType()); } } else { - options - .getLogger() - .log(INFO, "Current envelope %s is empty", currentSessionFile.getAbsolutePath()); + options.getLogger().log(INFO, "Current envelope is empty."); } + return null; + } + + private @Nullable Session readSessionFromDisk(final @NotNull File sessionFile) { + if (!sessionFile.exists()) { + return null; + } + try (final Reader reader = + new BufferedReader(new InputStreamReader(new FileInputStream(sessionFile), UTF_8))) { + return serializer.getValue().deserialize(reader, Session.class); + } catch (Exception e) { + options.getLogger().log(ERROR, "Failed to read session from disk.", e); + return null; + } + } + + private boolean isNewerUnhandledOrErrorSnapshot( + final @NotNull Session currentSession, final @NotNull Session startingSession) { + return (currentSession.hasNonTerminatingUnhandledError() + && !startingSession.hasNonTerminatingUnhandledError()) + || currentSession.errorCount() > startingSession.errorCount(); + } + + private boolean hasSameSessionId( + final @NotNull Session firstSession, final @NotNull Session secondSession) { + return firstSession.getSessionId() != null + && secondSession.getSessionId() != null + && Objects.equals(firstSession.getSessionId(), secondSession.getSessionId()); } private boolean writeEnvelopeToDisk( @@ -352,6 +406,13 @@ private void writeSessionToDisk(final @NotNull File file, final @NotNull Session } } + @ApiStatus.Internal + public void persistCurrentSession(final @NotNull Session session) { + try (final @NotNull ISentryLifecycleToken ignored = sessionLock.acquire()) { + writeSessionToDisk(getCurrentSessionFile(directory.getOrCreate().getAbsolutePath()), session); + } + } + @Override public void discard(final @NotNull SentryEnvelope envelope) { Objects.requireNonNull(envelope, "Envelope is required."); diff --git a/sentry/src/test/java/io/sentry/cache/EnvelopeCacheTest.kt b/sentry/src/test/java/io/sentry/cache/EnvelopeCacheTest.kt index dda06ee7e6..00102fdf4b 100644 --- a/sentry/src/test/java/io/sentry/cache/EnvelopeCacheTest.kt +++ b/sentry/src/test/java/io/sentry/cache/EnvelopeCacheTest.kt @@ -1,5 +1,6 @@ package io.sentry.cache +import com.google.common.truth.Truth.assertThat import io.sentry.DateUtils import io.sentry.Hint import io.sentry.ILogger @@ -160,6 +161,213 @@ class EnvelopeCacheTest { assertTrue(didStore) } + @Test + fun `delayed same SID SessionStart preserves newer unhandled snapshot`() { + val cache = fixture.getSUT() + val sid = SentryUUID.generateSentryId() + val currentSessionFile = EnvelopeCache.getCurrentSessionFile(fixture.options.cacheDirPath!!) + val previousSessionFile = EnvelopeCache.getPreviousSessionFile(fixture.options.cacheDirPath!!) + val newerSession = createSession(sessionId = sid) + newerSession.recordNonTerminatingUnhandledError() + cache.persistCurrentSession(newerSession) + + val delayedStart = createSession(sessionId = sid) + val envelope = SentryEnvelope.from(fixture.options.serializer, delayedStart, null) + cache.storeEnvelope(envelope, HintUtils.createWithTypeCheckHint(SessionStartHint())) + + val persistedSession = + fixture.options.serializer.deserialize( + currentSessionFile.bufferedReader(), + Session::class.java, + )!! + assertThat(persistedSession.sessionId).isEqualTo(sid) + assertThat(persistedSession.hasNonTerminatingUnhandledError()).isTrue() + assertThat(persistedSession.errorCount()).isEqualTo(1) + assertThat(previousSessionFile.exists()).isFalse() + } + + @Test + fun `delayed same SID SessionStart preserves newer error count snapshot`() { + val cache = fixture.getSUT() + val sid = SentryUUID.generateSentryId() + val currentSessionFile = EnvelopeCache.getCurrentSessionFile(fixture.options.cacheDirPath!!) + val previousSessionFile = EnvelopeCache.getPreviousSessionFile(fixture.options.cacheDirPath!!) + val newerSession = createSession(sessionId = sid) + newerSession.update(null, null, true) + cache.persistCurrentSession(newerSession) + + val delayedStart = createSession(sessionId = sid) + val envelope = SentryEnvelope.from(fixture.options.serializer, delayedStart, null) + cache.storeEnvelope(envelope, HintUtils.createWithTypeCheckHint(SessionStartHint())) + + val persistedSession = + fixture.options.serializer.deserialize( + currentSessionFile.bufferedReader(), + Session::class.java, + )!! + assertThat(persistedSession.sessionId).isEqualTo(sid) + assertThat(persistedSession.hasNonTerminatingUnhandledError()).isFalse() + assertThat(persistedSession.errorCount()).isEqualTo(1) + assertThat(previousSessionFile.exists()).isFalse() + } + + @Test + fun `null SIDs on SessionStart rotate instead of preserving as same session`() { + val cache = fixture.getSUT() + val currentSession = createSession(sessionId = null) + currentSession.update(null, null, true) + cache.persistCurrentSession(currentSession) + val startingSession = createSession(sessionId = null) + + val envelope = SentryEnvelope.from(fixture.options.serializer, startingSession, null) + cache.storeEnvelope(envelope, HintUtils.createWithTypeCheckHint(SessionStartHint())) + + val currentSessionFile = EnvelopeCache.getCurrentSessionFile(fixture.options.cacheDirPath!!) + val previousSessionFile = EnvelopeCache.getPreviousSessionFile(fixture.options.cacheDirPath!!) + val persistedCurrent = + fixture.options.serializer.deserialize( + currentSessionFile.bufferedReader(), + Session::class.java, + )!! + val persistedPrevious = + fixture.options.serializer.deserialize( + previousSessionFile.bufferedReader(), + Session::class.java, + )!! + assertThat(persistedCurrent.sessionId).isNull() + assertThat(persistedCurrent.errorCount()).isEqualTo(0) + assertThat(persistedPrevious.sessionId).isNull() + assertThat(persistedPrevious.errorCount()).isEqualTo(1) + } + + @Test + fun `different SID SessionStart rotates current session`() { + val cache = fixture.getSUT() + val currentSession = createSession() + cache.persistCurrentSession(currentSession) + val nextSession = createSession() + + val envelope = SentryEnvelope.from(fixture.options.serializer, nextSession, null) + cache.storeEnvelope(envelope, HintUtils.createWithTypeCheckHint(SessionStartHint())) + + val currentSessionFile = EnvelopeCache.getCurrentSessionFile(fixture.options.cacheDirPath!!) + val previousSessionFile = EnvelopeCache.getPreviousSessionFile(fixture.options.cacheDirPath!!) + val persistedCurrent = + fixture.options.serializer.deserialize( + currentSessionFile.bufferedReader(), + Session::class.java, + )!! + val persistedPrevious = + fixture.options.serializer.deserialize( + previousSessionFile.bufferedReader(), + Session::class.java, + )!! + assertThat(persistedCurrent.sessionId).isEqualTo(nextSession.sessionId) + assertThat(persistedPrevious.sessionId).isEqualTo(currentSession.sessionId) + } + + @Test + fun `matching SessionEnd deletes current session`() { + val cache = fixture.getSUT() + val session = createSession() + cache.persistCurrentSession(session) + + val envelope = SentryEnvelope.from(fixture.options.serializer, session, null) + cache.storeEnvelope(envelope, HintUtils.createWithTypeCheckHint(SessionEndHint())) + + assertThat(EnvelopeCache.getCurrentSessionFile(fixture.options.cacheDirPath!!).exists()) + .isFalse() + } + + @Test + fun `mismatching SessionEnd preserves newer unhandled current session`() { + val cache = fixture.getSUT() + val endingSession = createSession(started = Date(1_000)) + val currentSession = createSession(started = Date(2_000)) + currentSession.recordNonTerminatingUnhandledError() + cache.persistCurrentSession(currentSession) + + val envelope = SentryEnvelope.from(fixture.options.serializer, endingSession, null) + cache.storeEnvelope(envelope, HintUtils.createWithTypeCheckHint(SessionEndHint())) + + val currentSessionFile = EnvelopeCache.getCurrentSessionFile(fixture.options.cacheDirPath!!) + val persistedSession = + fixture.options.serializer.deserialize( + currentSessionFile.bufferedReader(), + Session::class.java, + )!! + assertThat(persistedSession.sessionId).isEqualTo(currentSession.sessionId) + } + + @Test + fun `mismatching newer SessionEnd deletes stale unhandled current session`() { + val cache = fixture.getSUT() + val currentSession = createSession(started = Date(1_000)) + currentSession.recordNonTerminatingUnhandledError() + cache.persistCurrentSession(currentSession) + val endingSession = createSession(started = Date(2_000)) + + val envelope = SentryEnvelope.from(fixture.options.serializer, endingSession, null) + cache.storeEnvelope(envelope, HintUtils.createWithTypeCheckHint(SessionEndHint())) + + assertThat(EnvelopeCache.getCurrentSessionFile(fixture.options.cacheDirPath!!).exists()) + .isFalse() + } + + @Test + fun `mismatching SessionEnd deletes current session without unhandled error`() { + val cache = fixture.getSUT() + val currentSession = createSession() + cache.persistCurrentSession(currentSession) + val endingSession = createSession() + + val envelope = SentryEnvelope.from(fixture.options.serializer, endingSession, null) + cache.storeEnvelope(envelope, HintUtils.createWithTypeCheckHint(SessionEndHint())) + + assertThat(EnvelopeCache.getCurrentSessionFile(fixture.options.cacheDirPath!!).exists()) + .isFalse() + } + + @Test + fun `null SIDs on SessionEnd delete current session`() { + val cache = fixture.getSUT() + val currentSession = createSession(sessionId = null) + cache.persistCurrentSession(currentSession) + val endingSession = createSession(sessionId = null) + + val envelope = SentryEnvelope.from(fixture.options.serializer, endingSession, null) + cache.storeEnvelope(envelope, HintUtils.createWithTypeCheckHint(SessionEndHint())) + + assertThat(EnvelopeCache.getCurrentSessionFile(fixture.options.cacheDirPath!!).exists()) + .isFalse() + } + + @Test + fun `malformed SessionEnd deletes current session`() { + val cache = fixture.getSUT() + val currentSession = createSession() + cache.persistCurrentSession(currentSession) + + val envelope = SentryEnvelope(null, null, emptyList()) + cache.storeEnvelope(envelope, HintUtils.createWithTypeCheckHint(SessionEndHint())) + + assertThat(EnvelopeCache.getCurrentSessionFile(fixture.options.cacheDirPath!!).exists()) + .isFalse() + } + + @Test + fun `unreadable current session on SessionEnd is deleted`() { + val cache = fixture.getSUT() + val currentSessionFile = EnvelopeCache.getCurrentSessionFile(fixture.options.cacheDirPath!!) + currentSessionFile.writeText("not-a-session") + + val endingSession = createSession() + val envelope = SentryEnvelope.from(fixture.options.serializer, endingSession, null) + cache.storeEnvelope(envelope, HintUtils.createWithTypeCheckHint(SessionEndHint())) + + assertThat(currentSessionFile.exists()).isFalse() + } + @Test fun `updates current file on session update and read it back`() { val cache = fixture.getSUT() @@ -346,6 +554,36 @@ class EnvelopeCacheTest { assertEquals(sessionExitedWithAbnormal, updatedSession!!.timestamp!!.time) } + @Test + fun `AbnormalExit hint keeps persisted unhandled session as abnormal`() { + val cache = fixture.getSUT() + + val previousSessionFile = EnvelopeCache.getPreviousSessionFile(fixture.options.cacheDirPath!!) + val session = createSession().apply { setNonTerminatingUnhandledError(true) } + fixture.options.serializer.serialize(session, previousSessionFile.bufferedWriter()) + + val envelope = SentryEnvelope.from(fixture.options.serializer, SentryEvent(), null) + val abnormalHint = + object : AbnormalExit { + override fun mechanism(): String = "abnormal_mechanism" + + override fun ignoreCurrentThread(): Boolean = false + + override fun timestamp(): Long = session.started!!.time + TimeUnit.HOURS.toMillis(1) + } + val hints = HintUtils.createWithTypeCheckHint(abnormalHint) + cache.storeEnvelope(envelope, hints) + + val updatedSession = + fixture.options.serializer.deserialize( + previousSessionFile.bufferedReader(), + Session::class.java, + ) + assertEquals(State.Abnormal, updatedSession!!.status) + assertEquals("abnormal_mechanism", updatedSession.abnormalMechanism) + assertTrue(updatedSession.hasNonTerminatingUnhandledError()) + } + @Test fun `when AbnormalExit happened before previous session start, does not mark as abnormal`() { val cache = fixture.getSUT() @@ -400,6 +638,29 @@ class EnvelopeCacheTest { assertEquals(nativeCrashTimestamp, updatedSession.timestamp!!.time) } + @Test + fun `NativeCrashExit hint keeps persisted unhandled session as crashed`() { + val cache = fixture.getSUT() + + val previousSessionFile = EnvelopeCache.getPreviousSessionFile(fixture.options.cacheDirPath!!) + val session = createSession().apply { setNonTerminatingUnhandledError(true) } + fixture.options.serializer.serialize(session, previousSessionFile.bufferedWriter()) + + val nativeCrashTimestamp = session.started!!.time + TimeUnit.HOURS.toMillis(1) + val envelope = SentryEnvelope.from(fixture.options.serializer, SentryEvent(), null) + val hints = HintUtils.createWithTypeCheckHint(NativeCrashExit { nativeCrashTimestamp }) + cache.storeEnvelope(envelope, hints) + + val updatedSession = + fixture.options.serializer.deserialize( + previousSessionFile.bufferedReader(), + Session::class.java, + ) + assertEquals(State.Crashed, updatedSession!!.status) + assertEquals(nativeCrashTimestamp, updatedSession.timestamp!!.time) + assertFalse(updatedSession.hasNonTerminatingUnhandledError()) + } + @Test fun `when NativeCrashExit happened before previous session start, does not mark as crashed`() { val cache = fixture.getSUT() @@ -438,14 +699,17 @@ class EnvelopeCacheTest { assertFalse(didStore) } - private fun createSession(started: Date? = null): Session = + private fun createSession( + started: Date? = null, + sessionId: String? = SentryUUID.generateSentryId(), + ): Session = Session( Ok, started ?: DateUtils.getCurrentDateTime(), DateUtils.getCurrentDateTime(), 0, "dis", - SentryUUID.generateSentryId(), + sessionId, true, null, null,