Skip to content

Commit cda00a7

Browse files
adinauerclaude
adinauer
and
claude
committed
fix(callbacks): Handle nonfatal callback throwables
Catch nonfatal Throwable failures from newly guarded customer callbacks while continuing to propagate fatal JVM errors. This keeps callback isolation consistent without introducing new fatal-error swallowing. Add regression coverage for nonfatal fallback behavior and fatal error propagation across the affected integrations. Refs #6081 Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 13094aa commit cda00a7

15 files changed

Lines changed: 156 additions & 23 deletions

File tree

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import io.sentry.android.replay.viewhierarchy.ViewHierarchyNode;
2020
import io.sentry.internal.eventprocessor.SentryEventProcessor;
2121
import io.sentry.protocol.SentryTransaction;
22+
import io.sentry.util.ExceptionUtils;
2223
import io.sentry.util.HintUtils;
2324
import io.sentry.util.Objects;
2425
import java.util.concurrent.CountDownLatch;
@@ -116,7 +117,8 @@ private boolean isMaskingEnabled() {
116117
if (!beforeCaptureCallback.execute(event, hint, shouldDebounce)) {
117118
return event;
118119
}
119-
} catch (Exception e) {
120+
} catch (Throwable e) {
121+
ExceptionUtils.rethrowIfFatal(e);
120122
options
121123
.getLogger()
122124
.log(

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import io.sentry.protocol.SentryTransaction;
2323
import io.sentry.protocol.ViewHierarchy;
2424
import io.sentry.protocol.ViewHierarchyNode;
25+
import io.sentry.util.ExceptionUtils;
2526
import io.sentry.util.HintUtils;
2627
import io.sentry.util.JsonSerializationUtils;
2728
import io.sentry.util.Objects;
@@ -95,7 +96,8 @@ public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options)
9596
if (!beforeCaptureCallback.execute(event, hint, shouldDebounce)) {
9697
return event;
9798
}
98-
} catch (Exception e) {
99+
} catch (Throwable e) {
100+
ExceptionUtils.rethrowIfFatal(e);
99101
options
100102
.getLogger()
101103
.log(

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

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import java.io.File
4444
import kotlin.test.BeforeTest
4545
import kotlin.test.Test
4646
import kotlin.test.assertEquals
47+
import kotlin.test.assertFailsWith
4748
import kotlin.test.assertFalse
4849
import kotlin.test.assertNotNull
4950
import kotlin.test.assertNull
@@ -315,12 +316,12 @@ class ScreenshotEventProcessorTest {
315316
}
316317

317318
@Test
318-
fun `when capture callback throws, skips screenshot and retains event`() {
319+
fun `when capture callback throws a nonfatal throwable, skips screenshot and retains event`() {
319320
CurrentActivityHolder.getInstance().setActivity(fixture.activity)
320321
val logger = mock<ILogger>()
321322
fixture.options.isDebug = true
322323
fixture.options.setLogger(logger)
323-
val failure = IllegalStateException("callback failed")
324+
val failure = AssertionError("callback failed")
324325
fixture.options.setBeforeScreenshotCaptureCallback { _, _, _ -> throw failure }
325326
val processor = fixture.getSut(true)
326327
val event = SentryEvent().apply { exceptions = listOf(SentryException()) }
@@ -341,6 +342,19 @@ class ScreenshotEventProcessorTest {
341342
assertThat(nextHint.screenshot).isNotNull()
342343
}
343344

345+
@Test
346+
fun `when capture callback throws a fatal error, it is not swallowed`() {
347+
CurrentActivityHolder.getInstance().setActivity(fixture.activity)
348+
val failure = OutOfMemoryError("callback failed")
349+
fixture.options.setBeforeScreenshotCaptureCallback { _, _, _ -> throw failure }
350+
val processor = fixture.getSut(true)
351+
val event = SentryEvent().apply { exceptions = listOf(SentryException()) }
352+
353+
val thrown = assertFailsWith<OutOfMemoryError> { processor.process(event, Hint()) }
354+
355+
assertThat(thrown).isSameInstanceAs(failure)
356+
}
357+
344358
@Test
345359
fun `when capture callback returns true, a screenshot should be captured`() {
346360
CurrentActivityHolder.getInstance().setActivity(fixture.activity)

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

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import java.io.Writer
1919
import kotlin.test.BeforeTest
2020
import kotlin.test.Test
2121
import kotlin.test.assertEquals
22+
import kotlin.test.assertFailsWith
2223
import kotlin.test.assertFalse
2324
import kotlin.test.assertNotNull
2425
import kotlin.test.assertNull
@@ -345,10 +346,10 @@ class ViewHierarchyEventProcessorTest {
345346
}
346347

347348
@Test
348-
fun `when capture callback throws, skips view hierarchy and retains event`() {
349+
fun `when capture callback throws a nonfatal throwable, skips view hierarchy and retains event`() {
349350
fixture.options.isDebug = true
350351
fixture.options.setLogger(fixture.logger)
351-
val failure = IllegalStateException("callback failed")
352+
val failure = AssertionError("callback failed")
352353
fixture.options.setBeforeViewHierarchyCaptureCallback { _, _, _ -> throw failure }
353354
val processor = fixture.getSut(true)
354355
val event = SentryEvent().apply { exceptions = listOf(SentryException()) }
@@ -369,6 +370,18 @@ class ViewHierarchyEventProcessorTest {
369370
assertThat(nextHint.viewHierarchy).isNotNull()
370371
}
371372

373+
@Test
374+
fun `when capture callback throws a fatal error, it is not swallowed`() {
375+
val failure = OutOfMemoryError("callback failed")
376+
fixture.options.setBeforeViewHierarchyCaptureCallback { _, _, _ -> throw failure }
377+
val processor = fixture.getSut(true)
378+
val event = SentryEvent().apply { exceptions = listOf(SentryException()) }
379+
380+
val thrown = assertFailsWith<OutOfMemoryError> { processor.process(event, Hint()) }
381+
382+
assertThat(thrown).isSameInstanceAs(failure)
383+
}
384+
372385
@Test
373386
fun `when capture callback returns true, a view hierarchy should be captured`() {
374387
fixture.options.setBeforeViewHierarchyCaptureCallback { _, _, _ -> true }

‎sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import io.sentry.SpanStatus
2626
import io.sentry.TypeCheckHint.APOLLO_REQUEST
2727
import io.sentry.TypeCheckHint.APOLLO_RESPONSE
2828
import io.sentry.clientreport.DiscardReason
29+
import io.sentry.util.ExceptionUtils
2930
import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion
3031
import io.sentry.util.SpanUtils
3132
import io.sentry.util.TracingUtils
@@ -180,7 +181,8 @@ class SentryApolloInterceptor(
180181
val wasSampled = span.isSampled == true
181182
try {
182183
newSpan = beforeSpan.execute(span, request, response)
183-
} catch (e: Exception) {
184+
} catch (@Suppress("TooGenericExceptionCaught") e: Throwable) {
185+
ExceptionUtils.rethrowIfFatal(e)
184186
span.spanContext.sampled = false
185187
if (wasSampled) {
186188
scopes.options.clientReportRecorder.recordLostEvent(

‎sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ package io.sentry.apollo
33
import com.apollographql.apollo.ApolloClient
44
import com.apollographql.apollo.coroutines.await
55
import com.apollographql.apollo.exception.ApolloException
6+
import com.apollographql.apollo.interceptor.ApolloInterceptor.CallBack
7+
import com.apollographql.apollo.interceptor.ApolloInterceptor.InterceptorRequest
8+
import com.apollographql.apollo.interceptor.ApolloInterceptor.InterceptorResponse
9+
import com.apollographql.apollo.interceptor.ApolloInterceptorChain
610
import com.google.common.truth.Truth.assertThat
711
import io.sentry.BaggageHeader
812
import io.sentry.Breadcrumb
@@ -29,6 +33,7 @@ import io.sentry.util.ApolloPlatformTestManipulator
2933
import java.util.concurrent.TimeUnit
3034
import kotlin.test.Test
3135
import kotlin.test.assertEquals
36+
import kotlin.test.assertFailsWith
3237
import kotlin.test.assertNotNull
3338
import kotlin.test.assertNull
3439
import kotlin.test.assertTrue
@@ -274,7 +279,7 @@ class SentryApolloInterceptorTest {
274279

275280
@Test
276281
fun `when beforeSpan throws, drops span and preserves response`(): Unit = runBlocking {
277-
val failure = IllegalStateException("callback failed")
282+
val failure = AssertionError("callback failed")
278283
val logger = mock<ILogger>()
279284
fixture.options.isDebug = true
280285
fixture.options.setLogger(logger)
@@ -308,6 +313,30 @@ class SentryApolloInterceptorTest {
308313
)
309314
}
310315

316+
@Test
317+
fun `fatal error from beforeSpan is not swallowed`() {
318+
val failure = OutOfMemoryError("callback failed")
319+
val tx =
320+
SentryTracer(TransactionContext("op", "desc", TracesSamplingDecision(true)), fixture.scopes)
321+
whenever(fixture.scopes.span).thenReturn(tx)
322+
val interceptor = SentryApolloInterceptor(fixture.scopes) { _, _, _ -> throw failure }
323+
val request = InterceptorRequest.builder(LaunchDetailsQuery.builder().id("83").build()).build()
324+
val chain = mock<ApolloInterceptorChain>()
325+
doAnswer {
326+
it.getArgument<CallBack>(2).onResponse(InterceptorResponse(null))
327+
null
328+
}
329+
.whenever(chain)
330+
.proceedAsync(any(), any(), any())
331+
332+
val thrown =
333+
assertFailsWith<OutOfMemoryError> {
334+
interceptor.interceptAsync(request, chain, { it.run() }, mock())
335+
}
336+
337+
assertThat(thrown).isSameInstanceAs(failure)
338+
}
339+
311340
@Test
312341
fun `adds breadcrumb when http calls succeeds`() {
313342
executeQuery()

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

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,11 @@ import io.sentry.graphql.SentryGraphqlInstrumentation
3535
import io.sentry.graphql.SentrySubscriptionHandler
3636
import java.lang.RuntimeException
3737
import java.util.concurrent.CompletableFuture
38+
import java.util.concurrent.CompletionException
3839
import kotlin.random.Random
3940
import kotlin.test.Test
4041
import kotlin.test.assertEquals
42+
import kotlin.test.assertFailsWith
4143
import kotlin.test.assertFalse
4244
import kotlin.test.assertNotNull
4345
import kotlin.test.assertTrue
@@ -210,8 +212,8 @@ class SentryInstrumentationTest {
210212
}
211213

212214
@Test
213-
fun `when beforeSpan throws, drops span and preserves result`() {
214-
val failure = IllegalStateException("callback failed")
215+
fun `when beforeSpan throws a nonfatal throwable, drops span and preserves result`() {
216+
val failure = AssertionError("callback failed")
215217
val logger = mock<ILogger>()
216218
val sut =
217219
fixture.getSut(
@@ -239,6 +241,18 @@ class SentryInstrumentationTest {
239241
}
240242
}
241243

244+
@Test
245+
fun `fatal error from beforeSpan is not swallowed`() {
246+
val failure = OutOfMemoryError("callback failed")
247+
val sut = fixture.getSut(beforeSpan = { _, _, _ -> throw failure })
248+
249+
withMockScopes {
250+
val thrown = assertFailsWith<CompletionException> { sut.execute("{ shows { id } }") }
251+
252+
assertThat(thrown.cause).isSameInstanceAs(failure)
253+
}
254+
}
255+
242256
@Test
243257
fun `when beforeSpan throws, drops async span and preserves result`() {
244258
val sut =

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import io.sentry.SpanStatus;
2929
import io.sentry.TypeCheckHint;
3030
import io.sentry.clientreport.DiscardReason;
31+
import io.sentry.util.ExceptionUtils;
3132
import io.sentry.util.StringUtils;
3233
import java.util.Arrays;
3334
import java.util.List;
@@ -290,7 +291,8 @@ private void finish(
290291
ISpan newSpan = span;
291292
try {
292293
newSpan = beforeSpan.execute(span, environment, result);
293-
} catch (Exception e) {
294+
} catch (Throwable e) {
295+
ExceptionUtils.rethrowIfFatal(e);
294296
span.getSpanContext().setSampled(false);
295297
if (wasSampled) {
296298
scopesFromContext(environment.getGraphQlContext())

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

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,11 @@ import io.sentry.TransactionContext
3131
import io.sentry.clientreport.DiscardReason
3232
import java.lang.RuntimeException
3333
import java.util.concurrent.CompletableFuture
34+
import java.util.concurrent.CompletionException
3435
import kotlin.random.Random
3536
import kotlin.test.Test
3637
import kotlin.test.assertEquals
38+
import kotlin.test.assertFailsWith
3739
import kotlin.test.assertFalse
3840
import kotlin.test.assertNotNull
3941
import kotlin.test.assertTrue
@@ -206,8 +208,8 @@ class SentryInstrumentationTest {
206208
}
207209

208210
@Test
209-
fun `when beforeSpan throws, drops span and preserves result`() {
210-
val failure = IllegalStateException("callback failed")
211+
fun `when beforeSpan throws a nonfatal throwable, drops span and preserves result`() {
212+
val failure = AssertionError("callback failed")
211213
val logger = mock<ILogger>()
212214
val sut =
213215
fixture.getSut(
@@ -235,6 +237,18 @@ class SentryInstrumentationTest {
235237
}
236238
}
237239

240+
@Test
241+
fun `fatal error from beforeSpan is not swallowed`() {
242+
val failure = OutOfMemoryError("callback failed")
243+
val sut = fixture.getSut(beforeSpan = { _, _, _ -> throw failure })
244+
245+
withMockScopes {
246+
val thrown = assertFailsWith<CompletionException> { sut.execute("{ shows { id } }") }
247+
248+
assertThat(thrown.cause).isSameInstanceAs(failure)
249+
}
250+
}
251+
238252
@Test
239253
fun `when beforeSpan throws, drops async span and preserves result`() {
240254
val sut =

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import io.sentry.SentryOptions
2222
import io.sentry.SpanStatus
2323
import io.sentry.clientreport.DiscardReason
2424
import io.sentry.kotlin.SentryContext
25+
import io.sentry.util.ExceptionUtils
2526
import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion
2627
import io.sentry.util.Platform
2728
import io.sentry.util.PropagationTargetsUtils
@@ -193,7 +194,8 @@ public val SentryKtorClientPlugin: ClientPlugin<SentryKtorClientPluginConfig> =
193194
result =
194195
try {
195196
beforeSpan.execute(span, request)
196-
} catch (e: Exception) {
197+
} catch (@Suppress("TooGenericExceptionCaught") e: Throwable) {
198+
ExceptionUtils.rethrowIfFatal(e)
197199
span.spanContext.sampled = false
198200
if (wasSampled) {
199201
(if (forceScopes) scopes else Sentry.getCurrentScopes())

0 commit comments

Comments
 (0)