Improve logging for failed JSON serialization (#1473)

getsentry · Feb 7, 2022 · fba1728 · fba1728
1 parent 8b4d317
commit fba1728
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,7 @@
 - Add Sentry to global usings when ImplicitUsings is enabled (`<ImplicitUsings>true</ImplicitUsings>`) ([#1398](https://github.com/getsentry/sentry-dotnet/pull/1398))
 - The implementation of the background worker can now be changed ([#1450](https://github.com/getsentry/sentry-dotnet/pull/1450))
 - Map reg key 528449 to net48 ([#1465](https://github.com/getsentry/sentry-dotnet/pull/1465))
+- Improve logging for failed JSON serialization ([#1473](https://github.com/getsentry/sentry-dotnet/pull/1473))
 
 ### Fixes
 

diff --git a/src/Sentry/Internal/Http/CachingTransport.cs b/src/Sentry/Internal/Http/CachingTransport.cs
@@ -183,10 +183,37 @@ private async Task ProcessCacheAsync(CancellationToken cancellationToken = defau
                 }
                 catch (Exception ex)
                 {
-                    _options.LogError(
-                        "Failed to send cached envelope: {0}, discarding cached envelope.",
-                        ex,
-                        envelopeFilePath);
+                    string? envelopeContents = null;
+                    try
+                    {
+                        if (File.Exists(envelopeFilePath))
+                        {
+#if NET461 || NETSTANDARD2_0
+                            envelopeContents = File.ReadAllText(envelopeFilePath);
+#else
+                            envelopeContents = await File.ReadAllTextAsync(envelopeFilePath, cancellationToken).ConfigureAwait(false);
+#endif
+                        }
+                    }
+                    catch
+                    {
+                    }
+
+                    if (envelopeContents == null)
+                    {
+                        _options.LogError(
+                            "Failed to send cached envelope: {0}, discarding cached envelope.",
+                            ex,
+                            envelopeFilePath);
+                    }
+                    else
+                    {
+                        _options.LogError(
+                            "Failed to send cached envelope: {0}, discarding cached envelope. Envelope contents: {1}",
+                            ex,
+                            envelopeFilePath,
+                            envelopeContents);
+                    }
                 }
 
                 // Envelope & file stream must be disposed prior to reaching this point

diff --git a/test/Sentry.Testing/TestOutputDiagnosticLogger.cs b/test/Sentry.Testing/TestOutputDiagnosticLogger.cs
@@ -18,6 +18,7 @@ public class LogEntry
         public SentryLevel Level { get; set; }
         public string Message { get; set; }
         public Exception Exception { get; set; }
+        public string RawMessage { get; set; }
     }
 
     public TestOutputDiagnosticLogger(
@@ -33,8 +34,14 @@ public TestOutputDiagnosticLogger(
     public void Log(SentryLevel logLevel, string message, Exception exception = null, params object[] args)
     {
         var formattedMessage = string.Format(message, args);
-        _entries.Enqueue(
-            new LogEntry { Level = logLevel, Message = formattedMessage, Exception = exception });
+        var entry = new LogEntry
+        {
+            Level = logLevel,
+            Message = formattedMessage,
+            RawMessage = message,
+            Exception = exception
+        };
+        _entries.Enqueue(entry);
 
         _testOutputHelper.WriteLine($@"
 [{logLevel}]: {formattedMessage}

diff --git a/test/Sentry.Tests/Internals/Http/CachingTransportTests.cs b/test/Sentry.Tests/Internals/Http/CachingTransportTests.cs
@@ -235,6 +235,45 @@ public async Task AwareOfExistingFiles()
         innerTransport.GetSentEnvelopes().Should().HaveCount(3);
     }
 
+    [Fact(Timeout = 7000)]
+    public async Task NonTransientExceptionShouldLog()
+    {
+        // Arrange
+        using var cacheDirectory = new TempDirectory();
+        var options = new SentryOptions
+        {
+            Dsn = DsnSamples.ValidDsnWithoutSecret,
+            DiagnosticLogger = _logger,
+            Debug = true,
+            CacheDirectoryPath = cacheDirectory.Path
+        };
+
+        var innerTransport = Substitute.For<ITransport>();
+
+        innerTransport
+            .SendEnvelopeAsync(Arg.Any<Envelope>(), Arg.Any<CancellationToken>())
+            .Returns(_ => Task.FromException(new Exception("The Message")));
+
+        await using var transport = new CachingTransport(innerTransport, options);
+
+        // Can't really reliably test this with a worker
+        await transport.StopWorkerAsync();
+
+        // Act
+        using var envelope = Envelope.FromEvent(new SentryEvent());
+        await transport.SendEnvelopeAsync(envelope);
+
+        await transport.FlushAsync();
+
+        var message = _logger.Entries
+            .Where(x => x.Level == SentryLevel.Error)
+            .Select(x => x.RawMessage)
+            .Single();
+
+        // Assert
+        Assert.Equal("Failed to send cached envelope: {0}, discarding cached envelope. Envelope contents: {1}", message);
+    }
+
     [Fact(Timeout = 7000)]
     public async Task DoesNotRetryOnNonTransientExceptions()
     {