Merge branch 'feat/unhandled-sessions-cache' into feat/unhandled-sessions-internal-api

Author: buenaflor

sentry/api/sentry.api

@@ -4290,12 +4290,12 @@ public final class io/sentry/Session : io/sentry/JsonSerializable, io/sentry/Jso
 	public fun getTimestamp ()Ljava/util/Date;
 	public fun getUnknown ()Ljava/util/Map;
 	public fun getUserAgent ()Ljava/lang/String;
-	public fun isPendingUnhandled ()Z
+	public fun hasNonTerminatingUnhandledError ()Z
 	public fun isTerminated ()Z
-	public fun markPendingUnhandled ()Z
+	public fun recordNonTerminatingUnhandledError ()Z
 	public fun serialize (Lio/sentry/ObjectWriter;Lio/sentry/ILogger;)V
 	public fun setInitAsTrue ()V
-	public fun setPendingUnhandled (Z)V
+	public fun setNonTerminatingUnhandledError (Z)V
 	public fun setUnknown (Ljava/util/Map;)V
 	public fun update (Lio/sentry/Session$State;Ljava/lang/String;Z)Z
 	public fun update (Lio/sentry/Session$State;Ljava/lang/String;ZLjava/lang/String;)Z
@@ -4316,7 +4316,7 @@ public final class io/sentry/Session$JsonKeys {
 	public static final field ERRORS Ljava/lang/String;
 	public static final field INIT Ljava/lang/String;
 	public static final field IP_ADDRESS Ljava/lang/String;
-	public static final field PENDING_UNHANDLED Ljava/lang/String;
+	public static final field NON_TERMINATING_UNHANDLED_ERROR Ljava/lang/String;
 	public static final field RELEASE Ljava/lang/String;
 	public static final field SEQ Ljava/lang/String;
 	public static final field SID Ljava/lang/String;

sentry/src/main/java/io/sentry/Session.java

@@ -68,12 +68,15 @@ public enum State {
   private @Nullable String abnormalMechanism;
 
   /**
-   * Whether the session experienced an unhandled (but non-terminal) exception. Kept locally and
-   * persisted with the session, but never sent as a status while the session is alive. On end() the
-   * session is finalized as {@link State#Unhandled} instead of {@link State#Exited} unless a crash
-   * escalated it to {@link State#Crashed}.
+   * Whether the session experienced an unhandled error that did <em>not</em> terminate the process,
+   * e.g. an unhandled Flutter exception. A native crash is also unhandled, but it kills the process
+   * and therefore ends the session as {@link State#Crashed} instead.
+   *
+   * <p>Kept locally and persisted with the session, but never sent as a status while the session is
+   * alive. On end() the session is finalized as {@link State#Unhandled} instead of {@link
+   * State#Exited}, unless a crash escalated it to {@link State#Crashed}.
    */
-  private boolean pendingUnhandled;
+  private boolean nonTerminatingUnhandledError;
 
   /** The session lock, ops should be atomic */
   private final @NotNull AutoClosableReentrantLock sessionLock = new AutoClosableReentrantLock();
@@ -198,32 +201,41 @@ public int errorCount() {
   }
 
   /**
-   * Whether the session has a pending unhandled (non-terminal) exception that hasn't been finalized
-   * yet.
+   * Whether the session experienced an unhandled error that did not terminate the process, and so
+   * finalizes as {@link State#Unhandled} rather than {@link State#Exited}.
    */
   @ApiStatus.Internal
-  public boolean isPendingUnhandled() {
-    return pendingUnhandled;
+  public boolean hasNonTerminatingUnhandledError() {
+    return nonTerminatingUnhandledError;
   }
 
   /**
-   * Marks the session as having experienced an unhandled (non-terminal) exception without ending
-   * it. On {@link #end()} the session will be finalized as {@link State#Unhandled} unless a crash
-   * escalated it to {@link State#Crashed} first.
+   * Restores the flag when rebuilding a session, i.e. from {@link #clone()} or the deserializer.
+   *
+   * <p>Not for use on a live session: unlike {@link #recordNonTerminatingUnhandledError()} this
+   * neither counts the error nor advances the session's sequence, so a session mutated through this
+   * setter would be sent as an out-of-date update.
    */
   @ApiStatus.Internal
-  public void setPendingUnhandled(final boolean pendingUnhandled) {
-    this.pendingUnhandled = pendingUnhandled;
+  public void setNonTerminatingUnhandledError(final boolean nonTerminatingUnhandledError) {
+    this.nonTerminatingUnhandledError = nonTerminatingUnhandledError;
   }
 
-  /** Marks an active session as having experienced an unhandled non-terminal exception. */
+  /**
+   * Records that an active session experienced an unhandled error which did not terminate the
+   * process, counting the error and advancing the session's sequence without ending it. On {@link
+   * #end()} the session is finalized as {@link State#Unhandled} unless a crash escalated it to
+   * {@link State#Crashed} first.
+   *
+   * @return whether the session was updated, i.e. false if it had already reached a terminal state
+   */
   @ApiStatus.Internal
-  public boolean markPendingUnhandled() {
+  public boolean recordNonTerminatingUnhandledError() {
     try (final @NotNull ISentryLifecycleToken ignored = sessionLock.acquire()) {
       if (status != State.Ok) {
         return false;
       }
-      pendingUnhandled = true;
+      nonTerminatingUnhandledError = true;
       errorCount.incrementAndGet();
       init = null;
       timestamp = DateUtils.getCurrentDateTime();
@@ -255,7 +267,7 @@ public void end(final @Nullable Date timestamp) {
       if (status == State.Ok) {
         // a session that experienced an unhandled (but non-terminal) exception is finalized as
         // Unhandled rather than Exited.
-        status = pendingUnhandled ? State.Unhandled : State.Exited;
+        status = nonTerminatingUnhandledError ? State.Unhandled : State.Exited;
       }
 
       if (timestamp != null) {
@@ -308,9 +320,9 @@ public boolean update(
       boolean sessionHasBeenUpdated = false;
       if (status != null) {
         this.status = status;
-        // a real crash takes precedence over a pending unhandled (non-terminal) exception.
+        // a crash terminates the process, so it takes precedence over a non-terminating one.
         if (status == State.Crashed) {
-          pendingUnhandled = false;
+          nonTerminatingUnhandledError = false;
         }
         sessionHasBeenUpdated = true;
       }
@@ -384,7 +396,7 @@ private long getSequenceTimestamp(final @NotNull Date timestamp) {
             environment,
             release,
             abnormalMechanism);
-    session.setPendingUnhandled(pendingUnhandled);
+    session.setNonTerminatingUnhandledError(nonTerminatingUnhandledError);
     return session;
   }
 
@@ -407,7 +419,7 @@ public static final class JsonKeys {
     public static final String IP_ADDRESS = "ip_address";
     public static final String USER_AGENT = "user_agent";
     public static final String ABNORMAL_MECHANISM = "abnormal_mechanism";
-    public static final String PENDING_UNHANDLED = "pending_unhandled";
+    public static final String NON_TERMINATING_UNHANDLED_ERROR = "non_terminating_unhandled_error";
   }
 
   @Override
@@ -438,8 +450,8 @@ public void serialize(final @NotNull ObjectWriter writer, final @NotNull ILogger
     if (abnormalMechanism != null) {
       writer.name(JsonKeys.ABNORMAL_MECHANISM).value(logger, abnormalMechanism);
     }
-    if (pendingUnhandled) {
-      writer.name(JsonKeys.PENDING_UNHANDLED).value(pendingUnhandled);
+    if (nonTerminatingUnhandledError) {
+      writer.name(JsonKeys.NON_TERMINATING_UNHANDLED_ERROR).value(nonTerminatingUnhandledError);
     }
     writer.name(JsonKeys.ATTRS);
     writer.beginObject();
@@ -497,7 +509,7 @@ public static final class Deserializer implements JsonDeserializer<Session> {
       String environment = null;
       String release = null; // @NotNull
       String abnormalMechanism = null;
-      boolean pendingUnhandled = false;
+      boolean nonTerminatingUnhandledError = false;
 
       Map<String, Object> unknown = null;
       while (reader.peek() == JsonToken.NAME) {
@@ -541,9 +553,10 @@ public static final class Deserializer implements JsonDeserializer<Session> {
           case JsonKeys.ABNORMAL_MECHANISM:
             abnormalMechanism = reader.nextStringOrNull();
             break;
-          case JsonKeys.PENDING_UNHANDLED:
-            final Boolean pendingUnhandledValue = reader.nextBooleanOrNull();
-            pendingUnhandled = pendingUnhandledValue != null && pendingUnhandledValue;
+          case JsonKeys.NON_TERMINATING_UNHANDLED_ERROR:
+            final Boolean nonTerminatingUnhandledErrorValue = reader.nextBooleanOrNull();
+            nonTerminatingUnhandledError =
+                nonTerminatingUnhandledErrorValue != null && nonTerminatingUnhandledErrorValue;
             break;
           case JsonKeys.ATTRS:
             reader.beginObject();
@@ -604,7 +617,7 @@ public static final class Deserializer implements JsonDeserializer<Session> {
               environment,
               release,
               abnormalMechanism);
-      session.setPendingUnhandled(pendingUnhandled);
+      session.setNonTerminatingUnhandledError(nonTerminatingUnhandledError);
       session.setUnknown(unknown);
       reader.endObject();
       return session;

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

@@ -122,17 +122,17 @@ private boolean storeInternal(final @NotNull SentryEnvelope envelope, final @Not
       try (final @NotNull ISentryLifecycleToken ignored = sessionLock.acquire()) {
         final @Nullable Session endingSession = readSessionFromEnvelope(envelope);
         final @Nullable Session currentSession = readSessionFromDisk(currentSessionFile);
-        final boolean preservePendingSession =
+        final boolean preserveCurrentSession =
             endingSession != null
                 && currentSession != null
-                && currentSession.isPendingUnhandled()
+                && currentSession.hasNonTerminatingUnhandledError()
                 && endingSession.getSessionId() != null
                 && currentSession.getSessionId() != null
                 && !Objects.equals(endingSession.getSessionId(), currentSession.getSessionId())
                 && endingSession.getStarted() != null
                 && currentSession.getStarted() != null
                 && currentSession.getStarted().after(endingSession.getStarted());
-        if (!preservePendingSession && !currentSessionFile.delete()) {
+        if (!preserveCurrentSession && !currentSessionFile.delete()) {
           options.getLogger().log(WARNING, "Current envelope doesn't exist.");
         }
       }
@@ -149,7 +149,7 @@ private boolean storeInternal(final @NotNull SentryEnvelope envelope, final @Not
         if (startingSession != null) {
           final @Nullable Session currentSession = readSessionFromDisk(currentSessionFile);
           if (currentSession != null && hasSameSessionId(currentSession, startingSession)) {
-            if (!isNewerPendingOrErrorSnapshot(currentSession, startingSession)) {
+            if (!isNewerUnhandledOrErrorSnapshot(currentSession, startingSession)) {
               writeSessionToDisk(currentSessionFile, startingSession);
             }
           } else {
@@ -355,9 +355,10 @@ private void writeCrashMarkerFile() {
     }
   }
 
-  private boolean isNewerPendingOrErrorSnapshot(
+  private boolean isNewerUnhandledOrErrorSnapshot(
       final @NotNull Session currentSession, final @NotNull Session startingSession) {
-    return (currentSession.isPendingUnhandled() && !startingSession.isPendingUnhandled())
+    return (currentSession.hasNonTerminatingUnhandledError()
+            && !startingSession.hasNonTerminatingUnhandledError())
         || currentSession.errorCount() > startingSession.errorCount();
   }
 

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

@@ -201,12 +201,14 @@ class PreviousSessionFinalizerTest {
   }
 
   @Test
-  fun `if previous session has pending unhandled and no crash marker, finalizes as unhandled`() {
+  fun `if previous session has a non-terminating unhandled error and no crash marker, finalizes as unhandled`() {
     val finalizer =
       fixture.getSut(
         tmpDir,
         session =
-          Session(null, null, null, "io.sentry.sample@1.0").apply { setPendingUnhandled(true) },
+          Session(null, null, null, "io.sentry.sample@1.0").apply {
+            setNonTerminatingUnhandledError(true)
+          },
       )
     finalizer.run()
 
@@ -216,18 +218,20 @@ class PreviousSessionFinalizerTest {
           val session = fixture.sessionFromEnvelope(this)
           session.release == "io.sentry.sample@1.0" &&
             session.status == Session.State.Unhandled &&
-            session.isPendingUnhandled
+            session.hasNonTerminatingUnhandledError()
         }
       )
   }
 
   @Test
-  fun `if previous session has pending unhandled but a native crash marker exists, finalizes as crashed`() {
+  fun `if previous session has a non-terminating unhandled error but a native crash marker exists, finalizes as crashed`() {
     val finalizer =
       fixture.getSut(
         tmpDir,
         session =
-          Session(null, null, null, "io.sentry.sample@1.0").apply { setPendingUnhandled(true) },
+          Session(null, null, null, "io.sentry.sample@1.0").apply {
+            setNonTerminatingUnhandledError(true)
+          },
         nativeCrashTimestamp = DateUtils.getDateTime("2023-10-01T00:00:00.000Z"),
       )
     finalizer.run()