Skip to content

Commit dea9190

Browse files
adinauerclaude
adinauer
and
claude
authored
fix(callbacks): [Callback Errors 10] Handle nonfatal throwables (#6251)
* 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> * changelog * fix(callbacks): Preserve Apollo 2 error handling Keep Apollo 2's existing Exception boundary unchanged so non-Exception errors retain their released behavior. Limit the broader nonfatal Throwable handling to callback guards introduced by the unreleased stack. Refs #6081 Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
2 parents 6e59919 + 55fd40d commit dea9190

14 files changed

Lines changed: 124 additions & 21 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
- Skip replay capture when `beforeErrorSampling` throws while retaining the error event ([#6165](https://github.com/getsentry/sentry-java/pull/6165)).
1818
- 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)).
1919
- Keep the Android feedback form available when its options configurator throws ([#6239](https://github.com/getsentry/sentry-java/pull/6239)).
20+
- Isolate nonfatal callback throwables while continuing to propagate fatal JVM errors ([#6251](https://github.com/getsentry/sentry-java/pull/6251)).
2021

2122
### Dependencies
2223

‎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-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())

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

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import java.util.concurrent.TimeUnit
3838
import kotlin.Unit
3939
import kotlin.test.Test
4040
import kotlin.test.assertEquals
41+
import kotlin.test.assertFailsWith
4142
import kotlin.test.assertNotNull
4243
import kotlin.test.assertNull
4344
import kotlin.test.assertTrue
@@ -451,7 +452,7 @@ class SentryKtorClientPluginTest {
451452
fixture.getSut(
452453
beforeSpan = { span, _ ->
453454
span.spanContext.sampled = false
454-
throw IllegalStateException("callback failed")
455+
error("callback failed")
455456
}
456457
)
457458
fixture.sentryTracer.spanContext.sampled = sampled
@@ -469,7 +470,7 @@ class SentryKtorClientPluginTest {
469470

470471
@Test
471472
fun `when beforeSpan throws, drops span and preserves response`(): Unit = runBlocking {
472-
val failure = IllegalStateException("callback failed")
473+
val failure = AssertionError("callback failed")
473474
val logger = mock<ILogger>()
474475
val sut =
475476
fixture.getSut(
@@ -498,6 +499,19 @@ class SentryKtorClientPluginTest {
498499
)
499500
}
500501

502+
@Test
503+
fun `fatal error from beforeSpan is not swallowed`(): Unit = runBlocking {
504+
val failure = OutOfMemoryError("callback failed")
505+
val sut = fixture.getSut(beforeSpan = { _, _ -> throw failure })
506+
507+
val thrown =
508+
assertFailsWith<OutOfMemoryError> {
509+
sut.use { sut.get(fixture.server.url("/hello").toString()) }
510+
}
511+
512+
assertThat(generateSequence<Throwable>(thrown) { it.cause }.last()).isSameInstanceAs(failure)
513+
}
514+
501515
@Test
502516
fun `beforeSpan can drop span`(): Unit = runBlocking {
503517
val sut = fixture.getSut(beforeSpan = { _, _ -> null })

0 commit comments

Comments
 (0)