Skip to content

Commit fa3ca45

Browse files
adinauerclaude
andcommitted
fix(core): [Callback Errors 3] Drop failed breadcrumbs
Drop breadcrumbs when beforeBreadcrumb throws instead of storing the breadcrumb with exception details. Keep observer state unchanged and release the callback reentrancy guard for later additions. Refs #6081 Co-Authored-By: Claude <noreply@anthropic.com>
1 parent d689526 commit fa3ca45

4 files changed

Lines changed: 48 additions & 18 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@
125125
- Fix SDK callback error handling ([#6140](https://github.com/getsentry/sentry-java/pull/6140))
126126
- Add `DiscardReason.CALLBACK_ERROR` and use it for telemetry dropped when a `beforeSend*` callback throws. `OnDiscardCallback` can now receive this value.
127127
- Drop telemetry and record `callback_error` when an event processor throws instead of continuing with a potentially partially processed item.
128+
- Drop breadcrumbs when `beforeBreadcrumb` throws instead of storing exception details on the breadcrumb.
128129
- Disable URL caching when reading `META-INF/MANIFEST.MF` files during version detection so that the SDK no longer keeps jar file handles open for the life of the process ([#6124](https://github.com/getsentry/sentry-java/pull/6124)
129130
- Keep the `EventListener` wrapped by `SentryOkHttpEventListener` per `Call` ([#6003](https://github.com/getsentry/sentry-java/pull/6003))
130131

‎sentry/src/main/java/io/sentry/Scope.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -472,12 +472,9 @@ public Queue<Breadcrumb> getBreadcrumbs() {
472472
.getLogger()
473473
.log(
474474
SentryLevel.ERROR,
475-
"The BeforeBreadcrumbCallback callback threw an exception. Exception details will be added to the breadcrumb.",
475+
"The BeforeBreadcrumb callback threw an exception. Dropping breadcrumb.",
476476
e);
477-
478-
if (e.getMessage() != null) {
479-
breadcrumb.setData("sentry:message", e.getMessage());
480-
}
477+
return null;
481478
}
482479
return breadcrumb;
483480
}

‎sentry/src/test/java/io/sentry/ScopeTest.kt‎

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package io.sentry
22

3+
import com.google.common.truth.Truth.assertThat
34
import io.sentry.SentryLevel.WARNING
5+
import io.sentry.clientreport.ClientReportTestHelper.Companion.assertClientReport
46
import io.sentry.protocol.Request
57
import io.sentry.protocol.SentryId
68
import io.sentry.protocol.User
@@ -334,16 +336,44 @@ class ScopeTest {
334336
}
335337

336338
@Test
337-
fun `when adding breadcrumb, executeBreadcrumb will be executed and throw, but breadcrumb will be added`() {
338-
val exception = Exception("test")
339+
fun `when beforeBreadcrumb throws, breadcrumb is dropped without notifying observers`() {
340+
val observer = mock<IScopeObserver>()
341+
val options =
342+
SentryOptions().apply {
343+
setBeforeBreadcrumb { _, _ -> throw Exception("test") }
344+
addScopeObserver(observer)
345+
}
339346

340-
val options = SentryOptions().apply { setBeforeBreadcrumb { _, _ -> throw exception } }
347+
val scope = Scope(options)
348+
val breadcrumb = Breadcrumb()
349+
scope.addBreadcrumb(breadcrumb)
350+
351+
assertThat(scope.breadcrumbs).isEmpty()
352+
assertThat(breadcrumb.data).doesNotContainKey("sentry:message")
353+
verifyNoInteractions(observer)
354+
assertClientReport(options.clientReportRecorder, emptyList())
355+
}
356+
357+
@Test
358+
fun `when beforeBreadcrumb throws, later breadcrumbs can still be added`() {
359+
var invocationCount = 0
360+
val options =
361+
SentryOptions().apply {
362+
setBeforeBreadcrumb { breadcrumb, _ ->
363+
invocationCount++
364+
if (invocationCount == 1) {
365+
throw Exception("test")
366+
}
367+
breadcrumb
368+
}
369+
}
341370

342371
val scope = Scope(options)
343-
val actual = Breadcrumb()
344-
scope.addBreadcrumb(actual)
372+
scope.addBreadcrumb(Breadcrumb("dropped"))
373+
scope.addBreadcrumb(Breadcrumb("kept"))
345374

346-
assertEquals("test", actual.data["sentry:message"])
375+
assertThat(invocationCount).isEqualTo(2)
376+
assertThat(scope.breadcrumbs.single().message).isEqualTo("kept")
347377
}
348378

349379
@Test

‎sentry/src/test/java/io/sentry/ScopesTest.kt‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package io.sentry
22

3+
import com.google.common.truth.Truth.assertThat
34
import io.sentry.backpressure.IBackpressureMonitor
45
import io.sentry.cache.EnvelopeCache
56
import io.sentry.clientreport.ClientReportTestHelper.Companion.assertClientReport
@@ -236,22 +237,23 @@ class ScopesTest {
236237
}
237238

238239
@Test
239-
fun `when beforeSend throws an exception, breadcrumb adds an entry to the data field with exception message`() {
240-
val exception = Exception("test")
241-
240+
fun `when beforeBreadcrumb throws an exception, breadcrumb is dropped`() {
242241
val options = SentryOptions()
243242
options.cacheDirPath = file.absolutePath
244243
options.beforeBreadcrumb = SentryOptions.BeforeBreadcrumbCallback { _: Breadcrumb, _: Any? ->
245-
throw exception
244+
throw Exception("test")
246245
}
247246
options.dsn = "https://key@sentry.io/proj"
248247
options.setSerializer(mock())
249248
val sut = createScopes(options)
250249

251-
val actual = Breadcrumb()
252-
sut.addBreadcrumb(actual)
250+
val breadcrumb = Breadcrumb()
251+
sut.addBreadcrumb(breadcrumb)
253252

254-
assertEquals("test", actual.data["sentry:message"])
253+
var breadcrumbs: Queue<Breadcrumb>? = null
254+
sut.configureScope { breadcrumbs = it.breadcrumbs }
255+
assertThat(breadcrumbs).isEmpty()
256+
assertThat(breadcrumb.data).doesNotContainKey("sentry:message")
255257
}
256258

257259
@Test

0 commit comments

Comments
 (0)