fix(callbacks): [Callback Errors 10] Handle nonfatal throwables

CHANGELOG.md

@@ -17,6 +17,7 @@
   - 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
 

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

@@ -19,6 +19,7 @@
 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;
@@ -116,7 +117,8 @@ private boolean isMaskingEnabled() {
         if (!beforeCaptureCallback.execute(event, hint, shouldDebounce)) {
           return event;
         }
-      } catch (Exception e) {
+      } catch (Throwable e) {
+        ExceptionUtils.rethrowIfFatal(e);
         options
             .getLogger()
             .log(

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

@@ -22,6 +22,7 @@
 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;
@@ -95,7 +96,8 @@ public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options)
         if (!beforeCaptureCallback.execute(event, hint, shouldDebounce)) {
           return event;
         }
-      } catch (Exception e) {
+      } catch (Throwable e) {
+        ExceptionUtils.rethrowIfFatal(e);
         options
             .getLogger()
             .log(

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

@@ -44,6 +44,7 @@ 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
@@ -315,12 +316,12 @@ class ScreenshotEventProcessorTest {
   }
 
   @Test
-  fun `when capture callback throws, skips screenshot and retains event`() {
+  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 = IllegalStateException("callback failed")
+    val failure = AssertionError("callback failed")
     fixture.options.setBeforeScreenshotCaptureCallback { _, _, _ -> throw failure }
     val processor = fixture.getSut(true)
     val event = SentryEvent().apply { exceptions = listOf(SentryException()) }
@@ -341,6 +342,19 @@ class ScreenshotEventProcessorTest {
     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)

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

@@ -19,6 +19,7 @@ import java.io.Writer
 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
@@ -345,10 +346,10 @@ class ViewHierarchyEventProcessorTest {
   }
 
   @Test
-  fun `when capture callback throws, skips view hierarchy and retains event`() {
+  fun `when capture callback throws a nonfatal throwable, skips view hierarchy and retains event`() {
     fixture.options.isDebug = true
     fixture.options.setLogger(fixture.logger)
-    val failure = IllegalStateException("callback failed")
+    val failure = AssertionError("callback failed")
     fixture.options.setBeforeViewHierarchyCaptureCallback { _, _, _ -> throw failure }
     val processor = fixture.getSut(true)
     val event = SentryEvent().apply { exceptions = listOf(SentryException()) }
@@ -369,6 +370,18 @@ class ViewHierarchyEventProcessorTest {
     assertThat(nextHint.viewHierarchy).isNotNull()
   }
 
+  @Test
+  fun `when capture callback throws a fatal error, it is not swallowed`() {
+    val failure = OutOfMemoryError("callback failed")
+    fixture.options.setBeforeViewHierarchyCaptureCallback { _, _, _ -> 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 view hierarchy should be captured`() {
     fixture.options.setBeforeViewHierarchyCaptureCallback { _, _, _ -> true }

sentry-graphql-22/src/test/kotlin/io/sentry/graphql22/SentryInstrumentationTest.kt

@@ -35,9 +35,11 @@ import io.sentry.graphql.SentryGraphqlInstrumentation
 import io.sentry.graphql.SentrySubscriptionHandler
 import java.lang.RuntimeException
 import java.util.concurrent.CompletableFuture
+import java.util.concurrent.CompletionException
 import kotlin.random.Random
 import kotlin.test.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertFailsWith
 import kotlin.test.assertFalse
 import kotlin.test.assertNotNull
 import kotlin.test.assertTrue
@@ -210,8 +212,8 @@ class SentryInstrumentationTest {
   }
 
   @Test
-  fun `when beforeSpan throws, drops span and preserves result`() {
-    val failure = IllegalStateException("callback failed")
+  fun `when beforeSpan throws a nonfatal throwable, drops span and preserves result`() {
+    val failure = AssertionError("callback failed")
     val logger = mock<ILogger>()
     val sut =
       fixture.getSut(
@@ -239,6 +241,18 @@ class SentryInstrumentationTest {
     }
   }
 
+  @Test
+  fun `fatal error from beforeSpan is not swallowed`() {
+    val failure = OutOfMemoryError("callback failed")
+    val sut = fixture.getSut(beforeSpan = { _, _, _ -> throw failure })
+
+    withMockScopes {
+      val thrown = assertFailsWith<CompletionException> { sut.execute("{ shows { id } }") }
+
+      assertThat(thrown.cause).isSameInstanceAs(failure)
+    }
+  }
+
   @Test
   fun `when beforeSpan throws, drops async span and preserves result`() {
     val sut =

sentry-graphql-core/src/main/java/io/sentry/graphql/SentryGraphqlInstrumentation.java

@@ -28,6 +28,7 @@
 import io.sentry.SpanStatus;
 import io.sentry.TypeCheckHint;
 import io.sentry.clientreport.DiscardReason;
+import io.sentry.util.ExceptionUtils;
 import io.sentry.util.StringUtils;
 import java.util.Arrays;
 import java.util.List;
@@ -290,7 +291,8 @@ private void finish(
       ISpan newSpan = span;
       try {
         newSpan = beforeSpan.execute(span, environment, result);
-      } catch (Exception e) {
+      } catch (Throwable e) {
+        ExceptionUtils.rethrowIfFatal(e);
         span.getSpanContext().setSampled(false);
         if (wasSampled) {
           scopesFromContext(environment.getGraphQlContext())

sentry-graphql/src/test/kotlin/io/sentry/graphql/SentryInstrumentationTest.kt

@@ -31,9 +31,11 @@ import io.sentry.TransactionContext
 import io.sentry.clientreport.DiscardReason
 import java.lang.RuntimeException
 import java.util.concurrent.CompletableFuture
+import java.util.concurrent.CompletionException
 import kotlin.random.Random
 import kotlin.test.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertFailsWith
 import kotlin.test.assertFalse
 import kotlin.test.assertNotNull
 import kotlin.test.assertTrue
@@ -206,8 +208,8 @@ class SentryInstrumentationTest {
   }
 
   @Test
-  fun `when beforeSpan throws, drops span and preserves result`() {
-    val failure = IllegalStateException("callback failed")
+  fun `when beforeSpan throws a nonfatal throwable, drops span and preserves result`() {
+    val failure = AssertionError("callback failed")
     val logger = mock<ILogger>()
     val sut =
       fixture.getSut(
@@ -235,6 +237,18 @@ class SentryInstrumentationTest {
     }
   }
 
+  @Test
+  fun `fatal error from beforeSpan is not swallowed`() {
+    val failure = OutOfMemoryError("callback failed")
+    val sut = fixture.getSut(beforeSpan = { _, _, _ -> throw failure })
+
+    withMockScopes {
+      val thrown = assertFailsWith<CompletionException> { sut.execute("{ shows { id } }") }
+
+      assertThat(thrown.cause).isSameInstanceAs(failure)
+    }
+  }
+
   @Test
   fun `when beforeSpan throws, drops async span and preserves result`() {
     val sut =

sentry-ktor-client/src/main/java/io/sentry/ktorClient/SentryKtorClientPlugin.kt

@@ -22,6 +22,7 @@ import io.sentry.SentryOptions
 import io.sentry.SpanStatus
 import io.sentry.clientreport.DiscardReason
 import io.sentry.kotlin.SentryContext
+import io.sentry.util.ExceptionUtils
 import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion
 import io.sentry.util.Platform
 import io.sentry.util.PropagationTargetsUtils
@@ -193,7 +194,8 @@ public val SentryKtorClientPlugin: ClientPlugin<SentryKtorClientPluginConfig> =
           result =
             try {
               beforeSpan.execute(span, request)
-            } catch (e: Exception) {
+            } catch (@Suppress("TooGenericExceptionCaught") e: Throwable) {
+              ExceptionUtils.rethrowIfFatal(e)
               span.spanContext.sampled = false
               if (wasSampled) {
                 (if (forceScopes) scopes else Sentry.getCurrentScopes())

sentry-ktor-client/src/test/java/io/sentry/ktorClient/SentryKtorClientPluginTest.kt

@@ -38,6 +38,7 @@ import java.util.concurrent.TimeUnit
 import kotlin.Unit
 import kotlin.test.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertFailsWith
 import kotlin.test.assertNotNull
 import kotlin.test.assertNull
 import kotlin.test.assertTrue
@@ -451,7 +452,7 @@ class SentryKtorClientPluginTest {
         fixture.getSut(
           beforeSpan = { span, _ ->
             span.spanContext.sampled = false
-            throw IllegalStateException("callback failed")
+            error("callback failed")
           }
         )
       fixture.sentryTracer.spanContext.sampled = sampled
@@ -469,7 +470,7 @@ class SentryKtorClientPluginTest {
 
   @Test
   fun `when beforeSpan throws, drops span and preserves response`(): Unit = runBlocking {
-    val failure = IllegalStateException("callback failed")
+    val failure = AssertionError("callback failed")
     val logger = mock<ILogger>()
     val sut =
       fixture.getSut(
@@ -498,6 +499,19 @@ class SentryKtorClientPluginTest {
       )
   }
 
+  @Test
+  fun `fatal error from beforeSpan is not swallowed`(): Unit = runBlocking {
+    val failure = OutOfMemoryError("callback failed")
+    val sut = fixture.getSut(beforeSpan = { _, _ -> throw failure })
+
+    val thrown =
+      assertFailsWith<OutOfMemoryError> {
+        sut.use { sut.get(fixture.server.url("/hello").toString()) }
+      }
+
+    assertThat(generateSequence<Throwable>(thrown) { it.cause }.last()).isSameInstanceAs(failure)
+  }
+
   @Test
   fun `beforeSpan can drop span`(): Unit = runBlocking {
     val sut = fixture.getSut(beforeSpan = { _, _ -> null })

sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpInterceptor.kt

@@ -22,6 +22,7 @@ import io.sentry.TypeCheckHint.SENTRY_REPLAY_NETWORK_DETAILS
 import io.sentry.clientreport.DiscardReason
 import io.sentry.okhttp.SentryOkHttpInterceptor.BeforeSpanCallback
 import io.sentry.transport.CurrentDateProvider
+import io.sentry.util.ExceptionUtils
 import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion
 import io.sentry.util.Platform
 import io.sentry.util.PropagationTargetsUtils
@@ -375,7 +376,8 @@ public open class SentryOkHttpInterceptor(
       val result =
         try {
           beforeSpan.execute(span, request, response)
-        } catch (e: Exception) {
+        } catch (@Suppress("TooGenericExceptionCaught") e: Throwable) {
+          ExceptionUtils.rethrowIfFatal(e)
           span.spanContext.sampled = false
           if (wasSampled) {
             scopes.options.clientReportRecorder.recordLostEvent(

sentry-okhttp/src/test/java/io/sentry/okhttp/SentryOkHttpInterceptorTest.kt

@@ -62,6 +62,7 @@ import org.mockito.kotlin.verify
 import org.mockito.kotlin.verifyNoMoreInteractions
 import org.mockito.kotlin.whenever
 
+@Suppress("LargeClass")
 class SentryOkHttpInterceptorTest {
   class Fixture {
     val scopes = mock<IScopes>()
@@ -463,7 +464,7 @@ class SentryOkHttpInterceptorTest {
         fixture.getSut(
           beforeSpan = { span, _, _ ->
             span.spanContext.sampled = false
-            throw IllegalStateException("callback failed")
+            error("callback failed")
           }
         )
       fixture.sentryTracer.spanContext.sampled = sampled
@@ -480,8 +481,8 @@ class SentryOkHttpInterceptorTest {
   }
 
   @Test
-  fun `when beforeSpan throws, drops span and preserves response`() {
-    val failure = IllegalStateException("callback failed")
+  fun `when beforeSpan throws a nonfatal throwable, drops span and preserves response`() {
+    val failure = AssertionError("callback failed")
     val logger = mock<ILogger>()
     val sut =
       fixture.getSut(
@@ -509,6 +510,16 @@ class SentryOkHttpInterceptorTest {
       )
   }
 
+  @Test
+  fun `fatal error from beforeSpan is not swallowed`() {
+    val failure = OutOfMemoryError("callback failed")
+    val sut = fixture.getSut(beforeSpan = { _, _, _ -> throw failure })
+
+    val thrown = assertFailsWith<OutOfMemoryError> { sut.newCall(getRequest()).execute() }
+
+    assertThat(thrown).isSameInstanceAs(failure)
+  }
+
   @Test
   fun `when beforeSpan throws, event listener still finishes span`() {
     val sut =

sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java

@@ -21,6 +21,7 @@
 import io.sentry.SpanStatus;
 import io.sentry.W3CTraceparentHeader;
 import io.sentry.clientreport.DiscardReason;
+import io.sentry.util.ExceptionUtils;
 import io.sentry.util.Objects;
 import io.sentry.util.SpanUtils;
 import io.sentry.util.TracingUtils;
@@ -102,7 +103,8 @@ public Response execute(final @NotNull Request request, final @NotNull Request.O
           ISpan result = span;
           try {
             result = beforeSpan.execute(span, request, response);
-          } catch (Exception e) {
+          } catch (Throwable e) {
+            ExceptionUtils.rethrowIfFatal(e);
             span.getSpanContext().setSampled(false);
             if (wasSampled) {
               scopes