Ensure cancelation works properly in PollingNetworkStatusListener (#3506

)
getsentry · Jul 31, 2024 · bf18a3a · bf18a3a
1 parent 43e28ad
commit bf18a3a
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 14 deletions.
diff --git a/src/Sentry/Internal/PollingNetworkStatusListener.cs b/src/Sentry/Internal/PollingNetworkStatusListener.cs
@@ -54,16 +54,23 @@ public async Task WaitForNetworkOnlineAsync(CancellationToken cancellationToken
     {
         while (!cancellationToken.IsCancellationRequested)
         {
-            await Task.Delay(_delayInMilliseconds, cancellationToken).ConfigureAwait(false);
-            var checkResult = await Ping.IsAvailableAsync().ConfigureAwait(false);
-            if (checkResult)
+            try
             {
-                Online = true;
-                return;
+                await Task.Delay(_delayInMilliseconds, cancellationToken).ConfigureAwait(false);
+                var checkResult = await Ping.IsAvailableAsync(cancellationToken).ConfigureAwait(false);
+                if (checkResult)
+                {
+                    Online = true;
+                    return;
+                }
+                if (_delayInMilliseconds < _maxDelayInMilliseconds)
+                {
+                    _delayInMilliseconds = _backoffFunction(_delayInMilliseconds);
+                }
             }
-            if (_delayInMilliseconds < _maxDelayInMilliseconds)
+            catch (OperationCanceledException)
             {
-                _delayInMilliseconds = _backoffFunction(_delayInMilliseconds);
+                break;
             }
         }
     }

diff --git a/src/Sentry/Internal/TcpPing.cs b/src/Sentry/Internal/TcpPing.cs
@@ -2,21 +2,29 @@ namespace Sentry.Internal;
 
 internal interface IPing
 {
-    Task<bool> IsAvailableAsync();
+    Task<bool> IsAvailableAsync(CancellationToken cancellationToken);
 }
 
 internal class TcpPing(string hostToCheck, int portToCheck = 443) : IPing
 {
     private readonly Ping _ping = new();
 
-    public async Task<bool> IsAvailableAsync()
+    public async Task<bool> IsAvailableAsync(CancellationToken cancellationToken)
     {
         try
         {
             using var tcpClient = new TcpClient();
+#if NET5_0_OR_GREATER
+            await tcpClient.ConnectAsync(hostToCheck, portToCheck, cancellationToken).ConfigureAwait(false);
+#else
             await tcpClient.ConnectAsync(hostToCheck, portToCheck).ConfigureAwait(false);
+#endif
             return true;
         }
+        catch (OperationCanceledException)
+        {
+            throw;
+        }
         catch (Exception)
         {
             return false;

diff --git a/test/Sentry.Tests/Internals/Http/CachingTransportTests.cs b/test/Sentry.Tests/Internals/Http/CachingTransportTests.cs
@@ -548,7 +548,7 @@ public async Task TestNetworkException(Exception exception)
         // Arrange - network unavailable
         using var cacheDirectory = new TempDirectory(_fileSystem);
         var pingHost = Substitute.For<IPing>();
-        pingHost.IsAvailableAsync().Returns(Task.FromResult(true));
+        pingHost.IsAvailableAsync(Arg.Any<CancellationToken>()).Returns(Task.FromResult(true));
         var options = new SentryOptions
         {
             Dsn = ValidDsn,

diff --git a/test/Sentry.Tests/Internals/PollingNetworkStatusListenerTests.cs b/test/Sentry.Tests/Internals/PollingNetworkStatusListenerTests.cs
@@ -9,7 +9,7 @@ public async Task HostAvailable_CheckOnlyRunsOnce()
         var initialDelay = 100;
         var pingHost = Substitute.For<IPing>();
         pingHost
-            .IsAvailableAsync()
+            .IsAvailableAsync(Arg.Any<CancellationToken>())
             .Returns(Task.FromResult(true));
 
         var pollingListener = new PollingNetworkStatusListener(pingHost, initialDelay);
@@ -23,7 +23,7 @@ public async Task HostAvailable_CheckOnlyRunsOnce()
         // Assert
         completedTask.Should().Be(waitForNetwork);
         pollingListener.Online.Should().Be(true);
-        await pingHost.Received(1).IsAvailableAsync();
+        await pingHost.Received(1).IsAvailableAsync(Arg.Any<CancellationToken>());
     }
 
     [Fact]
@@ -33,7 +33,7 @@ public async Task HostUnavailable_ShouldIncreaseDelay()
         var initialDelay = 100; // set initial delay to ease the testing
         var pingHost = Substitute.For<IPing>();
         pingHost
-            .IsAvailableAsync()
+            .IsAvailableAsync(Arg.Any<CancellationToken>())
             .Returns(Task.FromResult(false));
 
         var pollingListener = new PollingNetworkStatusListener(pingHost, initialDelay);
@@ -47,7 +47,35 @@ public async Task HostUnavailable_ShouldIncreaseDelay()
         // Assert
         completedTask.Should().Be(timeout);
         pollingListener.Online.Should().Be(false);
-        await pingHost.Received().IsAvailableAsync();
+        await pingHost.Received().IsAvailableAsync(Arg.Any<CancellationToken>());
         pollingListener._delayInMilliseconds.Should().BeGreaterThan(initialDelay);
     }
+
+    [Fact]
+    public async Task OperationCancelled_ShouldExitGracefully()
+    {
+        // Arrange
+        const int initialDelay = 10_000;
+        var pingHost = Substitute.For<IPing>();
+        pingHost
+            .IsAvailableAsync(Arg.Any<CancellationToken>())
+            .Returns(Task.FromResult(false));
+
+        var pollingListener = new PollingNetworkStatusListener(pingHost, initialDelay)
+        {
+            Online = false
+        };
+        var cts = new CancellationTokenSource();
+
+        // Act
+        var waitForNetwork = pollingListener.WaitForNetworkOnlineAsync(cts.Token);
+        var timeout = Task.Delay(2000);
+        cts.CancelAfter(100);
+        var completedTask = await Task.WhenAny(waitForNetwork, timeout);
+
+        // Assert
+        completedTask.Should().Be(waitForNetwork);
+        pollingListener.Online.Should().Be(false);
+        await completedTask; // Throws exception if the task is faulted
+    }
 }