fix(android): mark tombstone and ANR exits as reported when the event is dropped

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Fixes
+
+- Keep dropped tombstone and ANR events dropped, instead of reporting the same app exit again at every app start ([#6002](https://github.com/getsentry/sentry-java/pull/6002))
+
 ## 8.54.0
 
 ### Features

sentry-android-core/api/sentry-android-core.api

@@ -638,6 +638,7 @@ public class io/sentry/android/core/TombstoneIntegration$TombstonePolicy : io/se
 	public fun getLabel ()Ljava/lang/String;
 	public fun getLastReportedTimestamp ()Ljava/lang/Long;
 	public fun getTargetReason ()I
+	public fun markReported (J)V
 	public fun shouldReportHistorical ()Z
 }
 
@@ -749,6 +750,8 @@ public final class io/sentry/android/core/cache/AndroidEnvelopeCache : io/sentry
 	public static fun hasStartupCrashMarker (Lio/sentry/SentryOptions;)Z
 	public static fun lastReportedAnr (Lio/sentry/SentryOptions;)Ljava/lang/Long;
 	public static fun lastReportedTombstone (Lio/sentry/SentryOptions;)Ljava/lang/Long;
+	public static fun markAnrReported (Lio/sentry/SentryOptions;J)V
+	public static fun markTombstoneReported (Lio/sentry/SentryOptions;J)V
 	public fun store (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)V
 	public fun storeEnvelope (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)Z
 }

sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java

@@ -132,6 +132,11 @@ public boolean shouldReportHistorical() {
       return AndroidEnvelopeCache.lastReportedAnr(options);
     }
 
+    @Override
+    public void markReported(final long timestamp) {
+      AndroidEnvelopeCache.markAnrReported(options, timestamp);
+    }
+
     @Override
     public @Nullable ApplicationExitInfoHistoryDispatcher.Report buildReport(
         final @NotNull ApplicationExitInfo exitInfo, final boolean shouldEnrich) {

sentry-android-core/src/main/java/io/sentry/android/core/ApplicationExitInfoHistoryDispatcher.java

@@ -14,6 +14,7 @@
 import io.sentry.hints.BlockingFlushHint;
 import io.sentry.protocol.SentryId;
 import io.sentry.transport.ICurrentDateProvider;
+import io.sentry.util.HintUtils;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Iterator;
@@ -177,6 +178,7 @@ private void reportHistorical(
     }
   }
 
+  @RequiresApi(api = Build.VERSION_CODES.R)
   private void report(final @NotNull ApplicationExitInfo exitInfo, final boolean enrich) {
     final @Nullable Report report = policy.buildReport(exitInfo, enrich);
 
@@ -186,7 +188,28 @@ private void report(final @NotNull ApplicationExitInfo exitInfo, final boolean e
 
     final @NotNull SentryId sentryId = scopes.captureEvent(report.getEvent(), report.getHint());
     final boolean isEventDropped = sentryId.equals(SentryId.EMPTY_ID);
-    if (!isEventDropped) {
+    if (isEventDropped) {
+      // A dropped event never reaches the envelope disk cache, which is where the last reported
+      // marker is normally written. Without writing it here, the very same exit would be turned
+      // into an event again on the next app start, no matter why it was dropped. Only a technical
+      // failure to hand the event over keeps the exit eligible for another attempt.
+      if (HintUtils.isCaptureFailed(report.getHint())) {
+        options
+            .getLogger()
+            .log(
+                SentryLevel.DEBUG,
+                "Capturing the %s event failed, leaving the exit for the next app start.",
+                policy.getLabel());
+      } else {
+        options
+            .getLogger()
+            .log(
+                SentryLevel.DEBUG,
+                "%s event was dropped, marking the exit as reported.",
+                policy.getLabel());
+        policy.markReported(exitInfo.getTimestamp());
+      }
+    } else {
       final @Nullable BlockingFlushHint flushHint = report.getFlushHint();
       if (flushHint != null && !flushHint.waitFlush()) {
         options
@@ -211,6 +234,9 @@ interface ApplicationExitInfoPolicy {
     @Nullable
     Long getLastReportedTimestamp();
 
+    /** Records {@code timestamp} as the last reported exit, so it is not reported again. */
+    void markReported(long timestamp);
+
     @Nullable
     Report buildReport(@NotNull ApplicationExitInfo exitInfo, boolean enrich);
   }

sentry-android-core/src/main/java/io/sentry/android/core/TombstoneIntegration.java

@@ -147,6 +147,11 @@ public boolean shouldReportHistorical() {
       return AndroidEnvelopeCache.lastReportedTombstone(options);
     }
 
+    @Override
+    public void markReported(final long timestamp) {
+      AndroidEnvelopeCache.markTombstoneReported(options, timestamp);
+    }
+
     @RequiresApi(api = Build.VERSION_CODES.R)
     @Override
     public @Nullable ApplicationExitInfoHistoryDispatcher.Report buildReport(

sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java

@@ -85,7 +85,7 @@ private boolean storeInternalAndroid(@NotNull SentryEnvelope envelope, @NotNull
     }
 
     for (TimestampMarkerHandler<?> handler : TIMESTAMP_MARKER_HANDLERS) {
-      handler.handle(this, hint, options);
+      handler.handle(hint, options);
     }
 
     return didStore;
@@ -182,7 +182,8 @@ public static boolean hasStartupCrashMarker(final @NotNull SentryOptions options
     return null;
   }
 
-  private void writeLastReportedMarker(
+  private static void writeLastReportedMarker(
+      final @NotNull SentryOptions options,
       final @Nullable Long timestamp,
       @NotNull String reportFilename,
       @NotNull String markerCategory) {
@@ -215,6 +216,15 @@ private void writeLastReportedMarker(
     return lastReportedMarker(options, LAST_TOMBSTONE_REPORT, LAST_TOMBSTONE_MARKER_LABEL);
   }
 
+  public static void markAnrReported(final @NotNull SentryOptions options, final long timestamp) {
+    writeLastReportedMarker(options, timestamp, LAST_ANR_REPORT, LAST_ANR_MARKER_LABEL);
+  }
+
+  public static void markTombstoneReported(
+      final @NotNull SentryOptions options, final long timestamp) {
+    writeLastReportedMarker(options, timestamp, LAST_TOMBSTONE_REPORT, LAST_TOMBSTONE_MARKER_LABEL);
+  }
+
   private static final class TimestampMarkerHandler<T> {
     interface TimestampExtractor<T> {
       @NotNull
@@ -237,10 +247,7 @@ interface TimestampExtractor<T> {
       this.timestampProvider = timestampProvider;
     }
 
-    void handle(
-        final @NotNull AndroidEnvelopeCache cache,
-        final @NotNull Hint hint,
-        final @NotNull SentryAndroidOptions options) {
+    void handle(final @NotNull Hint hint, final @NotNull SentryAndroidOptions options) {
       HintUtils.runIfHasType(
           hint,
           type,
@@ -253,7 +260,7 @@ void handle(
                     "Writing last reported %s marker with timestamp %d",
                     label,
                     timestamp);
-            cache.writeLastReportedMarker(timestamp, reportFilename, label);
+            writeLastReportedMarker(options, timestamp, reportFilename, label);
           });
     }
   }

sentry-android-core/src/test/java/io/sentry/android/core/ApplicationExitIntegrationTestBase.kt

@@ -187,6 +187,33 @@ abstract class ApplicationExitIntegrationTestBase<THint : Any> {
       .log(any(), argThat { startsWith(config.flushLogPrefix) }, any<Any>())
   }
 
+  @Test
+  fun `when latest event was dropped, marks the exit as reported`() {
+    val integration =
+      fixture.getSut(tmpDir, lastReportedTimestamp = oldTimestamp, lastEventId = SentryId.EMPTY_ID)
+    fixture.addAppExitInfo(timestamp = newTimestamp)
+
+    integration.register(fixture.scopes, fixture.options)
+
+    assertEquals(newTimestamp.toString(), fixture.lastReportedFile.readText())
+  }
+
+  @Test
+  fun `when capturing the latest event failed, does not mark the exit as reported`() {
+    val integration =
+      fixture.getSut(
+        tmpDir,
+        lastReportedTimestamp = oldTimestamp,
+        lastEventId = SentryId.EMPTY_ID,
+        captureFailed = true,
+      )
+    fixture.addAppExitInfo(timestamp = newTimestamp)
+
+    integration.register(fixture.scopes, fixture.options)
+
+    assertEquals(oldTimestamp.toString(), fixture.lastReportedFile.readText())
+  }
+
   @Test
   fun `historical exits are reported non-enriched`() {
     val integration = fixture.getSut(tmpDir, lastReportedTimestamp = oldTimestamp)
@@ -403,6 +430,7 @@ abstract class ApplicationExitIntegrationTestBase<THint : Any> {
       sessionFlushTimeoutMillis: Long = 0L,
       lastReportedTimestamp: Long? = null,
       lastEventId: SentryId = SentryId(),
+      captureFailed: Boolean = false,
       sessionTrackingEnabled: Boolean = true,
       reportHistorical: Boolean = true,
       extraOptions: (SentryAndroidOptions) -> Unit = {},
@@ -426,7 +454,12 @@ abstract class ApplicationExitIntegrationTestBase<THint : Any> {
         lastReportedFile = File(cacheDir, config.lastReportedFileName)
         lastReportedFile.writeText(lastReportedTimestamp.toString())
       }
-      whenever(scopes.captureEvent(any(), anyOrNull<Hint>())).thenReturn(lastEventId)
+      whenever(scopes.captureEvent(any(), anyOrNull<Hint>())).thenAnswer { invocation ->
+        if (captureFailed) {
+          HintUtils.setCaptureFailed(invocation.getArgument(1))
+        }
+        lastEventId
+      }
       return config.createIntegration(context)
     }
 

sentry/api/sentry.api

@@ -4737,6 +4737,7 @@ public final class io/sentry/TypeCheckHint {
 	public static final field OPEN_FEIGN_REQUEST Ljava/lang/String;
 	public static final field OPEN_FEIGN_RESPONSE Ljava/lang/String;
 	public static final field REPLAY_FRAME_BITMAP Ljava/lang/String;
+	public static final field SENTRY_CAPTURE_FAILED Ljava/lang/String;
 	public static final field SENTRY_DART_SDK_NAME Ljava/lang/String;
 	public static final field SENTRY_DOTNET_SDK_NAME Ljava/lang/String;
 	public static final field SENTRY_EVENT_DROP_REASON Ljava/lang/String;
@@ -7762,11 +7763,13 @@ public final class io/sentry/util/HintUtils {
 	public static fun getEventDropReason (Lio/sentry/Hint;)Lio/sentry/hints/EventDropReason;
 	public static fun getSentrySdkHint (Lio/sentry/Hint;)Ljava/lang/Object;
 	public static fun hasType (Lio/sentry/Hint;Ljava/lang/Class;)Z
+	public static fun isCaptureFailed (Lio/sentry/Hint;)Z
 	public static fun isFromHybridSdk (Lio/sentry/Hint;)Z
 	public static fun runIfDoesNotHaveType (Lio/sentry/Hint;Ljava/lang/Class;Lio/sentry/util/HintUtils$SentryNullableConsumer;)V
 	public static fun runIfHasType (Lio/sentry/Hint;Ljava/lang/Class;Lio/sentry/util/HintUtils$SentryConsumer;)V
 	public static fun runIfHasType (Lio/sentry/Hint;Ljava/lang/Class;Lio/sentry/util/HintUtils$SentryConsumer;Lio/sentry/util/HintUtils$SentryHintFallback;)V
 	public static fun runIfHasTypeLogIfNot (Lio/sentry/Hint;Ljava/lang/Class;Lio/sentry/ILogger;Lio/sentry/util/HintUtils$SentryConsumer;)V
+	public static fun setCaptureFailed (Lio/sentry/Hint;)V
 	public static fun setEventDropReason (Lio/sentry/Hint;Lio/sentry/hints/EventDropReason;)V
 	public static fun setIsFromHybridSdk (Lio/sentry/Hint;Ljava/lang/String;)V
 	public static fun setTypeCheckHint (Lio/sentry/Hint;Ljava/lang/Object;)V

sentry/src/main/java/io/sentry/Scopes.java

@@ -152,8 +152,10 @@ public boolean isEnabled() {
           .getLogger()
           .log(
               SentryLevel.WARNING, "Instance is disabled and this 'captureEvent' call is a no-op.");
+      HintUtils.setCaptureFailed(hint);
     } else if (event == null) {
       getOptions().getLogger().log(SentryLevel.WARNING, "captureEvent called with null parameter.");
+      HintUtils.setCaptureFailed(hint);
     } else {
       try {
         assignTraceContext(event);
@@ -166,6 +168,7 @@ public boolean isEnabled() {
             .getLogger()
             .log(
                 SentryLevel.ERROR, "Error while capturing event with id: " + event.getEventId(), e);
+        HintUtils.setCaptureFailed(hint);
       }
     }
     return sentryId;

sentry/src/main/java/io/sentry/SentryClient.java

@@ -287,6 +287,7 @@ private boolean shouldApplyScopeData(final @NotNull CheckIn event, final @NotNul
       options.getLogger().log(SentryLevel.WARNING, e, "Capturing event %s failed.", sentryId);
 
       // if there was an error capturing the event, we return an emptyId
+      HintUtils.setCaptureFailed(hint);
       sentryId = SentryId.EMPTY_ID;
     }
 

sentry/src/main/java/io/sentry/TypeCheckHint.java

@@ -13,6 +13,8 @@ public final class TypeCheckHint {
   @ApiStatus.Internal
   public static final String SENTRY_EVENT_DROP_REASON = "sentry:eventDropReason";
 
+  @ApiStatus.Internal public static final String SENTRY_CAPTURE_FAILED = "sentry:captureFailed";
+
   @ApiStatus.Internal
   public static final String SENTRY_REPLAY_NETWORK_DETAILS = "sentry:replayNetworkDetails";
 

sentry/src/main/java/io/sentry/util/HintUtils.java

@@ -1,5 +1,6 @@
 package io.sentry.util;
 
+import static io.sentry.TypeCheckHint.SENTRY_CAPTURE_FAILED;
 import static io.sentry.TypeCheckHint.SENTRY_DART_SDK_NAME;
 import static io.sentry.TypeCheckHint.SENTRY_DOTNET_SDK_NAME;
 import static io.sentry.TypeCheckHint.SENTRY_EVENT_DROP_REASON;
@@ -45,6 +46,21 @@ public static EventDropReason getEventDropReason(final @NotNull Hint hint) {
     return hint.getAs(SENTRY_EVENT_DROP_REASON, EventDropReason.class);
   }
 
+  /**
+   * Marks the event as not captured because of a technical failure, as opposed to being dropped on
+   * purpose. Callers that keep an event around for a later attempt use this to tell the two apart:
+   * an empty event id alone does not, because almost every drop also returns one.
+   */
+  public static void setCaptureFailed(final @Nullable Hint hint) {
+    if (hint != null) {
+      hint.set(SENTRY_CAPTURE_FAILED, true);
+    }
+  }
+
+  public static boolean isCaptureFailed(final @NotNull Hint hint) {
+    return Boolean.TRUE.equals(hint.getAs(SENTRY_CAPTURE_FAILED, Boolean.class));
+  }
+
   public static Hint createWithTypeCheckHint(Object typeCheckHint) {
     Hint hint = new Hint();
     setTypeCheckHint(hint, typeCheckHint);

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

@@ -344,6 +344,17 @@ class ScopesTest {
     verify(mockClient, never()).captureEvent(any(), any<Hint>())
   }
 
+  @Test
+  fun `when captureEvent is called on disabled client, the hint is marked as capture failed`() {
+    val (sut, _) = getEnabledScopes()
+    sut.close()
+
+    val hint = Hint()
+    sut.captureEvent(SentryEvent(), hint)
+
+    assertTrue(HintUtils.isCaptureFailed(hint))
+  }
+
   @Test
   fun `when captureEvent is called with a valid argument, captureEvent on the client should be called`() {
     val (sut, mockClient) = getEnabledScopes()

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

@@ -1635,6 +1635,28 @@ class SentryClientTest {
     verify(fixture.transport).send(check { assertNotNull(it.header.sdkVersion) }, anyOrNull())
   }
 
+  @Test
+  fun `when capturing the event throws, the hint is marked as capture failed`() {
+    whenever(fixture.transport.send(any(), anyOrNull())).thenThrow(IOException())
+
+    val hint = Hint()
+    val sentryId = fixture.getSut().captureEvent(SentryEvent(), hint)
+
+    assertEquals(SentryId.EMPTY_ID, sentryId)
+    assertTrue(HintUtils.isCaptureFailed(hint))
+  }
+
+  @Test
+  fun `when the event is dropped, the hint is not marked as capture failed`() {
+    fixture.sentryOptions.setBeforeSend { _, _ -> null }
+
+    val hint = Hint()
+    val sentryId = fixture.getSut().captureEvent(SentryEvent(), hint)
+
+    assertEquals(SentryId.EMPTY_ID, sentryId)
+    assertFalse(HintUtils.isCaptureFailed(hint))
+  }
+
   @Test
   fun `when captureEnvelope and thres an exception, returns empty sentryId`() {
     whenever(fixture.transport.send(any(), anyOrNull())).thenThrow(IOException())