fix: avoid errors on log serialize exceptions

Farfetch · Mar 11, 2024 · 3414596 · 3414596
1 parent a9c40d8
commit 3414596
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 9 deletions.
diff --git a/src/KafkaFlow.Admin/TelemetryScheduler.cs b/src/KafkaFlow.Admin/TelemetryScheduler.cs
@@ -96,11 +96,7 @@ private void ProduceTelemetry(
         }
         catch (Exception e)
         {
-            _logHandler.Warning("Error producing telemetry data", new
-            {
-                Exception = e.Message,
-                e.StackTrace,
-            });
+            _logHandler.Warning("Error producing telemetry data", new { Exception = e });
         }
     }
 }
diff --git a/src/KafkaFlow.LogHandler.Microsoft/MicrosoftLogHandler.cs b/src/KafkaFlow.LogHandler.Microsoft/MicrosoftLogHandler.cs
@@ -6,6 +6,8 @@ namespace KafkaFlow;
 
 internal class MicrosoftLogHandler : ILogHandler
 {
+    private const string SerializationErrorMessage = "Log data serialization error.";
+
     private readonly ILogger _logger;
 
     public MicrosoftLogHandler(ILoggerFactory loggerFactory)
@@ -15,21 +17,37 @@ public MicrosoftLogHandler(ILoggerFactory loggerFactory)
 
     public void Error(string message, Exception ex, object data)
     {
-        _logger.LogError(ex, "{Message} | Data: {Data}", message, JsonSerializer.Serialize(data));
+        _logger.LogError(ex, "{Message} | Data: {Data}", message, SerializeData(data));
     }
 
     public void Warning(string message, object data)
     {
-        _logger.LogWarning("{Message} | Data: {Data}", message, JsonSerializer.Serialize(data));
+        _logger.LogWarning("{Message} | Data: {Data}", message, SerializeData(data));
     }
 
     public void Info(string message, object data)
     {
-        _logger.LogInformation("{Message} | Data: {Data}", message, JsonSerializer.Serialize(data));
+        _logger.LogInformation("{Message} | Data: {Data}", message, SerializeData(data));
     }
 
     public void Verbose(string message, object data)
     {
-        _logger.LogDebug("{Message} | Data: {Data}", message, JsonSerializer.Serialize(data));
+        _logger.LogDebug("{Message} | Data: {Data}", message, SerializeData(data));
+    }
+
+    private string SerializeData(object data)
+    {
+        try
+        {
+            return JsonSerializer.Serialize(data);
+        }
+        catch(Exception ex)
+        {
+            return JsonSerializer.Serialize(new
+            {
+                Error = SerializationErrorMessage,
+                Exception = $"Message: {ex.Message}, Trace: {ex.StackTrace}",
+            });
+        }
     }
 }
diff --git a/tests/KafkaFlow.UnitTests/LogHandlers/MicrosoftLogHandlerTests.cs b/tests/KafkaFlow.UnitTests/LogHandlers/MicrosoftLogHandlerTests.cs
@@ -1,6 +1,8 @@
 using Microsoft.Extensions.Logging;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using Moq;
+using System;
+using System.Linq;
 
 namespace KafkaFlow.UnitTests.LogHandlers;
 
@@ -21,4 +23,31 @@ public void Constructor_CreatesNamedLogger()
         // Assert
         loggerFactoryMock.Verify(x => x.CreateLogger("KafkaFlow"), Times.Once);
     }
+
+    [TestMethod]
+    public void LogWarning_WithNotSerializableData_DoesNotSerializeObject()
+    {
+        // Arrange
+        var expectedMessage = "Any message";
+        var expectedSerializationErrorMessage = "Log data serialization error.";
+        var loggerMock = new Mock<ILogger>();
+        var loggerFactoryMock = new Mock<ILoggerFactory>();
+
+        loggerFactoryMock
+            .Setup(x => x.CreateLogger("KafkaFlow"))
+            .Returns(loggerMock.Object);
+
+        var handler = new MicrosoftLogHandler(loggerFactoryMock.Object);
+
+        // Act
+        handler.Warning(expectedMessage, new
+        {
+            In = new IntPtr(1)
+        });
+
+        // Assert
+        Assert.AreEqual(1, loggerMock.Invocations.Count);
+        Assert.IsTrue(loggerMock.Invocations[0].Arguments.Any(x => x.ToString().Contains(expectedMessage)));
+        Assert.IsTrue(loggerMock.Invocations[0].Arguments.Any(x => x.ToString().Contains(expectedSerializationErrorMessage)));
+    }
 }