Skip to content

Commit 28af8f2

Browse files
adinauerclaude
adinauer
and
claude
committed
fix(core): [Callback Errors 1] Report callback failures
Add the callback_error discard reason and use it when a beforeSend callback throws. Preserve before_send for callbacks that intentionally return null and retain item, span, and byte accounting. Refs #6081 Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 468abd9 commit 28af8f2

6 files changed

Lines changed: 138 additions & 115 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,8 @@
122122

123123
### Fixes
124124

125+
- Fix SDK callback error handling ([#6140](https://github.com/getsentry/sentry-java/pull/6140))
126+
- Add `DiscardReason.CALLBACK_ERROR` and use it for telemetry dropped when a `beforeSend*` callback throws. `OnDiscardCallback` can now receive this value.
125127
- 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)
126128
- Keep the `EventListener` wrapped by `SentryOkHttpEventListener` per `Call` ([#6003](https://github.com/getsentry/sentry-java/pull/6003))
127129

‎sentry/api/sentry.api‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5148,6 +5148,7 @@ public final class io/sentry/clientreport/DiscardReason : java/lang/Enum {
51485148
public static final field BACKPRESSURE Lio/sentry/clientreport/DiscardReason;
51495149
public static final field BEFORE_SEND Lio/sentry/clientreport/DiscardReason;
51505150
public static final field CACHE_OVERFLOW Lio/sentry/clientreport/DiscardReason;
5151+
public static final field CALLBACK_ERROR Lio/sentry/clientreport/DiscardReason;
51515152
public static final field EVENT_PROCESSOR Lio/sentry/clientreport/DiscardReason;
51525153
public static final field NETWORK_ERROR Lio/sentry/clientreport/DiscardReason;
51535154
public static final field QUEUE_OVERFLOW Lio/sentry/clientreport/DiscardReason;

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

Lines changed: 100 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -171,9 +171,6 @@ private boolean shouldApplyScopeData(final @NotNull CheckIn event, final @NotNul
171171

172172
if (event == null) {
173173
options.getLogger().log(SentryLevel.DEBUG, "Event was dropped by beforeSend");
174-
options
175-
.getClientReportRecorder()
176-
.recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Error);
177174
}
178175
}
179176

@@ -345,9 +342,6 @@ private void finalizeTransaction(final @NotNull IScope scope, final @NotNull Hin
345342

346343
if (event == null) {
347344
options.getLogger().log(SentryLevel.DEBUG, "Event was dropped by beforeSendReplay");
348-
options
349-
.getClientReportRecorder()
350-
.recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Replay);
351345
}
352346
}
353347

@@ -530,6 +524,24 @@ private SentryEvent processEvent(
530524
return event;
531525
}
532526

527+
private void recordLostLogEvent(
528+
final @NotNull DiscardReason reason, final @NotNull SentryLogEvent event) {
529+
options.getClientReportRecorder().recordLostEvent(reason, DataCategory.LogItem);
530+
final long numberOfBytes =
531+
JsonSerializationUtils.byteSizeOf(options.getSerializer(), options.getLogger(), event);
532+
options.getClientReportRecorder().recordLostEvent(reason, DataCategory.LogByte, numberOfBytes);
533+
}
534+
535+
private void recordLostMetricsEvent(
536+
final @NotNull DiscardReason reason, final @NotNull SentryMetricsEvent event) {
537+
options.getClientReportRecorder().recordLostEvent(reason, DataCategory.TraceMetric);
538+
final long numberOfBytes =
539+
JsonSerializationUtils.byteSizeOf(options.getSerializer(), options.getLogger(), event);
540+
options
541+
.getClientReportRecorder()
542+
.recordLostEvent(reason, DataCategory.TraceMetricByte, numberOfBytes);
543+
}
544+
533545
@Nullable
534546
private SentryLogEvent processLogEvent(
535547
@NotNull SentryLogEvent event, final @NotNull List<EventProcessor> eventProcessors) {
@@ -554,16 +566,7 @@ private SentryLogEvent processLogEvent(
554566
SentryLevel.DEBUG,
555567
"Log event was dropped by a processor: %s",
556568
processor.getClass().getName());
557-
options
558-
.getClientReportRecorder()
559-
.recordLostEvent(DiscardReason.EVENT_PROCESSOR, DataCategory.LogItem);
560-
final long logEventNumberOfBytes =
561-
JsonSerializationUtils.byteSizeOf(
562-
options.getSerializer(), options.getLogger(), eventBeforeProcessor);
563-
options
564-
.getClientReportRecorder()
565-
.recordLostEvent(
566-
DiscardReason.EVENT_PROCESSOR, DataCategory.LogByte, logEventNumberOfBytes);
569+
recordLostLogEvent(DiscardReason.EVENT_PROCESSOR, eventBeforeProcessor);
567570
break;
568571
}
569572
}
@@ -596,18 +599,7 @@ private SentryMetricsEvent processMetricsEvent(
596599
SentryLevel.DEBUG,
597600
"Metrics event was dropped by a processor: %s",
598601
processor.getClass().getName());
599-
options
600-
.getClientReportRecorder()
601-
.recordLostEvent(DiscardReason.EVENT_PROCESSOR, DataCategory.TraceMetric);
602-
final long metricsEventNumberOfBytes =
603-
JsonSerializationUtils.byteSizeOf(
604-
options.getSerializer(), options.getLogger(), eventBeforeProcessor);
605-
options
606-
.getClientReportRecorder()
607-
.recordLostEvent(
608-
DiscardReason.EVENT_PROCESSOR,
609-
DataCategory.TraceMetricByte,
610-
metricsEventNumberOfBytes);
602+
recordLostMetricsEvent(DiscardReason.EVENT_PROCESSOR, eventBeforeProcessor);
611603
break;
612604
}
613605
}
@@ -1049,35 +1041,13 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint
10491041
return SentryId.EMPTY_ID;
10501042
}
10511043

1052-
final int spanCountBeforeCallback = transaction.getSpans().size();
10531044
transaction = executeBeforeSendTransaction(transaction, hint);
1054-
final int spanCountAfterCallback = transaction == null ? 0 : transaction.getSpans().size();
10551045

10561046
if (transaction == null) {
10571047
options
10581048
.getLogger()
10591049
.log(SentryLevel.DEBUG, "Transaction was dropped by beforeSendTransaction.");
1060-
options
1061-
.getClientReportRecorder()
1062-
.recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Transaction);
1063-
// If we drop a transaction, we are also dropping all its spans (+1 for the root span)
1064-
options
1065-
.getClientReportRecorder()
1066-
.recordLostEvent(
1067-
DiscardReason.BEFORE_SEND, DataCategory.Span, spanCountBeforeCallback + 1);
10681050
return SentryId.EMPTY_ID;
1069-
} else if (spanCountAfterCallback < spanCountBeforeCallback) {
1070-
// If the callback removed some spans, we report it
1071-
final int droppedSpanCount = spanCountBeforeCallback - spanCountAfterCallback;
1072-
options
1073-
.getLogger()
1074-
.log(
1075-
SentryLevel.DEBUG,
1076-
"%d spans were dropped by beforeSendTransaction.",
1077-
droppedSpanCount);
1078-
options
1079-
.getClientReportRecorder()
1080-
.recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Span, droppedSpanCount);
10811051
}
10821052

10831053
try {
@@ -1256,9 +1226,6 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint
12561226

12571227
if (event == null) {
12581228
options.getLogger().log(SentryLevel.DEBUG, "Event was dropped by beforeSend");
1259-
options
1260-
.getClientReportRecorder()
1261-
.recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Feedback);
12621229
}
12631230
}
12641231

@@ -1356,21 +1323,10 @@ public void captureLog(@Nullable SentryLogEvent logEvent, @Nullable IScope scope
13561323
}
13571324

13581325
if (logEvent != null) {
1359-
final @NotNull SentryLogEvent tmpLogEvent = logEvent;
13601326
logEvent = executeBeforeSendLog(logEvent);
13611327

13621328
if (logEvent == null) {
13631329
options.getLogger().log(SentryLevel.DEBUG, "Log Event was dropped by beforeSendLog");
1364-
options
1365-
.getClientReportRecorder()
1366-
.recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.LogItem);
1367-
final @NotNull long logEventNumberOfBytes =
1368-
JsonSerializationUtils.byteSizeOf(
1369-
options.getSerializer(), options.getLogger(), tmpLogEvent);
1370-
options
1371-
.getClientReportRecorder()
1372-
.recordLostEvent(
1373-
DiscardReason.BEFORE_SEND, DataCategory.LogByte, logEventNumberOfBytes);
13741330
return;
13751331
}
13761332

@@ -1419,23 +1375,12 @@ public void captureMetric(
14191375
}
14201376

14211377
if (metricsEvent != null) {
1422-
final @NotNull SentryMetricsEvent tmpMetricsEvent = metricsEvent;
14231378
metricsEvent = executeBeforeSendMetric(metricsEvent, hint);
14241379

14251380
if (metricsEvent == null) {
14261381
options
14271382
.getLogger()
14281383
.log(SentryLevel.DEBUG, "Metrics Event was dropped by beforeSendMetrics");
1429-
options
1430-
.getClientReportRecorder()
1431-
.recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.TraceMetric);
1432-
final long metricsEventNumberOfBytes =
1433-
JsonSerializationUtils.byteSizeOf(
1434-
options.getSerializer(), options.getLogger(), tmpMetricsEvent);
1435-
options
1436-
.getClientReportRecorder()
1437-
.recordLostEvent(
1438-
DiscardReason.BEFORE_SEND, DataCategory.TraceMetricByte, metricsEventNumberOfBytes);
14391384
return;
14401385
}
14411386

@@ -1673,11 +1618,17 @@ private void sortBreadcrumbsByDate(
16731618
.getLogger()
16741619
.log(
16751620
SentryLevel.ERROR,
1676-
"The BeforeSend callback threw an exception. It will be added as breadcrumb and continue.",
1621+
"The beforeSend callback threw an exception. Dropping event.",
16771622
e);
1678-
1679-
// drop event in case of an error in beforeSend due to PII concerns
1680-
event = null;
1623+
options
1624+
.getClientReportRecorder()
1625+
.recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Error);
1626+
return null;
1627+
}
1628+
if (event == null) {
1629+
options
1630+
.getClientReportRecorder()
1631+
.recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Error);
16811632
}
16821633
}
16831634
return event;
@@ -1688,18 +1639,48 @@ private void sortBreadcrumbsByDate(
16881639
final SentryOptions.BeforeSendTransactionCallback beforeSendTransaction =
16891640
options.getBeforeSendTransaction();
16901641
if (beforeSendTransaction != null) {
1642+
final int spanCountBeforeCallback = transaction.getSpans().size();
16911643
try (final @NotNull ISentryLifecycleToken ignored = SentryCallbackReentrancyGuard.enter()) {
16921644
transaction = beforeSendTransaction.execute(transaction, hint);
16931645
} catch (Throwable e) {
16941646
options
16951647
.getLogger()
16961648
.log(
16971649
SentryLevel.ERROR,
1698-
"The BeforeSendTransaction callback threw an exception. It will be added as breadcrumb and continue.",
1650+
"The beforeSendTransaction callback threw an exception. Dropping transaction.",
16991651
e);
1652+
options
1653+
.getClientReportRecorder()
1654+
.recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Transaction);
1655+
options
1656+
.getClientReportRecorder()
1657+
.recordLostEvent(
1658+
DiscardReason.CALLBACK_ERROR, DataCategory.Span, spanCountBeforeCallback + 1);
1659+
return null;
1660+
}
17001661

1701-
// drop transaction in case of an error in beforeSend due to PII concerns
1702-
transaction = null;
1662+
if (transaction == null) {
1663+
options
1664+
.getClientReportRecorder()
1665+
.recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Transaction);
1666+
options
1667+
.getClientReportRecorder()
1668+
.recordLostEvent(
1669+
DiscardReason.BEFORE_SEND, DataCategory.Span, spanCountBeforeCallback + 1);
1670+
} else {
1671+
final int spanCountAfterCallback = transaction.getSpans().size();
1672+
if (spanCountAfterCallback < spanCountBeforeCallback) {
1673+
final int droppedSpanCount = spanCountBeforeCallback - spanCountAfterCallback;
1674+
options
1675+
.getLogger()
1676+
.log(
1677+
SentryLevel.DEBUG,
1678+
"%d spans were dropped by beforeSendTransaction.",
1679+
droppedSpanCount);
1680+
options
1681+
.getClientReportRecorder()
1682+
.recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Span, droppedSpanCount);
1683+
}
17031684
}
17041685
}
17051686
return transaction;
@@ -1714,10 +1695,19 @@ private void sortBreadcrumbsByDate(
17141695
} catch (Throwable e) {
17151696
options
17161697
.getLogger()
1717-
.log(SentryLevel.ERROR, "The BeforeSendFeedback callback threw an exception.", e);
1718-
1719-
// drop feedback in case of an error in beforeSend due to PII concerns
1720-
event = null;
1698+
.log(
1699+
SentryLevel.ERROR,
1700+
"The beforeSendFeedback callback threw an exception. Dropping feedback.",
1701+
e);
1702+
options
1703+
.getClientReportRecorder()
1704+
.recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Feedback);
1705+
return null;
1706+
}
1707+
if (event == null) {
1708+
options
1709+
.getClientReportRecorder()
1710+
.recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Feedback);
17211711
}
17221712
}
17231713
return event;
@@ -1734,11 +1724,17 @@ private void sortBreadcrumbsByDate(
17341724
.getLogger()
17351725
.log(
17361726
SentryLevel.ERROR,
1737-
"The BeforeSendReplay callback threw an exception. It will be added as breadcrumb and continue.",
1727+
"The beforeSendReplay callback threw an exception. Dropping replay event.",
17381728
e);
1739-
1740-
// drop event in case of an error in beforeSend due to PII concerns
1741-
event = null;
1729+
options
1730+
.getClientReportRecorder()
1731+
.recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Replay);
1732+
return null;
1733+
}
1734+
if (event == null) {
1735+
options
1736+
.getClientReportRecorder()
1737+
.recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Replay);
17421738
}
17431739
}
17441740
return event;
@@ -1748,18 +1744,21 @@ private void sortBreadcrumbsByDate(
17481744
final SentryOptions.Logs.BeforeSendLogCallback beforeSendLog =
17491745
options.getLogs().getBeforeSend();
17501746
if (beforeSendLog != null) {
1747+
final @NotNull SentryLogEvent eventBeforeCallback = event;
17511748
try (final @NotNull ISentryLifecycleToken ignored = SentryCallbackReentrancyGuard.enter()) {
17521749
event = beforeSendLog.execute(event);
17531750
} catch (Throwable e) {
17541751
options
17551752
.getLogger()
17561753
.log(
17571754
SentryLevel.ERROR,
1758-
"The BeforeSendLog callback threw an exception. Dropping log event.",
1755+
"The beforeSendLog callback threw an exception. Dropping log event.",
17591756
e);
1760-
1761-
// drop event in case of an error in beforeSendLog due to PII concerns
1762-
event = null;
1757+
recordLostLogEvent(DiscardReason.CALLBACK_ERROR, eventBeforeCallback);
1758+
return null;
1759+
}
1760+
if (event == null) {
1761+
recordLostLogEvent(DiscardReason.BEFORE_SEND, eventBeforeCallback);
17631762
}
17641763
}
17651764
return event;
@@ -1770,18 +1769,21 @@ private void sortBreadcrumbsByDate(
17701769
final SentryOptions.Metrics.BeforeSendMetricCallback beforeSendMetric =
17711770
options.getMetrics().getBeforeSend();
17721771
if (beforeSendMetric != null) {
1772+
final @NotNull SentryMetricsEvent eventBeforeCallback = event;
17731773
try (final @NotNull ISentryLifecycleToken ignored = SentryCallbackReentrancyGuard.enter()) {
17741774
event = beforeSendMetric.execute(event, hint);
17751775
} catch (Throwable e) {
17761776
options
17771777
.getLogger()
17781778
.log(
17791779
SentryLevel.ERROR,
1780-
"The BeforeSendMetric callback threw an exception. Dropping metrics event.",
1780+
"The beforeSendMetric callback threw an exception. Dropping metrics event.",
17811781
e);
1782-
1783-
// drop event in case of an error in beforeSendMetric due to PII concerns
1784-
event = null;
1782+
recordLostMetricsEvent(DiscardReason.CALLBACK_ERROR, eventBeforeCallback);
1783+
return null;
1784+
}
1785+
if (event == null) {
1786+
recordLostMetricsEvent(DiscardReason.BEFORE_SEND, eventBeforeCallback);
17851787
}
17861788
}
17871789
return event;

‎sentry/src/main/java/io/sentry/clientreport/DiscardReason.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ public enum DiscardReason {
88
SEND_ERROR("send_error"),
99
SAMPLE_RATE("sample_rate"),
1010
BEFORE_SEND("before_send"),
11+
CALLBACK_ERROR("callback_error"),
1112
EVENT_PROCESSOR("event_processor"), // also for ignored exceptions
1213
BACKPRESSURE("backpressure");
1314

0 commit comments

Comments
 (0)