diff --git a/src/Sentry.Extensions.Logging/SentryLogger.cs b/src/Sentry.Extensions.Logging/SentryLogger.cs index 05013d0fef..9a25cfc0b5 100644 --- a/src/Sentry.Extensions.Logging/SentryLogger.cs +++ b/src/Sentry.Extensions.Logging/SentryLogger.cs @@ -1,4 +1,5 @@ using Microsoft.Extensions.Logging; +using Sentry.Extensibility; using Sentry.Infrastructure; using Sentry.Internal; @@ -51,7 +52,12 @@ public void Log( var message = formatter?.Invoke(state, exception); - if (ShouldCaptureEvent(logLevel, eventId, exception)) + if (IsFromSentry() || IsEfExceptionMessage(eventId) || IsFiltered(logLevel, eventId, exception)) + { + return; + } + + if (ShouldCaptureEvent(logLevel)) { var @event = CreateEvent(logLevel, eventId, state, exception, message, CategoryName); @@ -64,7 +70,7 @@ public void Log( } } - if (ShouldAddBreadcrumb(logLevel, eventId, exception)) + if (ShouldAddBreadcrumb(logLevel)) { var data = eventId.ToDictionaryOrNull(); @@ -153,35 +159,36 @@ internal static SentryEvent CreateEvent( return @event; } - private bool ShouldCaptureEvent( + private bool ShouldCaptureEvent(LogLevel logLevel) + => _options.MinimumEventLevel != LogLevel.None + && logLevel >= _options.MinimumEventLevel; + + private bool ShouldAddBreadcrumb(LogLevel logLevel) + => _options.MinimumBreadcrumbLevel != LogLevel.None + && logLevel >= _options.MinimumBreadcrumbLevel; + + private bool IsFiltered( LogLevel logLevel, EventId eventId, Exception? exception) - => _options.MinimumEventLevel != LogLevel.None - && logLevel >= _options.MinimumEventLevel - && !IsFromSentry() - && !IsEfExceptionMessage(eventId) - && _options.Filters.All( - f => !f.Filter( - CategoryName, - logLevel, - eventId, - exception)); - - private bool ShouldAddBreadcrumb( + => _options.Filters.Any(f => IsFiltered(f, logLevel, eventId, exception)); + + private bool IsFiltered( + ILogEntryFilter filter, LogLevel logLevel, EventId eventId, Exception? exception) - => _options.MinimumBreadcrumbLevel != LogLevel.None - && logLevel >= _options.MinimumBreadcrumbLevel - && !IsFromSentry() - && !IsEfExceptionMessage(eventId) - && _options.Filters.All( - f => !f.Filter( - CategoryName, - logLevel, - eventId, - exception)); + { + try + { + return filter.Filter(CategoryName, logLevel, eventId, exception); + } + catch (Exception e) + { + _options.LogError(e, "The {0} log filter callback failed. The log entry will be filtered out.", filter.GetType().Name); + return true; + } + } private bool IsFromSentry() => SentrySdkNamespaces.IsSentrySdk(CategoryName); diff --git a/src/Sentry.Maui/Internal/SentryMauiScreenshotProcessor.cs b/src/Sentry.Maui/Internal/SentryMauiScreenshotProcessor.cs index fbba3d457d..621b6ab95b 100644 --- a/src/Sentry.Maui/Internal/SentryMauiScreenshotProcessor.cs +++ b/src/Sentry.Maui/Internal/SentryMauiScreenshotProcessor.cs @@ -18,9 +18,23 @@ public SentryMauiScreenshotProcessor(SentryMauiOptions options) public SentryEvent? Process(SentryEvent @event, SentryHint hint) { - if (!_options.BeforeCaptureInternal?.Invoke(@event, hint) ?? false) + if (_options.BeforeScreenshotCaptureInternal is { } beforeCapture) { - return @event; + bool shouldCapture; + try + { + shouldCapture = beforeCapture.Invoke(@event, hint); + } + catch (Exception e) + { + _options.LogError(e, "BeforeScreenshotCapture callback failed."); + return @event; + } + + if (!shouldCapture) + { + return @event; + } } hint.Attachments.Add(new ScreenshotAttachment(_options)); diff --git a/src/Sentry.Maui/SentryMauiOptions.cs b/src/Sentry.Maui/SentryMauiOptions.cs index 17038bf747..6a7ecccb8c 100644 --- a/src/Sentry.Maui/SentryMauiOptions.cs +++ b/src/Sentry.Maui/SentryMauiOptions.cs @@ -76,11 +76,11 @@ public SentryMauiOptions() /// public bool AttachScreenshot { get; set; } - private Func? _beforeCapture; + private Func? _beforeScreenshotCapture; /// /// Action performed before attaching a screenshot /// - internal Func? BeforeCaptureInternal => _beforeCapture; + internal Func? BeforeScreenshotCaptureInternal => _beforeScreenshotCapture; /// /// Configures a callback function to be invoked before taking a screenshot @@ -90,7 +90,7 @@ public SentryMauiOptions() /// /// /// - ///options.SetBeforeCapture((@event, hint) => + ///options.SetBeforeScreenshotCapture((@event, hint) => ///{ /// // Return true to capture or false to prevent the capture /// return true; @@ -99,6 +99,6 @@ public SentryMauiOptions() /// Callback to be executed before taking a screenshot public void SetBeforeScreenshotCapture(Func beforeCapture) { - _beforeCapture = beforeCapture; + _beforeScreenshotCapture = beforeCapture; } } diff --git a/src/Sentry/Platforms/Android/Callbacks/BeforeBreadcrumbCallback.cs b/src/Sentry/Platforms/Android/Callbacks/BeforeBreadcrumbCallback.cs index ac2f149902..37629e821c 100644 --- a/src/Sentry/Platforms/Android/Callbacks/BeforeBreadcrumbCallback.cs +++ b/src/Sentry/Platforms/Android/Callbacks/BeforeBreadcrumbCallback.cs @@ -1,14 +1,19 @@ using Sentry.Android.Extensions; +using Sentry.Extensibility; namespace Sentry.Android.Callbacks; internal class BeforeBreadcrumbCallback : JavaObject, JavaSdk.SentryOptions.IBeforeBreadcrumbCallback { private readonly Func _beforeBreadcrumb; + private readonly SentryOptions _options; - public BeforeBreadcrumbCallback(Func beforeBreadcrumb) + public BeforeBreadcrumbCallback( + Func beforeBreadcrumb, + SentryOptions options) { _beforeBreadcrumb = beforeBreadcrumb; + _options = options; } public JavaSdk.Breadcrumb? Execute(JavaSdk.Breadcrumb b, JavaSdk.Hint h) @@ -18,7 +23,17 @@ public BeforeBreadcrumbCallback(Func before var breadcrumb = b.ToBreadcrumb(); var hint = h.ToHint(); - var result = _beforeBreadcrumb.Invoke(breadcrumb, hint); + + Breadcrumb? result; + try + { + result = _beforeBreadcrumb.Invoke(breadcrumb, hint); + } + catch (Exception exception) + { + _options.LogError(exception, "Android BeforeBreadcrumb callback failed."); + return null; + } if (result == breadcrumb) { diff --git a/src/Sentry/Platforms/Android/SentrySdk.cs b/src/Sentry/Platforms/Android/SentrySdk.cs index 9afb73dcc8..ab07387924 100644 --- a/src/Sentry/Platforms/Android/SentrySdk.cs +++ b/src/Sentry/Platforms/Android/SentrySdk.cs @@ -113,7 +113,7 @@ private static void InitSentryAndroidSdk(SentryOptions options) if (options.BeforeBreadcrumbInternal is { } beforeBreadcrumb) { - o.BeforeBreadcrumb = new BeforeBreadcrumbCallback(beforeBreadcrumb); + o.BeforeBreadcrumb = new BeforeBreadcrumbCallback(beforeBreadcrumb, options); } // These options we have behind feature flags diff --git a/src/Sentry/Platforms/Cocoa/SentrySdk.cs b/src/Sentry/Platforms/Cocoa/SentrySdk.cs index b55b44b3bf..539614f2e5 100644 --- a/src/Sentry/Platforms/Cocoa/SentrySdk.cs +++ b/src/Sentry/Platforms/Cocoa/SentrySdk.cs @@ -62,7 +62,17 @@ private static void InitSentryCocoaSdk(SentryOptions options) // See https://github.com/getsentry/sentry-cocoa/issues/2325 var hint = new SentryHint(); var breadcrumb = b.ToBreadcrumb(options.DiagnosticLogger); - var result = beforeBreadcrumb(breadcrumb, hint)?.ToCocoaBreadcrumb(); + + CocoaSdk.SentryObjCBreadcrumb? result; + try + { + result = beforeBreadcrumb(breadcrumb, hint)?.ToCocoaBreadcrumb(); + } + catch (Exception ex) + { + options.LogError(ex, "Cocoa BeforeBreadcrumb callback failed."); + result = null; + } // Note: Nullable result is allowed but delegate is generated incorrectly // See https://github.com/xamarin/xamarin-macios/issues/15299#issuecomment-1201863294 diff --git a/src/Sentry/Scope.cs b/src/Sentry/Scope.cs index d062290133..17b9df38ba 100644 --- a/src/Sentry/Scope.cs +++ b/src/Sentry/Scope.cs @@ -335,15 +335,24 @@ public void AddBreadcrumb(Breadcrumb breadcrumb, SentryHint hint) { hint.AddAttachmentsFromScope(this); - if (beforeBreadcrumb(breadcrumb, hint) is { } processedBreadcrumb) + Breadcrumb? processedBreadcrumb; + try { - breadcrumb = processedBreadcrumb; + processedBreadcrumb = beforeBreadcrumb(breadcrumb, hint); } - else + catch (Exception e) + { + Options.LogError(e, "BeforeBreadcrumb callback failed."); + return; + } + + if (processedBreadcrumb is null) { // Callback returned null, which means the breadcrumb should be dropped return; } + + breadcrumb = processedBreadcrumb; } if (Options.MaxBreadcrumbs <= 0) diff --git a/test/Sentry.Extensions.Logging.Tests/SentryLoggerTests.cs b/test/Sentry.Extensions.Logging.Tests/SentryLoggerTests.cs index 8a748b030a..58fd48ab14 100644 --- a/test/Sentry.Extensions.Logging.Tests/SentryLoggerTests.cs +++ b/test/Sentry.Extensions.Logging.Tests/SentryLoggerTests.cs @@ -214,6 +214,67 @@ public void LogCritical_NotMatchingFilter_CapturesEvent() .CaptureEvent(Arg.Any()); } + [Fact] + public void LogCritical_EventAndBreadcrumbLevelsBothMet_EvaluatesFilterOnce() + { + var invocations = 0; + _fixture.Options.MinimumEventLevel = LogLevel.Critical; + _fixture.Options.MinimumBreadcrumbLevel = LogLevel.Debug; + _fixture.Options.AddLogEntryFilter((_, _, _, _) => + { + invocations++; + return false; + }); + + var sut = _fixture.GetSut(); + + sut.LogCritical("message"); + + invocations.Should().Be(1); + } + + [Fact] + public void LogCritical_FilterThrows_DoesNotCaptureEventAndLogsError() + { + var exception = new InvalidOperationException("filter failed"); + _fixture.Options.AddLogEntryFilter((_, _, _, _) => throw exception); + _fixture.Options.AddDiagnosticLoggerSubstitute(); + + var sut = _fixture.GetSut(); + + sut.LogCritical("message"); + + _ = _fixture.Hub.DidNotReceive().CaptureEvent(Arg.Any()); + _fixture.Options.ReceivedLogError(exception, + "The {0} log filter callback failed. The log entry will be filtered out.", + nameof(DelegateLogEntryFilter)); + } + + [Fact] + public void LogCritical_FilterThrows_DoesNotAddBreadcrumb() + { + _fixture.Options.AddLogEntryFilter((_, _, _, _) => throw new InvalidOperationException("filter failed")); + _fixture.Options.AddDiagnosticLoggerSubstitute(); + + var sut = _fixture.GetSut(); + + sut.LogCritical("message"); + + _fixture.Scope.Breadcrumbs.Should().BeEmpty(); + } + + [Fact] + public void LogCritical_FilterThrows_DoesNotReachTheCaller() + { + _fixture.Options.AddLogEntryFilter((_, _, _, _) => throw new InvalidOperationException("filter failed")); + + var sut = _fixture.GetSut(); + + var log = () => sut.LogCritical("message"); + + log.Should().NotThrow(); + } + [Fact] public void LogCritical_DefaultOptions_CapturesEvent() { diff --git a/test/Sentry.Maui.Tests/Internal/SentryMauiScreenshotProcessorTests.cs b/test/Sentry.Maui.Tests/Internal/SentryMauiScreenshotProcessorTests.cs new file mode 100644 index 0000000000..438360a86a --- /dev/null +++ b/test/Sentry.Maui.Tests/Internal/SentryMauiScreenshotProcessorTests.cs @@ -0,0 +1,52 @@ +using Sentry.Maui.Internal; + +namespace Sentry.Maui.Tests.Internal; + +public class SentryMauiScreenshotProcessorTests +{ + [Fact] + public void Process_BeforeScreenshotCaptureThrows_KeepsEventAndSkipsScreenshot() + { + // Arrange + var exception = new InvalidOperationException("callback failed"); + var logger = new InMemoryDiagnosticLogger(); + var options = new SentryMauiOptions + { + Debug = true, + DiagnosticLogger = logger + }; + options.SetBeforeScreenshotCapture((_, _) => throw exception); + var processor = new SentryMauiScreenshotProcessor(options); + + var @event = new SentryEvent(); + var hint = new SentryHint(); + + // Act + var processed = processor.Process(@event, hint); + + // Assert + processed.Should().BeSameAs(@event); + hint.Attachments.Should().BeEmpty(); + logger.Entries.Should().ContainSingle(entry => + entry.Level == SentryLevel.Error && + entry.Exception == exception && + entry.Message == "BeforeScreenshotCapture callback failed."); + } + + [Fact] + public void Process_BeforeScreenshotCaptureReturnsTrue_AddsScreenshot() + { + // Arrange + var options = new SentryMauiOptions(); + options.SetBeforeScreenshotCapture((_, _) => true); + var processor = new SentryMauiScreenshotProcessor(options); + + var hint = new SentryHint(); + + // Act + processor.Process(new SentryEvent(), hint); + + // Assert + hint.Attachments.Should().ContainSingle(a => a.FileName == "screenshot.jpg"); + } +} diff --git a/test/Sentry.Maui.Tests/SentryMauiOptionsTests.cs b/test/Sentry.Maui.Tests/SentryMauiOptionsTests.cs index 1d2a9ae9ed..b3fee91394 100644 --- a/test/Sentry.Maui.Tests/SentryMauiOptionsTests.cs +++ b/test/Sentry.Maui.Tests/SentryMauiOptionsTests.cs @@ -92,7 +92,7 @@ public void HandlerStrategy_Set() #endif [Fact] - public void BeforeCaptureScreenshot_Set() + public void BeforeScreenshotCapture_Set() { // Arrange var options = GetSut(); @@ -105,17 +105,17 @@ public void BeforeCaptureScreenshot_Set() }); // Assert - Assert.NotNull(options.BeforeCaptureInternal); + Assert.NotNull(options.BeforeScreenshotCaptureInternal); } [Fact] - public void BeforeCaptureScreenshot_NotSet() + public void BeforeScreenshotCapture_NotSet() { // Arrange var options = GetSut(); options.AttachScreenshot = true; // Assert - Assert.Null(options.BeforeCaptureInternal); + Assert.Null(options.BeforeScreenshotCaptureInternal); } } diff --git a/test/Sentry.Maui.Tests/SentryMauiScreenshotTests.cs b/test/Sentry.Maui.Tests/SentryMauiScreenshotTests.cs index 89f9ad00af..1c9bf644fc 100644 --- a/test/Sentry.Maui.Tests/SentryMauiScreenshotTests.cs +++ b/test/Sentry.Maui.Tests/SentryMauiScreenshotTests.cs @@ -109,7 +109,7 @@ public async Task CaptureException_RemoveScreenshot_NotContainsScreenshotAttachm } [SkippableFact] - public async Task CaptureException_BeforeCaptureScreenshot_DisableCaptureAsync() + public async Task CaptureException_BeforeScreenshotCapture_DisableCaptureAsync() { #if __IOS__ Skip.If(true, "Flaky on iOS"); @@ -141,7 +141,7 @@ public async Task CaptureException_BeforeCaptureScreenshot_DisableCaptureAsync() // various static members like ActivityStateManager.Default: // https://github.com/dotnet/maui/blob/3c7b65264d2f341a48db32263a271fd8718cfd23/src/Essentials/src/Screenshot/Screenshot.android.cs#L28 [SkippableFact] - public async Task CaptureException_BeforeCaptureScreenshot_DefaultAsync() + public async Task CaptureException_BeforeScreenshotCapture_DefaultAsync() { #if __IOS__ Skip.If(true, "Flaky on iOS"); diff --git a/test/Sentry.Tests/Platforms/Android/BeforeBreadcrumbCallbackTests.cs b/test/Sentry.Tests/Platforms/Android/BeforeBreadcrumbCallbackTests.cs new file mode 100644 index 0000000000..2480623548 --- /dev/null +++ b/test/Sentry.Tests/Platforms/Android/BeforeBreadcrumbCallbackTests.cs @@ -0,0 +1,59 @@ +#if ANDROID +using Sentry.Android.Callbacks; + +namespace Sentry.Tests.Platforms.Android; + +public class BeforeBreadcrumbCallbackTests +{ + private static JavaSdk.Breadcrumb JavaBreadcrumb() => new() { Message = "test", Category = "test" }; + + [Fact] + public void Execute_CallbackThrows_ReturnsNullAndLogsError() + { + // Arrange + var exception = new InvalidOperationException("callback failed"); + var logger = new InMemoryDiagnosticLogger(); + var options = new SentryOptions { Debug = true, DiagnosticLogger = logger }; + using var sut = new BeforeBreadcrumbCallback((_, _) => throw exception, options); + + // Act + using var result = sut.Execute(JavaBreadcrumb(), new JavaSdk.Hint()); + + // Assert + result.Should().BeNull(); + logger.Entries.Should().ContainSingle(entry => + entry.Level == SentryLevel.Error && + entry.Exception == exception && + entry.Message == "Android BeforeBreadcrumb callback failed."); + } + + [Fact] + public void Execute_CallbackReturnsNull_ReturnsNull() + { + // Arrange + var options = new SentryOptions(); + using var sut = new BeforeBreadcrumbCallback((_, _) => null, options); + + // Act + using var result = sut.Execute(JavaBreadcrumb(), new JavaSdk.Hint()); + + // Assert + result.Should().BeNull(); + } + + [Fact] + public void Execute_CallbackReturnsInput_ReturnsOriginalJavaBreadcrumb() + { + // Arrange + var options = new SentryOptions(); + using var sut = new BeforeBreadcrumbCallback((breadcrumb, _) => breadcrumb, options); + using var javaBreadcrumb = JavaBreadcrumb(); + + // Act + var result = sut.Execute(javaBreadcrumb, new JavaSdk.Hint()); + + // Assert + result.Should().BeSameAs(javaBreadcrumb); + } +} +#endif diff --git a/test/Sentry.Tests/ScopeTests.cs b/test/Sentry.Tests/ScopeTests.cs index 0e04ec4b34..9f47603ced 100644 --- a/test/Sentry.Tests/ScopeTests.cs +++ b/test/Sentry.Tests/ScopeTests.cs @@ -534,6 +534,24 @@ public void AddBreadcrumb_BeforeAddBreadcrumb_ReceivesHint() receivedHint.Should().BeSameAs(expectedHint); } + [Fact] + public void AddBreadcrumb_BeforeBreadcrumbThrows_DropsBreadcrumbAndLogsError() + { + // Arrange + var exception = new InvalidOperationException("callback failed"); + var options = new SentryOptions(); + options.SetBeforeBreadcrumb((_, _) => throw exception); + options.AddDiagnosticLoggerSubstitute(); + var scope = new Scope(options); + + // Act + scope.AddBreadcrumb(new Breadcrumb()); + + // Assert + scope.Breadcrumbs.Should().BeEmpty(); + options.ReceivedLogError(exception, "BeforeBreadcrumb callback failed."); + } + [Fact] public void AddBreadcrumb_ScopeAttachments_Copied_To_Hint() {