fix(core): [Callback Errors 3] Drop failed breadcrumbs

CHANGELOG.md

@@ -125,6 +125,7 @@
 - Fix SDK callback error handling ([#6140](https://github.com/getsentry/sentry-java/pull/6140))
   - Add `DiscardReason.CALLBACK_ERROR` and use it for telemetry dropped when a `beforeSend*` callback throws. `OnDiscardCallback` can now receive this value.
   - Drop telemetry and record `callback_error` when an event processor throws instead of continuing with a potentially partially processed item.
+  - Drop breadcrumbs when `beforeBreadcrumb` throws instead of storing exception details on the breadcrumb.
 - 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)
 - Keep the `EventListener` wrapped by `SentryOkHttpEventListener` per `Call` ([#6003](https://github.com/getsentry/sentry-java/pull/6003))
 

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

@@ -472,12 +472,9 @@ public Queue<Breadcrumb> getBreadcrumbs() {
           .getLogger()
           .log(
               SentryLevel.ERROR,
-              "The BeforeBreadcrumbCallback callback threw an exception. Exception details will be added to the breadcrumb.",
+              "The BeforeBreadcrumb callback threw an exception. Dropping breadcrumb.",
               e);
-
-      if (e.getMessage() != null) {
-        breadcrumb.setData("sentry:message", e.getMessage());
-      }
+      return null;
     }
     return breadcrumb;
   }

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

@@ -1,6 +1,8 @@
 package io.sentry
 
+import com.google.common.truth.Truth.assertThat
 import io.sentry.SentryLevel.WARNING
+import io.sentry.clientreport.ClientReportTestHelper.Companion.assertClientReport
 import io.sentry.protocol.Request
 import io.sentry.protocol.SentryId
 import io.sentry.protocol.User
@@ -334,16 +336,44 @@ class ScopeTest {
   }
 
   @Test
-  fun `when adding breadcrumb, executeBreadcrumb will be executed and throw, but breadcrumb will be added`() {
-    val exception = Exception("test")
+  fun `when beforeBreadcrumb throws, breadcrumb is dropped without notifying observers`() {
+    val observer = mock<IScopeObserver>()
+    val options =
+      SentryOptions().apply {
+        setBeforeBreadcrumb { _, _ -> throw Exception("test") }
+        addScopeObserver(observer)
+      }
 
-    val options = SentryOptions().apply { setBeforeBreadcrumb { _, _ -> throw exception } }
+    val scope = Scope(options)
+    val breadcrumb = Breadcrumb()
+    scope.addBreadcrumb(breadcrumb)
+
+    assertThat(scope.breadcrumbs).isEmpty()
+    assertThat(breadcrumb.data).doesNotContainKey("sentry:message")
+    verifyNoInteractions(observer)
+    assertClientReport(options.clientReportRecorder, emptyList())
+  }
+
+  @Test
+  fun `when beforeBreadcrumb throws, later breadcrumbs can still be added`() {
+    var invocationCount = 0
+    val options =
+      SentryOptions().apply {
+        setBeforeBreadcrumb { breadcrumb, _ ->
+          invocationCount++
+          if (invocationCount == 1) {
+            throw Exception("test")
+          }
+          breadcrumb
+        }
+      }
 
     val scope = Scope(options)
-    val actual = Breadcrumb()
-    scope.addBreadcrumb(actual)
+    scope.addBreadcrumb(Breadcrumb("dropped"))
+    scope.addBreadcrumb(Breadcrumb("kept"))
 
-    assertEquals("test", actual.data["sentry:message"])
+    assertThat(invocationCount).isEqualTo(2)
+    assertThat(scope.breadcrumbs.single().message).isEqualTo("kept")
   }
 
   @Test

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

@@ -1,5 +1,6 @@
 package io.sentry
 
+import com.google.common.truth.Truth.assertThat
 import io.sentry.backpressure.IBackpressureMonitor
 import io.sentry.cache.EnvelopeCache
 import io.sentry.clientreport.ClientReportTestHelper.Companion.assertClientReport
@@ -236,22 +237,23 @@ class ScopesTest {
   }
 
   @Test
-  fun `when beforeSend throws an exception, breadcrumb adds an entry to the data field with exception message`() {
-    val exception = Exception("test")
-
+  fun `when beforeBreadcrumb throws an exception, breadcrumb is dropped`() {
     val options = SentryOptions()
     options.cacheDirPath = file.absolutePath
     options.beforeBreadcrumb = SentryOptions.BeforeBreadcrumbCallback { _: Breadcrumb, _: Any? ->
-      throw exception
+      throw Exception("test")
     }
     options.dsn = "https://key@sentry.io/proj"
     options.setSerializer(mock())
     val sut = createScopes(options)
 
-    val actual = Breadcrumb()
-    sut.addBreadcrumb(actual)
+    val breadcrumb = Breadcrumb()
+    sut.addBreadcrumb(breadcrumb)
 
-    assertEquals("test", actual.data["sentry:message"])
+    var breadcrumbs: Queue<Breadcrumb>? = null
+    sut.configureScope { breadcrumbs = it.breadcrumbs }
+    assertThat(breadcrumbs).isEmpty()
+    assertThat(breadcrumb.data).doesNotContainKey("sentry:message")
   }
 
   @Test