Update caching transport and tests (#1617)

Author: mattjohnsonpint

CHANGELOG.md

@@ -10,6 +10,7 @@
 
 - Workaround `System.Text.Json` issue with Unity IL2CPP. ([#1583](https://github.com/getsentry/sentry-dotnet/pull/1583))
 - Demystify stack traces for exceptions that fire in a `BeforeSend` callback. ([#1587](https://github.com/getsentry/sentry-dotnet/pull/1587))
+- Fix a minor issue in the caching transport related to recovery of files from previous session. ([#1617](https://github.com/getsentry/sentry-dotnet/pull/1617))
 
 ## 3.16.0
 

src/Sentry/Internal/Http/CachingTransport.cs

@@ -41,10 +41,10 @@ internal class CachingTransport : ITransport, IAsyncDisposable, IDisposable
         // Inner transport exposed internally primarily for testing
         internal ITransport InnerTransport => _innerTransport;
 
-        public static CachingTransport Create(ITransport innerTransport, SentryOptions options)
+        public static CachingTransport Create(ITransport innerTransport, SentryOptions options, bool startWorker = true)
         {
             var transport = new CachingTransport(innerTransport, options);
-            transport.Initialize();
+            transport.Initialize(startWorker);
             return transport;
         }
 
@@ -64,38 +64,21 @@ private CachingTransport(ITransport innerTransport, SentryOptions options)
             _processingDirectoryPath = Path.Combine(_isolatedCacheDirectoryPath, "__processing");
         }
 
-        private void Initialize()
+        private void Initialize(bool startWorker)
         {
+            // Restore any abandoned files from a previous session
+            MoveUnprocessedFilesBackToCache();
+
+            // Ensure directories exist
             Directory.CreateDirectory(_isolatedCacheDirectoryPath);
             Directory.CreateDirectory(_processingDirectoryPath);
 
-            _worker = Task.Run(CachedTransportBackgroundTaskAsync);
+            // Start a worker, if one is needed
+            _worker = startWorker ? Task.Run(CachedTransportBackgroundTaskAsync) : Task.CompletedTask;
         }
 
         private async Task CachedTransportBackgroundTaskAsync()
         {
-            try
-            {
-                // Processing directory may already contain some files left from previous session
-                // if the worker has been terminated unexpectedly.
-                // Move everything from that directory back to cache directory.
-                if (Directory.Exists(_processingDirectoryPath))
-                {
-                    foreach (var filePath in Directory.EnumerateFiles(_processingDirectoryPath))
-                    {
-                        var destinationPath = Path.Combine(_isolatedCacheDirectoryPath, Path.GetFileName(filePath));
-                        _options.LogDebug("Moving unprocessed file back to cache: {0} to {1}.",
-                            filePath, destinationPath);
-
-                        File.Move(filePath, destinationPath);
-                    }
-                }
-            }
-            catch (Exception e)
-            {
-                _options.LogError("Failed to move unprocessed files back to cache.", e);
-            }
-
             while (!_workerCts.IsCancellationRequested)
             {
                 try
@@ -130,6 +113,34 @@ private async Task CachedTransportBackgroundTaskAsync()
             _options.LogDebug("Background worker of CachingTransport has shutdown.");
         }
 
+        private void MoveUnprocessedFilesBackToCache()
+        {
+            // Processing directory may already contain some files left from previous session
+            // if the cache was working when the process terminated unexpectedly.
+            // Move everything from that directory back to cache directory.
+
+            if (!Directory.Exists(_processingDirectoryPath))
+            {
+                // nothing to do
+                return;
+            }
+
+            foreach (var filePath in Directory.EnumerateFiles(_processingDirectoryPath))
+            {
+                try
+                {
+                    var destinationPath = Path.Combine(_isolatedCacheDirectoryPath, Path.GetFileName(filePath));
+                    _options.LogDebug("Moving unprocessed file back to cache: {0} to {1}.",
+                        filePath, destinationPath);
+                    File.Move(filePath, destinationPath);
+                }
+                catch (Exception e)
+                {
+                    _options.LogError("Failed to move unprocessed file back to cache: {0}", e, filePath);
+                }
+            }
+        }
+
         private void EnsureFreeSpaceInCache()
         {
             // Trim files, leaving only (X - 1) of the newest ones.
@@ -206,6 +217,13 @@ private async Task InnerProcessCacheAsync(string file, CancellationToken cancell
                         // Let the worker catch, log, wait a bit and retry.
                         throw;
                     }
+
+                    if (ex.Source == "FakeFailingTransport")
+                    {
+                        // HACK: Deliberately sent from unit tests to avoid deleting the file from processing
+                        return;
+                    }
+
                     LogFailureWithDiscard(file, ex);
                 }
             }

test/Sentry.Testing/FakeFailingTransport.cs

@@ -6,6 +6,9 @@ public Task SendEnvelopeAsync(
         Envelope envelope,
         CancellationToken cancellationToken = default)
     {
-        throw new Exception("Expected transport failure has occured.");
+        throw new Exception("Expected transport failure has occured.")
+        {
+            Source = nameof(FakeFailingTransport)
+        };
     }
 }

test/Sentry.Testing/Waiter.cs

@@ -0,0 +1,24 @@
+namespace Sentry.Testing;
+
+public sealed class Waiter<T> : IDisposable
+{
+    private readonly TaskCompletionSource<object> _taskCompletionSource = new();
+    private readonly CancellationTokenSource _cancellationTokenSource = new();
+
+    public Waiter(Action<EventHandler<T>> callback)
+    {
+        _cancellationTokenSource.Token.Register(() => _taskCompletionSource.SetCanceled());
+        callback.Invoke((_, _) => _taskCompletionSource.SetResult(null));
+    }
+
+    public async Task WaitAsync(TimeSpan timeout)
+    {
+        _cancellationTokenSource.CancelAfter(timeout);
+        await _taskCompletionSource.Task;
+    }
+
+    public void Dispose()
+    {
+        _cancellationTokenSource.Dispose();
+    }
+}