From adce466ca73caa3a67a2d2084f8b124fb1907493 Mon Sep 17 00:00:00 2001 From: Artur Krajewski Date: Thu, 3 Oct 2019 22:38:24 +0200 Subject: [PATCH 1/2] Add test for dictionary with non-scalar key --- .../ExceptionDestructurerTest.cs | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/Tests/Serilog.Exceptions.Test/Destructurers/ExceptionDestructurerTest.cs b/Tests/Serilog.Exceptions.Test/Destructurers/ExceptionDestructurerTest.cs index ac08cc3c..5e2e46ff 100644 --- a/Tests/Serilog.Exceptions.Test/Destructurers/ExceptionDestructurerTest.cs +++ b/Tests/Serilog.Exceptions.Test/Destructurers/ExceptionDestructurerTest.cs @@ -1,6 +1,7 @@ namespace Serilog.Exceptions.Test.Destructurers { using System; + using System.Collections.Generic; using FluentAssertions; using Newtonsoft.Json.Linq; using NSubstitute; @@ -138,5 +139,36 @@ public void ArgumentException_ContainsType() var applicationException = new ArgumentException(); Test_LoggedExceptionContainsProperty(applicationException, "Type", "System.ArgumentException"); } + + [Fact] + public void WhenExceptionContainsDictionaryWithNonScalarValue_ShouldNotThrow() + { + // Arrange + var exception = new ExceptionWithDictNonScalarKey() + { + Reference = new Dictionary, object>() + { + { new List() { 1, 2, 3 }, "VALUE" } + } + }; + + // Act + var result = LogAndDestructureException(exception, new DestructuringOptionsBuilder()); + + // Assert + var exceptionDetails = ExtractExceptionDetails(result); + var referenceProperty = exceptionDetails.Should().BeOfType().Which + .Properties().Should().ContainSingle(x => x.Name == "Reference").Which; + + var referenceObject = referenceProperty.Value.Should().BeOfType().Which; + var kvp = referenceObject.Properties().Should().ContainSingle() + .Which.Should().BeOfType() + .Which.Name.Should().Be("System.Collections.Generic.List`1[System.Int32]"); + } + + public class ExceptionWithDictNonScalarKey : Exception + { + public Dictionary, object> Reference { get; set; } + } } } From b97242091e8f2a117dcd77790f1999074cecb7ea Mon Sep 17 00:00:00 2001 From: Artur Krajewski Date: Mon, 2 Dec 2019 07:11:59 +0100 Subject: [PATCH 2/2] Add WithoutReflectionBasedDestructurer destructuring option --- .../Core/DestructuringOptionsBuilder.cs | 19 ++++++++++- .../Core/ExceptionEnricher.cs | 29 +++++++++++----- .../Core/IDestructuringOptions.cs | 9 ++++- ...LoggerEnrichmentConfigurationExtensions.cs | 2 +- .../ExceptionDestructurerTest.cs | 33 +++++++++++++++++++ .../Destructurers/LogJsonOutputUtils.cs | 12 +++++-- 6 files changed, 89 insertions(+), 15 deletions(-) diff --git a/Source/Serilog.Exceptions/Core/DestructuringOptionsBuilder.cs b/Source/Serilog.Exceptions/Core/DestructuringOptionsBuilder.cs index f10a6f85..ce6f8e14 100644 --- a/Source/Serilog.Exceptions/Core/DestructuringOptionsBuilder.cs +++ b/Source/Serilog.Exceptions/Core/DestructuringOptionsBuilder.cs @@ -52,6 +52,11 @@ public class DestructuringOptionsBuilder : IDestructuringOptions /// public int DestructuringDepth { get; private set; } = 10; + /// + /// Disable reflection based destruturer. + /// + public bool DisableReflectionBasedDestructurer { get; private set; } = false; + /// /// Collection of destructurers that will be used to handle exception. /// @@ -149,5 +154,17 @@ public DestructuringOptionsBuilder WithDestructuringDepth(int destructuringDepth this.DestructuringDepth = destructuringDepth; return this; } + + /// + /// Disable reflection based destructurer. + /// You may want to disable this destructurer if you need full control + /// over the process of destructuring and want to provide all the destructurers yourself. + /// + /// Options builder for method chaining. + public DestructuringOptionsBuilder WithoutReflectionBasedDestructurer() + { + this.DisableReflectionBasedDestructurer = true; + return this; + } } -} \ No newline at end of file +} diff --git a/Source/Serilog.Exceptions/Core/ExceptionEnricher.cs b/Source/Serilog.Exceptions/Core/ExceptionEnricher.cs index 8b5c328c..495cb421 100644 --- a/Source/Serilog.Exceptions/Core/ExceptionEnricher.cs +++ b/Source/Serilog.Exceptions/Core/ExceptionEnricher.cs @@ -64,30 +64,41 @@ public void Enrich(LogEvent logEvent, ILogEventPropertyFactory propertyFactory) if (logEvent.Exception != null) { - logEvent.AddPropertyIfAbsent(propertyFactory.CreateProperty( - this.destructuringOptions.RootName, - this.DestructureException(logEvent.Exception), - true)); + var dataDictionary = this.DestructureException(logEvent.Exception); + + if (dataDictionary != null) + { + logEvent.AddPropertyIfAbsent(propertyFactory.CreateProperty( + this.destructuringOptions.RootName, + dataDictionary, + true)); + } } } private IReadOnlyDictionary DestructureException(Exception exception) { - var data = new ExceptionPropertiesBag(exception, this.destructuringOptions.Filter); - var exceptionType = exception.GetType(); if (this.destructurers.ContainsKey(exceptionType)) { + var data = new ExceptionPropertiesBag(exception, this.destructuringOptions.Filter); + var destructurer = this.destructurers[exceptionType]; destructurer.Destructure(exception, data, this.DestructureException); + + return data.GetResultDictionary(); } - else + else if (!this.destructuringOptions.DisableReflectionBasedDestructurer) { + var data = new ExceptionPropertiesBag(exception, this.destructuringOptions.Filter); + this.reflectionBasedDestructurer.Destructure(exception, data, this.DestructureException); + + return data.GetResultDictionary(); } - return data.GetResultDictionary(); + return null; } } -} \ No newline at end of file +} diff --git a/Source/Serilog.Exceptions/Core/IDestructuringOptions.cs b/Source/Serilog.Exceptions/Core/IDestructuringOptions.cs index 5a20761d..d42c75f1 100644 --- a/Source/Serilog.Exceptions/Core/IDestructuringOptions.cs +++ b/Source/Serilog.Exceptions/Core/IDestructuringOptions.cs @@ -33,5 +33,12 @@ public interface IDestructuringOptions /// Filter is applied to every property regardless of which destructurer was used. /// IExceptionPropertyFilter Filter { get; } + + /// + /// Decides whether to disable reflection based destructurer. + /// You may want to disable this destructurer if you need full control + /// over the process of destructuring and want to provide all the destructurers yourself. + /// + bool DisableReflectionBasedDestructurer { get; } } -} \ No newline at end of file +} diff --git a/Source/Serilog.Exceptions/LoggerEnrichmentConfigurationExtensions.cs b/Source/Serilog.Exceptions/LoggerEnrichmentConfigurationExtensions.cs index 4394e1cb..5a743586 100644 --- a/Source/Serilog.Exceptions/LoggerEnrichmentConfigurationExtensions.cs +++ b/Source/Serilog.Exceptions/LoggerEnrichmentConfigurationExtensions.cs @@ -11,7 +11,7 @@ namespace Serilog.Exceptions public static class LoggerEnrichmentConfigurationExtensions { /// - /// Enrich logger output with a destuctured object containing exception's public properties. Default + /// Enrich logger output with a destructured object containing exception's public properties. Default /// destructurers are registered. and Exception.TargetSite /// are omitted by the destructuring process because Serilog already attaches them to log event. /// diff --git a/Tests/Serilog.Exceptions.Test/Destructurers/ExceptionDestructurerTest.cs b/Tests/Serilog.Exceptions.Test/Destructurers/ExceptionDestructurerTest.cs index 5e2e46ff..7a664729 100644 --- a/Tests/Serilog.Exceptions.Test/Destructurers/ExceptionDestructurerTest.cs +++ b/Tests/Serilog.Exceptions.Test/Destructurers/ExceptionDestructurerTest.cs @@ -120,6 +120,39 @@ public void PassedFilter_IsCalledWithCorrectArguments() filter.Received().ShouldPropertyBeFiltered(exception, "StackTrace", null); } + [Fact] + public void WithoutReflectionBasedDestructurer_CustomExceptionIsNotLogged() + { + // Arrange + var exception = new ExceptionWithDictNonScalarKey(); + var options = new DestructuringOptionsBuilder().WithoutReflectionBasedDestructurer(); + + // Act + var rootObject = LogAndDestructureException(exception, options); + + // Assert + rootObject.Properties().Should().NotContain(x => x.Name == "Properties"); + } + + [Fact] + public void WithoutReflectionBasedDestructurerAndCustomRootName_StandardExceptionIsLogged() + { + // Arrange + var exception = new ArgumentException("ARG", "arg"); + var options = new DestructuringOptionsBuilder() + .WithDefaultDestructurers() + .WithoutReflectionBasedDestructurer() + .WithRootName("CUSTOM-ROOT"); + + // Act + var rootObject = LogAndDestructureException(exception, options); + + // Assert + var exceptionObject = ExtractExceptionDetails(rootObject, "CUSTOM-ROOT"); + var paramObject = exceptionObject.Properties().Should().ContainSingle(x => x.Name == "ParamName").Which; + paramObject.Value.Should().BeOfType().Which.Value.Should().Be("arg"); + } + [Fact] public void ArgumentException_WithStackTrace_ContainsStackTrace() { diff --git a/Tests/Serilog.Exceptions.Test/Destructurers/LogJsonOutputUtils.cs b/Tests/Serilog.Exceptions.Test/Destructurers/LogJsonOutputUtils.cs index 5f2f260c..f0f2ea5c 100644 --- a/Tests/Serilog.Exceptions.Test/Destructurers/LogJsonOutputUtils.cs +++ b/Tests/Serilog.Exceptions.Test/Destructurers/LogJsonOutputUtils.cs @@ -54,10 +54,9 @@ public static JArray ExtractInnerExceptionsProperty(JObject jObject) return innerExceptionsValue; } - public static JObject ExtractExceptionDetails(JObject jObject, string rootName = "ExceptionDetail") + public static JObject ExtractExceptionDetails(JObject rootObject, string rootName = "ExceptionDetail") { - var propertiesProperty = Assert.Single(jObject.Properties(), x => x.Name == "Properties"); - var propertiesObject = Assert.IsType(propertiesProperty.Value); + var propertiesObject = ExtractPropertiesObject(rootObject); var exceptionDetailProperty = Assert.Single(propertiesObject.Properties(), x => x.Name == rootName); var exceptionDetailValue = Assert.IsType(exceptionDetailProperty.Value); @@ -65,6 +64,13 @@ public static JObject ExtractExceptionDetails(JObject jObject, string rootName = return exceptionDetailValue; } + public static JObject ExtractPropertiesObject(JObject rootObject) + { + var propertiesProperty = Assert.Single(rootObject.Properties(), x => x.Name == "Properties"); + var propertiesObject = Assert.IsType(propertiesProperty.Value); + return propertiesObject; + } + public static void Assert_ContainsPropertyWithValue( JObject jObject, string propertyKey,