From a73e643c70ef30cf4ed935f19ebbe973e57e91b3 Mon Sep 17 00:00:00 2001 From: Release-Agent <> Date: Fri, 2 May 2025 19:15:39 +0000 Subject: [PATCH] '' --- .../Client/ConnectionService.cs | 2 +- .../DataverseClient/Client/ServiceClient.cs | 18 +++-- .../DataverseClient/Client/Utils/Utils.cs | 11 ++- .../ServiceClientTests.cs | 77 +++++++++++++++++++ ...Platform.Dataverse.Client.ReleaseNotes.txt | 3 + 5 files changed, 103 insertions(+), 8 deletions(-) diff --git a/src/GeneralTools/DataverseClient/Client/ConnectionService.cs b/src/GeneralTools/DataverseClient/Client/ConnectionService.cs index ef39b09..d870a66 100644 --- a/src/GeneralTools/DataverseClient/Client/ConnectionService.cs +++ b/src/GeneralTools/DataverseClient/Client/ConnectionService.cs @@ -2383,7 +2383,7 @@ internal async Task 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 { diff --git a/src/GeneralTools/DataverseClient/Client/ServiceClient.cs b/src/GeneralTools/DataverseClient/Client/ServiceClient.cs index ab180a8..19f6a53 100644 --- a/src/GeneralTools/DataverseClient/Client/ServiceClient.cs +++ b/src/GeneralTools/DataverseClient/Client/ServiceClient.cs @@ -1876,10 +1876,10 @@ internal async Task 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 { @@ -1887,6 +1887,12 @@ internal async Task Command_ExecuteAsyncImpl(OrganizationR _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 { diff --git a/src/GeneralTools/DataverseClient/Client/Utils/Utils.cs b/src/GeneralTools/DataverseClient/Client/Utils/Utils.cs index e8542b9..4067f91 100644 --- a/src/GeneralTools/DataverseClient/Client/Utils/Utils.cs +++ b/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 /// retryCount /// when set indicated this was caused by a Throttle /// + /// internal static async Task 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; } diff --git a/src/GeneralTools/DataverseClient/UnitTests/CdsClient_Core_Tests/ServiceClientTests.cs b/src/GeneralTools/DataverseClient/UnitTests/CdsClient_Core_Tests/ServiceClientTests.cs index 602801a..4791d8a 100644 --- a/src/GeneralTools/DataverseClient/UnitTests/CdsClient_Core_Tests/ServiceClientTests.cs +++ b/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 cancelledRequest = async() => + { + await cli.ExecuteAsync(new WhoAmIRequest(), cts.Token).ConfigureAwait(false); + }; + + await cancelledRequest.Should().ThrowAsync().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 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().ConfigureAwait(false); + testwatch.Elapsed.Should().BeLessThan(delay, "Task should return before its delay timer can complete due to cancellation"); + } + #region LiveConnectedTests [SkippableConnectionTest] diff --git a/src/nuspecs/Microsoft.PowerPlatform.Dataverse.Client.ReleaseNotes.txt b/src/nuspecs/Microsoft.PowerPlatform.Dataverse.Client.ReleaseNotes.txt index 11fc07a..fd6f3e8 100644 --- a/src/nuspecs/Microsoft.PowerPlatform.Dataverse.Client.ReleaseNotes.txt +++ b/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.