fix(core): Handle callback errors

CHANGELOG.md

@@ -2,6 +2,23 @@
 
 ## Unreleased
 
+### Fixes
+
+- Fix SDK callback error handling ([#6140](https://github.com/getsentry/sentry-java/pull/6140))
+  - Drop telemetry when a `beforeSend*` callback throws and report it using `DiscardReason.CALLBACK_ERROR`.
+  - Drop transactions when `tracesSampler` throws instead of inheriting the parent sampling decision or falling back to `tracesSampleRate` ([#6163](https://github.com/getsentry/sentry-java/pull/6163)).
+  - Drop spans when `beforeSpan` throws without disrupting requests, covering OkHttp, OpenFeign, GraphQL, Ktor, and Apollo—including Apollo 5 ([#6167](https://github.com/getsentry/sentry-java/pull/6167), [#6238](https://github.com/getsentry/sentry-java/pull/6238)).
+  - Drop telemetry and record `callback_error` when a customer event processor throws instead of continuing with a potentially partially processed item. SDK-owned processor failures are logged and processing continues.
+  - Drop breadcrumbs when `beforeBreadcrumb` throws instead of storing exception details on the breadcrumb.
+  - No longer fail Spring requests when a `TransactionNameProvider` throws, falling back to `<unlabeled transaction>` ([#6241](https://github.com/getsentry/sentry-java/pull/6241)).
+  - No longer fail Spring requests when a `SentryUserProvider` throws and overwrite the user with an empty user object ([#6240](https://github.com/getsentry/sentry-java/pull/6240)).
+  - Disable profiling when `profilesSampler` throws instead of falling back to `profilesSampleRate` or inheriting the parent profiling decision ([#6164](https://github.com/getsentry/sentry-java/pull/6164)).
+  - Report attached profiles dropped by transaction callback errors as `callback_error` in client reports and `OnDiscardCallback` ([#6166](https://github.com/getsentry/sentry-java/pull/6166)).
+  - Skip replay capture when `beforeErrorSampling` throws while retaining the error event ([#6165](https://github.com/getsentry/sentry-java/pull/6165)).
+  - Skip Android screenshots or view hierarchies when their capture callbacks throw while retaining the error event ([#6167](https://github.com/getsentry/sentry-java/pull/6167)).
+  - Keep the Android feedback form available when its options configurator throws ([#6239](https://github.com/getsentry/sentry-java/pull/6239)).
+  - Isolate nonfatal callback throwables while continuing to propagate fatal JVM errors ([#6251](https://github.com/getsentry/sentry-java/pull/6251)).
+
 ### Dependencies
 
 - Bump Native SDK from v0.17.2 to v0.17.3 ([#6250](https://github.com/getsentry/sentry-java/pull/6250))

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

@@ -233,7 +233,7 @@ public final class io/sentry/android/core/AppState$LifecycleObserver : androidx/
 	public fun onStop (Landroidx/lifecycle/LifecycleOwner;)V
 }
 
-public final class io/sentry/android/core/ApplicationExitInfoEventProcessor : io/sentry/BackfillingEventProcessor {
+public final class io/sentry/android/core/ApplicationExitInfoEventProcessor : io/sentry/BackfillingEventProcessor, io/sentry/internal/eventprocessor/SentryEventProcessor {
 	public fun <init> (Landroid/content/Context;Lio/sentry/android/core/SentryAndroidOptions;Lio/sentry/android/core/BuildInfoProvider;)V
 	public fun getOrder ()Ljava/lang/Long;
 	public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent;
@@ -401,7 +401,7 @@ public class io/sentry/android/core/PerfettoProfiler {
 	public fun endAndCollect (Ljava/util/function/Consumer;)V
 }
 
-public final class io/sentry/android/core/ScreenshotEventProcessor : io/sentry/EventProcessor {
+public final class io/sentry/android/core/ScreenshotEventProcessor : io/sentry/internal/eventprocessor/SentryEventProcessor {
 	public fun <init> (Lio/sentry/android/core/SentryAndroidOptions;Lio/sentry/android/core/BuildInfoProvider;Z)V
 	public fun getOrder ()Ljava/lang/Long;
 	public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent;
@@ -680,7 +680,7 @@ public final class io/sentry/android/core/UserInteractionIntegration : android/a
 	public fun register (Lio/sentry/IScopes;Lio/sentry/SentryOptions;)V
 }
 
-public final class io/sentry/android/core/ViewHierarchyEventProcessor : io/sentry/EventProcessor {
+public final class io/sentry/android/core/ViewHierarchyEventProcessor : io/sentry/internal/eventprocessor/SentryEventProcessor {
 	public fun <init> (Lio/sentry/android/core/SentryAndroidOptions;)V
 	public fun getOrder ()Ljava/lang/Long;
 	public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent;

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

@@ -52,6 +52,7 @@
 import io.sentry.hints.AbnormalExit;
 import io.sentry.hints.Backfillable;
 import io.sentry.hints.NativeCrashExit;
+import io.sentry.internal.eventprocessor.SentryEventProcessor;
 import io.sentry.protocol.App;
 import io.sentry.protocol.Contexts;
 import io.sentry.protocol.DebugImage;
@@ -89,7 +90,8 @@
  */
 @ApiStatus.Internal
 @WorkerThread
-public final class ApplicationExitInfoEventProcessor implements BackfillingEventProcessor {
+public final class ApplicationExitInfoEventProcessor
+    implements BackfillingEventProcessor, SentryEventProcessor {
 
   private final @NotNull Context context;
 

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

@@ -8,6 +8,7 @@
 import io.sentry.android.core.internal.util.AndroidThreadChecker;
 import io.sentry.android.core.performance.AppStartMetrics;
 import io.sentry.android.core.performance.TimeSpan;
+import io.sentry.internal.eventprocessor.SentryEventProcessor;
 import io.sentry.protocol.App;
 import io.sentry.protocol.OperatingSystem;
 import io.sentry.protocol.SentryException;
@@ -32,7 +33,7 @@
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.annotations.TestOnly;
 
-final class DefaultAndroidEventProcessor implements EventProcessor {
+final class DefaultAndroidEventProcessor implements SentryEventProcessor {
 
   @TestOnly final Context context;
 

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

@@ -7,7 +7,6 @@
 import static io.sentry.android.core.ActivityLifecycleIntegration.STANDALONE_APP_START_OP;
 import static io.sentry.android.core.ActivityLifecycleIntegration.UI_LOAD_OP;
 
-import io.sentry.EventProcessor;
 import io.sentry.Hint;
 import io.sentry.ISentryLifecycleToken;
 import io.sentry.MeasurementUnit;
@@ -20,6 +19,7 @@
 import io.sentry.android.core.internal.util.AndroidThreadChecker;
 import io.sentry.android.core.performance.AppStartMetrics;
 import io.sentry.android.core.performance.TimeSpan;
+import io.sentry.internal.eventprocessor.SentryEventProcessor;
 import io.sentry.protocol.App;
 import io.sentry.protocol.MeasurementValue;
 import io.sentry.protocol.SentryId;
@@ -36,7 +36,7 @@
 import org.jetbrains.annotations.Nullable;
 
 /** Event Processor responsible for adding Android metrics to transactions */
-final class PerformanceAndroidEventProcessor implements EventProcessor {
+final class PerformanceAndroidEventProcessor implements SentryEventProcessor {
 
   private static final String APP_METRICS_ORIGIN = "auto.ui";
 

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

@@ -8,7 +8,6 @@
 import android.graphics.Bitmap;
 import android.view.View;
 import io.sentry.Attachment;
-import io.sentry.EventProcessor;
 import io.sentry.Hint;
 import io.sentry.SentryEvent;
 import io.sentry.SentryLevel;
@@ -18,7 +17,9 @@
 import io.sentry.android.replay.util.MaskRenderer;
 import io.sentry.android.replay.util.ViewsKt;
 import io.sentry.android.replay.viewhierarchy.ViewHierarchyNode;
+import io.sentry.internal.eventprocessor.SentryEventProcessor;
 import io.sentry.protocol.SentryTransaction;
+import io.sentry.util.ExceptionUtils;
 import io.sentry.util.HintUtils;
 import io.sentry.util.Objects;
 import java.util.concurrent.CountDownLatch;
@@ -34,7 +35,7 @@
  * captured.
  */
 @ApiStatus.Internal
-public final class ScreenshotEventProcessor implements EventProcessor {
+public final class ScreenshotEventProcessor implements SentryEventProcessor {
 
   private final @NotNull SentryAndroidOptions options;
   private final @NotNull BuildInfoProvider buildInfoProvider;
@@ -112,7 +113,18 @@ private boolean isMaskingEnabled() {
     final @Nullable SentryAndroidOptions.BeforeCaptureCallback beforeCaptureCallback =
         options.getBeforeScreenshotCaptureCallback();
     if (beforeCaptureCallback != null) {
-      if (!beforeCaptureCallback.execute(event, hint, shouldDebounce)) {
+      try {
+        if (!beforeCaptureCallback.execute(event, hint, shouldDebounce)) {
+          return event;
+        }
+      } catch (Throwable e) {
+        ExceptionUtils.rethrowIfFatal(e);
+        options
+            .getLogger()
+            .log(
+                SentryLevel.ERROR,
+                "The beforeScreenshotCapture callback threw an exception. Skipping screenshot capture.",
+                e);
         return event;
       }
     } else if (shouldDebounce) {

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

@@ -62,14 +62,37 @@ public class SentryUserFeedbackForm extends AlertDialog {
       final @Nullable SentryFeedbackOptions.OptionsConfigurator configurator) {
     super(context, themeResId);
     this.associatedEventId = associatedEventId;
-    this.resolvedFeedbackOptions =
-        new SentryFeedbackOptions(Sentry.getCurrentScopes().getOptions().getFeedbackOptions());
+    final @NotNull SentryOptions options = Sentry.getCurrentScopes().getOptions();
+    @NotNull
+    SentryFeedbackOptions feedbackOptions = new SentryFeedbackOptions(options.getFeedbackOptions());
     if (configuration != null) {
-      configuration.configure(context, resolvedFeedbackOptions);
+      final @NotNull SentryFeedbackOptions configuredOptions =
+          new SentryFeedbackOptions(feedbackOptions);
+      try {
+        configuration.configure(context, configuredOptions);
+        feedbackOptions = configuredOptions;
+      } catch (Throwable e) {
+        ExceptionUtils.rethrowIfFatal(e);
+        options
+            .getLogger()
+            .log(
+                SentryLevel.ERROR, "Feedback OptionsConfiguration callback threw an exception.", e);
+      }
     }
     if (configurator != null) {
-      configurator.configure(resolvedFeedbackOptions);
+      final @NotNull SentryFeedbackOptions configuredOptions =
+          new SentryFeedbackOptions(feedbackOptions);
+      try {
+        configurator.configure(configuredOptions);
+        feedbackOptions = configuredOptions;
+      } catch (Throwable e) {
+        ExceptionUtils.rethrowIfFatal(e);
+        options
+            .getLogger()
+            .log(SentryLevel.ERROR, "Feedback OptionsConfigurator callback threw an exception.", e);
+      }
     }
+    this.resolvedFeedbackOptions = feedbackOptions;
     SentryIntegrationPackageStorage.getInstance().addIntegration("UserFeedbackWidget");
     maybeStartShakeDetection(context);
   }

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

@@ -7,7 +7,6 @@
 import android.view.ViewGroup;
 import android.view.Window;
 import io.sentry.Attachment;
-import io.sentry.EventProcessor;
 import io.sentry.Hint;
 import io.sentry.ILogger;
 import io.sentry.ISerializer;
@@ -18,10 +17,12 @@
 import io.sentry.android.core.internal.util.AndroidThreadChecker;
 import io.sentry.android.core.internal.util.ClassUtil;
 import io.sentry.android.core.internal.util.Debouncer;
+import io.sentry.internal.eventprocessor.SentryEventProcessor;
 import io.sentry.internal.viewhierarchy.ViewHierarchyExporter;
 import io.sentry.protocol.SentryTransaction;
 import io.sentry.protocol.ViewHierarchy;
 import io.sentry.protocol.ViewHierarchyNode;
+import io.sentry.util.ExceptionUtils;
 import io.sentry.util.HintUtils;
 import io.sentry.util.JsonSerializationUtils;
 import io.sentry.util.Objects;
@@ -37,7 +38,7 @@
 
 /** ViewHierarchyEventProcessor responsible for taking a snapshot of the current view hierarchy. */
 @ApiStatus.Internal
-public final class ViewHierarchyEventProcessor implements EventProcessor {
+public final class ViewHierarchyEventProcessor implements SentryEventProcessor {
 
   private final @NotNull SentryAndroidOptions options;
   private final @NotNull Debouncer debouncer;
@@ -91,7 +92,18 @@ public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options)
     final @Nullable SentryAndroidOptions.BeforeCaptureCallback beforeCaptureCallback =
         options.getBeforeViewHierarchyCaptureCallback();
     if (beforeCaptureCallback != null) {
-      if (!beforeCaptureCallback.execute(event, hint, shouldDebounce)) {
+      try {
+        if (!beforeCaptureCallback.execute(event, hint, shouldDebounce)) {
+          return event;
+        }
+      } catch (Throwable e) {
+        ExceptionUtils.rethrowIfFatal(e);
+        options
+            .getLogger()
+            .log(
+                SentryLevel.ERROR,
+                "The beforeViewHierarchyCapture callback threw an exception. Skipping view hierarchy capture.",
+                e);
         return event;
       }
     } else if (shouldDebounce) {

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

@@ -29,25 +29,30 @@ import androidx.compose.ui.text.style.TextOverflow
 import androidx.compose.ui.unit.dp
 import androidx.compose.ui.unit.sp
 import androidx.test.ext.junit.runners.AndroidJUnit4
+import com.google.common.truth.Truth.assertThat
 import io.sentry.Attachment
 import io.sentry.Hint
+import io.sentry.ILogger
 import io.sentry.MainEventProcessor
 import io.sentry.SentryEvent
 import io.sentry.SentryIntegrationPackageStorage
+import io.sentry.SentryLevel
 import io.sentry.TypeCheckHint.ANDROID_ACTIVITY
 import io.sentry.protocol.SentryException
 import io.sentry.util.thread.IThreadChecker
 import java.io.File
 import kotlin.test.BeforeTest
 import kotlin.test.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertFailsWith
 import kotlin.test.assertFalse
 import kotlin.test.assertNotNull
 import kotlin.test.assertNull
 import kotlin.test.assertSame
 import kotlin.test.assertTrue
 import org.junit.runner.RunWith
 import org.mockito.kotlin.mock
+import org.mockito.kotlin.verify
 import org.mockito.kotlin.whenever
 import org.robolectric.Robolectric.buildActivity
 import org.robolectric.Shadows.shadowOf
@@ -310,11 +315,51 @@ class ScreenshotEventProcessorTest {
     assertNull(hint.screenshot)
   }
 
+  @Test
+  fun `when capture callback throws a nonfatal throwable, skips screenshot and retains event`() {
+    CurrentActivityHolder.getInstance().setActivity(fixture.activity)
+    val logger = mock<ILogger>()
+    fixture.options.isDebug = true
+    fixture.options.setLogger(logger)
+    val failure = AssertionError("callback failed")
+    fixture.options.setBeforeScreenshotCaptureCallback { _, _, _ -> throw failure }
+    val processor = fixture.getSut(true)
+    val event = SentryEvent().apply { exceptions = listOf(SentryException()) }
+    val hint = Hint()
+
+    assertThat(processor.process(event, hint)).isSameInstanceAs(event)
+    assertThat(hint.screenshot).isNull()
+    verify(logger)
+      .log(
+        SentryLevel.ERROR,
+        "The beforeScreenshotCapture callback threw an exception. Skipping screenshot capture.",
+        failure,
+      )
+
+    fixture.options.setBeforeScreenshotCaptureCallback { _, _, _ -> true }
+    val nextHint = Hint()
+    assertThat(processor.process(event, nextHint)).isSameInstanceAs(event)
+    assertThat(nextHint.screenshot).isNotNull()
+  }
+
+  @Test
+  fun `when capture callback throws a fatal error, it is not swallowed`() {
+    CurrentActivityHolder.getInstance().setActivity(fixture.activity)
+    val failure = OutOfMemoryError("callback failed")
+    fixture.options.setBeforeScreenshotCaptureCallback { _, _, _ -> throw failure }
+    val processor = fixture.getSut(true)
+    val event = SentryEvent().apply { exceptions = listOf(SentryException()) }
+
+    val thrown = assertFailsWith<OutOfMemoryError> { processor.process(event, Hint()) }
+
+    assertThat(thrown).isSameInstanceAs(failure)
+  }
+
   @Test
   fun `when capture callback returns true, a screenshot should be captured`() {
     CurrentActivityHolder.getInstance().setActivity(fixture.activity)
 
-    fixture.options.setBeforeViewHierarchyCaptureCallback { _, _, _ -> true }
+    fixture.options.setBeforeScreenshotCaptureCallback { _, _, _ -> true }
     val processor = fixture.getSut(true)
 
     val event = SentryEvent().apply { exceptions = listOf(SentryException()) }