From 06f0d7e9090cfaa68a52701832f001183fa05319 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 23 Mar 2023 12:31:44 +0100 Subject: [PATCH 01/16] Mark previous session as abnormal if an ANR has happened --- .../sentry/android/core/AnrV2Integration.java | 102 +++++++++++------ .../core/cache/AndroidEnvelopeCache.java | 14 ++- .../core/cache/AndroidEnvelopeCacheTest.kt | 4 +- sentry/src/main/java/io/sentry/Hub.java | 3 +- .../src/main/java/io/sentry/OutboxSender.java | 2 + .../java/io/sentry/cache/EnvelopeCache.java | 107 +++++++++++++++--- .../java/io/sentry/hints/AbnormalExit.java | 6 + .../io/sentry/hints/AllSessionsEndHint.java | 5 + .../io/sentry/hints/PreviousSessionEnd.java | 5 + .../sentry/hints/PreviousSessionEndHint.java | 4 + .../java/io/sentry/cache/EnvelopeCacheTest.kt | 8 +- 11 files changed, 198 insertions(+), 62 deletions(-) create mode 100644 sentry/src/main/java/io/sentry/hints/AllSessionsEndHint.java create mode 100644 sentry/src/main/java/io/sentry/hints/PreviousSessionEnd.java create mode 100644 sentry/src/main/java/io/sentry/hints/PreviousSessionEndHint.java diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java index e62f4088b3..640b0756da 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java @@ -14,9 +14,14 @@ import io.sentry.SentryLevel; import io.sentry.SentryOptions; import io.sentry.android.core.cache.AndroidEnvelopeCache; +import io.sentry.cache.EnvelopeCache; +import io.sentry.cache.IEnvelopeCache; import io.sentry.exception.ExceptionMechanismException; +import io.sentry.hints.AbnormalExit; import io.sentry.hints.Backfillable; import io.sentry.hints.BlockingFlushHint; +import io.sentry.hints.PreviousSessionEnd; +import io.sentry.hints.PreviousSessionEndHint; import io.sentry.protocol.Mechanism; import io.sentry.protocol.SentryId; import io.sentry.transport.CurrentDateProvider; @@ -75,26 +80,14 @@ public void register(@NotNull IHub hub, @NotNull SentryOptions options) { } if (this.options.isAnrEnabled()) { - final ActivityManager activityManager = - (ActivityManager) context.getSystemService(Context.ACTIVITY_SERVICE); - - List applicationExitInfoList = - activityManager.getHistoricalProcessExitReasons(null, 0, 0); - - if (applicationExitInfoList.size() != 0) { - options - .getExecutorService() - .submit( - new AnrProcessor( - new ArrayList<>( - applicationExitInfoList), // just making a deep copy to be safe, as we're - // modifying the list - hub, - this.options, - dateProvider)); - } else { - options.getLogger().log(SentryLevel.DEBUG, "No records in historical exit reasons."); - } + options + .getExecutorService() + .submit( + new AnrProcessor( + context, + hub, + this.options, + dateProvider)); options.getLogger().log(SentryLevel.DEBUG, "AnrV2Integration installed."); addIntegrationToSdkVersion(); } @@ -109,17 +102,17 @@ public void close() throws IOException { static class AnrProcessor implements Runnable { - final @NotNull List exitInfos; + private final @NotNull Context context; private final @NotNull IHub hub; private final @NotNull SentryAndroidOptions options; private final long threshold; AnrProcessor( - final @NotNull List exitInfos, + final @NotNull Context context, final @NotNull IHub hub, final @NotNull SentryAndroidOptions options, final @NotNull ICurrentDateProvider dateProvider) { - this.exitInfos = exitInfos; + this.context = context; this.hub = hub; this.options = options; this.threshold = dateProvider.getCurrentTimeMillis() - NINETY_DAYS_THRESHOLD; @@ -128,7 +121,20 @@ static class AnrProcessor implements Runnable { @SuppressLint("NewApi") // we check this in AnrIntegrationFactory @Override public void run() { - final long lastReportedAnrTimestamp = AndroidEnvelopeCache.lastReportedAnr(options); + final ActivityManager activityManager = + (ActivityManager) context.getSystemService(Context.ACTIVITY_SERVICE); + + List applicationExitInfoList = + activityManager.getHistoricalProcessExitReasons(null, 0, 0); + if (applicationExitInfoList.size() == 0) { + endPreviousSession(); + options.getLogger().log(SentryLevel.DEBUG, "No records in historical exit reasons."); + return; + } + + // making a deep copy as we're modifying the list + final List exitInfos = new ArrayList<>(applicationExitInfoList); + final @Nullable Long lastReportedAnrTimestamp = AndroidEnvelopeCache.lastReportedAnr(options); // search for the latest ANR to report it separately as we're gonna enrich it. The latest // ANR will be first in the list, as it's filled last-to-first in order of appearance @@ -143,6 +149,7 @@ public void run() { } if (latestAnr == null) { + endPreviousSession(); options .getLogger() .log(SentryLevel.DEBUG, "No ANRs have been found in the historical exit reasons list."); @@ -150,13 +157,15 @@ public void run() { } if (latestAnr.getTimestamp() < threshold) { + endPreviousSession(); options .getLogger() .log(SentryLevel.DEBUG, "Latest ANR happened too long ago, returning early."); return; } - if (latestAnr.getTimestamp() <= lastReportedAnrTimestamp) { + if (lastReportedAnrTimestamp != null && latestAnr.getTimestamp() <= lastReportedAnrTimestamp) { + endPreviousSession(); options .getLogger() .log(SentryLevel.DEBUG, "Latest ANR has already been reported, returning early."); @@ -172,7 +181,7 @@ public void run() { } private void reportNonEnrichedHistoricalAnrs( - final @NotNull List exitInfos, final long lastReportedAnr) { + final @NotNull List exitInfos, final @Nullable Long lastReportedAnr) { // we reverse the list, because the OS puts errors in order of appearance, last-to-first // and we want to write a marker file after each ANR has been processed, so in case the app // gets killed meanwhile, we can proceed from the last reported ANR and not process the entire @@ -187,7 +196,7 @@ private void reportNonEnrichedHistoricalAnrs( continue; } - if (applicationExitInfo.getTimestamp() <= lastReportedAnr) { + if (lastReportedAnr != null && applicationExitInfo.getTimestamp() <= lastReportedAnr) { options .getLogger() .log(SentryLevel.DEBUG, "ANR has already been reported %s.", applicationExitInfo); @@ -202,10 +211,12 @@ private void reportNonEnrichedHistoricalAnrs( private void reportAsSentryEvent( final @NotNull ApplicationExitInfo exitInfo, final boolean shouldEnrich) { final long anrTimestamp = exitInfo.getTimestamp(); - final Throwable anrThrowable = buildAnrThrowable(exitInfo); + final boolean isBackground = + exitInfo.getImportance() != ActivityManager.RunningAppProcessInfo.IMPORTANCE_FOREGROUND; + final Throwable anrThrowable = buildAnrThrowable(exitInfo, isBackground); final AnrV2Hint anrHint = new AnrV2Hint( - options.getFlushTimeoutMillis(), options.getLogger(), anrTimestamp, shouldEnrich); + options.getFlushTimeoutMillis(), options.getLogger(), anrTimestamp, shouldEnrich, isBackground); final Hint hint = HintUtils.createWithTypeCheckHint(anrHint); @@ -228,10 +239,10 @@ private void reportAsSentryEvent( } } - private @NotNull Throwable buildAnrThrowable(final @NotNull ApplicationExitInfo exitInfo) { - final boolean isBackground = - exitInfo.getImportance() != ActivityManager.RunningAppProcessInfo.IMPORTANCE_FOREGROUND; - + private @NotNull Throwable buildAnrThrowable( + final @NotNull ApplicationExitInfo exitInfo, + final boolean isBackground + ) { String message = "ANR"; if (isBackground) { message = "Background " + message; @@ -245,26 +256,40 @@ private void reportAsSentryEvent( mechanism.setType("ANRv2"); return new ExceptionMechanismException(mechanism, error, error.getThread(), true); } + + private void endPreviousSession() { + final IEnvelopeCache envelopeCache = options.getEnvelopeDiskCache(); + if (envelopeCache instanceof EnvelopeCache) { + final Hint hint = HintUtils.createWithTypeCheckHint(new PreviousSessionEndHint()); + ((EnvelopeCache) envelopeCache).endPreviousSession(hint); + } + } } @ApiStatus.Internal - public static final class AnrV2Hint extends BlockingFlushHint implements Backfillable { + public static final class AnrV2Hint extends BlockingFlushHint implements Backfillable, + AbnormalExit, PreviousSessionEnd { private final long timestamp; private final boolean shouldEnrich; + private final boolean isBackgroundAnr; + public AnrV2Hint( final long flushTimeoutMillis, final @NotNull ILogger logger, final long timestamp, - final boolean shouldEnrich) { + final boolean shouldEnrich, + final boolean isBackgroundAnr) { super(flushTimeoutMillis, logger); this.timestamp = timestamp; this.shouldEnrich = shouldEnrich; + this.isBackgroundAnr = isBackgroundAnr; } - public long timestamp() { + @Override + public Long timestamp() { return timestamp; } @@ -272,5 +297,10 @@ public long timestamp() { public boolean shouldEnrich() { return shouldEnrich; } + + @Override + public String mechanism() { + return isBackgroundAnr ? "anr_background" : "anr_foreground"; + } } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java b/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java index 18f8bd05fa..102d8f5688 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java @@ -13,6 +13,7 @@ import io.sentry.android.core.SentryAndroidOptions; import io.sentry.android.core.internal.util.AndroidCurrentDateProvider; import io.sentry.cache.EnvelopeCache; +import io.sentry.hints.AbnormalExit; import io.sentry.transport.ICurrentDateProvider; import io.sentry.util.FileUtils; import io.sentry.util.HintUtils; @@ -22,6 +23,7 @@ import java.io.OutputStream; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; @ApiStatus.Internal @@ -70,7 +72,7 @@ public void store(@NotNull SentryEnvelope envelope, @NotNull Hint hint) { hint, AnrV2Integration.AnrV2Hint.class, (anrHint) -> { - final long timestamp = anrHint.timestamp(); + final @Nullable Long timestamp = anrHint.timestamp(); options .getLogger() .log( @@ -78,7 +80,7 @@ public void store(@NotNull SentryEnvelope envelope, @NotNull Hint hint) { "Writing last reported ANR marker with timestamp %d", timestamp); - writeLastReportedAnrMarker(anrHint.timestamp()); + writeLastReportedAnrMarker(timestamp); }); } @@ -136,7 +138,7 @@ public static boolean hasStartupCrashMarker(final @NotNull SentryOptions options return false; } - public static long lastReportedAnr(final @NotNull SentryOptions options) { + public static @Nullable Long lastReportedAnr(final @NotNull SentryOptions options) { final String cacheDirPath = Objects.requireNonNull( options.getCacheDirPath(), "Cache dir path should be set for getting ANRs reported"); @@ -147,7 +149,7 @@ public static long lastReportedAnr(final @NotNull SentryOptions options) { final String content = FileUtils.readText(lastAnrMarker); // we wrapped into try-catch already //noinspection ConstantConditions - return Long.parseLong(content.trim()); + return content.equals("null") ? null : Long.parseLong(content.trim()); } else { options .getLogger() @@ -156,10 +158,10 @@ public static long lastReportedAnr(final @NotNull SentryOptions options) { } catch (Throwable e) { options.getLogger().log(ERROR, "Error reading last ANR marker", e); } - return 0L; + return null; } - private void writeLastReportedAnrMarker(final long timestamp) { + private void writeLastReportedAnrMarker(final @Nullable Long timestamp) { final String cacheDirPath = options.getCacheDirPath(); if (cacheDirPath == null) { options.getLogger().log(DEBUG, "Cache dir path is null, the ANR marker will not be written"); diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt index 3eee455d98..7eb8e0baab 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt @@ -136,7 +136,7 @@ class AndroidEnvelopeCacheTest { 0, NoOpLogger.getInstance(), 12345678L, - false + false, false ) ) cache.store(fixture.envelope, hints) @@ -153,7 +153,7 @@ class AndroidEnvelopeCacheTest { 0, NoOpLogger.getInstance(), 12345678L, - false + false, false ) ) cache.store(fixture.envelope, hints) diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index 6d485a6b15..4e44f5f115 100644 --- a/sentry/src/main/java/io/sentry/Hub.java +++ b/sentry/src/main/java/io/sentry/Hub.java @@ -2,6 +2,7 @@ import io.sentry.Stack.StackItem; import io.sentry.clientreport.DiscardReason; +import io.sentry.hints.AllSessionsEndHint; import io.sentry.hints.SessionEndHint; import io.sentry.hints.SessionStartHint; import io.sentry.protocol.SentryId; @@ -319,7 +320,7 @@ public void endSession() { final StackItem item = this.stack.peek(); final Session previousSession = item.getScope().endSession(); if (previousSession != null) { - final Hint hint = HintUtils.createWithTypeCheckHint(new SessionEndHint()); + final Hint hint = HintUtils.createWithTypeCheckHint(new AllSessionsEndHint()); item.getClient().captureSession(previousSession, hint); } diff --git a/sentry/src/main/java/io/sentry/OutboxSender.java b/sentry/src/main/java/io/sentry/OutboxSender.java index fcf21fb121..9e704f0a0e 100644 --- a/sentry/src/main/java/io/sentry/OutboxSender.java +++ b/sentry/src/main/java/io/sentry/OutboxSender.java @@ -2,6 +2,7 @@ import static io.sentry.SentryLevel.ERROR; import static io.sentry.cache.EnvelopeCache.PREFIX_CURRENT_SESSION_FILE; +import static io.sentry.cache.EnvelopeCache.PREFIX_PREVIOUS_SESSION_FILE; import io.sentry.cache.EnvelopeCache; import io.sentry.hints.Flushable; @@ -99,6 +100,7 @@ protected boolean isRelevantFileName(final @Nullable String fileName) { // ignore current.envelope return fileName != null && !fileName.startsWith(PREFIX_CURRENT_SESSION_FILE) + && !fileName.startsWith(PREFIX_PREVIOUS_SESSION_FILE) && !fileName.startsWith(EnvelopeCache.STARTUP_CRASH_MARKER_FILE); // TODO: Use an extension to filter out relevant files } diff --git a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java index 2e7f71f46d..e3e1318b0e 100644 --- a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java +++ b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java @@ -9,6 +9,7 @@ import com.jakewharton.nopen.annotation.Open; import io.sentry.DateUtils; import io.sentry.Hint; +import io.sentry.ISerializer; import io.sentry.SentryCrashLastRunState; import io.sentry.SentryEnvelope; import io.sentry.SentryEnvelopeItem; @@ -17,6 +18,8 @@ import io.sentry.SentryOptions; import io.sentry.Session; import io.sentry.UncaughtExceptionHandlerIntegration; +import io.sentry.hints.AbnormalExit; +import io.sentry.hints.PreviousSessionEnd; import io.sentry.hints.SessionEnd; import io.sentry.hints.SessionStart; import io.sentry.transport.NoOpEnvelopeCache; @@ -56,7 +59,9 @@ public class EnvelopeCache extends CacheStrategy implements IEnvelopeCache { public static final String SUFFIX_ENVELOPE_FILE = ".envelope"; public static final String PREFIX_CURRENT_SESSION_FILE = "session"; - static final String SUFFIX_CURRENT_SESSION_FILE = ".json"; + + public static final String PREFIX_PREVIOUS_SESSION_FILE = "previous_session"; + static final String SUFFIX_SESSION_FILE = ".json"; public static final String CRASH_MARKER_FILE = "last_crash"; public static final String NATIVE_CRASH_MARKER_FILE = ".sentry-native/" + CRASH_MARKER_FILE; @@ -96,6 +101,14 @@ public void store(final @NotNull SentryEnvelope envelope, final @NotNull Hint hi } } + endPreviousSession(hint); + + /** + * Common cases when previous session is not ended with a SessionStart hint for envelope: + * - The previous session experienced Abnormal exit (ANR, OS kills app, User kills app) + * - The previous session experienced native crash + * - The previous session hasn't been ended properly (e.g. session was started in .init() and then in onForeground() after sessionTrackingIntervalMillis) + */ if (HintUtils.hasType(hint, SessionStart.class)) { boolean crashedLastRun = false; @@ -119,13 +132,12 @@ public void store(final @NotNull SentryEnvelope envelope, final @NotNull Hint hi } else { final File crashMarkerFile = new File(options.getCacheDirPath(), NATIVE_CRASH_MARKER_FILE); - Date timestamp = null; if (crashMarkerFile.exists()) { options .getLogger() .log(INFO, "Crash marker file exists, last Session is gonna be Crashed."); - timestamp = getTimestampFromCrashMarkerFile(crashMarkerFile); + final Date timestamp = getTimestampFromCrashMarkerFile(crashMarkerFile); crashedLastRun = true; if (!crashMarkerFile.delete()) { @@ -137,22 +149,24 @@ public void store(final @NotNull SentryEnvelope envelope, final @NotNull Hint hi crashMarkerFile.getAbsolutePath()); } session.update(Session.State.Crashed, null, true); + session.end(timestamp); + // if it was a native crash, we are safe to send session in an envelope, meaning + // there was no abnormal exit + saveSessionToEnvelope(session); + } else { + // if there was no native crash, the session has potentially experienced Abnormal exit + // so we end it with the current timestamp, but do not send it yet, as other envelopes + // may come later and change its attributes (status, etc.). We just save it as previous_session.json + session.end(); + writeSessionToDisk(getPreviousSessionFile(), session); } - - session.end(timestamp); - // if the App. has been upgraded and there's a new version of the SDK running, - // SdkVersion will be outdated. - final SentryEnvelope fromSession = - SentryEnvelope.from(serializer, session, options.getSdkVersion()); - final File fileFromSession = getEnvelopeFile(fromSession); - writeEnvelopeToDisk(fileFromSession, fromSession); } } catch (Throwable e) { options.getLogger().log(SentryLevel.ERROR, "Error processing session.", e); } // at this point the leftover session and its current session file already became a new - // envelope file to be sent + // envelope file to be sent or became a previous_session file // so deleting it as the new session will take place. if (!currentSessionFile.delete()) { options.getLogger().log(WARNING, "Failed to delete the current session file."); @@ -209,6 +223,68 @@ public void store(final @NotNull SentryEnvelope envelope, final @NotNull Hint hi } } + /** + * Attempts to end previous session, relying on PreviousSessionEnd hint. If the hint is also + * AbnormalExit, marks session as abnormal with abnormal mechanism and takes its timestamp. + *

+ * If there was no abnormal exit, the previous session will be captured with the current session + * at latest, preserving the original end timestamp. + *

+ * Otherwise, callers might also call it directly when necessary. + * + * @param hint a hint coming with the envelope + */ + public void endPreviousSession(final @NotNull Hint hint) { + if (HintUtils.hasType(hint, PreviousSessionEnd.class)) { + final File previousSessionFile = getPreviousSessionFile(); + if (previousSessionFile.exists()) { + options.getLogger().log(WARNING, "Previous session is not ended, we'd need to end it."); + + final ISerializer serializer = options.getSerializer(); + try (final Reader reader = + new BufferedReader( + new InputStreamReader(new FileInputStream(previousSessionFile), UTF_8))) { + final Session previousSession = serializer.deserialize(reader, Session.class); + if (previousSession != null) { + final Object sdkHint = HintUtils.getSentrySdkHint(hint); + if (sdkHint instanceof AbnormalExit) { + final Long abnormalExitTimestamp = ((AbnormalExit) sdkHint).timestamp(); + if (abnormalExitTimestamp != null) { + final Date timestamp = DateUtils.getDateTime(abnormalExitTimestamp); + // sanity check if the abnormal exit actually happened when the session was alive + final Date sessionStart = previousSession.getStarted(); + if (sessionStart == null || timestamp.before(sessionStart)) { + options.getLogger().log(WARNING, "Abnormal exit happened before previous session start, not ending the session."); + return; + } + } + final String abnormalMechanism = ((AbnormalExit) sdkHint).mechanism(); + previousSession.update(Session.State.Abnormal, null, true, abnormalMechanism); + } + saveSessionToEnvelope(previousSession); + + // at this point the previous session and its file already became a new envelope, so + // it's safe to delete it + if (!previousSessionFile.delete()) { + options.getLogger().log(WARNING, "Failed to delete the previous session file."); + } + } + } catch (Throwable e) { + options.getLogger().log(SentryLevel.ERROR, "Error processing previous session.", e); + } + } + } + } + + private void saveSessionToEnvelope(final @NotNull Session session) throws IOException { + // if the App. has been upgraded and there's a new version of the SDK running, + // SdkVersion will be outdated. + final SentryEnvelope fromSession = + SentryEnvelope.from(serializer, session, options.getSdkVersion()); + final File fileFromSession = getEnvelopeFile(fromSession); + writeEnvelopeToDisk(fileFromSession, fromSession); + } + private void writeCrashMarkerFile() { final File crashMarkerFile = new File(options.getCacheDirPath(), CRASH_MARKER_FILE); try (final OutputStream outputStream = new FileOutputStream(crashMarkerFile)) { @@ -367,7 +443,12 @@ public void discard(final @NotNull SentryEnvelope envelope) { private @NotNull File getCurrentSessionFile() { return new File( - directory.getAbsolutePath(), PREFIX_CURRENT_SESSION_FILE + SUFFIX_CURRENT_SESSION_FILE); + directory.getAbsolutePath(), PREFIX_CURRENT_SESSION_FILE + SUFFIX_SESSION_FILE); + } + + private @NotNull File getPreviousSessionFile() { + return new File( + directory.getAbsolutePath(), PREFIX_PREVIOUS_SESSION_FILE + SUFFIX_SESSION_FILE); } @Override diff --git a/sentry/src/main/java/io/sentry/hints/AbnormalExit.java b/sentry/src/main/java/io/sentry/hints/AbnormalExit.java index 6b3f0472a0..d8d5d30454 100644 --- a/sentry/src/main/java/io/sentry/hints/AbnormalExit.java +++ b/sentry/src/main/java/io/sentry/hints/AbnormalExit.java @@ -13,4 +13,10 @@ public interface AbnormalExit { default boolean ignoreCurrentThread() { return false; } + + /** When exactly the abnormal exit happened */ + @Nullable + default Long timestamp() { + return null; + } } diff --git a/sentry/src/main/java/io/sentry/hints/AllSessionsEndHint.java b/sentry/src/main/java/io/sentry/hints/AllSessionsEndHint.java new file mode 100644 index 0000000000..c423b90e99 --- /dev/null +++ b/sentry/src/main/java/io/sentry/hints/AllSessionsEndHint.java @@ -0,0 +1,5 @@ +package io.sentry.hints; + +/** An aggregator hint which marks envelopes to end all pending sessions */ +public class AllSessionsEndHint implements SessionEnd, PreviousSessionEnd { +} diff --git a/sentry/src/main/java/io/sentry/hints/PreviousSessionEnd.java b/sentry/src/main/java/io/sentry/hints/PreviousSessionEnd.java new file mode 100644 index 0000000000..0aaa305cf9 --- /dev/null +++ b/sentry/src/main/java/io/sentry/hints/PreviousSessionEnd.java @@ -0,0 +1,5 @@ +package io.sentry.hints; + +/** Hint that shows this is a previous session end envelope */ +public interface PreviousSessionEnd { +} diff --git a/sentry/src/main/java/io/sentry/hints/PreviousSessionEndHint.java b/sentry/src/main/java/io/sentry/hints/PreviousSessionEndHint.java new file mode 100644 index 0000000000..e87b9832c8 --- /dev/null +++ b/sentry/src/main/java/io/sentry/hints/PreviousSessionEndHint.java @@ -0,0 +1,4 @@ +package io.sentry.hints; + +public class PreviousSessionEndHint implements PreviousSessionEnd { +} diff --git a/sentry/src/test/java/io/sentry/cache/EnvelopeCacheTest.kt b/sentry/src/test/java/io/sentry/cache/EnvelopeCacheTest.kt index 2a4ee2dc40..a00c6c9e6b 100644 --- a/sentry/src/test/java/io/sentry/cache/EnvelopeCacheTest.kt +++ b/sentry/src/test/java/io/sentry/cache/EnvelopeCacheTest.kt @@ -11,7 +11,7 @@ import io.sentry.SentryOptions import io.sentry.Session import io.sentry.UncaughtExceptionHandlerIntegration.UncaughtExceptionHint import io.sentry.cache.EnvelopeCache.PREFIX_CURRENT_SESSION_FILE -import io.sentry.cache.EnvelopeCache.SUFFIX_CURRENT_SESSION_FILE +import io.sentry.cache.EnvelopeCache.SUFFIX_SESSION_FILE import io.sentry.hints.SessionEndHint import io.sentry.hints.SessionStartHint import io.sentry.protocol.User @@ -96,7 +96,7 @@ class EnvelopeCacheTest { val hints = HintUtils.createWithTypeCheckHint(SessionStartHint()) cache.store(envelope, hints) - val currentFile = File(fixture.options.cacheDirPath!!, "$PREFIX_CURRENT_SESSION_FILE$SUFFIX_CURRENT_SESSION_FILE") + val currentFile = File(fixture.options.cacheDirPath!!, "$PREFIX_CURRENT_SESSION_FILE$SUFFIX_SESSION_FILE") assertTrue(currentFile.exists()) file.deleteRecursively() @@ -113,7 +113,7 @@ class EnvelopeCacheTest { val hints = HintUtils.createWithTypeCheckHint(SessionStartHint()) cache.store(envelope, hints) - val currentFile = File(fixture.options.cacheDirPath!!, "$PREFIX_CURRENT_SESSION_FILE$SUFFIX_CURRENT_SESSION_FILE") + val currentFile = File(fixture.options.cacheDirPath!!, "$PREFIX_CURRENT_SESSION_FILE$SUFFIX_SESSION_FILE") assertTrue(currentFile.exists()) HintUtils.setTypeCheckHint(hints, SessionEndHint()) @@ -134,7 +134,7 @@ class EnvelopeCacheTest { val hints = HintUtils.createWithTypeCheckHint(SessionStartHint()) cache.store(envelope, hints) - val currentFile = File(fixture.options.cacheDirPath!!, "$PREFIX_CURRENT_SESSION_FILE$SUFFIX_CURRENT_SESSION_FILE") + val currentFile = File(fixture.options.cacheDirPath!!, "$PREFIX_CURRENT_SESSION_FILE$SUFFIX_SESSION_FILE") assertTrue(currentFile.exists()) val session = fixture.serializer.deserialize(currentFile.bufferedReader(Charsets.UTF_8), Session::class.java) From ab73382a99dba98ecbd4a72ff4c5f4866956600a Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 23 Mar 2023 12:40:44 +0100 Subject: [PATCH 02/16] Fix tests --- .../test/java/io/sentry/android/core/AnrV2IntegrationTest.kt | 4 ++-- .../io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt | 4 ++-- sentry/src/main/java/io/sentry/cache/EnvelopeCache.java | 1 + sentry/src/main/java/io/sentry/hints/AllSessionsEndHint.java | 3 +-- .../src/main/java/io/sentry/hints/PreviousSessionEndHint.java | 3 +-- 5 files changed, 7 insertions(+), 8 deletions(-) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AnrV2IntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AnrV2IntegrationTest.kt index 6a8474ba9c..648a514ab0 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AnrV2IntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AnrV2IntegrationTest.kt @@ -135,11 +135,11 @@ class AnrV2IntegrationTest { @Test fun `when historical exit list is empty, does not process historical exits`() { - val integration = fixture.getSut(tmpDir, useImmediateExecutorService = false) + val integration = fixture.getSut(tmpDir) integration.register(fixture.hub, fixture.options) - verify(fixture.options.executorService, never()).submit(any()) + verify(fixture.hub, never()).captureEvent(any(), anyOrNull()) } @Test diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt index 7eb8e0baab..fafaf9028d 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt @@ -176,12 +176,12 @@ class AndroidEnvelopeCacheTest { } @Test - fun `when last reported anr file does not exist, returns 0 upon reading`() { + fun `when last reported anr file does not exist, returns null upon reading`() { fixture.getSut(tmpDir) val lastReportedAnr = AndroidEnvelopeCache.lastReportedAnr(fixture.options) - assertEquals(0L, lastReportedAnr) + assertEquals(null, lastReportedAnr) } @Test diff --git a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java index e3e1318b0e..75121723e8 100644 --- a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java +++ b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java @@ -234,6 +234,7 @@ public void store(final @NotNull SentryEnvelope envelope, final @NotNull Hint hi * * @param hint a hint coming with the envelope */ + @SuppressWarnings("JavaUtilDate") public void endPreviousSession(final @NotNull Hint hint) { if (HintUtils.hasType(hint, PreviousSessionEnd.class)) { final File previousSessionFile = getPreviousSessionFile(); diff --git a/sentry/src/main/java/io/sentry/hints/AllSessionsEndHint.java b/sentry/src/main/java/io/sentry/hints/AllSessionsEndHint.java index c423b90e99..792a23aaea 100644 --- a/sentry/src/main/java/io/sentry/hints/AllSessionsEndHint.java +++ b/sentry/src/main/java/io/sentry/hints/AllSessionsEndHint.java @@ -1,5 +1,4 @@ package io.sentry.hints; /** An aggregator hint which marks envelopes to end all pending sessions */ -public class AllSessionsEndHint implements SessionEnd, PreviousSessionEnd { -} +public final class AllSessionsEndHint implements SessionEnd, PreviousSessionEnd {} diff --git a/sentry/src/main/java/io/sentry/hints/PreviousSessionEndHint.java b/sentry/src/main/java/io/sentry/hints/PreviousSessionEndHint.java index e87b9832c8..78ad92da7d 100644 --- a/sentry/src/main/java/io/sentry/hints/PreviousSessionEndHint.java +++ b/sentry/src/main/java/io/sentry/hints/PreviousSessionEndHint.java @@ -1,4 +1,3 @@ package io.sentry.hints; -public class PreviousSessionEndHint implements PreviousSessionEnd { -} +public final class PreviousSessionEndHint implements PreviousSessionEnd {} From 63a8f7ef563dd0e8aa1315a55ab318d169855574 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Thu, 23 Mar 2023 11:46:12 +0000 Subject: [PATCH 03/16] Format code --- .../sentry/android/core/AnrV2Integration.java | 30 ++++++++-------- .../core/cache/AndroidEnvelopeCache.java | 1 - .../core/cache/AndroidEnvelopeCacheTest.kt | 6 ++-- .../java/io/sentry/cache/EnvelopeCache.java | 35 +++++++++++-------- .../io/sentry/hints/PreviousSessionEnd.java | 3 +- 5 files changed, 39 insertions(+), 36 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java index 640b0756da..8fce3cb50b 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java @@ -82,12 +82,7 @@ public void register(@NotNull IHub hub, @NotNull SentryOptions options) { if (this.options.isAnrEnabled()) { options .getExecutorService() - .submit( - new AnrProcessor( - context, - hub, - this.options, - dateProvider)); + .submit(new AnrProcessor(context, hub, this.options, dateProvider)); options.getLogger().log(SentryLevel.DEBUG, "AnrV2Integration installed."); addIntegrationToSdkVersion(); } @@ -122,10 +117,10 @@ static class AnrProcessor implements Runnable { @Override public void run() { final ActivityManager activityManager = - (ActivityManager) context.getSystemService(Context.ACTIVITY_SERVICE); + (ActivityManager) context.getSystemService(Context.ACTIVITY_SERVICE); List applicationExitInfoList = - activityManager.getHistoricalProcessExitReasons(null, 0, 0); + activityManager.getHistoricalProcessExitReasons(null, 0, 0); if (applicationExitInfoList.size() == 0) { endPreviousSession(); options.getLogger().log(SentryLevel.DEBUG, "No records in historical exit reasons."); @@ -164,7 +159,8 @@ public void run() { return; } - if (lastReportedAnrTimestamp != null && latestAnr.getTimestamp() <= lastReportedAnrTimestamp) { + if (lastReportedAnrTimestamp != null + && latestAnr.getTimestamp() <= lastReportedAnrTimestamp) { endPreviousSession(); options .getLogger() @@ -212,11 +208,15 @@ private void reportAsSentryEvent( final @NotNull ApplicationExitInfo exitInfo, final boolean shouldEnrich) { final long anrTimestamp = exitInfo.getTimestamp(); final boolean isBackground = - exitInfo.getImportance() != ActivityManager.RunningAppProcessInfo.IMPORTANCE_FOREGROUND; + exitInfo.getImportance() != ActivityManager.RunningAppProcessInfo.IMPORTANCE_FOREGROUND; final Throwable anrThrowable = buildAnrThrowable(exitInfo, isBackground); final AnrV2Hint anrHint = new AnrV2Hint( - options.getFlushTimeoutMillis(), options.getLogger(), anrTimestamp, shouldEnrich, isBackground); + options.getFlushTimeoutMillis(), + options.getLogger(), + anrTimestamp, + shouldEnrich, + isBackground); final Hint hint = HintUtils.createWithTypeCheckHint(anrHint); @@ -240,9 +240,7 @@ private void reportAsSentryEvent( } private @NotNull Throwable buildAnrThrowable( - final @NotNull ApplicationExitInfo exitInfo, - final boolean isBackground - ) { + final @NotNull ApplicationExitInfo exitInfo, final boolean isBackground) { String message = "ANR"; if (isBackground) { message = "Background " + message; @@ -267,8 +265,8 @@ private void endPreviousSession() { } @ApiStatus.Internal - public static final class AnrV2Hint extends BlockingFlushHint implements Backfillable, - AbnormalExit, PreviousSessionEnd { + public static final class AnrV2Hint extends BlockingFlushHint + implements Backfillable, AbnormalExit, PreviousSessionEnd { private final long timestamp; diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java b/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java index 102d8f5688..bd3e0809b5 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java @@ -13,7 +13,6 @@ import io.sentry.android.core.SentryAndroidOptions; import io.sentry.android.core.internal.util.AndroidCurrentDateProvider; import io.sentry.cache.EnvelopeCache; -import io.sentry.hints.AbnormalExit; import io.sentry.transport.ICurrentDateProvider; import io.sentry.util.FileUtils; import io.sentry.util.HintUtils; diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt index fafaf9028d..095da5f32a 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt @@ -136,7 +136,8 @@ class AndroidEnvelopeCacheTest { 0, NoOpLogger.getInstance(), 12345678L, - false, false + false, + false ) ) cache.store(fixture.envelope, hints) @@ -153,7 +154,8 @@ class AndroidEnvelopeCacheTest { 0, NoOpLogger.getInstance(), 12345678L, - false, false + false, + false ) ) cache.store(fixture.envelope, hints) diff --git a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java index 75121723e8..ebaaeeb2da 100644 --- a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java +++ b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java @@ -104,10 +104,11 @@ public void store(final @NotNull SentryEnvelope envelope, final @NotNull Hint hi endPreviousSession(hint); /** - * Common cases when previous session is not ended with a SessionStart hint for envelope: - * - The previous session experienced Abnormal exit (ANR, OS kills app, User kills app) - * - The previous session experienced native crash - * - The previous session hasn't been ended properly (e.g. session was started in .init() and then in onForeground() after sessionTrackingIntervalMillis) + * Common cases when previous session is not ended with a SessionStart hint for envelope: - The + * previous session experienced Abnormal exit (ANR, OS kills app, User kills app) - The previous + * session experienced native crash - The previous session hasn't been ended properly (e.g. + * session was started in .init() and then in onForeground() after + * sessionTrackingIntervalMillis) */ if (HintUtils.hasType(hint, SessionStart.class)) { boolean crashedLastRun = false; @@ -156,7 +157,8 @@ public void store(final @NotNull SentryEnvelope envelope, final @NotNull Hint hi } else { // if there was no native crash, the session has potentially experienced Abnormal exit // so we end it with the current timestamp, but do not send it yet, as other envelopes - // may come later and change its attributes (status, etc.). We just save it as previous_session.json + // may come later and change its attributes (status, etc.). We just save it as + // previous_session.json session.end(); writeSessionToDisk(getPreviousSessionFile(), session); } @@ -226,11 +228,11 @@ public void store(final @NotNull SentryEnvelope envelope, final @NotNull Hint hi /** * Attempts to end previous session, relying on PreviousSessionEnd hint. If the hint is also * AbnormalExit, marks session as abnormal with abnormal mechanism and takes its timestamp. - *

- * If there was no abnormal exit, the previous session will be captured with the current session - * at latest, preserving the original end timestamp. - *

- * Otherwise, callers might also call it directly when necessary. + * + *

If there was no abnormal exit, the previous session will be captured with the current + * session at latest, preserving the original end timestamp. + * + *

Otherwise, callers might also call it directly when necessary. * * @param hint a hint coming with the envelope */ @@ -243,8 +245,8 @@ public void endPreviousSession(final @NotNull Hint hint) { final ISerializer serializer = options.getSerializer(); try (final Reader reader = - new BufferedReader( - new InputStreamReader(new FileInputStream(previousSessionFile), UTF_8))) { + new BufferedReader( + new InputStreamReader(new FileInputStream(previousSessionFile), UTF_8))) { final Session previousSession = serializer.deserialize(reader, Session.class); if (previousSession != null) { final Object sdkHint = HintUtils.getSentrySdkHint(hint); @@ -255,7 +257,11 @@ public void endPreviousSession(final @NotNull Hint hint) { // sanity check if the abnormal exit actually happened when the session was alive final Date sessionStart = previousSession.getStarted(); if (sessionStart == null || timestamp.before(sessionStart)) { - options.getLogger().log(WARNING, "Abnormal exit happened before previous session start, not ending the session."); + options + .getLogger() + .log( + WARNING, + "Abnormal exit happened before previous session start, not ending the session."); return; } } @@ -443,8 +449,7 @@ public void discard(final @NotNull SentryEnvelope envelope) { } private @NotNull File getCurrentSessionFile() { - return new File( - directory.getAbsolutePath(), PREFIX_CURRENT_SESSION_FILE + SUFFIX_SESSION_FILE); + return new File(directory.getAbsolutePath(), PREFIX_CURRENT_SESSION_FILE + SUFFIX_SESSION_FILE); } private @NotNull File getPreviousSessionFile() { diff --git a/sentry/src/main/java/io/sentry/hints/PreviousSessionEnd.java b/sentry/src/main/java/io/sentry/hints/PreviousSessionEnd.java index 0aaa305cf9..088a21b2f8 100644 --- a/sentry/src/main/java/io/sentry/hints/PreviousSessionEnd.java +++ b/sentry/src/main/java/io/sentry/hints/PreviousSessionEnd.java @@ -1,5 +1,4 @@ package io.sentry.hints; /** Hint that shows this is a previous session end envelope */ -public interface PreviousSessionEnd { -} +public interface PreviousSessionEnd {} From a8efc613a19b8830c790723ac094bc9ee5ec6840 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 29 Mar 2023 18:59:31 +0200 Subject: [PATCH 04/16] 2nd attempt --- .../android/core/AnrV2EventProcessor.java | 4 +- .../sentry/android/core/AnrV2Integration.java | 11 +- .../io/sentry/android/core/SentryAndroid.java | 6 +- sentry/src/main/java/io/sentry/Hub.java | 19 +- .../io/sentry/PreviousSessionFinalizer.java | 130 +++++++++++ .../main/java/io/sentry/SentryEnvelope.java | 6 + .../java/io/sentry/cache/CacheStrategy.java | 2 +- .../java/io/sentry/cache/EnvelopeCache.java | 217 +++++++----------- .../io/sentry/hints/AllSessionsEndHint.java | 4 - 9 files changed, 247 insertions(+), 152 deletions(-) create mode 100644 sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java delete mode 100644 sentry/src/main/java/io/sentry/hints/AllSessionsEndHint.java diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2EventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2EventProcessor.java index 80d8faaf9d..466d4dc8c3 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2EventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2EventProcessor.java @@ -300,7 +300,7 @@ private void setApp(final @NotNull SentryBaseEvent event) { } catch (Throwable e) { options .getLogger() - .log(SentryLevel.ERROR, "Failed to parse release from scope cache: %s", release); + .log(SentryLevel.WARNING, "Failed to parse release from scope cache: %s", release); } } @@ -361,7 +361,7 @@ private void setDist(final @NotNull SentryBaseEvent event) { } catch (Throwable e) { options .getLogger() - .log(SentryLevel.ERROR, "Failed to parse release from scope cache: %s", release); + .log(SentryLevel.WARNING, "Failed to parse release from scope cache: %s", release); } } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java index 640b0756da..d99293e313 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java @@ -5,11 +5,13 @@ import android.app.ApplicationExitInfo; import android.content.Context; import android.os.Looper; +import android.util.Log; import io.sentry.DateUtils; import io.sentry.Hint; import io.sentry.IHub; import io.sentry.ILogger; import io.sentry.Integration; +import io.sentry.SentryEnvelope; import io.sentry.SentryEvent; import io.sentry.SentryLevel; import io.sentry.SentryOptions; @@ -29,6 +31,7 @@ import io.sentry.util.HintUtils; import io.sentry.util.Objects; import java.io.Closeable; +import java.io.File; import java.io.IOException; import java.util.ArrayList; import java.util.Collections; @@ -261,7 +264,13 @@ private void endPreviousSession() { final IEnvelopeCache envelopeCache = options.getEnvelopeDiskCache(); if (envelopeCache instanceof EnvelopeCache) { final Hint hint = HintUtils.createWithTypeCheckHint(new PreviousSessionEndHint()); - ((EnvelopeCache) envelopeCache).endPreviousSession(hint); + final SentryEnvelope sessionEnvelope = + ((EnvelopeCache) envelopeCache).endPreviousSession(SentryEnvelope.empty(), hint); + + // if there was no ANRs, we just capture the previous session asap, so it's not delayed for long + if (sessionEnvelope.getHeader().getEventId() != SentryId.EMPTY_ID) { + hub.captureEnvelope(sessionEnvelope); + } } } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java index eb33039a71..b90562f87e 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java @@ -125,8 +125,10 @@ public static synchronized void init( final @NotNull IHub hub = Sentry.getCurrentHub(); if (hub.getOptions().isEnableAutoSessionTracking() && ContextUtils.isForegroundImportance(context)) { - hub.addBreadcrumb(BreadcrumbFactory.forSession("session.start")); - hub.startSession(); + //hub.getOptions().getExecutorService().submit(() -> { + hub.addBreadcrumb(BreadcrumbFactory.forSession("session.start")); + hub.startSession(); + //}); } } catch (IllegalAccessException e) { logger.log(SentryLevel.FATAL, "Fatal error during SentryAndroid.init(...)", e); diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index 63ef82803b..6a7b5292e7 100644 --- a/sentry/src/main/java/io/sentry/Hub.java +++ b/sentry/src/main/java/io/sentry/Hub.java @@ -2,7 +2,6 @@ import io.sentry.Stack.StackItem; import io.sentry.clientreport.DiscardReason; -import io.sentry.hints.AllSessionsEndHint; import io.sentry.hints.SessionEndHint; import io.sentry.hints.SessionStartHint; import io.sentry.protocol.SentryId; @@ -318,12 +317,16 @@ public void endSession() { .log(SentryLevel.WARNING, "Instance is disabled and this 'endSession' call is a no-op."); } else { final StackItem item = this.stack.peek(); - final Session previousSession = item.getScope().endSession(); - if (previousSession != null) { - final Hint hint = HintUtils.createWithTypeCheckHint(new AllSessionsEndHint()); + endSessionInternal(item); + } + } - item.getClient().captureSession(previousSession, hint); - } + private void endSessionInternal(final @NotNull StackItem item) { + final Session previousSession = item.getScope().endSession(); + if (previousSession != null) { + final Hint hint = HintUtils.createWithTypeCheckHint(new SessionEndHint()); + + item.getClient().captureSession(previousSession, hint); } } @@ -344,7 +347,9 @@ public void close() { // Close the top-most client final StackItem item = stack.peek(); - // TODO: should we end session before closing client? + + // end session before closing the client, close() of the client will wait for it to be flushed + endSessionInternal(item); item.getClient().close(); } catch (Throwable e) { options.getLogger().log(SentryLevel.ERROR, "Error while closing the Hub.", e); diff --git a/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java b/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java new file mode 100644 index 0000000000..c8ed7598f7 --- /dev/null +++ b/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java @@ -0,0 +1,130 @@ +package io.sentry; + +import io.sentry.cache.EnvelopeCache; +import java.io.BufferedReader; +import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; +import java.io.InputStreamReader; +import java.io.Reader; +import java.nio.charset.Charset; +import java.util.Date; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import static io.sentry.SentryLevel.DEBUG; +import static io.sentry.SentryLevel.ERROR; +import static io.sentry.SentryLevel.INFO; +import static io.sentry.SentryLevel.WARNING; +import static io.sentry.cache.EnvelopeCache.NATIVE_CRASH_MARKER_FILE; + +/** + * Common cases when previous session is not ended properly (app background or crash): + * - The previous session experienced Abnormal exit (ANR, OS kills app, User kills app) + * - The previous session experienced native crash + */ +final class PreviousSessionFinalizer implements Runnable { + + @SuppressWarnings("CharsetObjectCanBeUsed") + private static final Charset UTF_8 = Charset.forName("UTF-8"); + + private final @NotNull SentryOptions options; + + private final @NotNull IHub hub; + + PreviousSessionFinalizer(final @NotNull SentryOptions options, final @NotNull IHub hub) { + this.options = options; + this.hub = hub; + } + + @Override public void run() { + final String cacheDirPath = options.getCacheDirPath(); + if (cacheDirPath == null) { + options.getLogger().log(INFO, "Cache dir is not set, not finalizing the previous session."); + return; + } + + final File previousSessionFile = EnvelopeCache.getPreviousSessionFile(cacheDirPath); + final ISerializer serializer = options.getSerializer(); + + if (previousSessionFile.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(previousSessionFile), UTF_8))) { + + final Session session = serializer.deserialize(reader, Session.class); + if (session == null) { + options + .getLogger() + .log( + SentryLevel.ERROR, + "Stream from path %s resulted in a null envelope.", + previousSessionFile.getAbsolutePath()); + } else { + final File crashMarkerFile = + new File(options.getCacheDirPath(), NATIVE_CRASH_MARKER_FILE); + if (crashMarkerFile.exists()) { + options + .getLogger() + .log(INFO, "Crash marker file exists, last Session is gonna be Crashed."); + + final Date timestamp = getTimestampFromCrashMarkerFile(crashMarkerFile); + + if (!crashMarkerFile.delete()) { + options + .getLogger() + .log( + ERROR, + "Failed to delete the crash marker file. %s.", + crashMarkerFile.getAbsolutePath()); + } + session.update(Session.State.Crashed, null, true); + session.end(timestamp); + // if the App. has been upgraded and there's a new version of the SDK running, + // SdkVersion will be outdated. + final SentryEnvelope fromSession = + SentryEnvelope.from(serializer, session, options.getSdkVersion()); + hub.captureEnvelope(fromSession); + } else { + // if there was no native crash, the session has potentially experienced Abnormal exit + // so we end it with the current timestamp, but do not send it yet, as other envelopes + // may come later and change its attributes (status, etc.). We just save it as previous_session.json + session.end(); + writeSessionToDisk(getPreviousSessionFile(), session); + } + } + } catch (Throwable e) { + options.getLogger().log(SentryLevel.ERROR, "Error processing previous session.", e); + } + + // at this point the leftover session and its current session file already became a new + // envelope file to be sent or became a previous_session file + // so deleting it as the new session will take place. + if (!previousSessionFile.delete()) { + options.getLogger().log(WARNING, "Failed to delete the previous session file."); + } + } + } + + /** + * Reads the crash marker file and returns the timestamp as Date written in there + * + * @param markerFile the marker file + * @return the timestamp as Date + */ + private @Nullable Date getTimestampFromCrashMarkerFile(final @NotNull File markerFile) { + try (final BufferedReader reader = + new BufferedReader(new InputStreamReader(new FileInputStream(markerFile), UTF_8))) { + final String timestamp = reader.readLine(); + options.getLogger().log(DEBUG, "Crash marker file has %s timestamp.", timestamp); + return DateUtils.getDateTime(timestamp); + } catch (IOException e) { + options.getLogger().log(ERROR, "Error reading the crash marker file.", e); + } catch (IllegalArgumentException e) { + options.getLogger().log(SentryLevel.ERROR, e, "Error converting the crash timestamp."); + } + return null; + } +} diff --git a/sentry/src/main/java/io/sentry/SentryEnvelope.java b/sentry/src/main/java/io/sentry/SentryEnvelope.java index 5c3f141baf..90bba8c901 100644 --- a/sentry/src/main/java/io/sentry/SentryEnvelope.java +++ b/sentry/src/main/java/io/sentry/SentryEnvelope.java @@ -3,9 +3,11 @@ import io.sentry.exception.SentryEnvelopeException; import io.sentry.protocol.SdkVersion; import io.sentry.protocol.SentryId; +import io.sentry.util.CollectionUtils; import io.sentry.util.Objects; import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @@ -92,4 +94,8 @@ public SentryEnvelope( sdkVersion, SentryEnvelopeItem.fromProfilingTrace(profilingTraceData, maxTraceFileSize, serializer)); } + + public static @NotNull SentryEnvelope empty() { + return new SentryEnvelope(new SentryEnvelopeHeader(SentryId.EMPTY_ID), Collections.emptyList()); + } } diff --git a/sentry/src/main/java/io/sentry/cache/CacheStrategy.java b/sentry/src/main/java/io/sentry/cache/CacheStrategy.java index dbb6a49c19..a162a6b357 100644 --- a/sentry/src/main/java/io/sentry/cache/CacheStrategy.java +++ b/sentry/src/main/java/io/sentry/cache/CacheStrategy.java @@ -276,7 +276,7 @@ private void saveNewEnvelope( } } - private @NotNull SentryEnvelope buildNewEnvelope( + protected @NotNull SentryEnvelope buildNewEnvelope( final @NotNull SentryEnvelope envelope, final @NotNull SentryEnvelopeItem sessionItem) { final List newEnvelopeItems = new ArrayList<>(); diff --git a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java index 75121723e8..9340ce1b50 100644 --- a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java +++ b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java @@ -9,7 +9,6 @@ import com.jakewharton.nopen.annotation.Open; import io.sentry.DateUtils; import io.sentry.Hint; -import io.sentry.ISerializer; import io.sentry.SentryCrashLastRunState; import io.sentry.SentryEnvelope; import io.sentry.SentryEnvelopeItem; @@ -88,12 +87,12 @@ public EnvelopeCache( } @Override - public void store(final @NotNull SentryEnvelope envelope, final @NotNull Hint hint) { + public void store(@NotNull SentryEnvelope envelope, final @NotNull Hint hint) { Objects.requireNonNull(envelope, "Envelope is required."); rotateCacheIfNeeded(allEnvelopeFiles()); - final File currentSessionFile = getCurrentSessionFile(); + final File currentSessionFile = getCurrentSessionFile(directory.getAbsolutePath()); if (HintUtils.hasType(hint, SessionEnd.class)) { if (!currentSessionFile.delete()) { @@ -101,78 +100,17 @@ public void store(final @NotNull SentryEnvelope envelope, final @NotNull Hint hi } } - endPreviousSession(hint); - - /** - * Common cases when previous session is not ended with a SessionStart hint for envelope: - * - The previous session experienced Abnormal exit (ANR, OS kills app, User kills app) - * - The previous session experienced native crash - * - The previous session hasn't been ended properly (e.g. session was started in .init() and then in onForeground() after sessionTrackingIntervalMillis) - */ + // TODO: get rid of this, move it to the executor service, but after ANR, do native crash handling there, also endSession on close(), + // TODO: rename current session synchronously on init if (HintUtils.hasType(hint, SessionStart.class)) { - boolean crashedLastRun = false; - - // TODO: should we move this to AppLifecycleIntegration? and do on SDK init? but it's too much - // on main-thread - 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.deserialize(reader, Session.class); - if (session == null) { - options - .getLogger() - .log( - SentryLevel.ERROR, - "Stream from path %s resulted in a null envelope.", - currentSessionFile.getAbsolutePath()); - } else { - final File crashMarkerFile = - new File(options.getCacheDirPath(), NATIVE_CRASH_MARKER_FILE); - if (crashMarkerFile.exists()) { - options - .getLogger() - .log(INFO, "Crash marker file exists, last Session is gonna be Crashed."); - - final Date timestamp = getTimestampFromCrashMarkerFile(crashMarkerFile); - - crashedLastRun = true; - if (!crashMarkerFile.delete()) { - options - .getLogger() - .log( - ERROR, - "Failed to delete the crash marker file. %s.", - crashMarkerFile.getAbsolutePath()); - } - session.update(Session.State.Crashed, null, true); - session.end(timestamp); - // if it was a native crash, we are safe to send session in an envelope, meaning - // there was no abnormal exit - saveSessionToEnvelope(session); - } else { - // if there was no native crash, the session has potentially experienced Abnormal exit - // so we end it with the current timestamp, but do not send it yet, as other envelopes - // may come later and change its attributes (status, etc.). We just save it as previous_session.json - session.end(); - writeSessionToDisk(getPreviousSessionFile(), session); - } - } - } catch (Throwable e) { - options.getLogger().log(SentryLevel.ERROR, "Error processing session.", e); - } + updateCurrentSession(currentSessionFile, envelope); - // at this point the leftover session and its current session file already became a new - // envelope file to be sent or became a previous_session file - // so deleting it as the new session will take place. - if (!currentSessionFile.delete()) { - options.getLogger().log(WARNING, "Failed to delete the current session file."); - } + boolean crashedLastRun = false; + final File crashMarkerFile = + new File(options.getCacheDirPath(), NATIVE_CRASH_MARKER_FILE); + if (crashMarkerFile.exists()) { + crashedLastRun = true; } - updateCurrentSession(currentSessionFile, envelope); // check java marker file if the native marker isnt there if (!crashedLastRun) { @@ -233,57 +171,86 @@ public void store(final @NotNull SentryEnvelope envelope, final @NotNull Hint hi * Otherwise, callers might also call it directly when necessary. * * @param hint a hint coming with the envelope + * @param envelope an original envelope that is being stored + * @return SentryEnvelope returns either a new envelope containing previous session in it, or an + * old one, if the previous session should not be sent with the current envelope. */ @SuppressWarnings("JavaUtilDate") - public void endPreviousSession(final @NotNull Hint hint) { + public @NotNull SentryEnvelope endPreviousSession( + final @NotNull SentryEnvelope envelope, + final @NotNull Hint hint + ) { if (HintUtils.hasType(hint, PreviousSessionEnd.class)) { - final File previousSessionFile = getPreviousSessionFile(); + final File previousSessionFile = getPreviousSessionFile(directory.getAbsolutePath()); + final Object sdkHint = HintUtils.getSentrySdkHint(hint); + if (previousSessionFile.exists()) { options.getLogger().log(WARNING, "Previous session is not ended, we'd need to end it."); - final ISerializer serializer = options.getSerializer(); - try (final Reader reader = - new BufferedReader( - new InputStreamReader(new FileInputStream(previousSessionFile), UTF_8))) { - final Session previousSession = serializer.deserialize(reader, Session.class); - if (previousSession != null) { - final Object sdkHint = HintUtils.getSentrySdkHint(hint); - if (sdkHint instanceof AbnormalExit) { - final Long abnormalExitTimestamp = ((AbnormalExit) sdkHint).timestamp(); - if (abnormalExitTimestamp != null) { - final Date timestamp = DateUtils.getDateTime(abnormalExitTimestamp); - // sanity check if the abnormal exit actually happened when the session was alive - final Date sessionStart = previousSession.getStarted(); - if (sessionStart == null || timestamp.before(sessionStart)) { - options.getLogger().log(WARNING, "Abnormal exit happened before previous session start, not ending the session."); - return; - } - } - final String abnormalMechanism = ((AbnormalExit) sdkHint).mechanism(); - previousSession.update(Session.State.Abnormal, null, true, abnormalMechanism); - } - saveSessionToEnvelope(previousSession); + return endSessionIntoEnvelope(previousSessionFile, envelope, sdkHint); + } else { + options.getLogger().log(DEBUG, "No session file to end."); + } + } + return envelope; + } - // at this point the previous session and its file already became a new envelope, so - // it's safe to delete it - if (!previousSessionFile.delete()) { - options.getLogger().log(WARNING, "Failed to delete the previous session file."); + @SuppressWarnings("JavaUtilDate") + private @NotNull SentryEnvelope endSessionIntoEnvelope( + final @NotNull File sessionFile, + final @NotNull SentryEnvelope envelope, + final @Nullable Object sdkHint + ) { + try (final Reader reader = + new BufferedReader( + new InputStreamReader(new FileInputStream(sessionFile), UTF_8))) { + final Session session = serializer.deserialize(reader, Session.class); + if (session != null) { + if (sdkHint instanceof AbnormalExit) { + final AbnormalExit abnormalHint = (AbnormalExit) sdkHint; + final @Nullable Long abnormalExitTimestamp = abnormalHint.timestamp(); + Date timestamp = null; + + if (abnormalExitTimestamp != null) { + timestamp = DateUtils.getDateTime(abnormalExitTimestamp); + // sanity check if the abnormal exit actually happened when the session was alive + final Date sessionStart = session.getStarted(); + if (sessionStart == null || timestamp.before(sessionStart)) { + options.getLogger().log(WARNING, "Abnormal exit happened before previous session start, not ending the session."); + return envelope; } } - } catch (Throwable e) { - options.getLogger().log(SentryLevel.ERROR, "Error processing previous session.", e); + + final String abnormalMechanism = abnormalHint.mechanism(); + session.update(Session.State.Abnormal, null, true, abnormalMechanism); + // we have to use the actual timestamp of the Abnormal Exit here to mark the session + // as finished at the time it happened + session.end(timestamp); + } + + final SentryEnvelope newEnvelope; + // if an envelope has items, we send the session in the same envelope, otherwise + // it's a dummy envelope and we build a new envelope and return it + if (envelope.getItems().iterator().hasNext()) { + final SentryEnvelopeItem sessionItem = + SentryEnvelopeItem.fromSession(serializer, session); + newEnvelope = buildNewEnvelope(envelope, sessionItem); + } else { + newEnvelope = + SentryEnvelope.from(serializer, session, options.getSdkVersion()); } + + // at this point the session and its file already became a new envelope, so + // it's safe to delete it + if (!sessionFile.delete()) { + options.getLogger().log(WARNING, "Failed to delete the session file."); + } + return newEnvelope; } + } catch (Throwable e) { + options.getLogger().log(SentryLevel.ERROR, "Error processing previous session.", e); } - } - - private void saveSessionToEnvelope(final @NotNull Session session) throws IOException { - // if the App. has been upgraded and there's a new version of the SDK running, - // SdkVersion will be outdated. - final SentryEnvelope fromSession = - SentryEnvelope.from(serializer, session, options.getSdkVersion()); - final File fileFromSession = getEnvelopeFile(fromSession); - writeEnvelopeToDisk(fileFromSession, fromSession); + return envelope; } private void writeCrashMarkerFile() { @@ -297,26 +264,6 @@ private void writeCrashMarkerFile() { } } - /** - * Reads the crash marker file and returns the timestamp as Date written in there - * - * @param markerFile the marker file - * @return the timestamp as Date - */ - private @Nullable Date getTimestampFromCrashMarkerFile(final @NotNull File markerFile) { - try (final BufferedReader reader = - new BufferedReader(new InputStreamReader(new FileInputStream(markerFile), UTF_8))) { - final String timestamp = reader.readLine(); - options.getLogger().log(DEBUG, "Crash marker file has %s timestamp.", timestamp); - return DateUtils.getDateTime(timestamp); - } catch (IOException e) { - options.getLogger().log(ERROR, "Error reading the crash marker file.", e); - } catch (IllegalArgumentException e) { - options.getLogger().log(SentryLevel.ERROR, e, "Error converting the crash timestamp."); - } - return null; - } - private void updateCurrentSession( final @NotNull File currentSessionFile, final @NotNull SentryEnvelope envelope) { final Iterable items = envelope.getItems(); @@ -442,14 +389,14 @@ public void discard(final @NotNull SentryEnvelope envelope) { return new File(directory.getAbsolutePath(), fileName); } - private @NotNull File getCurrentSessionFile() { + public static @NotNull File getCurrentSessionFile(final @NotNull String cacheDirPath) { return new File( - directory.getAbsolutePath(), PREFIX_CURRENT_SESSION_FILE + SUFFIX_SESSION_FILE); + cacheDirPath, PREFIX_CURRENT_SESSION_FILE + SUFFIX_SESSION_FILE); } - private @NotNull File getPreviousSessionFile() { + public static @NotNull File getPreviousSessionFile(final @NotNull String cacheDirPath) { return new File( - directory.getAbsolutePath(), PREFIX_PREVIOUS_SESSION_FILE + SUFFIX_SESSION_FILE); + cacheDirPath, PREFIX_PREVIOUS_SESSION_FILE + SUFFIX_SESSION_FILE); } @Override diff --git a/sentry/src/main/java/io/sentry/hints/AllSessionsEndHint.java b/sentry/src/main/java/io/sentry/hints/AllSessionsEndHint.java deleted file mode 100644 index 792a23aaea..0000000000 --- a/sentry/src/main/java/io/sentry/hints/AllSessionsEndHint.java +++ /dev/null @@ -1,4 +0,0 @@ -package io.sentry.hints; - -/** An aggregator hint which marks envelopes to end all pending sessions */ -public final class AllSessionsEndHint implements SessionEnd, PreviousSessionEnd {} From bce19aec4f4198b602d4e6aab7f6a00045a13037 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 30 Mar 2023 15:27:31 +0200 Subject: [PATCH 05/16] 3rd attempt --- .../sentry/android/core/AnrV2Integration.java | 34 ++-- sentry/src/main/java/io/sentry/Hub.java | 18 +-- .../io/sentry/PreviousSessionFinalizer.java | 36 +++-- sentry/src/main/java/io/sentry/Sentry.java | 19 +++ .../java/io/sentry/cache/EnvelopeCache.java | 150 ++++++++++-------- .../io/sentry/hints/PreviousSessionEnd.java | 5 - .../sentry/hints/PreviousSessionEndHint.java | 3 - 7 files changed, 135 insertions(+), 130 deletions(-) delete mode 100644 sentry/src/main/java/io/sentry/hints/PreviousSessionEnd.java delete mode 100644 sentry/src/main/java/io/sentry/hints/PreviousSessionEndHint.java diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java index d99293e313..16b47bd004 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java @@ -5,25 +5,20 @@ import android.app.ApplicationExitInfo; import android.content.Context; import android.os.Looper; -import android.util.Log; import io.sentry.DateUtils; import io.sentry.Hint; import io.sentry.IHub; import io.sentry.ILogger; import io.sentry.Integration; -import io.sentry.SentryEnvelope; import io.sentry.SentryEvent; import io.sentry.SentryLevel; import io.sentry.SentryOptions; import io.sentry.android.core.cache.AndroidEnvelopeCache; import io.sentry.cache.EnvelopeCache; -import io.sentry.cache.IEnvelopeCache; import io.sentry.exception.ExceptionMechanismException; import io.sentry.hints.AbnormalExit; import io.sentry.hints.Backfillable; import io.sentry.hints.BlockingFlushHint; -import io.sentry.hints.PreviousSessionEnd; -import io.sentry.hints.PreviousSessionEndHint; import io.sentry.protocol.Mechanism; import io.sentry.protocol.SentryId; import io.sentry.transport.CurrentDateProvider; @@ -31,7 +26,6 @@ import io.sentry.util.HintUtils; import io.sentry.util.Objects; import java.io.Closeable; -import java.io.File; import java.io.IOException; import java.util.ArrayList; import java.util.Collections; @@ -130,11 +124,18 @@ public void run() { List applicationExitInfoList = activityManager.getHistoricalProcessExitReasons(null, 0, 0); if (applicationExitInfoList.size() == 0) { - endPreviousSession(); options.getLogger().log(SentryLevel.DEBUG, "No records in historical exit reasons."); return; } + if (!EnvelopeCache.waitPreviousSessionFlush()) { + options + .getLogger() + .log( + SentryLevel.WARNING, + "Timed out waiting to flush previous session to its own file."); + } + // making a deep copy as we're modifying the list final List exitInfos = new ArrayList<>(applicationExitInfoList); final @Nullable Long lastReportedAnrTimestamp = AndroidEnvelopeCache.lastReportedAnr(options); @@ -152,7 +153,6 @@ public void run() { } if (latestAnr == null) { - endPreviousSession(); options .getLogger() .log(SentryLevel.DEBUG, "No ANRs have been found in the historical exit reasons list."); @@ -160,7 +160,6 @@ public void run() { } if (latestAnr.getTimestamp() < threshold) { - endPreviousSession(); options .getLogger() .log(SentryLevel.DEBUG, "Latest ANR happened too long ago, returning early."); @@ -168,7 +167,6 @@ public void run() { } if (lastReportedAnrTimestamp != null && latestAnr.getTimestamp() <= lastReportedAnrTimestamp) { - endPreviousSession(); options .getLogger() .log(SentryLevel.DEBUG, "Latest ANR has already been reported, returning early."); @@ -259,25 +257,11 @@ private void reportAsSentryEvent( mechanism.setType("ANRv2"); return new ExceptionMechanismException(mechanism, error, error.getThread(), true); } - - private void endPreviousSession() { - final IEnvelopeCache envelopeCache = options.getEnvelopeDiskCache(); - if (envelopeCache instanceof EnvelopeCache) { - final Hint hint = HintUtils.createWithTypeCheckHint(new PreviousSessionEndHint()); - final SentryEnvelope sessionEnvelope = - ((EnvelopeCache) envelopeCache).endPreviousSession(SentryEnvelope.empty(), hint); - - // if there was no ANRs, we just capture the previous session asap, so it's not delayed for long - if (sessionEnvelope.getHeader().getEventId() != SentryId.EMPTY_ID) { - hub.captureEnvelope(sessionEnvelope); - } - } - } } @ApiStatus.Internal public static final class AnrV2Hint extends BlockingFlushHint implements Backfillable, - AbnormalExit, PreviousSessionEnd { + AbnormalExit { private final long timestamp; diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index 6a7b5292e7..e22259195d 100644 --- a/sentry/src/main/java/io/sentry/Hub.java +++ b/sentry/src/main/java/io/sentry/Hub.java @@ -317,16 +317,12 @@ public void endSession() { .log(SentryLevel.WARNING, "Instance is disabled and this 'endSession' call is a no-op."); } else { final StackItem item = this.stack.peek(); - endSessionInternal(item); - } - } - - private void endSessionInternal(final @NotNull StackItem item) { - final Session previousSession = item.getScope().endSession(); - if (previousSession != null) { - final Hint hint = HintUtils.createWithTypeCheckHint(new SessionEndHint()); + final Session previousSession = item.getScope().endSession(); + if (previousSession != null) { + final Hint hint = HintUtils.createWithTypeCheckHint(new SessionEndHint()); - item.getClient().captureSession(previousSession, hint); + item.getClient().captureSession(previousSession, hint); + } } } @@ -347,9 +343,7 @@ public void close() { // Close the top-most client final StackItem item = stack.peek(); - - // end session before closing the client, close() of the client will wait for it to be flushed - endSessionInternal(item); + // TODO: should we end session before closing client? item.getClient().close(); } catch (Throwable e) { options.getLogger().log(SentryLevel.ERROR, "Error while closing the Hub.", e); diff --git a/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java b/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java index c8ed7598f7..01b9475c67 100644 --- a/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java +++ b/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java @@ -44,6 +44,15 @@ final class PreviousSessionFinalizer implements Runnable { return; } + if (!EnvelopeCache.waitPreviousSessionFlush()) { + options + .getLogger() + .log( + SentryLevel.WARNING, + "Timed out waiting to flush previous session to its own file in session finalizer."); + return; + } + final File previousSessionFile = EnvelopeCache.getPreviousSessionFile(cacheDirPath); final ISerializer serializer = options.getSerializer(); @@ -63,6 +72,7 @@ final class PreviousSessionFinalizer implements Runnable { "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()) { @@ -70,7 +80,7 @@ final class PreviousSessionFinalizer implements Runnable { .getLogger() .log(INFO, "Crash marker file exists, last Session is gonna be Crashed."); - final Date timestamp = getTimestampFromCrashMarkerFile(crashMarkerFile); + timestamp = getTimestampFromCrashMarkerFile(crashMarkerFile); if (!crashMarkerFile.delete()) { options @@ -81,27 +91,21 @@ final class PreviousSessionFinalizer implements Runnable { crashMarkerFile.getAbsolutePath()); } session.update(Session.State.Crashed, null, true); - session.end(timestamp); - // if the App. has been upgraded and there's a new version of the SDK running, - // SdkVersion will be outdated. - final SentryEnvelope fromSession = - SentryEnvelope.from(serializer, session, options.getSdkVersion()); - hub.captureEnvelope(fromSession); - } else { - // if there was no native crash, the session has potentially experienced Abnormal exit - // so we end it with the current timestamp, but do not send it yet, as other envelopes - // may come later and change its attributes (status, etc.). We just save it as previous_session.json - session.end(); - writeSessionToDisk(getPreviousSessionFile(), session); } + session.end(timestamp); + + // if the App. has been upgraded and there's a new version of the SDK running, + // SdkVersion will be outdated. + final SentryEnvelope fromSession = + SentryEnvelope.from(serializer, session, options.getSdkVersion()); + hub.captureEnvelope(fromSession); } } catch (Throwable e) { options.getLogger().log(SentryLevel.ERROR, "Error processing previous session.", e); } - // at this point the leftover session and its current session file already became a new - // envelope file to be sent or became a previous_session file - // so deleting it as the new session will take place. + // at this point the previous session and its session file already became a new envelope file + // to be sent, so deleting it if (!previousSessionFile.delete()) { options.getLogger().log(WARNING, "Failed to delete the previous session file."); } diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 27abd369fc..fd6e71f430 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -17,6 +17,7 @@ import io.sentry.util.thread.NoOpMainThreadChecker; import java.io.File; import java.lang.reflect.InvocationTargetException; +import java.nio.file.Files; import java.util.Arrays; import java.util.List; import org.jetbrains.annotations.ApiStatus; @@ -224,9 +225,27 @@ private static synchronized void init( integration.register(HubAdapter.getInstance(), options); } + finalizePreviousSession(options, HubAdapter.getInstance()); + notifyOptionsObservers(options); } + private static void finalizePreviousSession( + final @NotNull SentryOptions options, + final @NotNull IHub hub + ) { + // enqueue a task to finalize previous session. Since the executor + // is single-threaded, this task will be enqueued sequentially after all integrations that have + // to modify the previous session have done their work, even if they do that async. + try { + options + .getExecutorService() + .submit(new PreviousSessionFinalizer(options, hub)); + } catch (Throwable e) { + options.getLogger().log(SentryLevel.DEBUG, "Failed to notify options observers.", e); + } + } + @SuppressWarnings("FutureReturnValueIgnored") private static void notifyOptionsObservers(final @NotNull SentryOptions options) { // enqueue a task to trigger the static options change for the observers. Since the executor diff --git a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java index 9340ce1b50..317126f760 100644 --- a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java +++ b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java @@ -18,7 +18,6 @@ import io.sentry.Session; import io.sentry.UncaughtExceptionHandlerIntegration; import io.sentry.hints.AbnormalExit; -import io.sentry.hints.PreviousSessionEnd; import io.sentry.hints.SessionEnd; import io.sentry.hints.SessionStart; import io.sentry.transport.NoOpEnvelopeCache; @@ -46,6 +45,8 @@ import java.util.Map; import java.util.UUID; import java.util.WeakHashMap; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -66,6 +67,8 @@ public class EnvelopeCache extends CacheStrategy implements IEnvelopeCache { public static final String STARTUP_CRASH_MARKER_FILE = "startup_crash"; + private static final CountDownLatch previousSessionLatch = new CountDownLatch(1); + private final @NotNull Map fileNameMap = new WeakHashMap<>(); public static @NotNull IEnvelopeCache create(final @NotNull SentryOptions options) { @@ -93,6 +96,7 @@ public void store(@NotNull SentryEnvelope envelope, final @NotNull Hint hint) { rotateCacheIfNeeded(allEnvelopeFiles()); final File currentSessionFile = getCurrentSessionFile(directory.getAbsolutePath()); + final File previousSessionFile = getPreviousSessionFile(directory.getAbsolutePath()); if (HintUtils.hasType(hint, SessionEnd.class)) { if (!currentSessionFile.delete()) { @@ -100,9 +104,25 @@ public void store(@NotNull SentryEnvelope envelope, final @NotNull Hint hint) { } } - // TODO: get rid of this, move it to the executor service, but after ANR, do native crash handling there, also endSession on close(), - // TODO: rename current session synchronously on init + envelope = endPreviousSessionForAbnormalExit(envelope, hint); + 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.deserialize(reader, Session.class); + if (session != null) { + writeSessionToDisk(previousSessionFile, session); + } + } catch (Throwable e) { + options.getLogger().log(SentryLevel.ERROR, "Error processing session.", e); + } + } + previousSessionLatch.countDown(); + updateCurrentSession(currentSessionFile, envelope); boolean crashedLastRun = false; @@ -162,13 +182,10 @@ public void store(@NotNull SentryEnvelope envelope, final @NotNull Hint hint) { } /** - * Attempts to end previous session, relying on PreviousSessionEnd hint. If the hint is also - * AbnormalExit, marks session as abnormal with abnormal mechanism and takes its timestamp. - *

- * If there was no abnormal exit, the previous session will be captured with the current session - * at latest, preserving the original end timestamp. + * Attempts to end previous session, relying on AbnormalExit hint, marks session as abnormal with + * abnormal mechanism and takes its timestamp. *

- * Otherwise, callers might also call it directly when necessary. + * If there was no abnormal exit, the previous session will be captured by PreviousSessionFinalizer. * * @param hint a hint coming with the envelope * @param envelope an original envelope that is being stored @@ -176,79 +193,62 @@ public void store(@NotNull SentryEnvelope envelope, final @NotNull Hint hint) { * old one, if the previous session should not be sent with the current envelope. */ @SuppressWarnings("JavaUtilDate") - public @NotNull SentryEnvelope endPreviousSession( + public @NotNull SentryEnvelope endPreviousSessionForAbnormalExit( final @NotNull SentryEnvelope envelope, final @NotNull Hint hint ) { - if (HintUtils.hasType(hint, PreviousSessionEnd.class)) { + final Object sdkHint = HintUtils.getSentrySdkHint(hint); + if (sdkHint instanceof AbnormalExit) { final File previousSessionFile = getPreviousSessionFile(directory.getAbsolutePath()); - final Object sdkHint = HintUtils.getSentrySdkHint(hint); if (previousSessionFile.exists()) { options.getLogger().log(WARNING, "Previous session is not ended, we'd need to end it."); - return endSessionIntoEnvelope(previousSessionFile, envelope, sdkHint); - } else { - options.getLogger().log(DEBUG, "No session file to end."); - } - } - return envelope; - } + try (final Reader reader = + new BufferedReader( + new InputStreamReader(new FileInputStream(previousSessionFile), UTF_8))) { + final Session session = serializer.deserialize(reader, Session.class); + if (session != null) { + final AbnormalExit abnormalHint = (AbnormalExit) sdkHint; + final @Nullable Long abnormalExitTimestamp = abnormalHint.timestamp(); + Date timestamp = null; + + if (abnormalExitTimestamp != null) { + timestamp = DateUtils.getDateTime(abnormalExitTimestamp); + // sanity check if the abnormal exit actually happened when the session was alive + final Date sessionStart = session.getStarted(); + if (sessionStart == null || timestamp.before(sessionStart)) { + options.getLogger() + .log(WARNING, + "Abnormal exit happened before previous session start, not ending the session."); + return envelope; + } + } - @SuppressWarnings("JavaUtilDate") - private @NotNull SentryEnvelope endSessionIntoEnvelope( - final @NotNull File sessionFile, - final @NotNull SentryEnvelope envelope, - final @Nullable Object sdkHint - ) { - try (final Reader reader = - new BufferedReader( - new InputStreamReader(new FileInputStream(sessionFile), UTF_8))) { - final Session session = serializer.deserialize(reader, Session.class); - if (session != null) { - if (sdkHint instanceof AbnormalExit) { - final AbnormalExit abnormalHint = (AbnormalExit) sdkHint; - final @Nullable Long abnormalExitTimestamp = abnormalHint.timestamp(); - Date timestamp = null; - - if (abnormalExitTimestamp != null) { - timestamp = DateUtils.getDateTime(abnormalExitTimestamp); - // sanity check if the abnormal exit actually happened when the session was alive - final Date sessionStart = session.getStarted(); - if (sessionStart == null || timestamp.before(sessionStart)) { - options.getLogger().log(WARNING, "Abnormal exit happened before previous session start, not ending the session."); - return envelope; + final String abnormalMechanism = abnormalHint.mechanism(); + session.update(Session.State.Abnormal, null, true, abnormalMechanism); + // we have to use the actual timestamp of the Abnormal Exit here to mark the session + // as finished at the time it happened + session.end(timestamp); + + final SentryEnvelopeItem sessionItem = + SentryEnvelopeItem.fromSession(serializer, session); + // send session in the same envelope as abnormal exit event + final SentryEnvelope newEnvelope = buildNewEnvelope(envelope, sessionItem); + + // at this point the session and its file already became a new envelope, so + // it's safe to delete it + if (!previousSessionFile.delete()) { + options.getLogger().log(WARNING, "Failed to delete the previous session file."); } + return newEnvelope; } - - final String abnormalMechanism = abnormalHint.mechanism(); - session.update(Session.State.Abnormal, null, true, abnormalMechanism); - // we have to use the actual timestamp of the Abnormal Exit here to mark the session - // as finished at the time it happened - session.end(timestamp); - } - - final SentryEnvelope newEnvelope; - // if an envelope has items, we send the session in the same envelope, otherwise - // it's a dummy envelope and we build a new envelope and return it - if (envelope.getItems().iterator().hasNext()) { - final SentryEnvelopeItem sessionItem = - SentryEnvelopeItem.fromSession(serializer, session); - newEnvelope = buildNewEnvelope(envelope, sessionItem); - } else { - newEnvelope = - SentryEnvelope.from(serializer, session, options.getSdkVersion()); - } - - // at this point the session and its file already became a new envelope, so - // it's safe to delete it - if (!sessionFile.delete()) { - options.getLogger().log(WARNING, "Failed to delete the session file."); + } catch (Throwable e) { + options.getLogger().log(SentryLevel.ERROR, "Error processing previous session.", e); } - return newEnvelope; + } else { + options.getLogger().log(DEBUG, "No previous session file to end."); } - } catch (Throwable e) { - options.getLogger().log(SentryLevel.ERROR, "Error processing previous session.", e); } return envelope; } @@ -440,4 +440,16 @@ public void discard(final @NotNull SentryEnvelope envelope) { } return new File[] {}; } + + /** + * Awaits until the previous session (if any) is flushed to its own file. + */ + public static boolean waitPreviousSessionFlush() { + try { + return previousSessionLatch.await(30, TimeUnit.SECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + return false; + } } diff --git a/sentry/src/main/java/io/sentry/hints/PreviousSessionEnd.java b/sentry/src/main/java/io/sentry/hints/PreviousSessionEnd.java deleted file mode 100644 index 0aaa305cf9..0000000000 --- a/sentry/src/main/java/io/sentry/hints/PreviousSessionEnd.java +++ /dev/null @@ -1,5 +0,0 @@ -package io.sentry.hints; - -/** Hint that shows this is a previous session end envelope */ -public interface PreviousSessionEnd { -} diff --git a/sentry/src/main/java/io/sentry/hints/PreviousSessionEndHint.java b/sentry/src/main/java/io/sentry/hints/PreviousSessionEndHint.java deleted file mode 100644 index 78ad92da7d..0000000000 --- a/sentry/src/main/java/io/sentry/hints/PreviousSessionEndHint.java +++ /dev/null @@ -1,3 +0,0 @@ -package io.sentry.hints; - -public final class PreviousSessionEndHint implements PreviousSessionEnd {} From de79fbe424082e9d9cadf965e3a5fd5e1ff3bf85 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 30 Mar 2023 15:31:58 +0200 Subject: [PATCH 06/16] Formatting --- .../sentry/android/core/AnrV2Integration.java | 8 +-- .../io/sentry/android/core/SentryAndroid.java | 8 +-- .../io/sentry/PreviousSessionFinalizer.java | 63 ++++++++++--------- sentry/src/main/java/io/sentry/Sentry.java | 9 +-- .../main/java/io/sentry/SentryEnvelope.java | 1 - .../java/io/sentry/cache/EnvelopeCache.java | 42 ++++++------- 6 files changed, 61 insertions(+), 70 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java index 9759174386..2d52daad53 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java @@ -125,10 +125,10 @@ public void run() { if (!EnvelopeCache.waitPreviousSessionFlush()) { options - .getLogger() - .log( - SentryLevel.WARNING, - "Timed out waiting to flush previous session to its own file."); + .getLogger() + .log( + SentryLevel.WARNING, + "Timed out waiting to flush previous session to its own file."); } // making a deep copy as we're modifying the list diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java index b90562f87e..4a0fad3154 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java @@ -125,10 +125,10 @@ public static synchronized void init( final @NotNull IHub hub = Sentry.getCurrentHub(); if (hub.getOptions().isEnableAutoSessionTracking() && ContextUtils.isForegroundImportance(context)) { - //hub.getOptions().getExecutorService().submit(() -> { - hub.addBreadcrumb(BreadcrumbFactory.forSession("session.start")); - hub.startSession(); - //}); + // hub.getOptions().getExecutorService().submit(() -> { + hub.addBreadcrumb(BreadcrumbFactory.forSession("session.start")); + hub.startSession(); + // }); } } catch (IllegalAccessException e) { logger.log(SentryLevel.FATAL, "Fatal error during SentryAndroid.init(...)", e); diff --git a/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java b/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java index 01b9475c67..22f06aa256 100644 --- a/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java +++ b/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java @@ -1,5 +1,11 @@ package io.sentry; +import static io.sentry.SentryLevel.DEBUG; +import static io.sentry.SentryLevel.ERROR; +import static io.sentry.SentryLevel.INFO; +import static io.sentry.SentryLevel.WARNING; +import static io.sentry.cache.EnvelopeCache.NATIVE_CRASH_MARKER_FILE; + import io.sentry.cache.EnvelopeCache; import java.io.BufferedReader; import java.io.File; @@ -12,16 +18,10 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import static io.sentry.SentryLevel.DEBUG; -import static io.sentry.SentryLevel.ERROR; -import static io.sentry.SentryLevel.INFO; -import static io.sentry.SentryLevel.WARNING; -import static io.sentry.cache.EnvelopeCache.NATIVE_CRASH_MARKER_FILE; - /** - * Common cases when previous session is not ended properly (app background or crash): - * - The previous session experienced Abnormal exit (ANR, OS kills app, User kills app) - * - The previous session experienced native crash + * Common cases when previous session is not ended properly (app background or crash): - The + * previous session experienced Abnormal exit (ANR, OS kills app, User kills app) - The previous + * session experienced native crash */ final class PreviousSessionFinalizer implements Runnable { @@ -37,7 +37,8 @@ final class PreviousSessionFinalizer implements Runnable { this.hub = hub; } - @Override public void run() { + @Override + public void run() { final String cacheDirPath = options.getCacheDirPath(); if (cacheDirPath == null) { options.getLogger().log(INFO, "Cache dir is not set, not finalizing the previous session."); @@ -46,10 +47,10 @@ final class PreviousSessionFinalizer implements Runnable { if (!EnvelopeCache.waitPreviousSessionFlush()) { options - .getLogger() - .log( - SentryLevel.WARNING, - "Timed out waiting to flush previous session to its own file in session finalizer."); + .getLogger() + .log( + SentryLevel.WARNING, + "Timed out waiting to flush previous session to its own file in session finalizer."); return; } @@ -60,35 +61,35 @@ final class PreviousSessionFinalizer implements Runnable { 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(previousSessionFile), UTF_8))) { + new BufferedReader( + new InputStreamReader(new FileInputStream(previousSessionFile), UTF_8))) { final Session session = serializer.deserialize(reader, Session.class); if (session == null) { options - .getLogger() - .log( - SentryLevel.ERROR, - "Stream from path %s resulted in a null envelope.", - previousSessionFile.getAbsolutePath()); + .getLogger() + .log( + SentryLevel.ERROR, + "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); + new File(options.getCacheDirPath(), NATIVE_CRASH_MARKER_FILE); if (crashMarkerFile.exists()) { options - .getLogger() - .log(INFO, "Crash marker file exists, last Session is gonna be Crashed."); + .getLogger() + .log(INFO, "Crash marker file exists, last Session is gonna be Crashed."); timestamp = getTimestampFromCrashMarkerFile(crashMarkerFile); if (!crashMarkerFile.delete()) { options - .getLogger() - .log( - ERROR, - "Failed to delete the crash marker file. %s.", - crashMarkerFile.getAbsolutePath()); + .getLogger() + .log( + ERROR, + "Failed to delete the crash marker file. %s.", + crashMarkerFile.getAbsolutePath()); } session.update(Session.State.Crashed, null, true); } @@ -97,7 +98,7 @@ final class PreviousSessionFinalizer implements Runnable { // if the App. has been upgraded and there's a new version of the SDK running, // SdkVersion will be outdated. final SentryEnvelope fromSession = - SentryEnvelope.from(serializer, session, options.getSdkVersion()); + SentryEnvelope.from(serializer, session, options.getSdkVersion()); hub.captureEnvelope(fromSession); } } catch (Throwable e) { @@ -120,7 +121,7 @@ final class PreviousSessionFinalizer implements Runnable { */ private @Nullable Date getTimestampFromCrashMarkerFile(final @NotNull File markerFile) { try (final BufferedReader reader = - new BufferedReader(new InputStreamReader(new FileInputStream(markerFile), UTF_8))) { + new BufferedReader(new InputStreamReader(new FileInputStream(markerFile), UTF_8))) { final String timestamp = reader.readLine(); options.getLogger().log(DEBUG, "Crash marker file has %s timestamp.", timestamp); return DateUtils.getDateTime(timestamp); diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index fd6e71f430..8df308ddeb 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -17,7 +17,6 @@ import io.sentry.util.thread.NoOpMainThreadChecker; import java.io.File; import java.lang.reflect.InvocationTargetException; -import java.nio.file.Files; import java.util.Arrays; import java.util.List; import org.jetbrains.annotations.ApiStatus; @@ -231,16 +230,12 @@ private static synchronized void init( } private static void finalizePreviousSession( - final @NotNull SentryOptions options, - final @NotNull IHub hub - ) { + final @NotNull SentryOptions options, final @NotNull IHub hub) { // enqueue a task to finalize previous session. Since the executor // is single-threaded, this task will be enqueued sequentially after all integrations that have // to modify the previous session have done their work, even if they do that async. try { - options - .getExecutorService() - .submit(new PreviousSessionFinalizer(options, hub)); + options.getExecutorService().submit(new PreviousSessionFinalizer(options, hub)); } catch (Throwable e) { options.getLogger().log(SentryLevel.DEBUG, "Failed to notify options observers.", e); } diff --git a/sentry/src/main/java/io/sentry/SentryEnvelope.java b/sentry/src/main/java/io/sentry/SentryEnvelope.java index 90bba8c901..7d8a01cca3 100644 --- a/sentry/src/main/java/io/sentry/SentryEnvelope.java +++ b/sentry/src/main/java/io/sentry/SentryEnvelope.java @@ -3,7 +3,6 @@ import io.sentry.exception.SentryEnvelopeException; import io.sentry.protocol.SdkVersion; import io.sentry.protocol.SentryId; -import io.sentry.util.CollectionUtils; import io.sentry.util.Objects; import java.io.IOException; import java.util.ArrayList; diff --git a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java index 317126f760..13f6159fa9 100644 --- a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java +++ b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java @@ -111,8 +111,8 @@ public void store(@NotNull SentryEnvelope envelope, final @NotNull Hint hint) { 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))) { + new BufferedReader( + new InputStreamReader(new FileInputStream(currentSessionFile), UTF_8))) { final Session session = serializer.deserialize(reader, Session.class); if (session != null) { writeSessionToDisk(previousSessionFile, session); @@ -126,8 +126,7 @@ public void store(@NotNull SentryEnvelope envelope, final @NotNull Hint hint) { updateCurrentSession(currentSessionFile, envelope); boolean crashedLastRun = false; - final File crashMarkerFile = - new File(options.getCacheDirPath(), NATIVE_CRASH_MARKER_FILE); + final File crashMarkerFile = new File(options.getCacheDirPath(), NATIVE_CRASH_MARKER_FILE); if (crashMarkerFile.exists()) { crashedLastRun = true; } @@ -184,19 +183,18 @@ public void store(@NotNull SentryEnvelope envelope, final @NotNull Hint hint) { /** * Attempts to end previous session, relying on AbnormalExit hint, marks session as abnormal with * abnormal mechanism and takes its timestamp. - *

- * If there was no abnormal exit, the previous session will be captured by PreviousSessionFinalizer. + * + *

If there was no abnormal exit, the previous session will be captured by + * PreviousSessionFinalizer. * * @param hint a hint coming with the envelope * @param envelope an original envelope that is being stored * @return SentryEnvelope returns either a new envelope containing previous session in it, or an - * old one, if the previous session should not be sent with the current envelope. + * old one, if the previous session should not be sent with the current envelope. */ @SuppressWarnings("JavaUtilDate") public @NotNull SentryEnvelope endPreviousSessionForAbnormalExit( - final @NotNull SentryEnvelope envelope, - final @NotNull Hint hint - ) { + final @NotNull SentryEnvelope envelope, final @NotNull Hint hint) { final Object sdkHint = HintUtils.getSentrySdkHint(hint); if (sdkHint instanceof AbnormalExit) { final File previousSessionFile = getPreviousSessionFile(directory.getAbsolutePath()); @@ -205,8 +203,8 @@ public void store(@NotNull SentryEnvelope envelope, final @NotNull Hint hint) { options.getLogger().log(WARNING, "Previous session is not ended, we'd need to end it."); try (final Reader reader = - new BufferedReader( - new InputStreamReader(new FileInputStream(previousSessionFile), UTF_8))) { + new BufferedReader( + new InputStreamReader(new FileInputStream(previousSessionFile), UTF_8))) { final Session session = serializer.deserialize(reader, Session.class); if (session != null) { final AbnormalExit abnormalHint = (AbnormalExit) sdkHint; @@ -218,9 +216,11 @@ public void store(@NotNull SentryEnvelope envelope, final @NotNull Hint hint) { // sanity check if the abnormal exit actually happened when the session was alive final Date sessionStart = session.getStarted(); if (sessionStart == null || timestamp.before(sessionStart)) { - options.getLogger() - .log(WARNING, - "Abnormal exit happened before previous session start, not ending the session."); + options + .getLogger() + .log( + WARNING, + "Abnormal exit happened before previous session start, not ending the session."); return envelope; } } @@ -232,7 +232,7 @@ public void store(@NotNull SentryEnvelope envelope, final @NotNull Hint hint) { session.end(timestamp); final SentryEnvelopeItem sessionItem = - SentryEnvelopeItem.fromSession(serializer, session); + SentryEnvelopeItem.fromSession(serializer, session); // send session in the same envelope as abnormal exit event final SentryEnvelope newEnvelope = buildNewEnvelope(envelope, sessionItem); @@ -390,13 +390,11 @@ public void discard(final @NotNull SentryEnvelope envelope) { } public static @NotNull File getCurrentSessionFile(final @NotNull String cacheDirPath) { - return new File( - cacheDirPath, PREFIX_CURRENT_SESSION_FILE + SUFFIX_SESSION_FILE); + return new File(cacheDirPath, PREFIX_CURRENT_SESSION_FILE + SUFFIX_SESSION_FILE); } public static @NotNull File getPreviousSessionFile(final @NotNull String cacheDirPath) { - return new File( - cacheDirPath, PREFIX_PREVIOUS_SESSION_FILE + SUFFIX_SESSION_FILE); + return new File(cacheDirPath, PREFIX_PREVIOUS_SESSION_FILE + SUFFIX_SESSION_FILE); } @Override @@ -441,9 +439,7 @@ public void discard(final @NotNull SentryEnvelope envelope) { return new File[] {}; } - /** - * Awaits until the previous session (if any) is flushed to its own file. - */ + /** Awaits until the previous session (if any) is flushed to its own file. */ public static boolean waitPreviousSessionFlush() { try { return previousSessionLatch.await(30, TimeUnit.SECONDS); From 5d09a5afc23484017a901f34a8635722ba92dd1c Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 31 Mar 2023 09:44:23 +0200 Subject: [PATCH 07/16] It works --- .../sentry/android/core/AnrV2Integration.java | 12 ++++--- .../io/sentry/android/core/SentryAndroid.java | 2 -- .../io/sentry/PreviousSessionFinalizer.java | 26 +++++++++----- sentry/src/main/java/io/sentry/Sentry.java | 1 + .../main/java/io/sentry/SentryEnvelope.java | 5 --- .../java/io/sentry/cache/EnvelopeCache.java | 36 ++++++------------- 6 files changed, 37 insertions(+), 45 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java index 2d52daad53..67529e7cf5 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java @@ -15,6 +15,7 @@ import io.sentry.SentryOptions; import io.sentry.android.core.cache.AndroidEnvelopeCache; import io.sentry.cache.EnvelopeCache; +import io.sentry.cache.IEnvelopeCache; import io.sentry.exception.ExceptionMechanismException; import io.sentry.hints.AbnormalExit; import io.sentry.hints.Backfillable; @@ -123,12 +124,15 @@ public void run() { return; } - if (!EnvelopeCache.waitPreviousSessionFlush()) { - options + final IEnvelopeCache cache = options.getEnvelopeDiskCache(); + if (cache instanceof EnvelopeCache) { + if (!((EnvelopeCache) cache).waitPreviousSessionFlush()) { + options .getLogger() .log( - SentryLevel.WARNING, - "Timed out waiting to flush previous session to its own file."); + SentryLevel.WARNING, + "Timed out waiting to flush previous session to its own file."); + } } // making a deep copy as we're modifying the list diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java index 4a0fad3154..eb33039a71 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java @@ -125,10 +125,8 @@ public static synchronized void init( final @NotNull IHub hub = Sentry.getCurrentHub(); if (hub.getOptions().isEnableAutoSessionTracking() && ContextUtils.isForegroundImportance(context)) { - // hub.getOptions().getExecutorService().submit(() -> { hub.addBreadcrumb(BreadcrumbFactory.forSession("session.start")); hub.startSession(); - // }); } } catch (IllegalAccessException e) { logger.log(SentryLevel.FATAL, "Fatal error during SentryAndroid.init(...)", e); diff --git a/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java b/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java index 22f06aa256..d26fcd8d45 100644 --- a/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java +++ b/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java @@ -7,6 +7,7 @@ import static io.sentry.cache.EnvelopeCache.NATIVE_CRASH_MARKER_FILE; import io.sentry.cache.EnvelopeCache; +import io.sentry.cache.IEnvelopeCache; import java.io.BufferedReader; import java.io.File; import java.io.FileInputStream; @@ -19,9 +20,9 @@ import org.jetbrains.annotations.Nullable; /** - * Common cases when previous session is not ended properly (app background or crash): - The - * previous session experienced Abnormal exit (ANR, OS kills app, User kills app) - The previous - * session experienced native crash + * Common cases when previous session is not ended properly (app background or crash): + *

- The previous session experienced Abnormal exit (ANR, OS kills app, User kills app) + *

- The previous session experienced native crash */ final class PreviousSessionFinalizer implements Runnable { @@ -45,13 +46,16 @@ public void run() { return; } - if (!EnvelopeCache.waitPreviousSessionFlush()) { - options + final IEnvelopeCache cache = options.getEnvelopeDiskCache(); + if (cache instanceof EnvelopeCache) { + if (!((EnvelopeCache) cache).waitPreviousSessionFlush()) { + options .getLogger() .log( - SentryLevel.WARNING, - "Timed out waiting to flush previous session to its own file in session finalizer."); - return; + SentryLevel.WARNING, + "Timed out waiting to flush previous session to its own file in session finalizer."); + return; + } } final File previousSessionFile = EnvelopeCache.getPreviousSessionFile(cacheDirPath); @@ -93,7 +97,11 @@ public void run() { } session.update(Session.State.Crashed, null, true); } - session.end(timestamp); + // 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, // SdkVersion will be outdated. diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 8df308ddeb..0d4ad1c235 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -229,6 +229,7 @@ private static synchronized void init( notifyOptionsObservers(options); } + @SuppressWarnings("FutureReturnValueIgnored") private static void finalizePreviousSession( final @NotNull SentryOptions options, final @NotNull IHub hub) { // enqueue a task to finalize previous session. Since the executor diff --git a/sentry/src/main/java/io/sentry/SentryEnvelope.java b/sentry/src/main/java/io/sentry/SentryEnvelope.java index 7d8a01cca3..5c3f141baf 100644 --- a/sentry/src/main/java/io/sentry/SentryEnvelope.java +++ b/sentry/src/main/java/io/sentry/SentryEnvelope.java @@ -6,7 +6,6 @@ import io.sentry.util.Objects; import java.io.IOException; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @@ -93,8 +92,4 @@ public SentryEnvelope( sdkVersion, SentryEnvelopeItem.fromProfilingTrace(profilingTraceData, maxTraceFileSize, serializer)); } - - public static @NotNull SentryEnvelope empty() { - return new SentryEnvelope(new SentryEnvelopeHeader(SentryId.EMPTY_ID), Collections.emptyList()); - } } diff --git a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java index 13f6159fa9..125d046fd9 100644 --- a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java +++ b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java @@ -67,7 +67,7 @@ public class EnvelopeCache extends CacheStrategy implements IEnvelopeCache { public static final String STARTUP_CRASH_MARKER_FILE = "startup_crash"; - private static final CountDownLatch previousSessionLatch = new CountDownLatch(1); + private final CountDownLatch previousSessionLatch; private final @NotNull Map fileNameMap = new WeakHashMap<>(); @@ -87,6 +87,7 @@ public EnvelopeCache( final @NotNull String cacheDirPath, final int maxCacheItems) { super(options, cacheDirPath, maxCacheItems); + previousSessionLatch = new CountDownLatch(1); } @Override @@ -104,7 +105,7 @@ public void store(@NotNull SentryEnvelope envelope, final @NotNull Hint hint) { } } - envelope = endPreviousSessionForAbnormalExit(envelope, hint); + endPreviousSessionForAbnormalExit(hint); if (HintUtils.hasType(hint, SessionStart.class)) { if (currentSessionFile.exists()) { @@ -121,8 +122,6 @@ public void store(@NotNull SentryEnvelope envelope, final @NotNull Hint hint) { options.getLogger().log(SentryLevel.ERROR, "Error processing session.", e); } } - previousSessionLatch.countDown(); - updateCurrentSession(currentSessionFile, envelope); boolean crashedLastRun = false; @@ -152,6 +151,8 @@ public void store(@NotNull SentryEnvelope envelope, final @NotNull Hint hint) { } SentryCrashLastRunState.getInstance().setCrashedLastRun(crashedLastRun); + + previousSessionLatch.countDown(); } // TODO: probably we need to update the current session file for session updates to because of @@ -188,13 +189,9 @@ public void store(@NotNull SentryEnvelope envelope, final @NotNull Hint hint) { * PreviousSessionFinalizer. * * @param hint a hint coming with the envelope - * @param envelope an original envelope that is being stored - * @return SentryEnvelope returns either a new envelope containing previous session in it, or an - * old one, if the previous session should not be sent with the current envelope. */ @SuppressWarnings("JavaUtilDate") - public @NotNull SentryEnvelope endPreviousSessionForAbnormalExit( - final @NotNull SentryEnvelope envelope, final @NotNull Hint hint) { + public void endPreviousSessionForAbnormalExit(final @NotNull Hint hint) { final Object sdkHint = HintUtils.getSentrySdkHint(hint); if (sdkHint instanceof AbnormalExit) { final File previousSessionFile = getPreviousSessionFile(directory.getAbsolutePath()); @@ -221,7 +218,7 @@ public void store(@NotNull SentryEnvelope envelope, final @NotNull Hint hint) { .log( WARNING, "Abnormal exit happened before previous session start, not ending the session."); - return envelope; + return; } } @@ -230,18 +227,7 @@ public void store(@NotNull SentryEnvelope envelope, final @NotNull Hint hint) { // we have to use the actual timestamp of the Abnormal Exit here to mark the session // as finished at the time it happened session.end(timestamp); - - final SentryEnvelopeItem sessionItem = - SentryEnvelopeItem.fromSession(serializer, session); - // send session in the same envelope as abnormal exit event - final SentryEnvelope newEnvelope = buildNewEnvelope(envelope, sessionItem); - - // at this point the session and its file already became a new envelope, so - // it's safe to delete it - if (!previousSessionFile.delete()) { - options.getLogger().log(WARNING, "Failed to delete the previous session file."); - } - return newEnvelope; + writeSessionToDisk(previousSessionFile, session); } } catch (Throwable e) { options.getLogger().log(SentryLevel.ERROR, "Error processing previous session.", e); @@ -250,7 +236,6 @@ public void store(@NotNull SentryEnvelope envelope, final @NotNull Hint hint) { options.getLogger().log(DEBUG, "No previous session file to end."); } } - return envelope; } private void writeCrashMarkerFile() { @@ -440,11 +425,12 @@ public void discard(final @NotNull SentryEnvelope envelope) { } /** Awaits until the previous session (if any) is flushed to its own file. */ - public static boolean waitPreviousSessionFlush() { + public boolean waitPreviousSessionFlush() { try { - return previousSessionLatch.await(30, TimeUnit.SECONDS); + return previousSessionLatch.await(60_000L, TimeUnit.MILLISECONDS); } catch (InterruptedException e) { Thread.currentThread().interrupt(); + options.getLogger().log(DEBUG, "Timed out waiting for previous session to flush."); } return false; } From 67d444d632ade3687a8eefbcb80044462fc65793 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 31 Mar 2023 09:44:48 +0200 Subject: [PATCH 08/16] Formatting --- .../io/sentry/android/core/AnrV2Integration.java | 8 ++++---- .../java/io/sentry/PreviousSessionFinalizer.java | 14 ++++++++------ 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java index 67529e7cf5..ebb872afad 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java @@ -128,10 +128,10 @@ public void run() { if (cache instanceof EnvelopeCache) { if (!((EnvelopeCache) cache).waitPreviousSessionFlush()) { options - .getLogger() - .log( - SentryLevel.WARNING, - "Timed out waiting to flush previous session to its own file."); + .getLogger() + .log( + SentryLevel.WARNING, + "Timed out waiting to flush previous session to its own file."); } } diff --git a/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java b/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java index d26fcd8d45..def1d7e695 100644 --- a/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java +++ b/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java @@ -21,8 +21,10 @@ /** * Common cases when previous session is not ended properly (app background or crash): - *

- The previous session experienced Abnormal exit (ANR, OS kills app, User kills app) - *

- The previous session experienced native crash + * + *

- The previous session experienced Abnormal exit (ANR, OS kills app, User kills app) + * + *

- The previous session experienced native crash */ final class PreviousSessionFinalizer implements Runnable { @@ -50,10 +52,10 @@ public void run() { if (cache instanceof EnvelopeCache) { if (!((EnvelopeCache) cache).waitPreviousSessionFlush()) { options - .getLogger() - .log( - SentryLevel.WARNING, - "Timed out waiting to flush previous session to its own file in session finalizer."); + .getLogger() + .log( + SentryLevel.WARNING, + "Timed out waiting to flush previous session to its own file in session finalizer."); return; } } From a8104a6636add3be30ae81fd0bc085b27622d53e Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 31 Mar 2023 23:30:28 +0200 Subject: [PATCH 09/16] Tests --- .../android/core/AnrV2IntegrationTest.kt | 63 +++++- sentry/src/main/java/io/sentry/Sentry.java | 4 +- .../java/io/sentry/cache/CacheStrategy.java | 2 +- .../java/io/sentry/cache/EnvelopeCache.java | 4 +- .../test/java/io/sentry/OutboxSenderTest.kt | 5 + .../io/sentry/PreviousSessionFinalizerTest.kt | 162 ++++++++++++++ sentry/src/test/java/io/sentry/SentryTest.kt | 78 +++++++ .../java/io/sentry/cache/EnvelopeCacheTest.kt | 202 +++++++++++------- 8 files changed, 435 insertions(+), 85 deletions(-) create mode 100644 sentry/src/test/java/io/sentry/PreviousSessionFinalizerTest.kt diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AnrV2IntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AnrV2IntegrationTest.kt index 648a514ab0..e9d20e34f4 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AnrV2IntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AnrV2IntegrationTest.kt @@ -8,11 +8,14 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 import io.sentry.Hint import io.sentry.IHub import io.sentry.ILogger +import io.sentry.SentryEnvelope import io.sentry.SentryLevel import io.sentry.android.core.AnrV2Integration.AnrV2Hint import io.sentry.android.core.cache.AndroidEnvelopeCache +import io.sentry.cache.EnvelopeCache import io.sentry.exception.ExceptionMechanismException import io.sentry.hints.DiskFlushNotification +import io.sentry.hints.SessionStartHint import io.sentry.protocol.SentryId import io.sentry.test.ImmediateExecutorService import io.sentry.util.HintUtils @@ -73,6 +76,7 @@ class AnrV2IntegrationTest { if (useImmediateExecutorService) ImmediateExecutorService() else mock() this.isAnrEnabled = isAnrEnabled this.flushTimeoutMillis = flushTimeoutMillis + setEnvelopeDiskCache(EnvelopeCache.create(this)) } options.cacheDirPath?.let { cacheDir -> lastReportedAnrFile = File(cacheDir, AndroidEnvelopeCache.LAST_ANR_REPORT) @@ -175,6 +179,16 @@ class AnrV2IntegrationTest { verify(fixture.hub, never()).captureEvent(any(), anyOrNull()) } + @Test + fun `when no ANRs have ever been reported, captures events`() { + val integration = fixture.getSut(tmpDir, lastReportedAnrTimestamp = null) + fixture.addAppExitInfo(timestamp = oldTimestamp) + + integration.register(fixture.hub, fixture.options) + + verify(fixture.hub).captureEvent(any(), anyOrNull()) + } + @Test fun `when latest ANR has not been reported, captures event with enriching`() { val integration = fixture.getSut(tmpDir, lastReportedAnrTimestamp = oldTimestamp) @@ -244,7 +258,7 @@ class AnrV2IntegrationTest { // shouldn't fall into timed out state, because we marked event as flushed on another thread verify(fixture.logger, never()).log( any(), - argThat { startsWith("Timed out") }, + argThat { startsWith("Timed out waiting to flush ANR event to disk.") }, any() ) } @@ -265,7 +279,7 @@ class AnrV2IntegrationTest { // we drop the event, it should not even come to this if-check verify(fixture.logger, never()).log( any(), - argThat { startsWith("Timed out") }, + argThat { startsWith("Timed out waiting to flush ANR event to disk.") }, any() ) } @@ -331,4 +345,49 @@ class AnrV2IntegrationTest { } ) } + + @Test + fun `abnormal mechanism is passed with the hint`() { + val integration = fixture.getSut(tmpDir, lastReportedAnrTimestamp = oldTimestamp) + fixture.addAppExitInfo(timestamp = newTimestamp) + + integration.register(fixture.hub, fixture.options) + + verify(fixture.hub).captureEvent( + any(), + argThat { + val hint = HintUtils.getSentrySdkHint(this) + (hint as AnrV2Hint).mechanism() == "anr_background" + } + ) + } + + @Test + fun `awaits for previous session flush if cache is EnvelopeCache`() { + val integration = fixture.getSut( + tmpDir, + lastReportedAnrTimestamp = oldTimestamp, + flushTimeoutMillis = 3000L + ) + fixture.addAppExitInfo(timestamp = newTimestamp) + + thread { + Thread.sleep(1000L) + val sessionHint = HintUtils.createWithTypeCheckHint(SessionStartHint()) + fixture.options.envelopeDiskCache.store( + SentryEnvelope(SentryId.EMPTY_ID, null, emptyList()), + sessionHint + ) + } + + integration.register(fixture.hub, fixture.options) + + // we store envelope with StartSessionHint on different thread after some delay, which + // triggers the previous session flush, so no timeout + verify(fixture.logger, never()).log( + any(), + argThat { startsWith("Timed out waiting to flush previous session to its own file.") }, + any() + ) + } } diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 0d4ad1c235..362b0beee3 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -224,9 +224,9 @@ private static synchronized void init( integration.register(HubAdapter.getInstance(), options); } - finalizePreviousSession(options, HubAdapter.getInstance()); - notifyOptionsObservers(options); + + finalizePreviousSession(options, HubAdapter.getInstance()); } @SuppressWarnings("FutureReturnValueIgnored") diff --git a/sentry/src/main/java/io/sentry/cache/CacheStrategy.java b/sentry/src/main/java/io/sentry/cache/CacheStrategy.java index a162a6b357..dbb6a49c19 100644 --- a/sentry/src/main/java/io/sentry/cache/CacheStrategy.java +++ b/sentry/src/main/java/io/sentry/cache/CacheStrategy.java @@ -276,7 +276,7 @@ private void saveNewEnvelope( } } - protected @NotNull SentryEnvelope buildNewEnvelope( + private @NotNull SentryEnvelope buildNewEnvelope( final @NotNull SentryEnvelope envelope, final @NotNull SentryEnvelopeItem sessionItem) { final List newEnvelopeItems = new ArrayList<>(); diff --git a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java index 125d046fd9..2b647737d6 100644 --- a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java +++ b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java @@ -91,7 +91,7 @@ public EnvelopeCache( } @Override - public void store(@NotNull SentryEnvelope envelope, final @NotNull Hint hint) { + public void store(final @NotNull SentryEnvelope envelope, final @NotNull Hint hint) { Objects.requireNonNull(envelope, "Envelope is required."); rotateCacheIfNeeded(allEnvelopeFiles()); @@ -427,7 +427,7 @@ public void discard(final @NotNull SentryEnvelope envelope) { /** Awaits until the previous session (if any) is flushed to its own file. */ public boolean waitPreviousSessionFlush() { try { - return previousSessionLatch.await(60_000L, TimeUnit.MILLISECONDS); + return previousSessionLatch.await(options.getFlushTimeoutMillis(), TimeUnit.MILLISECONDS); } catch (InterruptedException e) { Thread.currentThread().interrupt(); options.getLogger().log(DEBUG, "Timed out waiting for previous session to flush."); diff --git a/sentry/src/test/java/io/sentry/OutboxSenderTest.kt b/sentry/src/test/java/io/sentry/OutboxSenderTest.kt index 3b24dcc620..4f8f1d9860 100644 --- a/sentry/src/test/java/io/sentry/OutboxSenderTest.kt +++ b/sentry/src/test/java/io/sentry/OutboxSenderTest.kt @@ -322,6 +322,11 @@ class OutboxSenderTest { assertFalse(fixture.getSut().isRelevantFileName(EnvelopeCache.STARTUP_CRASH_MARKER_FILE)) } + @Test + fun `when file name is previous session file, should be ignored`() { + assertFalse(fixture.getSut().isRelevantFileName(EnvelopeCache.PREFIX_PREVIOUS_SESSION_FILE)) + } + @Test fun `when file name is relevant, should return true`() { assertTrue(fixture.getSut().isRelevantFileName("123.envelope")) diff --git a/sentry/src/test/java/io/sentry/PreviousSessionFinalizerTest.kt b/sentry/src/test/java/io/sentry/PreviousSessionFinalizerTest.kt new file mode 100644 index 0000000000..ab5553fac7 --- /dev/null +++ b/sentry/src/test/java/io/sentry/PreviousSessionFinalizerTest.kt @@ -0,0 +1,162 @@ +package io.sentry + +import io.sentry.Session.State.Crashed +import io.sentry.cache.EnvelopeCache +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import org.mockito.kotlin.any +import org.mockito.kotlin.argThat +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.verify +import java.io.File +import java.util.Date +import kotlin.test.assertFalse + +class PreviousSessionFinalizerTest { + + @get:Rule + val tmpDir = TemporaryFolder() + + class Fixture { + val options = SentryOptions() + val hub = mock() + val logger = mock() + lateinit var sessionFile: File + + internal fun getSut( + dir: TemporaryFolder?, + flushTimeoutMillis: Long = 0L, + sessionFileExists: Boolean = true, + session: Session? = null, + nativeCrashTimestamp: Date? = null + ): PreviousSessionFinalizer { + options.run { + setLogger(this@Fixture.logger) + isDebug = true + cacheDirPath = dir?.newFolder()?.absolutePath + this.flushTimeoutMillis = flushTimeoutMillis + } + options.cacheDirPath?.let { + sessionFile = EnvelopeCache.getPreviousSessionFile(it) + sessionFile.parentFile.mkdirs() + if (sessionFileExists) { + sessionFile.createNewFile() + } + if (session != null) { + options.serializer.serialize(session, sessionFile.bufferedWriter()) + } + if (nativeCrashTimestamp != null) { + val nativeCrashMarker = File(it, EnvelopeCache.NATIVE_CRASH_MARKER_FILE) + nativeCrashMarker.parentFile.mkdirs() + nativeCrashMarker.writeText(nativeCrashTimestamp.toString()) + } + } + return PreviousSessionFinalizer(options, hub) + } + + fun sessionFromEnvelope(envelope: SentryEnvelope): Session { + val sessionItem = envelope.items.find { it.header.type == SentryItemType.Session } + return options.serializer.deserialize( + sessionItem!!.data.inputStream().bufferedReader(), + Session::class.java + )!! + } + } + + private val fixture = Fixture() + + @Test + fun `if cacheDir is not set, does not send session update`() { + val finalizer = fixture.getSut(null) + finalizer.run() + + verify(fixture.hub, never()).captureEnvelope(any()) + } + + @Test + fun `if previous session file does not exist, does not send session update`() { + val finalizer = fixture.getSut(tmpDir, sessionFileExists = false) + finalizer.run() + + verify(fixture.hub, never()).captureEnvelope(any()) + } + + @Test + fun `if previous session file exists, but session is null, does not send session update`() { + val finalizer = fixture.getSut(tmpDir, sessionFileExists = true, session = null) + finalizer.run() + + verify(fixture.hub, never()).captureEnvelope(any()) + } + + @Test + fun `if previous session exists, sends session update with current end time`() { + val finalizer = fixture.getSut( + tmpDir, + session = Session(null, null, null, "io.sentry.sample@1.0") + ) + finalizer.run() + + verify(fixture.hub).captureEnvelope(argThat { + val session = fixture.sessionFromEnvelope(this) + session.release == "io.sentry.sample@1.0" && + session.timestamp!!.time - DateUtils.getCurrentDateTime().time < 1000 + }) + } + + @Test + fun `if previous session exists with abnormal mechanism, sends session update without chaging end timestamp`() { + val abnormalEndDate = Date(2023, 10, 1) + val finalizer = fixture.getSut( + tmpDir, + session = Session( + null, + null, + null, + "io.sentry.sample@1.0" + ).apply { + update(null, null, false, "mechanism") + end(abnormalEndDate) + } + ) + finalizer.run() + + verify(fixture.hub).captureEnvelope(argThat { + val session = fixture.sessionFromEnvelope(this) + session.release == "io.sentry.sample@1.0" && + session.timestamp!! == abnormalEndDate + }) + } + + @Test + fun `if native crash marker exists, marks previous session as crashed`() { + val finalizer = fixture.getSut( + tmpDir, + session = Session( + null, + null, + null, + "io.sentry.sample@1.0" + ), + nativeCrashTimestamp = Date(2023, 10, 1) + ) + finalizer.run() + + verify(fixture.hub).captureEnvelope(argThat { + val session = fixture.sessionFromEnvelope(this) + session.release == "io.sentry.sample@1.0" && + session.status == Crashed + }) + } + + @Test + fun `if previous session file exists, deletes previous session file`() { + val finalizer = fixture.getSut(tmpDir, sessionFileExists = true) + finalizer.run() + + verify(fixture.hub, never()).captureEnvelope(any()) + assertFalse(fixture.sessionFile.exists()) + } +} diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt index b43d9c3491..0f2700a339 100644 --- a/sentry/src/test/java/io/sentry/SentryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTest.kt @@ -10,6 +10,7 @@ import io.sentry.test.ImmediateExecutorService import io.sentry.util.thread.IMainThreadChecker import io.sentry.util.thread.MainThreadChecker import org.awaitility.kotlin.await +import org.junit.Rule import org.junit.rules.TemporaryFolder import org.mockito.kotlin.any import org.mockito.kotlin.argThat @@ -33,6 +34,9 @@ class SentryTest { private val dsn = "http://key@localhost/proj" + @get:Rule + val tmpDir = TemporaryFolder() + @BeforeTest @AfterTest fun beforeTest() { @@ -571,6 +575,80 @@ class SentryTest { assertEquals("debug", optionsObserver.environment) } + @Test + fun `init finalizes previous session`() { + lateinit var previousSessionFile: File + + Sentry.init { + it.dsn = dsn + + it.release = "io.sentry.sample@2.0" + it.cacheDirPath = tmpDir.newFolder().absolutePath + + it.executorService = ImmediateExecutorService() + + previousSessionFile = EnvelopeCache.getPreviousSessionFile(it.cacheDirPath!!) + previousSessionFile.parentFile.mkdirs() + it.serializer.serialize( + Session(null, null, "release", "io.sentry.samples@2.0"), + previousSessionFile.bufferedWriter() + ) + assertEquals( + "release", + it.serializer.deserialize(previousSessionFile.bufferedReader(), Session::class.java)!!.environment + ) + + it.addIntegration { hub, _ -> + // this is just a hack to trigger the previousSessionFlush latch, so the finalizer + // does not time out waiting. We have to do it as integration, because this is where + // the hub is already initialized + hub.startSession() + } + } + + + assertFalse(previousSessionFile.exists()) + } + + @Test + fun `if there is work enqueued, init finalizes previous session after that work is done`() { + lateinit var previousSessionFile: File + val triggered = AtomicBoolean(false) + + Sentry.init { + it.dsn = dsn + + it.release = "io.sentry.sample@2.0" + it.cacheDirPath = tmpDir.newFolder().absolutePath + + previousSessionFile = EnvelopeCache.getPreviousSessionFile(it.cacheDirPath!!) + previousSessionFile.parentFile.mkdirs() + it.serializer.serialize( + Session(null, null, "release", "io.sentry.sample@1.0"), + previousSessionFile.bufferedWriter() + ) + + it.executorService.submit { + // here the previous session should still exist. Sentry.init will submit another runnable + // to finalize the previous session, but because the executor is single-threaded, the + // work will be enqueued and the previous session will be finalized after current work is + // finished, ensuring that even if something is using the previous session from a + // different thread, it will still be able to access it. + Thread.sleep(1000L) + val session = it.serializer.deserialize(previousSessionFile.bufferedReader(), Session::class.java) + assertEquals("io.sentry.sample@1.0", session!!.release) + assertEquals("release", session.environment) + triggered.set(true) + } + } + + // to trigger previous session flush + Sentry.startSession() + + await.untilTrue(triggered) + assertFalse(previousSessionFile.exists()) + } + private class InMemoryOptionsObserver : IOptionsObserver { var release: String? = null private set diff --git a/sentry/src/test/java/io/sentry/cache/EnvelopeCacheTest.kt b/sentry/src/test/java/io/sentry/cache/EnvelopeCacheTest.kt index a00c6c9e6b..08ab1c2ba5 100644 --- a/sentry/src/test/java/io/sentry/cache/EnvelopeCacheTest.kt +++ b/sentry/src/test/java/io/sentry/cache/EnvelopeCacheTest.kt @@ -1,29 +1,30 @@ package io.sentry.cache +import io.sentry.DateUtils import io.sentry.ILogger -import io.sentry.ISerializer import io.sentry.NoOpLogger import io.sentry.SentryCrashLastRunState import io.sentry.SentryEnvelope import io.sentry.SentryEvent -import io.sentry.SentryLevel import io.sentry.SentryOptions import io.sentry.Session +import io.sentry.Session.State +import io.sentry.Session.State.Ok import io.sentry.UncaughtExceptionHandlerIntegration.UncaughtExceptionHint import io.sentry.cache.EnvelopeCache.PREFIX_CURRENT_SESSION_FILE import io.sentry.cache.EnvelopeCache.SUFFIX_SESSION_FILE +import io.sentry.hints.AbnormalExit import io.sentry.hints.SessionEndHint import io.sentry.hints.SessionStartHint import io.sentry.protocol.User import io.sentry.util.HintUtils -import org.mockito.kotlin.any -import org.mockito.kotlin.eq import org.mockito.kotlin.mock -import org.mockito.kotlin.verify -import org.mockito.kotlin.whenever import java.io.File import java.nio.file.Files import java.nio.file.Path +import java.util.Date +import java.util.UUID +import java.util.concurrent.TimeUnit import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals @@ -34,22 +35,16 @@ import kotlin.test.assertTrue class EnvelopeCacheTest { private class Fixture { val dir: Path = Files.createTempDirectory("sentry-session-cache-test") - val serializer = mock() val options = SentryOptions() val logger = mock() - fun getSUT(): IEnvelopeCache { + fun getSUT(): EnvelopeCache { options.cacheDirPath = dir.toAbsolutePath().toFile().absolutePath - whenever(serializer.deserialize(any(), eq(Session::class.java))).thenAnswer { - Session("dis", User(), "env", "rel") - } - options.setLogger(logger) - options.setSerializer(serializer) options.setDebug(true) - return EnvelopeCache.create(options) + return EnvelopeCache.create(options) as EnvelopeCache } } @@ -69,7 +64,7 @@ class EnvelopeCacheTest { assertEquals(0, nofFiles()) - cache.store(SentryEnvelope.from(fixture.serializer, createSession(), null)) + cache.store(SentryEnvelope.from(fixture.options.serializer, createSession(), null)) assertEquals(1, nofFiles()) @@ -80,7 +75,7 @@ class EnvelopeCacheTest { fun `tolerates discarding unknown envelope`() { val cache = fixture.getSUT() - cache.discard(SentryEnvelope.from(fixture.serializer, createSession(), null)) + cache.discard(SentryEnvelope.from(fixture.options.serializer, createSession(), null)) // no exception thrown } @@ -91,7 +86,7 @@ class EnvelopeCacheTest { val file = File(fixture.options.cacheDirPath!!) - val envelope = SentryEnvelope.from(fixture.serializer, createSession(), null) + val envelope = SentryEnvelope.from(fixture.options.serializer, createSession(), null) val hints = HintUtils.createWithTypeCheckHint(SessionStartHint()) cache.store(envelope, hints) @@ -108,7 +103,7 @@ class EnvelopeCacheTest { val file = File(fixture.options.cacheDirPath!!) - val envelope = SentryEnvelope.from(fixture.serializer, createSession(), null) + val envelope = SentryEnvelope.from(fixture.options.serializer, createSession(), null) val hints = HintUtils.createWithTypeCheckHint(SessionStartHint()) cache.store(envelope, hints) @@ -129,7 +124,7 @@ class EnvelopeCacheTest { val file = File(fixture.options.cacheDirPath!!) - val envelope = SentryEnvelope.from(fixture.serializer, createSession(), null) + val envelope = SentryEnvelope.from(fixture.options.serializer, createSession(), null) val hints = HintUtils.createWithTypeCheckHint(SessionStartHint()) cache.store(envelope, hints) @@ -137,7 +132,7 @@ class EnvelopeCacheTest { val currentFile = File(fixture.options.cacheDirPath!!, "$PREFIX_CURRENT_SESSION_FILE$SUFFIX_SESSION_FILE") assertTrue(currentFile.exists()) - val session = fixture.serializer.deserialize(currentFile.bufferedReader(Charsets.UTF_8), Session::class.java) + val session = fixture.options.serializer.deserialize(currentFile.bufferedReader(Charsets.UTF_8), Session::class.java) assertNotNull(session) currentFile.delete() @@ -146,126 +141,177 @@ class EnvelopeCacheTest { } @Test - fun `when session start and current file already exist, close session and start a new one`() { + fun `when native crash marker file exist, mark isCrashedLastRun`() { val cache = fixture.getSUT() - val envelope = SentryEnvelope.from(fixture.serializer, createSession(), null) + val file = File(fixture.options.cacheDirPath!!) + val markerFile = File(fixture.options.cacheDirPath!!, EnvelopeCache.NATIVE_CRASH_MARKER_FILE) + markerFile.mkdirs() + assertTrue(markerFile.exists()) + + val envelope = SentryEnvelope.from(fixture.options.serializer, createSession(), null) val hints = HintUtils.createWithTypeCheckHint(SessionStartHint()) cache.store(envelope, hints) - val newEnvelope = SentryEnvelope.from(fixture.serializer, createSession(), null) + val newEnvelope = SentryEnvelope.from(fixture.options.serializer, createSession(), null) + + // since the first store call would set as readCrashedLastRun=true + SentryCrashLastRunState.getInstance().reset() cache.store(newEnvelope, hints) - verify(fixture.logger).log(eq(SentryLevel.WARNING), eq("Current session is not ended, we'd need to end it.")) + file.deleteRecursively() + + // passing empty string since readCrashedLastRun is already set + assertTrue(SentryCrashLastRunState.getInstance().isCrashedLastRun("", false)!!) } @Test - fun `when session start, current file already exist and crash marker file exist, end session and delete marker file`() { + fun `when java crash marker file exist, mark isCrashedLastRun`() { val cache = fixture.getSUT() - val file = File(fixture.options.cacheDirPath!!) - val markerFile = File(fixture.options.cacheDirPath!!, EnvelopeCache.NATIVE_CRASH_MARKER_FILE) + val markerFile = File(fixture.options.cacheDirPath!!, EnvelopeCache.CRASH_MARKER_FILE) markerFile.mkdirs() assertTrue(markerFile.exists()) - val envelope = SentryEnvelope.from(fixture.serializer, createSession(), null) + val envelope = SentryEnvelope.from(fixture.options.serializer, createSession(), null) val hints = HintUtils.createWithTypeCheckHint(SessionStartHint()) cache.store(envelope, hints) - val newEnvelope = SentryEnvelope.from(fixture.serializer, createSession(), null) + // passing empty string since readCrashedLastRun is already set + assertTrue(SentryCrashLastRunState.getInstance().isCrashedLastRun("", false)!!) + assertFalse(markerFile.exists()) + } - cache.store(newEnvelope, hints) - verify(fixture.logger).log(eq(SentryLevel.INFO), eq("Crash marker file exists, last Session is gonna be Crashed.")) + @Test + fun `write java marker file to disk when uncaught exception hint`() { + val cache = fixture.getSUT() + + val markerFile = File(fixture.options.cacheDirPath!!, EnvelopeCache.CRASH_MARKER_FILE) assertFalse(markerFile.exists()) - file.deleteRecursively() + + val envelope = SentryEnvelope.from(fixture.options.serializer, SentryEvent(), null) + + val hints = HintUtils.createWithTypeCheckHint(UncaughtExceptionHint(0, NoOpLogger.getInstance())) + cache.store(envelope, hints) + + assertTrue(markerFile.exists()) } @Test - fun `when session start, current file already exist and crash marker file exist, end session with given timestamp`() { + fun `store with StartSession hint flushes previous session`() { val cache = fixture.getSUT() - val file = File(fixture.options.cacheDirPath!!) - val markerFile = File(fixture.options.cacheDirPath!!, EnvelopeCache.NATIVE_CRASH_MARKER_FILE) - File(fixture.options.cacheDirPath!!, ".sentry-native").mkdirs() - markerFile.createNewFile() - val date = "2020-02-07T14:16:00.000Z" - markerFile.writeText(charset = Charsets.UTF_8, text = date) - val envelope = SentryEnvelope.from(fixture.serializer, createSession(), null) + val envelope = SentryEnvelope.from(fixture.options.serializer, SentryEvent(), null) val hints = HintUtils.createWithTypeCheckHint(SessionStartHint()) cache.store(envelope, hints) - val newEnvelope = SentryEnvelope.from(fixture.serializer, createSession(), null) - - cache.store(newEnvelope, hints) - assertFalse(markerFile.exists()) - file.deleteRecursively() - File(fixture.options.cacheDirPath!!).deleteRecursively() + assertTrue(cache.waitPreviousSessionFlush()) } @Test - fun `when native crash marker file exist, mark isCrashedLastRun`() { + fun `SessionStart hint saves unfinished session to previous_session file`() { val cache = fixture.getSUT() - val file = File(fixture.options.cacheDirPath!!) - val markerFile = File(fixture.options.cacheDirPath!!, EnvelopeCache.NATIVE_CRASH_MARKER_FILE) - markerFile.mkdirs() - assertTrue(markerFile.exists()) + val previousSessionFile = EnvelopeCache.getPreviousSessionFile(fixture.options.cacheDirPath!!) + val currentSessionFile = EnvelopeCache.getCurrentSessionFile(fixture.options.cacheDirPath!!) + val session = createSession() + fixture.options.serializer.serialize(session, currentSessionFile.bufferedWriter()) - val envelope = SentryEnvelope.from(fixture.serializer, createSession(), null) + assertFalse(previousSessionFile.exists()) + val envelope = SentryEnvelope.from(fixture.options.serializer, SentryEvent(), null) val hints = HintUtils.createWithTypeCheckHint(SessionStartHint()) cache.store(envelope, hints) - val newEnvelope = SentryEnvelope.from(fixture.serializer, createSession(), null) + assertTrue(previousSessionFile.exists()) + val persistedSession = fixture.options.serializer.deserialize(previousSessionFile.bufferedReader(), Session::class.java) + assertEquals("dis", persistedSession!!.distinctId) + } - // since the first store call would set as readCrashedLastRun=true - SentryCrashLastRunState.getInstance().reset() + @Test + fun `AbnormalExit hint marks previous session as abnormal with abnormal mechanism and current timestamp`() { + val cache = fixture.getSUT() - cache.store(newEnvelope, hints) - verify(fixture.logger).log(eq(SentryLevel.INFO), eq("Crash marker file exists, last Session is gonna be Crashed.")) - assertFalse(markerFile.exists()) - file.deleteRecursively() + val previousSessionFile = EnvelopeCache.getPreviousSessionFile(fixture.options.cacheDirPath!!) + val session = createSession() + fixture.options.serializer.serialize(session, previousSessionFile.bufferedWriter()) - // passing empty string since readCrashedLastRun is already set - assertTrue(SentryCrashLastRunState.getInstance().isCrashedLastRun("", false)!!) + val envelope = SentryEnvelope.from(fixture.options.serializer, SentryEvent(), null) + val abnormalHint = AbnormalExit { "abnormal_mechanism" } + val hints = HintUtils.createWithTypeCheckHint(abnormalHint) + cache.store(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.timestamp!!.time - DateUtils.getCurrentDateTime().time < 1000 } } @Test - fun `when java crash marker file exist, mark isCrashedLastRun`() { + fun `previous session uses AbnormalExit hint timestamp when available`() { val cache = fixture.getSUT() - val markerFile = File(fixture.options.cacheDirPath!!, EnvelopeCache.CRASH_MARKER_FILE) - markerFile.mkdirs() - assertTrue(markerFile.exists()) + val previousSessionFile = EnvelopeCache.getPreviousSessionFile(fixture.options.cacheDirPath!!) + val sessionStarted = Date(2023, 10, 1) + val sessionExitedWithAbnormal = sessionStarted.time + TimeUnit.HOURS.toMillis(3) + val session = createSession(sessionStarted) + fixture.options.serializer.serialize(session, previousSessionFile.bufferedWriter()) - val envelope = SentryEnvelope.from(fixture.serializer, createSession(), null) + val envelope = SentryEnvelope.from(fixture.options.serializer, SentryEvent(), null) + val abnormalHint = object : AbnormalExit { + override fun mechanism(): String = "abnormal_mechanism" - val hints = HintUtils.createWithTypeCheckHint(SessionStartHint()) + override fun timestamp(): Long = sessionExitedWithAbnormal + } + val hints = HintUtils.createWithTypeCheckHint(abnormalHint) cache.store(envelope, hints) - // passing empty string since readCrashedLastRun is already set - assertTrue(SentryCrashLastRunState.getInstance().isCrashedLastRun("", false)!!) - assertFalse(markerFile.exists()) + val updatedSession = fixture.options.serializer.deserialize(previousSessionFile.bufferedReader(), Session::class.java) + assertEquals(sessionExitedWithAbnormal, updatedSession!!.timestamp!!.time) } @Test - fun `write java marker file to disk when uncaught exception hint`() { + fun `when AbnormalExit happened before previous session start, does not mark as abnormal`() { val cache = fixture.getSUT() - val markerFile = File(fixture.options.cacheDirPath!!, EnvelopeCache.CRASH_MARKER_FILE) - assertFalse(markerFile.exists()) + val previousSessionFile = EnvelopeCache.getPreviousSessionFile(fixture.options.cacheDirPath!!) + val sessionStarted = Date(2023, 10, 1) + val sessionExitedWithAbnormal = sessionStarted.time - TimeUnit.HOURS.toMillis(3) + val session = createSession(sessionStarted) + fixture.options.serializer.serialize(session, previousSessionFile.bufferedWriter()) - val envelope = SentryEnvelope.from(fixture.serializer, SentryEvent(), null) + val envelope = SentryEnvelope.from(fixture.options.serializer, SentryEvent(), null) + val abnormalHint = object : AbnormalExit { + override fun mechanism(): String = "abnormal_mechanism" - val hints = HintUtils.createWithTypeCheckHint(UncaughtExceptionHint(0, NoOpLogger.getInstance())) + override fun timestamp(): Long = sessionExitedWithAbnormal + } + val hints = HintUtils.createWithTypeCheckHint(abnormalHint) cache.store(envelope, hints) - assertTrue(markerFile.exists()) + val updatedSession = fixture.options.serializer.deserialize(previousSessionFile.bufferedReader(), Session::class.java) + assertEquals(Ok, updatedSession!!.status) + assertEquals(null, updatedSession.abnormalMechanism) } - private fun createSession(): Session { - return Session("dis", User(), "env", "rel") + private fun createSession(started: Date? = null): Session { + return Session( + Ok, + started ?: DateUtils.getCurrentDateTime(), + DateUtils.getCurrentDateTime(), + 0, + "dis", + UUID.randomUUID(), + true, + null, + null, + null, + null, + "env", + "rel", + null + ) } } From 7da3a7371cc9a7dbc45f93e2bc9e3f4fd25ddde7 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 31 Mar 2023 23:30:48 +0200 Subject: [PATCH 10/16] Spotless --- .../io/sentry/PreviousSessionFinalizerTest.kt | 36 +++++++++++-------- sentry/src/test/java/io/sentry/SentryTest.kt | 1 - .../java/io/sentry/cache/EnvelopeCacheTest.kt | 1 - 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/sentry/src/test/java/io/sentry/PreviousSessionFinalizerTest.kt b/sentry/src/test/java/io/sentry/PreviousSessionFinalizerTest.kt index ab5553fac7..56b1d044c1 100644 --- a/sentry/src/test/java/io/sentry/PreviousSessionFinalizerTest.kt +++ b/sentry/src/test/java/io/sentry/PreviousSessionFinalizerTest.kt @@ -99,11 +99,13 @@ class PreviousSessionFinalizerTest { ) finalizer.run() - verify(fixture.hub).captureEnvelope(argThat { - val session = fixture.sessionFromEnvelope(this) - session.release == "io.sentry.sample@1.0" && - session.timestamp!!.time - DateUtils.getCurrentDateTime().time < 1000 - }) + verify(fixture.hub).captureEnvelope( + argThat { + val session = fixture.sessionFromEnvelope(this) + session.release == "io.sentry.sample@1.0" && + session.timestamp!!.time - DateUtils.getCurrentDateTime().time < 1000 + } + ) } @Test @@ -123,11 +125,13 @@ class PreviousSessionFinalizerTest { ) finalizer.run() - verify(fixture.hub).captureEnvelope(argThat { - val session = fixture.sessionFromEnvelope(this) - session.release == "io.sentry.sample@1.0" && - session.timestamp!! == abnormalEndDate - }) + verify(fixture.hub).captureEnvelope( + argThat { + val session = fixture.sessionFromEnvelope(this) + session.release == "io.sentry.sample@1.0" && + session.timestamp!! == abnormalEndDate + } + ) } @Test @@ -144,11 +148,13 @@ class PreviousSessionFinalizerTest { ) finalizer.run() - verify(fixture.hub).captureEnvelope(argThat { - val session = fixture.sessionFromEnvelope(this) - session.release == "io.sentry.sample@1.0" && - session.status == Crashed - }) + verify(fixture.hub).captureEnvelope( + argThat { + val session = fixture.sessionFromEnvelope(this) + session.release == "io.sentry.sample@1.0" && + session.status == Crashed + } + ) } @Test diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt index 0f2700a339..176c1700fa 100644 --- a/sentry/src/test/java/io/sentry/SentryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTest.kt @@ -606,7 +606,6 @@ class SentryTest { } } - assertFalse(previousSessionFile.exists()) } diff --git a/sentry/src/test/java/io/sentry/cache/EnvelopeCacheTest.kt b/sentry/src/test/java/io/sentry/cache/EnvelopeCacheTest.kt index 08ab1c2ba5..33970f4093 100644 --- a/sentry/src/test/java/io/sentry/cache/EnvelopeCacheTest.kt +++ b/sentry/src/test/java/io/sentry/cache/EnvelopeCacheTest.kt @@ -16,7 +16,6 @@ import io.sentry.cache.EnvelopeCache.SUFFIX_SESSION_FILE import io.sentry.hints.AbnormalExit import io.sentry.hints.SessionEndHint import io.sentry.hints.SessionStartHint -import io.sentry.protocol.User import io.sentry.util.HintUtils import org.mockito.kotlin.mock import java.io.File From bc9dbe1a910b5fff8e91479773be9ee9bc718e41 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 31 Mar 2023 23:32:04 +0200 Subject: [PATCH 11/16] api dump --- sentry-android-core/api/sentry-android-core.api | 9 +++++---- sentry/api/sentry.api | 5 +++++ sentry/src/main/java/io/sentry/cache/EnvelopeCache.java | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 9a8c074679..e3e51346f8 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -71,10 +71,11 @@ public class io/sentry/android/core/AnrV2Integration : io/sentry/Integration, ja public fun register (Lio/sentry/IHub;Lio/sentry/SentryOptions;)V } -public final class io/sentry/android/core/AnrV2Integration$AnrV2Hint : io/sentry/hints/BlockingFlushHint, io/sentry/hints/Backfillable { - public fun (JLio/sentry/ILogger;JZ)V +public final class io/sentry/android/core/AnrV2Integration$AnrV2Hint : io/sentry/hints/BlockingFlushHint, io/sentry/hints/AbnormalExit, io/sentry/hints/Backfillable { + public fun (JLio/sentry/ILogger;JZZ)V + public fun mechanism ()Ljava/lang/String; public fun shouldEnrich ()Z - public fun timestamp ()J + public fun timestamp ()Ljava/lang/Long; } public final class io/sentry/android/core/AppComponentsBreadcrumbsIntegration : android/content/ComponentCallbacks2, io/sentry/Integration, java/io/Closeable { @@ -297,7 +298,7 @@ public final class io/sentry/android/core/cache/AndroidEnvelopeCache : io/sentry public fun (Lio/sentry/android/core/SentryAndroidOptions;)V public fun getDirectory ()Ljava/io/File; public static fun hasStartupCrashMarker (Lio/sentry/SentryOptions;)Z - public static fun lastReportedAnr (Lio/sentry/SentryOptions;)J + public static fun lastReportedAnr (Lio/sentry/SentryOptions;)Ljava/lang/Long; public fun store (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)V } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 16ef8858d2..25170b8ddc 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -2265,14 +2265,18 @@ public class io/sentry/cache/EnvelopeCache : io/sentry/cache/IEnvelopeCache { public static final field CRASH_MARKER_FILE Ljava/lang/String; public static final field NATIVE_CRASH_MARKER_FILE Ljava/lang/String; public static final field PREFIX_CURRENT_SESSION_FILE Ljava/lang/String; + public static final field PREFIX_PREVIOUS_SESSION_FILE Ljava/lang/String; public static final field STARTUP_CRASH_MARKER_FILE Ljava/lang/String; public static final field SUFFIX_ENVELOPE_FILE Ljava/lang/String; protected static final field UTF_8 Ljava/nio/charset/Charset; public fun (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 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 store (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)V + public fun waitPreviousSessionFlush ()Z } public abstract interface class io/sentry/cache/IEnvelopeCache : java/lang/Iterable { @@ -2454,6 +2458,7 @@ public final class io/sentry/exception/SentryHttpClientException : java/lang/Exc public abstract interface class io/sentry/hints/AbnormalExit { public fun ignoreCurrentThread ()Z public abstract fun mechanism ()Ljava/lang/String; + public fun timestamp ()Ljava/lang/Long; } public abstract interface class io/sentry/hints/ApplyScopeData { diff --git a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java index 2b647737d6..4646e8cea1 100644 --- a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java +++ b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java @@ -191,7 +191,7 @@ public void store(final @NotNull SentryEnvelope envelope, final @NotNull Hint hi * @param hint a hint coming with the envelope */ @SuppressWarnings("JavaUtilDate") - public void endPreviousSessionForAbnormalExit(final @NotNull Hint hint) { + private void endPreviousSessionForAbnormalExit(final @NotNull Hint hint) { final Object sdkHint = HintUtils.getSentrySdkHint(hint); if (sdkHint instanceof AbnormalExit) { final File previousSessionFile = getPreviousSessionFile(directory.getAbsolutePath()); From 6c09cd06c5e9ff0b99e3c65eb2db4db0141c190b Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Mon, 3 Apr 2023 11:50:33 +0200 Subject: [PATCH 12/16] Fix integration tests --- .../sentry/android/core/AnrV2Integration.java | 8 ++- .../android/core/AnrV2IntegrationTest.kt | 52 +++++++++++++++++-- .../uitest/android/AutomaticSpansTest.kt | 1 - .../io/sentry/PreviousSessionFinalizer.java | 9 ++++ .../java/io/sentry/cache/EnvelopeCache.java | 4 ++ .../io/sentry/PreviousSessionFinalizerTest.kt | 40 +++++++++++++- 6 files changed, 106 insertions(+), 8 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java index ebb872afad..ed28e29b8e 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java @@ -36,6 +36,8 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import static io.sentry.SentryLevel.DEBUG; + @SuppressLint("NewApi") // we check this in AnrIntegrationFactory public class AnrV2Integration implements Integration, Closeable { @@ -126,12 +128,16 @@ public void run() { final IEnvelopeCache cache = options.getEnvelopeDiskCache(); if (cache instanceof EnvelopeCache) { - if (!((EnvelopeCache) cache).waitPreviousSessionFlush()) { + if (options.isEnableAutoSessionTracking() && !((EnvelopeCache) cache).waitPreviousSessionFlush()) { options .getLogger() .log( SentryLevel.WARNING, "Timed out waiting to flush previous session to its own file."); + + // if we timed out waiting here, we can already flush the latch, because the timeout is big + // enough to wait for it only once and we don't have to wait again in PreviousSessionFinalizer + ((EnvelopeCache) cache).flushPreviousSession(); } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AnrV2IntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AnrV2IntegrationTest.kt index e9d20e34f4..b09abe9e14 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AnrV2IntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AnrV2IntegrationTest.kt @@ -66,7 +66,8 @@ class AnrV2IntegrationTest { isAnrEnabled: Boolean = true, flushTimeoutMillis: Long = 0L, lastReportedAnrTimestamp: Long? = null, - lastEventId: SentryId = SentryId() + lastEventId: SentryId = SentryId(), + sessionTrackingEnabled: Boolean = true ): AnrV2Integration { options.run { setLogger(this@Fixture.logger) @@ -76,6 +77,7 @@ class AnrV2IntegrationTest { if (useImmediateExecutorService) ImmediateExecutorService() else mock() this.isAnrEnabled = isAnrEnabled this.flushTimeoutMillis = flushTimeoutMillis + this.isEnableAutoSessionTracking = sessionTrackingEnabled setEnvelopeDiskCache(EnvelopeCache.create(this)) } options.cacheDirPath?.let { cacheDir -> @@ -238,7 +240,7 @@ class AnrV2IntegrationTest { val integration = fixture.getSut( tmpDir, lastReportedAnrTimestamp = oldTimestamp, - flushTimeoutMillis = 3000L + flushTimeoutMillis = 1000L ) fixture.addAppExitInfo(timestamp = newTimestamp) @@ -246,7 +248,7 @@ class AnrV2IntegrationTest { val hint = HintUtils.getSentrySdkHint(invocation.getArgument(1)) as DiskFlushNotification thread { - Thread.sleep(1000L) + Thread.sleep(500L) hint.markFlushed() } SentryId() @@ -367,12 +369,12 @@ class AnrV2IntegrationTest { val integration = fixture.getSut( tmpDir, lastReportedAnrTimestamp = oldTimestamp, - flushTimeoutMillis = 3000L + flushTimeoutMillis = 1000L ) fixture.addAppExitInfo(timestamp = newTimestamp) thread { - Thread.sleep(1000L) + Thread.sleep(500L) val sessionHint = HintUtils.createWithTypeCheckHint(SessionStartHint()) fixture.options.envelopeDiskCache.store( SentryEnvelope(SentryId.EMPTY_ID, null, emptyList()), @@ -390,4 +392,44 @@ class AnrV2IntegrationTest { any() ) } + + @Test + fun `does not await for previous session flush, if session tracking is disabled`() { + val integration = fixture.getSut( + tmpDir, + lastReportedAnrTimestamp = oldTimestamp, + flushTimeoutMillis = 500L, + sessionTrackingEnabled = false + ) + fixture.addAppExitInfo(timestamp = newTimestamp) + + integration.register(fixture.hub, fixture.options) + + verify(fixture.logger, never()).log( + any(), + argThat { startsWith("Timed out waiting to flush previous session to its own file.") }, + any() + ) + verify(fixture.hub).captureEvent(any(), any()) + } + + @Test + fun `flushes previous session latch, if timed out waiting`() { + val integration = fixture.getSut( + tmpDir, + lastReportedAnrTimestamp = oldTimestamp, + flushTimeoutMillis = 500L + ) + fixture.addAppExitInfo(timestamp = newTimestamp) + + integration.register(fixture.hub, fixture.options) + + verify(fixture.logger).log( + any(), + argThat { startsWith("Timed out waiting to flush previous session to its own file.") }, + any() + ) + // should return true, because latch is 0 now + assertTrue((fixture.options.envelopeDiskCache as EnvelopeCache).waitPreviousSessionFlush()) + } } diff --git a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/AutomaticSpansTest.kt b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/AutomaticSpansTest.kt index a2524c44a8..ce3be459cb 100644 --- a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/AutomaticSpansTest.kt +++ b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/AutomaticSpansTest.kt @@ -26,7 +26,6 @@ class AutomaticSpansTest : BaseUiTest() { options.tracesSampleRate = 1.0 options.profilesSampleRate = 1.0 options.isEnableAutoActivityLifecycleTracing = true - options.beforeSendTransaction options.isEnableTimeToFullDisplayTracing = true options.beforeSendTransaction = SentryOptions.BeforeSendTransactionCallback { transaction, _ -> diff --git a/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java b/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java index def1d7e695..01ac02063e 100644 --- a/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java +++ b/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java @@ -48,6 +48,15 @@ public void run() { return; } + if (!options.isEnableAutoSessionTracking()) { + options + .getLogger() + .log( + DEBUG, + "Session tracking is disabled, bailing from previous session finalizer."); + return; + } + final IEnvelopeCache cache = options.getEnvelopeDiskCache(); if (cache instanceof EnvelopeCache) { if (!((EnvelopeCache) cache).waitPreviousSessionFlush()) { diff --git a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java index 4646e8cea1..57a6b3844a 100644 --- a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java +++ b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java @@ -434,4 +434,8 @@ public boolean waitPreviousSessionFlush() { } return false; } + + public void flushPreviousSession() { + previousSessionLatch.countDown(); + } } diff --git a/sentry/src/test/java/io/sentry/PreviousSessionFinalizerTest.kt b/sentry/src/test/java/io/sentry/PreviousSessionFinalizerTest.kt index 56b1d044c1..48c8709d6e 100644 --- a/sentry/src/test/java/io/sentry/PreviousSessionFinalizerTest.kt +++ b/sentry/src/test/java/io/sentry/PreviousSessionFinalizerTest.kt @@ -30,13 +30,20 @@ class PreviousSessionFinalizerTest { flushTimeoutMillis: Long = 0L, sessionFileExists: Boolean = true, session: Session? = null, - nativeCrashTimestamp: Date? = null + nativeCrashTimestamp: Date? = null, + sessionTrackingEnabled: Boolean = true, + shouldAwait: Boolean = false ): PreviousSessionFinalizer { options.run { setLogger(this@Fixture.logger) isDebug = true cacheDirPath = dir?.newFolder()?.absolutePath this.flushTimeoutMillis = flushTimeoutMillis + isEnableAutoSessionTracking = sessionTrackingEnabled + setEnvelopeDiskCache(EnvelopeCache.create(this)) + if (!shouldAwait) { + (envelopeDiskCache as? EnvelopeCache)?.flushPreviousSession() + } } options.cacheDirPath?.let { sessionFile = EnvelopeCache.getPreviousSessionFile(it) @@ -165,4 +172,35 @@ class PreviousSessionFinalizerTest { verify(fixture.hub, never()).captureEnvelope(any()) assertFalse(fixture.sessionFile.exists()) } + + @Test + fun `if session tracking is disabled, does not wait for previous session flush`() { + val finalizer = fixture.getSut( + tmpDir, + flushTimeoutMillis = 500L, + sessionTrackingEnabled = false, + shouldAwait = true + ) + finalizer.run() + + verify(fixture.logger, never()).log( + any(), + argThat { startsWith("Timed out waiting to flush previous session to its own file in session finalizer.") }, + any() + ) + verify(fixture.hub, never()).captureEnvelope(any()) + } + + @Test + fun `awaits for previous session flush`() { + val finalizer = fixture.getSut(tmpDir, flushTimeoutMillis = 500L, shouldAwait = true) + finalizer.run() + + verify(fixture.logger).log( + any(), + argThat { startsWith("Timed out waiting to flush previous session to its own file in session finalizer.") }, + any() + ) + verify(fixture.hub, never()).captureEnvelope(any()) + } } From 5857dd0c722b3474627e2fbeef00136bdf850c7e Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Mon, 3 Apr 2023 09:53:01 +0000 Subject: [PATCH 13/16] Format code --- .../java/io/sentry/android/core/AnrV2Integration.java | 11 ++++++----- .../main/java/io/sentry/PreviousSessionFinalizer.java | 6 ++---- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java index ed28e29b8e..dc5f793041 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java @@ -36,8 +36,6 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import static io.sentry.SentryLevel.DEBUG; - @SuppressLint("NewApi") // we check this in AnrIntegrationFactory public class AnrV2Integration implements Integration, Closeable { @@ -128,15 +126,18 @@ public void run() { final IEnvelopeCache cache = options.getEnvelopeDiskCache(); if (cache instanceof EnvelopeCache) { - if (options.isEnableAutoSessionTracking() && !((EnvelopeCache) cache).waitPreviousSessionFlush()) { + if (options.isEnableAutoSessionTracking() + && !((EnvelopeCache) cache).waitPreviousSessionFlush()) { options .getLogger() .log( SentryLevel.WARNING, "Timed out waiting to flush previous session to its own file."); - // if we timed out waiting here, we can already flush the latch, because the timeout is big - // enough to wait for it only once and we don't have to wait again in PreviousSessionFinalizer + // if we timed out waiting here, we can already flush the latch, because the timeout is + // big + // enough to wait for it only once and we don't have to wait again in + // PreviousSessionFinalizer ((EnvelopeCache) cache).flushPreviousSession(); } } diff --git a/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java b/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java index 01ac02063e..458c6532ba 100644 --- a/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java +++ b/sentry/src/main/java/io/sentry/PreviousSessionFinalizer.java @@ -50,10 +50,8 @@ public void run() { if (!options.isEnableAutoSessionTracking()) { options - .getLogger() - .log( - DEBUG, - "Session tracking is disabled, bailing from previous session finalizer."); + .getLogger() + .log(DEBUG, "Session tracking is disabled, bailing from previous session finalizer."); return; } From f1f4bb243dec05e303747ac8f240fe913cac864e Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Mon, 3 Apr 2023 12:13:29 +0200 Subject: [PATCH 14/16] Api dump --- sentry/api/sentry.api | 1 + 1 file changed, 1 insertion(+) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 25170b8ddc..d2c365025d 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -2272,6 +2272,7 @@ public class io/sentry/cache/EnvelopeCache : io/sentry/cache/IEnvelopeCache { public fun (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; From 9b823d32889e10a7bb9646c79414718d8cd521cd Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 4 Apr 2023 16:34:19 +0200 Subject: [PATCH 15/16] Address PR review --- .../main/java/io/sentry/android/core/AnrIntegration.java | 3 ++- .../java/io/sentry/android/core/AnrV2Integration.java | 2 +- sentry/src/main/java/io/sentry/SentryClient.java | 3 ++- sentry/src/main/java/io/sentry/cache/EnvelopeCache.java | 6 ++++-- sentry/src/main/java/io/sentry/hints/TransactionEnd.java | 5 +++++ .../test/java/io/sentry/PreviousSessionFinalizerTest.kt | 2 +- sentry/src/test/java/io/sentry/SentryClientTest.kt | 9 +++++---- 7 files changed, 20 insertions(+), 10 deletions(-) create mode 100644 sentry/src/main/java/io/sentry/hints/TransactionEnd.java diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AnrIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/AnrIntegration.java index 210f2128eb..4c6ac6373a 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AnrIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AnrIntegration.java @@ -10,6 +10,7 @@ import io.sentry.SentryOptions; import io.sentry.exception.ExceptionMechanismException; import io.sentry.hints.AbnormalExit; +import io.sentry.hints.TransactionEnd; import io.sentry.protocol.Mechanism; import io.sentry.util.HintUtils; import io.sentry.util.Objects; @@ -141,7 +142,7 @@ public void close() throws IOException { * href="https://develop.sentry.dev/sdk/sessions/#crashed-abnormal-vs-errored">Develop Docs * because we don't know whether the app has recovered after it or not. */ - static final class AnrHint implements AbnormalExit { + static final class AnrHint implements AbnormalExit, TransactionEnd { private final boolean isBackgroundAnr; diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java index dc5f793041..f56c6a26cd 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java @@ -117,7 +117,7 @@ public void run() { final ActivityManager activityManager = (ActivityManager) context.getSystemService(Context.ACTIVITY_SERVICE); - List applicationExitInfoList = + final List applicationExitInfoList = activityManager.getHistoricalProcessExitReasons(null, 0, 0); if (applicationExitInfoList.size() == 0) { options.getLogger().log(SentryLevel.DEBUG, "No records in historical exit reasons."); diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index e47c534e14..7d66fb0c85 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -4,6 +4,7 @@ import io.sentry.exception.SentryEnvelopeException; import io.sentry.hints.AbnormalExit; import io.sentry.hints.Backfillable; +import io.sentry.hints.TransactionEnd; import io.sentry.protocol.Contexts; import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; @@ -196,7 +197,7 @@ private boolean shouldApplyScopeData( @Nullable ITransaction transaction = scope.getTransaction(); if (transaction != null) { // TODO if we want to do the same for crashes, e.g. check for event.isCrashed() - if (HintUtils.hasType(hint, AbnormalExit.class)) { + if (HintUtils.hasType(hint, TransactionEnd.class)) { transaction.forceFinish(SpanStatus.ABORTED, false); } } diff --git a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java index 57a6b3844a..f5c531628c 100644 --- a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java +++ b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java @@ -105,7 +105,9 @@ public void store(final @NotNull SentryEnvelope envelope, final @NotNull Hint hi } } - endPreviousSessionForAbnormalExit(hint); + if (HintUtils.hasType(hint, AbnormalExit.class)) { + tryEndPreviousSession(hint); + } if (HintUtils.hasType(hint, SessionStart.class)) { if (currentSessionFile.exists()) { @@ -191,7 +193,7 @@ public void store(final @NotNull SentryEnvelope envelope, final @NotNull Hint hi * @param hint a hint coming with the envelope */ @SuppressWarnings("JavaUtilDate") - private void endPreviousSessionForAbnormalExit(final @NotNull Hint hint) { + private void tryEndPreviousSession(final @NotNull Hint hint) { final Object sdkHint = HintUtils.getSentrySdkHint(hint); if (sdkHint instanceof AbnormalExit) { final File previousSessionFile = getPreviousSessionFile(directory.getAbsolutePath()); diff --git a/sentry/src/main/java/io/sentry/hints/TransactionEnd.java b/sentry/src/main/java/io/sentry/hints/TransactionEnd.java new file mode 100644 index 0000000000..95e0f29efd --- /dev/null +++ b/sentry/src/main/java/io/sentry/hints/TransactionEnd.java @@ -0,0 +1,5 @@ +package io.sentry.hints; + +/** Marker interface for events that should trigger finish of the current transaction on Scope */ +public interface TransactionEnd { +} diff --git a/sentry/src/test/java/io/sentry/PreviousSessionFinalizerTest.kt b/sentry/src/test/java/io/sentry/PreviousSessionFinalizerTest.kt index 48c8709d6e..8e27662b5b 100644 --- a/sentry/src/test/java/io/sentry/PreviousSessionFinalizerTest.kt +++ b/sentry/src/test/java/io/sentry/PreviousSessionFinalizerTest.kt @@ -116,7 +116,7 @@ class PreviousSessionFinalizerTest { } @Test - fun `if previous session exists with abnormal mechanism, sends session update without chaging end timestamp`() { + fun `if previous session exists with abnormal mechanism, sends session update without changing end timestamp`() { val abnormalEndDate = Date(2023, 10, 1) val finalizer = fixture.getSut( tmpDir, diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index bf8664ce68..19dff0a504 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -9,6 +9,7 @@ import io.sentry.hints.AbnormalExit import io.sentry.hints.ApplyScopeData import io.sentry.hints.Backfillable import io.sentry.hints.Cached +import io.sentry.hints.TransactionEnd import io.sentry.protocol.Contexts import io.sentry.protocol.Mechanism import io.sentry.protocol.Request @@ -2043,7 +2044,7 @@ class SentryClientTest { } @Test - fun `AbnormalExits automatically trigger force-stop of any running transaction`() { + fun `TransactionEnds automatically trigger force-stop of any running transaction`() { val sut = fixture.getSut() // build up a running transaction @@ -2059,10 +2060,10 @@ class SentryClientTest { whenever(scope.extras).thenReturn(emptyMap()) whenever(scope.contexts).thenReturn(Contexts()) - val abnormalExit = AbnormalExit { "ANR" } - val abnormalExitHint = HintUtils.createWithTypeCheckHint(abnormalExit) + val transactionEnd = object : TransactionEnd {} + val transactionEndHint = HintUtils.createWithTypeCheckHint(transactionEnd) - sut.captureEvent(SentryEvent(), scope, abnormalExitHint) + sut.captureEvent(SentryEvent(), scope, transactionEndHint) verify(transaction).forceFinish(SpanStatus.ABORTED, false) verify(fixture.transport).send( From 642462aea46101b9836f9672e9a2f175530a14ab Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 4 Apr 2023 16:35:05 +0200 Subject: [PATCH 16/16] Api dump --- sentry/api/sentry.api | 3 +++ sentry/src/main/java/io/sentry/hints/TransactionEnd.java | 3 +-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 3f2390f321..d10f74b40b 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -2529,6 +2529,9 @@ public abstract interface class io/sentry/hints/SubmissionResult { public abstract fun setResult (Z)V } +public abstract interface class io/sentry/hints/TransactionEnd { +} + public final class io/sentry/instrumentation/file/SentryFileInputStream : java/io/FileInputStream { public fun (Ljava/io/File;)V public fun (Ljava/io/FileDescriptor;)V diff --git a/sentry/src/main/java/io/sentry/hints/TransactionEnd.java b/sentry/src/main/java/io/sentry/hints/TransactionEnd.java index 95e0f29efd..d13027e712 100644 --- a/sentry/src/main/java/io/sentry/hints/TransactionEnd.java +++ b/sentry/src/main/java/io/sentry/hints/TransactionEnd.java @@ -1,5 +1,4 @@ package io.sentry.hints; /** Marker interface for events that should trigger finish of the current transaction on Scope */ -public interface TransactionEnd { -} +public interface TransactionEnd {}