fix(core): [Callback Errors 8] Report dropped profiles

CHANGELOG.md

@@ -124,6 +124,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.
+  - Report attached profiles dropped by transaction callback errors as `callback_error` in client reports and `OnDiscardCallback` ([#6166](https://github.com/getsentry/sentry-java/pull/6166))
   - 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))

sentry/src/main/java/io/sentry/SentryClient.java

@@ -624,7 +624,8 @@ private SentryMetricsEvent processMetricsEvent(
   private @Nullable SentryTransaction processTransaction(
       @NotNull SentryTransaction transaction,
       final @NotNull Hint hint,
-      final @NotNull List<EventProcessor> eventProcessors) {
+      final @NotNull List<EventProcessor> eventProcessors,
+      final boolean hasProfile) {
     for (final EventProcessor processor : eventProcessors) {
       final int spanCountBeforeProcessor = transaction.getSpans().size();
       try {
@@ -645,6 +646,11 @@ private SentryMetricsEvent processMetricsEvent(
               .getClientReportRecorder()
               .recordLostEvent(
                   DiscardReason.CALLBACK_ERROR, DataCategory.Span, spanCountBeforeProcessor + 1);
+          if (hasProfile) {
+            options
+                .getClientReportRecorder()
+                .recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Profile);
+          }
           return null;
         }
       }
@@ -1061,7 +1067,9 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint
       transaction = applyScope(transaction, scope, HintUtils.hasType(hint, Cached.class));
 
       if (transaction != null && scope != null) {
-        transaction = processTransaction(transaction, hint, scope.getEventProcessors());
+        transaction =
+            processTransaction(
+                transaction, hint, scope.getEventProcessors(), profilingTraceData != null);
       }
 
       if (transaction == null) {
@@ -1070,15 +1078,17 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint
     }
 
     if (transaction != null) {
-      transaction = processTransaction(transaction, hint, options.getEventProcessors());
+      transaction =
+          processTransaction(
+              transaction, hint, options.getEventProcessors(), profilingTraceData != null);
     }
 
     if (transaction == null) {
       options.getLogger().log(SentryLevel.DEBUG, "Transaction was dropped by Event processors.");
       return SentryId.EMPTY_ID;
     }
 
-    transaction = executeBeforeSendTransaction(transaction, hint);
+    transaction = executeBeforeSendTransaction(transaction, hint, profilingTraceData != null);
 
     if (transaction == null) {
       options
@@ -1672,7 +1682,7 @@ private void sortBreadcrumbsByDate(
   }
 
   private @Nullable SentryTransaction executeBeforeSendTransaction(
-      @NotNull SentryTransaction transaction, final @NotNull Hint hint) {
+      @NotNull SentryTransaction transaction, final @NotNull Hint hint, final boolean hasProfile) {
     final SentryOptions.BeforeSendTransactionCallback beforeSendTransaction =
         options.getBeforeSendTransaction();
     if (beforeSendTransaction != null) {
@@ -1693,6 +1703,11 @@ private void sortBreadcrumbsByDate(
             .getClientReportRecorder()
             .recordLostEvent(
                 DiscardReason.CALLBACK_ERROR, DataCategory.Span, spanCountBeforeCallback + 1);
+        if (hasProfile) {
+          options
+              .getClientReportRecorder()
+              .recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Profile);
+        }
         return null;
       }
 

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

@@ -100,6 +100,35 @@ class SentryClientInternalEventProcessorTest(private val onScope: Boolean) {
     assertFailureLoggedWithoutLoss("transaction")
   }
 
+  @Test
+  fun `SDK transaction processor failure keeps attached profile without reporting loss`() {
+    val transaction = SentryTransaction(fixture.sentryTracer)
+    whenever(processor.process(any<SentryTransaction>(), any())).thenThrow(failure)
+    whenever(nextProcessor.process(any<SentryTransaction>(), any())).thenAnswer { it.arguments[0] }
+
+    val id =
+      fixture
+        .getSut()
+        .captureTransaction(
+          transaction,
+          fixture.sentryTracer.traceContext(),
+          scope,
+          null,
+          fixture.profilingTraceData,
+        )
+
+    assertThat(id).isEqualTo(transaction.eventId)
+    verify(fixture.transport)
+      .send(
+        check {
+          assertThat(it.items.map { item -> item.header.type })
+            .containsExactly(SentryItemType.Transaction, SentryItemType.Profile)
+        },
+        anyOrNull(),
+      )
+    assertFailureLoggedWithoutLoss("transaction")
+  }
+
   @Test
   fun `SDK feedback processor failure keeps feedback and runs remaining callbacks`() {
     val feedback = Feedback("message")

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

@@ -0,0 +1,84 @@
+package io.sentry
+
+import com.google.common.truth.Truth.assertThat
+import io.sentry.clientreport.ClientReportTestHelper.Companion.assertClientReport
+import io.sentry.clientreport.DiscardReason
+import io.sentry.clientreport.DiscardedEvent
+import io.sentry.protocol.SentryId
+import io.sentry.protocol.SentryTransaction
+import kotlin.test.Test
+import org.junit.runner.RunWith
+import org.junit.runners.Parameterized
+import org.mockito.kotlin.any
+import org.mockito.kotlin.anyOrNull
+import org.mockito.kotlin.mock
+import org.mockito.kotlin.never
+import org.mockito.kotlin.verify
+import org.mockito.kotlin.verifyNoMoreInteractions
+import org.mockito.kotlin.whenever
+
+@RunWith(Parameterized::class)
+class SentryClientTransactionProfileTest(
+  private val callback: String,
+  private val hasProfile: Boolean,
+) {
+  companion object {
+    @JvmStatic
+    @Parameterized.Parameters(name = "callback={0}, hasProfile={1}")
+    fun data(): List<Array<Any>> =
+      listOf("scope processor", "options processor", "beforeSendTransaction").flatMap { callback ->
+        listOf(false, true).map { hasProfile -> arrayOf<Any>(callback, hasProfile) }
+      }
+  }
+
+  @Test
+  fun `transaction callback failure reports attached profile exactly once`() {
+    val fixture = SentryClientTest.Fixture()
+    val options = fixture.sentryOptions
+    options.eventProcessors.clear()
+    val scope = Scope(options)
+    val onDiscard = mock<SentryOptions.OnDiscardCallback>()
+    options.onDiscard = onDiscard
+    val failure = IllegalStateException("callback failed")
+    if (callback == "beforeSendTransaction") {
+      options.setBeforeSendTransaction { _, _ -> throw failure }
+    } else {
+      val processor = mock<EventProcessor>()
+      whenever(processor.process(any<SentryTransaction>(), any())).thenThrow(failure)
+      if (callback == "scope processor") {
+        scope.addEventProcessor(processor)
+      } else {
+        options.addEventProcessor(processor)
+      }
+    }
+
+    val id =
+      fixture
+        .getSut()
+        .captureTransaction(
+          SentryTransaction(fixture.sentryTracer),
+          fixture.sentryTracer.traceContext(),
+          scope,
+          null,
+          if (hasProfile) fixture.profilingTraceData else null,
+        )
+
+    assertThat(id).isEqualTo(SentryId.EMPTY_ID)
+    verify(fixture.transport, never()).send(any(), anyOrNull())
+    val expected =
+      mutableListOf(
+        DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Transaction.category, 1),
+        DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Span.category, 2),
+      )
+    if (hasProfile) {
+      expected.add(
+        DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Profile.category, 1)
+      )
+      verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Profile, 1)
+    }
+    assertClientReport(options.clientReportRecorder, expected)
+    verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Transaction, 1)
+    verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Span, 2)
+    verifyNoMoreInteractions(onDiscard)
+  }
+}