diff --git a/src/Sentry/Internal/SentryEventHelper.cs b/src/Sentry/Internal/SentryEventHelper.cs index 561ba6aea9..cc12cbb40c 100644 --- a/src/Sentry/Internal/SentryEventHelper.cs +++ b/src/Sentry/Internal/SentryEventHelper.cs @@ -38,9 +38,6 @@ internal static class SentryEventHelper return processedEvent; } -#if NET6_0_OR_GREATER - [UnconditionalSuppressMessage("Trimming", "IL2026: RequiresUnreferencedCode", Justification = AotHelper.AvoidAtRuntime)] -#endif public static SentryEvent? DoBeforeSend(SentryEvent? @event, SentryHint hint, SentryOptions options) { if (@event is null || options.BeforeSendInternal is null) @@ -60,26 +57,9 @@ internal static class SentryEventHelper } catch (Exception e) { - if (!AotHelper.IsTrimmed) - { - // Attempt to demystify exceptions before adding them as breadcrumbs. - e.Demystify(); - } - - options.LogError(e, "The BeforeSend callback threw an exception. It will be added as breadcrumb and continue."); - var data = new Dictionary - { - {"message", e.Message} - }; - if (e.StackTrace is not null) - { - data.Add("stackTrace", e.StackTrace); - } - @event?.AddBreadcrumb( - "BeforeSend callback failed.", - category: "SentryClient", - data: data, - level: BreadcrumbLevel.Error); + options.ClientReportRecorder.RecordDiscardedEvent(DiscardReason.CallbackError, DataCategory.Error); + options.LogError(e, "The BeforeSend callback threw an exception. The event will be dropped."); + return null; } return @event; diff --git a/src/Sentry/Platforms/Android/SentrySdk.cs b/src/Sentry/Platforms/Android/SentrySdk.cs index ab07387924..5e2fc52ce5 100644 --- a/src/Sentry/Platforms/Android/SentrySdk.cs +++ b/src/Sentry/Platforms/Android/SentrySdk.cs @@ -238,9 +238,21 @@ private static void InitSentryAndroidSdk(SentryOptions options) } // Call the user defined BeforeSend callback, if it's defined - otherwise return the event as-is - return (options.Native.EnableBeforeSend && options.BeforeSendInternal is { } beforeSend) - ? beforeSend(evt, hint) - : evt; + if (!options.Native.EnableBeforeSend || options.BeforeSendInternal is not { } beforeSend) + { + return evt; + } + + try + { + return beforeSend(evt, hint); + } + catch (Exception e) + { + // Dropping the event is left to the Java SDK, which also records the client report for it. + options.LogError(e, "Android BeforeSend callback failed."); + return null; + } }; } diff --git a/src/Sentry/SentryClient.cs b/src/Sentry/SentryClient.cs index 95adc72db0..d49ac28b5b 100644 --- a/src/Sentry/SentryClient.cs +++ b/src/Sentry/SentryClient.cs @@ -233,12 +233,9 @@ public void CaptureTransaction(SentryTransaction transaction, Scope? scope, Sent } } - processedTransaction = BeforeSendTransaction(processedTransaction, hint); + processedTransaction = BeforeSendTransaction(processedTransaction, hint, spanCount); if (processedTransaction is null) // Rejected transaction { - _options.ClientReportRecorder.RecordDiscardedEvent(DiscardReason.BeforeSend, DataCategory.Transaction); - _options.ClientReportRecorder.RecordDiscardedEvent(DiscardReason.BeforeSend, DataCategory.Span, spanCount); - _options.LogInfo("Transaction dropped by BeforeSendTransaction callback."); return; } @@ -253,10 +250,7 @@ public void CaptureTransaction(SentryTransaction transaction, Scope? scope, Sent CaptureEnvelope(Envelope.FromTransaction(processedTransaction, _options.DiagnosticLogger, attachments)); } -#if NET6_0_OR_GREATER - [UnconditionalSuppressMessage("Trimming", "IL2026: RequiresUnreferencedCode", Justification = AotHelper.AvoidAtRuntime)] -#endif - private SentryTransaction? BeforeSendTransaction(SentryTransaction transaction, SentryHint hint) + private SentryTransaction? BeforeSendTransaction(SentryTransaction transaction, SentryHint hint, int spanCount) { if (_options.BeforeSendTransactionInternal is null) { @@ -267,36 +261,23 @@ public void CaptureTransaction(SentryTransaction transaction, Scope? scope, Sent try { - return _options.BeforeSendTransactionInternal?.Invoke(transaction, hint); - } - catch (Exception e) - { - if (!AotHelper.IsTrimmed) + var processedTransaction = _options.BeforeSendTransactionInternal.Invoke(transaction, hint); + if (processedTransaction is null) // Rejected transaction { - // Attempt to demystify exceptions before adding them as breadcrumbs. - e.Demystify(); + _options.ClientReportRecorder.RecordDiscardedEvent(DiscardReason.BeforeSend, DataCategory.Transaction); + _options.ClientReportRecorder.RecordDiscardedEvent(DiscardReason.BeforeSend, DataCategory.Span, spanCount); + _options.LogInfo("Transaction dropped by BeforeSendTransaction callback."); } - _options.LogError(e, "The BeforeSendTransaction callback threw an exception. It will be added as breadcrumb and continue."); - - var data = new Dictionary - { - {"message", e.Message} - }; - - if (e.StackTrace is not null) - { - data.Add("stackTrace", e.StackTrace); - } - - transaction.AddBreadcrumb( - message: "BeforeSendTransaction callback failed.", - category: "SentryClient", - data: data, - level: BreadcrumbLevel.Error); + return processedTransaction; + } + catch (Exception e) + { + _options.ClientReportRecorder.RecordDiscardedEvent(DiscardReason.CallbackError, DataCategory.Transaction); + _options.ClientReportRecorder.RecordDiscardedEvent(DiscardReason.CallbackError, DataCategory.Span, spanCount); + _options.LogError(e, "The BeforeSendTransaction callback threw an exception. The transaction will be dropped."); + return null; } - - return transaction; } /// diff --git a/test/Sentry.Tests/Platforms/Android/SentrySdkTests.cs b/test/Sentry.Tests/Platforms/Android/SentrySdkTests.cs index 96bde8e62b..6dfd12dc9b 100644 --- a/test/Sentry.Tests/Platforms/Android/SentrySdkTests.cs +++ b/test/Sentry.Tests/Platforms/Android/SentrySdkTests.cs @@ -112,5 +112,28 @@ public void BeforeSendWrapper_NativeBeforeSendDisabled_ReturnsEvent() beforeSend.DidNotReceive().Invoke(Arg.Any(), Arg.Any()); result.Should().Be(evt); } + + [Fact] + public void BeforeSendWrapper_BeforeSendThrows_ReturnsNullAndLogsError() + { + // Arrange + var exception = new InvalidOperationException("callback failed"); + var logger = new InMemoryDiagnosticLogger(); + var options = new SentryOptions { Debug = true, DiagnosticLogger = logger }; + options.Native.EnableBeforeSend = true; + options.SetBeforeSend((SentryEvent _, SentryHint _) => throw exception); + var evt = new SentryEvent(); + var hint = new SentryHint(); + + // Act + var result = SentrySdk.BeforeSendWrapper(options).Invoke(evt, hint); + + // Assert + result.Should().BeNull(); + logger.Entries.Should().ContainSingle(entry => + entry.Level == SentryLevel.Error && + entry.Exception == exception && + entry.Message == "Android BeforeSend callback failed."); + } } #endif diff --git a/test/Sentry.Tests/SentryClientTests.CaptureEvent_BeforeEventThrows_ErrorToEventBreadcrumb.Net4_8.verified.txt b/test/Sentry.Tests/SentryClientTests.CaptureEvent_BeforeEventThrows_ErrorToEventBreadcrumb.Net4_8.verified.txt deleted file mode 100644 index 6ce33e1e01..0000000000 --- a/test/Sentry.Tests/SentryClientTests.CaptureEvent_BeforeEventThrows_ErrorToEventBreadcrumb.Net4_8.verified.txt +++ /dev/null @@ -1,14 +0,0 @@ -[ - { - Timestamp: DateTimeOffset_1, - Message: BeforeSend callback failed., - Data: { - message: Exception message!, - stackTrace: -at Task Sentry.Tests.SentryClientTests.CaptureEvent_BeforeEventThrows_ErrorToEventBreadcrumb() -at SentryEvent Sentry.SentryClient.BeforeSend(...) - }, - Category: SentryClient, - Level: error - } -] \ No newline at end of file diff --git a/test/Sentry.Tests/SentryClientTests.CaptureEvent_BeforeEventThrows_ErrorToEventBreadcrumb.verified.txt b/test/Sentry.Tests/SentryClientTests.CaptureEvent_BeforeEventThrows_ErrorToEventBreadcrumb.verified.txt deleted file mode 100644 index 97e165f68b..0000000000 --- a/test/Sentry.Tests/SentryClientTests.CaptureEvent_BeforeEventThrows_ErrorToEventBreadcrumb.verified.txt +++ /dev/null @@ -1,14 +0,0 @@ -[ - { - Timestamp: DateTimeOffset_1, - Message: BeforeSend callback failed., - Data: { - message: Exception message!, - stackTrace: -at Task Sentry.Tests.SentryClientTests.CaptureEvent_BeforeEventThrows_ErrorToEventBreadcrumb() -at SentryEvent Sentry.Internal.SentryEventHelper.DoBeforeSend(...) - }, - Category: SentryClient, - Level: error - } -] \ No newline at end of file diff --git a/test/Sentry.Tests/SentryClientTests.CaptureTransaction_BeforeSendTransactionThrows_ErrorToEventBreadcrumb.verified.txt b/test/Sentry.Tests/SentryClientTests.CaptureTransaction_BeforeSendTransactionThrows_ErrorToEventBreadcrumb.verified.txt deleted file mode 100644 index c6f3a16345..0000000000 --- a/test/Sentry.Tests/SentryClientTests.CaptureTransaction_BeforeSendTransactionThrows_ErrorToEventBreadcrumb.verified.txt +++ /dev/null @@ -1,14 +0,0 @@ -[ - { - Timestamp: DateTimeOffset_1, - Message: BeforeSendTransaction callback failed., - Data: { - message: Exception message!, - stackTrace: -at Task Sentry.Tests.SentryClientTests.CaptureTransaction_BeforeSendTransactionThrows_ErrorToEventBreadcrumb() -at SentryTransaction Sentry.SentryClient.BeforeSendTransaction(...) - }, - Category: SentryClient, - Level: error - } -] \ No newline at end of file diff --git a/test/Sentry.Tests/SentryClientTests.cs b/test/Sentry.Tests/SentryClientTests.cs index 6e50e735c2..1eb558c1d4 100644 --- a/test/Sentry.Tests/SentryClientTests.cs +++ b/test/Sentry.Tests/SentryClientTests.cs @@ -372,6 +372,36 @@ public void CaptureEvent_BeforeEvent_RejectEvent_RecordsDiscard() .RecordDiscardedEvent(DiscardReason.BeforeSend, DataCategory.Error); } + [Fact] + public void CaptureEvent_BeforeSendThrows_DropsEventAndRecordsDiscard() + { + var exception = new InvalidOperationException("callback failed"); + _fixture.SentryOptions.SetBeforeSend((_, _) => throw exception); + _fixture.SentryOptions.AddDiagnosticLoggerSubstitute(); + var @event = new SentryEvent(); + + var sut = _fixture.GetSut(); + var id = sut.CaptureEvent(@event); + + id.Should().Be(SentryId.Empty); + _ = _fixture.BackgroundWorker.DidNotReceive().EnqueueEnvelope(Arg.Any()); + _fixture.ClientReportRecorder.Received(1) + .RecordDiscardedEvent(DiscardReason.CallbackError, DataCategory.Error); + _fixture.SentryOptions.ReceivedLogError(exception, "The BeforeSend callback threw an exception. The event will be dropped."); + } + + [Fact] + public void CaptureEvent_BeforeSendThrows_DoesNotAttachFailureToEvent() + { + _fixture.SentryOptions.SetBeforeSend((_, _) => throw new InvalidOperationException("callback failed")); + var @event = new SentryEvent(); + + var sut = _fixture.GetSut(); + _ = sut.CaptureEvent(@event); + + @event.Breadcrumbs.Should().BeEmpty(); + } + [Fact] public void CaptureEvent_EventProcessor_RejectEvent_RecordsDiscard() { @@ -1773,6 +1803,34 @@ public void CaptureTransaction_TransactionProcessorThrows_DropsTransactionAndRec _fixture.ClientReportRecorder.Received(1).RecordDiscardedEvent(reason, DataCategory.Span, expectedDroppedSpanCount); } + [Fact] + public void CaptureTransaction_BeforeSendTransactionThrows_DropsTransactionAndRecordsDiscard() + { + // Arrange + var exception = new InvalidOperationException("callback failed"); + _fixture.SentryOptions.SetBeforeSendTransaction((_, _) => throw exception); + _fixture.SentryOptions.AddDiagnosticLoggerSubstitute(); + + var hub = Substitute.For(); + var tracer = new TransactionTracer(hub, "test name", "test operation"); + tracer.StartChild("span1"); + tracer.StartChild("span2"); + tracer.EndTimestamp = DateTimeOffset.Now; // finished + var transaction = new SentryTransaction(tracer) { IsSampled = true }; + + // Act + _fixture.GetSut().CaptureTransaction(transaction); + + // Assert + _ = _fixture.BackgroundWorker.DidNotReceive().EnqueueEnvelope(Arg.Any()); + _fixture.ClientReportRecorder.Received(1) + .RecordDiscardedEvent(DiscardReason.CallbackError, DataCategory.Transaction); + _fixture.ClientReportRecorder.Received(1) + .RecordDiscardedEvent(DiscardReason.CallbackError, DataCategory.Span, transaction.Spans.Count + 1); + _fixture.SentryOptions.ReceivedLogError(exception, "The BeforeSendTransaction callback threw an exception. The transaction will be dropped."); + transaction.Breadcrumbs.Should().BeEmpty(); + } + [Fact] public void CaptureTransaction_BeforeSendTransaction_GetsHint() { diff --git a/test/Sentry.Tests/SentryClientTests.verify.cs b/test/Sentry.Tests/SentryClientTests.verify.cs deleted file mode 100644 index 329f8065d0..0000000000 --- a/test/Sentry.Tests/SentryClientTests.verify.cs +++ /dev/null @@ -1,35 +0,0 @@ -namespace Sentry.Tests; - -public partial class SentryClientTests -{ - [Fact] - public Task CaptureEvent_BeforeEventThrows_ErrorToEventBreadcrumb() - { - var error = new Exception("Exception message!"); - _fixture.SentryOptions.SetBeforeSend((_, _) => throw error); - - var @event = new SentryEvent(); - - var sut = _fixture.GetSut(); - _ = sut.CaptureEvent(@event); - - return Verify(@event.Breadcrumbs); - } - - [Fact] - public Task CaptureTransaction_BeforeSendTransactionThrows_ErrorToEventBreadcrumb() - { - var error = new Exception("Exception message!"); - _fixture.SentryOptions.SetBeforeSendTransaction((_, _) => throw error); - - var transaction = new SentryTransaction("name", "operation") - { - IsSampled = true - }; - - var sut = _fixture.GetSut(); - sut.CaptureTransaction(transaction); - - return Verify(transaction.Breadcrumbs); - } -} diff --git a/test/Sentry.Tests/SentrySdkTests.cs b/test/Sentry.Tests/SentrySdkTests.cs index c0dde5651f..402a644492 100644 --- a/test/Sentry.Tests/SentrySdkTests.cs +++ b/test/Sentry.Tests/SentrySdkTests.cs @@ -1462,6 +1462,44 @@ public void ProcessOnBeforeSend_EventProcessorsInvoked() // Assert eventProcessor.Invoked.Should().BeTrue(); } + + [Fact] + public void ProcessOnBeforeSend_BeforeSendThrows_DropsNativeEvent() + { + // Arrange + var exception = new InvalidOperationException("callback failed"); + var logger = new InMemoryDiagnosticLogger(); + var options = new SentryOptions { Debug = true, DiagnosticLogger = logger }; + options.SetBeforeSend((SentryEvent _, SentryHint _) => throw exception); + + var native = new Sentry.CocoaSdk.SentryObjCEvent(); + + // Act + var result = SentrySdk.ProcessOnBeforeSend(options, native, Substitute.For()); + + // Assert + result.Should().BeNull(); + logger.Entries.Should().ContainSingle(entry => + entry.Level == SentryLevel.Error && + entry.Exception == exception && + entry.Message == "The BeforeSend callback threw an exception. The event will be dropped."); + } + + [Fact] + public void ProcessOnBeforeSend_BeforeSendReturnsEvent_KeepsNativeEvent() + { + // Arrange + var options = new SentryOptions(); + options.SetBeforeSend((SentryEvent @event, SentryHint _) => @event); + + var native = new Sentry.CocoaSdk.SentryObjCEvent(); + + // Act + var result = SentrySdk.ProcessOnBeforeSend(options, native, Substitute.For()); + + // Assert + result.Should().BeSameAs(native); + } #endif public void Dispose()