sprinkle with TODOs to highlight next PR steps

Author: supervacuus

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

@@ -122,6 +122,12 @@ public AnrV2EventProcessor(
       return event;
     }
 
+    // TODO: right now, any event with a Backfillable Hint will get here. This is fine if the
+    //       enrichment code is generically applicable to any Backfilling Event. However some parts
+    //       in the ANRv2EventProcessor are specific to ANRs and currently override the tombstone
+    //       events. Similar to the Integration we should find an abstraction split that allows to
+    //       separate the generic from the specific.
+
     // we always set exception values, platform, os and device even if the ANR is not enrich-able
     // even though the OS context may change in the meantime (OS update), we consider this an
     // edge-case
@@ -255,6 +261,7 @@ private void setFingerprints(final @NotNull SentryEvent event, final @NotNull Ob
     // sentry does not yet have a capability to provide default server-side fingerprint rules,
     // so we're doing this on the SDK side to group background and foreground ANRs separately
     // even if they have similar stacktraces
+    // TODO: this will always set the fingerprint of tombstones to foreground-anr
     final boolean isBackgroundAnr = isBackgroundAnr(hint);
     if (event.getFingerprints() == null) {
       event.setFingerprints(
@@ -385,6 +392,8 @@ private void setApp(final @NotNull SentryBaseEvent event, final @NotNull Object
     // TODO: not entirely correct, because we define background ANRs as not the ones of
     //  IMPORTANCE_FOREGROUND, but this doesn't mean the app was in foreground when an ANR happened
     //  but it's our best effort for now. We could serialize AppState in theory.
+
+    // TODO: this will always be true of tombstones
     app.setInForeground(!isBackgroundAnr(hint));
 
     final PackageInfo packageInfo = ContextUtils.getPackageInfo(context, buildInfoProvider);
@@ -533,6 +542,9 @@ private void setStaticValues(final @NotNull SentryEvent event) {
   private void setPlatform(final @NotNull SentryBaseEvent event) {
     if (event.getPlatform() == null) {
       // this actually means JVM related.
+      // TODO: since we write this from the tombstone parser we are current unaffected. It is good
+      //       that it doesn't overwrite previous platform values, however we still rely on the
+      //       order in which each event processor was called.
       event.setPlatform(SentryBaseEvent.DEFAULT_PLATFORM);
     }
   }
@@ -564,12 +576,16 @@ private void setExceptions(final @NotNull SentryEvent event, final @NotNull Obje
     // and make an exception out of its stacktrace
     final Mechanism mechanism = new Mechanism();
     if (!((Backfillable) hint).shouldEnrich()) {
+      // TODO: this currently overrides the signalhandler mechanism we set in the TombstoneParser
+      //       with a new type (can be the right choice down the road, but currently is
+      //       unintentional, and it might be better to leave it close to the Native SDK)
       // we only enrich the latest ANR in the list, so this is historical
       mechanism.setType("HistoricalAppExitInfo");
     } else {
       mechanism.setType("AppExitInfo");
     }
 
+    // TODO: this currently overrides the tombstone exceptions
     final boolean isBackgroundAnr = isBackgroundAnr(hint);
     String message = "ANR";
     if (isBackgroundAnr) {

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

@@ -40,6 +40,8 @@
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
+// TODO: This is very very close to ANRv2Integration in terms of mechanism. Find an abstraction
+//       split that separates the equivalent mechanism from the differing policy between the two.
 @ApiStatus.Internal
 public class TombstoneIntegration implements Integration, Closeable {
   static final long NINETY_DAYS_THRESHOLD = TimeUnit.DAYS.toMillis(91);

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

@@ -122,13 +122,15 @@ private boolean storeInternal(final @NotNull SentryEnvelope envelope, final @Not
     if (HintUtils.hasType(hint, AbnormalExit.class)) {
       tryEndPreviousSession(hint);
     }
+    // TODO: adapt       tryEndPreviousSession(hint); to CrashExit.class
 
     if (HintUtils.hasType(hint, SessionStart.class)) {
       movePreviousSession(currentSessionFile, previousSessionFile);
       updateCurrentSession(currentSessionFile, envelope);
 
       boolean crashedLastRun = false;
       final File crashMarkerFile = new File(options.getCacheDirPath(), NATIVE_CRASH_MARKER_FILE);
+      // TODO: this should probably check whether the Native SDK integration is currently enabled or remove the marker file if it isn't. Otherwise, application that disable the Native SDK, will report a crash for the last run forever.
       if (crashMarkerFile.exists()) {
         crashedLastRun = true;
       }
@@ -153,6 +155,7 @@ private boolean storeInternal(final @NotNull SentryEnvelope envelope, final @Not
         }
       }
 
+      // TODO: maybe set crashLastRun for tombstone too?
       SentryCrashLastRunState.getInstance().setCrashedLastRun(crashedLastRun);
 
       flushPreviousSession();