Flush caching transport with main flush (#1890)

* Flush caching transport with main flush * Update CHANGELOG.md * Refactor to fix broken test * .
getsentry · Sep 1, 2022 · cb735d1 · cb735d1
1 parent 1bc3869
commit cb735d1
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 25 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -26,6 +26,7 @@
 - Fix environment name casing issue ([#1861](https://github.com/getsentry/sentry-dotnet/pull/1861))
 - Null check HttpContext in SystemWebVersionLocator ([#1881](https://github.com/getsentry/sentry-dotnet/pull/1881))
 - Fix detection of .NET Framework 4.8.1 ([#1885](https://github.com/getsentry/sentry-dotnet/pull/1885))
+- Flush caching transport with main flush ([#1890](https://github.com/getsentry/sentry-dotnet/pull/1890))
 
 ## 3.20.1
 

diff --git a/src/Sentry/Internal/BackgroundWorker.cs b/src/Sentry/Internal/BackgroundWorker.cs
@@ -5,6 +5,7 @@
 using System.Threading.Tasks;
 using Sentry.Extensibility;
 using Sentry.Internal.Extensions;
+using Sentry.Internal.Http;
 using Sentry.Protocol.Envelopes;
 
 namespace Sentry.Internal
@@ -244,12 +245,6 @@ private async Task DoFlushAsync(CancellationToken cancellationToken)
                 return;
             }
 
-            if (_queue.IsEmpty)
-            {
-                _options.LogDebug("No events to flush.");
-                return;
-            }
-
             var completionSource = new TaskCompletionSource<bool>();
             cancellationToken.Register(() => completionSource.TrySetCanceled());
 
@@ -270,24 +265,28 @@ void EventFlushedCallback(object? _, EventArgs __)
 
             try
             {
+                // now we're subscribed and counting, make sure it's not already empty.
                 var trackedDepth = _queue.Count;
-                if (trackedDepth == 0) // now we're subscribed and counting, make sure it's not already empty.
+                if (trackedDepth != 0)
                 {
-                    return;
+                    Interlocked.Exchange(ref depth, trackedDepth);
+                    _options.LogDebug("Tracking depth: {0}.", trackedDepth);
+
+                    // Check if the worker didn't finish flushing before we set the depth
+                    if (counter < depth)
+                    {
+                        // Await until event is flushed (or we have cancelled)
+                        await completionSource.Task.ConfigureAwait(false);
+                    }
                 }
 
-                Interlocked.Exchange(ref depth, trackedDepth);
-                _options.LogDebug("Tracking depth: {0}.", trackedDepth);
+                _options.LogDebug("Successfully flushed all events up to call to FlushAsync.");
 
-                if (counter >= depth) // When the worker finished flushing before we set the depth
+                if (_transport is CachingTransport cachingTransport && !cancellationToken.IsCancellationRequested)
                 {
-                    return;
+                    _options.LogDebug("Flushing caching transport with remaining flush time.");
+                    await cachingTransport.FlushAsync(cancellationToken).ConfigureAwait(false);
                 }
-
-                // Await until event is flushed (or we have cancelled)
-                await completionSource.Task.ConfigureAwait(false);
-
-                _options.LogDebug("Successfully flushed all events up to call to FlushAsync.");
             }
             catch (OperationCanceledException)
             {

diff --git a/test/Sentry.Tests/Internals/BackgroundWorkerTests.cs b/test/Sentry.Tests/Internals/BackgroundWorkerTests.cs
@@ -1,6 +1,7 @@
 using System.Collections.Concurrent;
 using System.Diagnostics;
 using NSubstitute.ExceptionExtensions;
+using Sentry.Internal.Http;
 using Sentry.Testing;
 
 namespace Sentry.Tests.Internals;
@@ -41,6 +42,7 @@ public Fixture(ITestOutputHelper outputHelper)
                     return token.IsCancellationRequested ? Task.FromCanceled(token) : Task.CompletedTask;
                 });
 
+            SentryOptions.Dsn = ValidDsn;
             SentryOptions.Debug = true;
             SentryOptions.DiagnosticLogger = Logger;
             SentryOptions.ClientReportRecorder = ClientReportRecorder;
@@ -319,14 +321,6 @@ public async Task FlushAsync_DisposedWorker_LogsAndReturns()
         _fixture.Logger.Received(1).Log(SentryLevel.Debug, "Worker disposed. Nothing to flush.");
     }
 
-    [Fact]
-    public async Task FlushAsync_EmptyQueue_LogsAndReturns()
-    {
-        using var sut = _fixture.GetSut();
-        await sut.FlushAsync(TimeSpan.MaxValue);
-        _fixture.Logger.Received(1).Log(SentryLevel.Debug, "No events to flush.");
-    }
-
     [Fact]
     public async Task FlushAsync_SingleEvent_FlushReturnsAfterEventSent()
     {
@@ -469,4 +463,31 @@ await _fixture.Transport.Received(1).SendEnvelopeAsync(
             Arg.Is<Envelope>(e => e.Items.Count == 1 && e.Items[0].TryGetType() == "client_report"),
             Arg.Any<CancellationToken>());
     }
+
+    [Fact]
+    public async Task FlushAsync_Calls_CachingTransport_FlushAsync()
+    {
+        // Arrange
+        var fileSystem = new FakeFileSystem();
+        using var tempDir = new TempDirectory(fileSystem);
+
+        var options = _fixture.SentryOptions;
+        options.FileSystem = fileSystem;
+        options.CacheDirectoryPath = tempDir.Path;
+
+        var innerTransport = _fixture.Transport;
+        _fixture.Transport = CachingTransport.Create(innerTransport, options, startWorker: false);
+
+        using var sut = _fixture.GetSut();
+        var envelope = Envelope.FromEvent(new SentryEvent());
+
+        // Act
+        sut.EnqueueEnvelope(envelope, process: false);
+        sut.ProcessQueuedItems(1);
+        await sut.FlushAsync(Timeout.InfiniteTimeSpan);
+
+        // Assert
+        _fixture.Logger.Received(1)
+            .Log(SentryLevel.Debug, "External FlushAsync invocation: flushing cached envelopes.");
+    }
 }