Minor fix for CancellationToken handling between retries

src/GeneralTools/DataverseClient/Client/ConnectionService.cs

@@ -2383,7 +2383,7 @@ internal async Task<HttpResponseMessage> Command_WebExecuteAsync(string queryStr
                         retry = ShouldRetryWebAPI(ex, retryCount, maxRetryCount, retryPauseTime, out isThrottled);
                         if (retry)
                         {
-                            retryCount = await Utilities.RetryRequest(null, requestTrackingId, TimeSpan.Zero, logDt, logEntry, SessionTrackingId, disableConnectionLocking, _retryPauseTimeRunning, ex, errorStringCheck, retryCount, isThrottled, webUriReq: $"{method} {queryString}").ConfigureAwait(false);
+                            retryCount = await Utilities.RetryRequest(null, requestTrackingId, TimeSpan.Zero, logDt, logEntry, SessionTrackingId, disableConnectionLocking, _retryPauseTimeRunning, ex, errorStringCheck, retryCount, isThrottled, webUriReq: $"{method} {queryString}", cancellationToken).ConfigureAwait(false);
                         }
                         else
                         {

src/GeneralTools/DataverseClient/Client/ServiceClient.cs

@@ -1876,17 +1876,23 @@ internal async Task<OrganizationResponse> Command_ExecuteAsyncImpl(OrganizationR
                 catch (Exception ex)
                 {
                     bool isThrottled = false;
-                    retry = ShouldRetry(req, ex, retryCount, out isThrottled);
+                    retry = ShouldRetry(req, ex, retryCount, out isThrottled) || !cancellationToken.IsCancellationRequested;
                     if (retry)
                     {
-                        retryCount = await Utilities.RetryRequest(req, requestTrackingId, LockWait, logDt, _logEntry, SessionTrackingId, _disableConnectionLocking, _retryPauseTimeRunning, ex, errorStringCheck, retryCount, isThrottled).ConfigureAwait(false);
+                        retryCount = await Utilities.RetryRequest(req, requestTrackingId, LockWait, logDt, _logEntry, SessionTrackingId, _disableConnectionLocking, _retryPauseTimeRunning, ex, errorStringCheck, retryCount, isThrottled, cancellationToken: cancellationToken).ConfigureAwait(false);
                     }
                     else
                     {
                         _logEntry.LogRetry(retryCount, req, _retryPauseTimeRunning, true, isThrottled: isThrottled);
                         _logEntry.LogException(req, ex, errorStringCheck);
                         //keep it in end so that LastError could be a better message.
                         _logEntry.LogFailure(req, requestTrackingId, SessionTrackingId, _disableConnectionLocking, LockWait, logDt, ex, errorStringCheck, true);
+
+                        // Callers which cancel should expect to handle a OperationCanceledException
+                        if (ex is OperationCanceledException)
+                            throw;
+                        else
+                            cancellationToken.ThrowIfCancellationRequested();
                     }
                     resp = null;
                 }
@@ -2022,6 +2028,8 @@ private bool ShouldRetry(OrganizationRequest req, Exception ex, int retryCount,
             isThrottlingRetry = false;
             if (retryCount >= _configuration.Value.MaxRetryCount)
                 return false;
+            else if (ex is OperationCanceledException)
+                return false;
             else if (((string.Equals(req.RequestName.ToLower(), "retrieve"))
                 && ((Utilities.ShouldAutoRetryRetrieveByEntityName(((Microsoft.Xrm.Sdk.EntityReference)req.Parameters["Target"]).LogicalName))))
                 || (string.Equals(req.RequestName.ToLower(), "retrievemultiple")
@@ -2047,10 +2055,10 @@ private bool ShouldRetry(OrganizationRequest req, Exception ex, int retryCount,
                     OrgEx.Detail.ErrorCode == ErrorCodes.ThrottlingTimeExceededError ||
                     OrgEx.Detail.ErrorCode == ErrorCodes.ThrottlingConcurrencyLimitExceededError)
                 {
-                     // Use Retry-After delay when specified
+                    // Use Retry-After delay when specified
                     if (OrgEx.Detail.ErrorDetails.TryGetValue("Retry-After", out var retryAfter) && retryAfter is TimeSpan retryAsTimeSpan)
-                    { 
-                         _retryPauseTimeRunning = retryAsTimeSpan;
+                    {
+                        _retryPauseTimeRunning = retryAsTimeSpan;
                     }
                     else
                     {

src/GeneralTools/DataverseClient/Client/Utils/Utils.cs

@@ -19,6 +19,7 @@
 using System.Net.Http;
 using System.Reflection;
 using System.Text;
+using System.Threading;
 using System.Threading.Tasks;
 #endregion
 
@@ -445,14 +446,20 @@ internal static string ConstructWebApiRequestUrl(OrganizationRequest request, Ht
         /// <param name="retryCount">retryCount</param>
         /// <param name="isThrottled">when set indicated this was caused by a Throttle</param>
         /// <param name="webUriReq"></param>
+        /// <param name="cancellationToken"></param>
         internal static async Task<int> RetryRequest(OrganizationRequest req, Guid requestTrackingId, TimeSpan LockWait, Stopwatch logDt,
             DataverseTraceLogger logEntry, Guid? sessionTrackingId, bool disableConnectionLocking, TimeSpan retryPauseTimeRunning,
-            Exception ex, string errorStringCheck, int retryCount, bool isThrottled, string webUriReq = "")
+            Exception ex, string errorStringCheck, int retryCount, bool isThrottled, string webUriReq = "", CancellationToken cancellationToken = default)
         {
             retryCount++;
             logEntry.LogFailure(req, requestTrackingId, sessionTrackingId, disableConnectionLocking, LockWait, logDt, ex, errorStringCheck, webUriMessageReq: webUriReq);
             logEntry.LogRetry(retryCount, req, retryPauseTimeRunning, isThrottled: isThrottled);
-            await Task.Delay(retryPauseTimeRunning).ConfigureAwait(false);
+
+            // Task.Delay returns early if the CancellationToken is cancelled, but throws a TaskCancelledException if
+            // we pass it an already-cancelled CancellationToken.  If case that occurs, skip the call for consistent behavior.
+            if (!cancellationToken.IsCancellationRequested) 
+                await Task.Delay(retryPauseTimeRunning, cancellationToken).ConfigureAwait(false);
+
             return retryCount;
         }
 

src/GeneralTools/DataverseClient/UnitTests/CdsClient_Core_Tests/ServiceClientTests.cs

@@ -26,6 +26,7 @@
 using System.Net.Http;
 using System.Runtime.CompilerServices;
 using System.Security;
+using System.Threading;
 using System.Threading.Tasks;
 using Xunit;
 using Xunit.Abstractions;
@@ -104,6 +105,23 @@ public void TestThrowDisposedOperationCheck()
             });
         }
 
+        [Fact]
+        public async Task ThrowsOperationCanceledExceptionWhenCancelled()
+        {
+            testSupport.SetupMockAndSupport(out var orgSvc, out var fakHttpMethodHander, out var cli);
+            var cts = new CancellationTokenSource();
+
+            var cancelledToken = cts.Token;
+            cts.Cancel();
+
+            Func<Task> cancelledRequest = async() =>
+            {
+                await cli.ExecuteAsync(new WhoAmIRequest(), cts.Token).ConfigureAwait(false);
+            };
+
+            await cancelledRequest.Should().ThrowAsync<OperationCanceledException>().ConfigureAwait(false);
+        }
+
         [Fact]
         public void ExecuteMessageTests()
         {
@@ -807,6 +825,65 @@ public async void RetryOperationAyncTest()
             Assert.True(retrycount == 2);
         }
 
+        [Fact]
+        public async Task RetryOperationCancelledDuringDelayTest()
+        {
+            testSupport.SetupMockAndSupport(out var orgSvc, out var fakHttpMethodHander, out var cli);
+
+            Guid testGuid = Guid.NewGuid();
+
+            CreateRequest exampleRequest = new CreateRequest();
+            exampleRequest.Target = new Entity("account");
+            exampleRequest.Target.Attributes.Add("id", testGuid);
+
+            Stopwatch testwatch = Stopwatch.StartNew();
+
+            var cts = new CancellationTokenSource();
+            var cancellationToken = cts.Token;
+
+            var delay = TimeSpan.FromSeconds(5);
+
+            var retryTask = Task.Run(async () =>
+            {
+                await Utilities.RetryRequest(exampleRequest, testGuid, new TimeSpan(0), testwatch, cli._logEntry, null, false, delay, new Exception("Fake_TEST_MSG"), "test retry logic", 0, false, null, cancellationToken: cancellationToken).ConfigureAwait(false);
+            });
+
+            await Task.Delay(TimeSpan.FromMilliseconds(100)).ConfigureAwait(false);
+            cts.Cancel();
+            await Task.Delay(TimeSpan.FromMilliseconds(50)).ConfigureAwait(false);
+
+            retryTask.IsCompleted.Should().BeTrue("Task.Delay within Utilities.RetryRequest should just return early, allowing the task to complete");
+            testwatch.Elapsed.Should().BeLessThan(delay, "Task should return before its delay timer can complete due to cancellation");
+        }
+
+        [Fact]
+        public async Task RetryOperationShouldNotThrowWhenAlreadyCanceledTest()
+        {
+            testSupport.SetupMockAndSupport(out var orgSvc, out var fakHttpMethodHander, out var cli);
+
+            Guid testGuid = Guid.NewGuid();
+
+            CreateRequest exampleRequest = new CreateRequest();
+            exampleRequest.Target = new Entity("account");
+            exampleRequest.Target.Attributes.Add("id", testGuid);
+
+            Stopwatch testwatch = Stopwatch.StartNew();
+
+            var cts = new CancellationTokenSource();
+            var cancellationToken = cts.Token;
+            cts.Cancel(); // Cancel before the token is even provided to the method
+
+            var delay = TimeSpan.FromSeconds(5);
+
+            Func<Task> cancelledRetry = async () =>
+            {
+                await Utilities.RetryRequest(exampleRequest, testGuid, new TimeSpan(0), testwatch, cli._logEntry, null, false, delay, new Exception("Fake_TEST_MSG"), "test retry logic", 0, false, null, cancellationToken: cancellationToken).ConfigureAwait(false);
+            };
+
+            await cancelledRetry.Should().NotThrowAsync<TaskCanceledException>().ConfigureAwait(false);
+            testwatch.Elapsed.Should().BeLessThan(delay, "Task should return before its delay timer can complete due to cancellation");
+        }
+
         #region LiveConnectedTests
 
         [SkippableConnectionTest]

src/nuspecs/Microsoft.PowerPlatform.Dataverse.Client.ReleaseNotes.txt

@@ -7,6 +7,9 @@ Notice:
     Note: Only AD on FullFramework, OAuth, Certificate, ClientSecret Authentication types are supported at this time.
 
 ++CURRENTRELEASEID++
+Fix for CancellationToken not canceling retries during delays Git: https://github.com/microsoft/PowerPlatform-DataverseServiceClient/issues/508
+
+1.2.5
 Fix for Null ILogger passed to the AzAuth Client Creators causing a fault.  Git: https://github.com/microsoft/PowerPlatform-DataverseServiceClient/issues/481
 Fix for Memory Leak in Retrieve Multiple. Git: https://github.com/microsoft/PowerPlatform-DataverseServiceClient/issues/463
 Fix for Memory bloat issue caused by not releasing lastErrorMessage property.