diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b48c92798..9504883d42 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/sentry/src/main/java/io/sentry/Scope.java b/sentry/src/main/java/io/sentry/Scope.java index 54e8b89355..42aef2821a 100644 --- a/sentry/src/main/java/io/sentry/Scope.java +++ b/sentry/src/main/java/io/sentry/Scope.java @@ -472,12 +472,9 @@ public Queue 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; } diff --git a/sentry/src/test/java/io/sentry/ScopeTest.kt b/sentry/src/test/java/io/sentry/ScopeTest.kt index be6a22516c..69f19b6d42 100644 --- a/sentry/src/test/java/io/sentry/ScopeTest.kt +++ b/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() + 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 diff --git a/sentry/src/test/java/io/sentry/ScopesTest.kt b/sentry/src/test/java/io/sentry/ScopesTest.kt index d1cb38c649..e6db21bc79 100644 --- a/sentry/src/test/java/io/sentry/ScopesTest.kt +++ b/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? = null + sut.configureScope { breadcrumbs = it.breadcrumbs } + assertThat(breadcrumbs).isEmpty() + assertThat(breadcrumb.data).doesNotContainKey("sentry:message") } @Test