diff --git a/CHANGELOG.md b/CHANGELOG.md index 4948cb46f7..1f51017b6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,7 @@ - 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 a customer event processor throws instead of continuing with a potentially partially processed item. SDK-owned processor failures are logged and processing continues without a `callback_error` client report. - Drop breadcrumbs when `beforeBreadcrumb` throws instead of storing exception details on the breadcrumb. + - Skip replay capture when `beforeErrorSampling` throws, while still sending the error event ([#6165](https://github.com/getsentry/sentry-java/pull/6165)) - When `tracesSampler` throws, drop the transaction and record `callback_error` instead of inheriting the parent sampling decision or falling back to `tracesSampleRate` ([#6163](https://github.com/getsentry/sentry-java/pull/6163)) - When `profilesSampler` throws, disable profiling instead of falling back to `profilesSampleRate` or inheriting the parent's profiling decision. Trace sampling is unchanged ([#6164](https://github.com/getsentry/sentry-java/pull/6164)) - 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) diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 2fdbff39e0..b113323fef 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -233,9 +233,14 @@ private boolean shouldApplyScopeData(final @NotNull CheckIn event, final @NotNul .getLogger() .log( SentryLevel.ERROR, - "The beforeErrorSampling callback threw an exception. Proceeding with replay capture.", + "The beforeErrorSampling callback threw an exception. Skipping replay capture.", e); - shouldCaptureReplay = true; + if (!SentryId.EMPTY_ID.equals(options.getReplayController().getReplayId())) { + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Replay); + } + shouldCaptureReplay = false; } } if (shouldCaptureReplay) { diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index d7d484ce2c..e3bc59c549 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -3973,24 +3973,78 @@ class SentryClientTest { } @Test - fun `beforeErrorSampling throwing exception proceeds with captureReplay`() { - var called = false - fixture.sentryOptions.setReplayController( - object : ReplayController by NoOpReplayController.getInstance() { - override fun captureReplay(isTerminating: Boolean?): SentryId { - called = true - return SentryId.EMPTY_ID + fun `beforeErrorSampling throwing exception skips replay but still sends errors and crashes`() { + val replayController = mock() + whenever(replayController.replayId).thenReturn(SentryId()) + whenever(replayController.captureReplay(anyOrNull())).thenReturn(SentryId.EMPTY_ID) + fixture.sentryOptions.setReplayController(replayController) + val logger = mock() + fixture.sentryOptions.setLogger(logger) + val exception = RuntimeException("test") + fixture.sentryOptions.sessionReplay.beforeErrorSampling = + SentryReplayOptions.BeforeErrorSamplingCallback { _, _ -> + throw exception + } + val sut = fixture.getSut() + val events = + listOf(true, false).map { handled -> + SentryEvent().apply { + exceptions = + listOf( + SentryException().apply { mechanism = Mechanism().apply { isHandled = handled } } + ) } } + + events.forEach { event -> + assertThat(sut.captureEvent(event)).isEqualTo(event.eventId) + } + + verify(replayController, never()).captureReplay(anyOrNull()) + val envelopes = argumentCaptor() + verify(fixture.transport, times(2)).send(envelopes.capture(), anyOrNull()) + assertThat(envelopes.allValues.map { getEventFromData(it.items.first().data).eventId }) + .containsExactlyElementsIn(events.map { it.eventId }) + .inOrder() + verify(logger, times(2)) + .log( + SentryLevel.ERROR, + "The beforeErrorSampling callback threw an exception. Skipping replay capture.", + exception, + ) + assertClientReport( + fixture.sentryOptions.clientReportRecorder, + listOf(DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Replay.category, 2)), ) + } + + @Test + fun `beforeErrorSampling throwing exception does not prevent replay capture for subsequent events`() { + val replayController = mock() + whenever(replayController.replayId).thenReturn(SentryId.EMPTY_ID) + whenever(replayController.captureReplay(anyOrNull())).thenReturn(SentryId.EMPTY_ID) + fixture.sentryOptions.setReplayController(replayController) + var invocations = 0 fixture.sentryOptions.sessionReplay.beforeErrorSampling = SentryReplayOptions.BeforeErrorSamplingCallback { _, _ -> - throw RuntimeException("test") + invocations++ + if (invocations == 1) { + throw RuntimeException("test") + } + true } val sut = fixture.getSut() sut.captureEvent(SentryEvent().apply { exceptions = listOf(SentryException()) }) - assertTrue(called) + verify(replayController, never()).captureReplay(anyOrNull()) + + val event = SentryEvent().apply { exceptions = listOf(SentryException()) } + assertThat(sut.captureEvent(event)).isEqualTo(event.eventId) + + assertThat(invocations).isEqualTo(2) + verify(replayController).captureReplay(false) + verify(fixture.transport, times(2)).send(any(), anyOrNull()) + assertClientReport(fixture.sentryOptions.clientReportRecorder, emptyList()) } @Test