dh: Fix ExponentialBackoffWithJitterRetryOnSignalHandler for handled …

…ex on last attempt
rmandvikar · Sep 26, 2023 · 59895fd · 59895fd
1 parent 5e1fde4
commit 59895fd
Show file tree

Hide file tree

Showing 2 changed files with 81 additions and 5 deletions.
diff --git a/src/rm.DelegatingHandlers/ExponentialBackoffWithJitterRetryOnSignalHandler.cs b/src/rm.DelegatingHandlers/ExponentialBackoffWithJitterRetryOnSignalHandler.cs
@@ -20,12 +20,13 @@ namespace rm.DelegatingHandlers;
 public class ExponentialBackoffWithJitterRetryOnSignalHandler : DelegatingHandler
 {
 	private readonly AsyncRetryPolicy<(HttpRequestMessage request, HttpResponseMessage response)> retryPolicy;
+	private readonly IRetrySettings retrySettings;
 
 	/// <inheritdoc cref="ExponentialBackoffWithJitterRetryOnSignalHandler" />
 	public ExponentialBackoffWithJitterRetryOnSignalHandler(
 		IRetrySettings retrySettings)
 	{
-		_ = retrySettings
+		this.retrySettings = retrySettings
 			?? throw new ArgumentNullException(nameof(retrySettings));
 
 		var sleepDurationsWithJitter = Backoff.DecorrelatedJitterBackoffV2(
@@ -68,6 +69,10 @@ protected override async Task<HttpResponseMessage> SendAsync(
 				catch (Exception)
 					when (request.Properties.TryGetValue(RequestProperties.RetrySignal, out var retrySignaledObj) && (bool)retrySignaledObj)
 				{
+					if (retrySettings.RetryCount == (int)retryAttempt)
+					{
+						throw;
+					}
 					// swallow if retry signaled on ex
 					return (request, null);
 				}

diff --git a/tests/rm.DelegatingHandlersTest/ExponentialBackoffWithJitterRetryOnSignalHandlerTests.cs b/tests/rm.DelegatingHandlersTest/ExponentialBackoffWithJitterRetryOnSignalHandlerTests.cs
@@ -17,10 +17,8 @@ public async Task Retries_On_Signal()
 		var shortCircuitingCannedActionsHandler = new ShortCircuitingCannedActionsHandler(
 			(request) => new HttpResponseMessage() { StatusCode = (HttpStatusCode)404 }, // retry
 			(request) => new HttpResponseMessage() { StatusCode = (HttpStatusCode)200, Content = new StringContent("yawn!") }, // retry
-			(request) => throw new TaskCanceledException("timeout!"), // retry
-			(request) => new HttpResponseMessage() { StatusCode = (HttpStatusCode)200 }, // NO retry
-			(request) => new HttpResponseMessage() { StatusCode = (HttpStatusCode)200 }, // not used
-			(request) => new HttpResponseMessage() { StatusCode = (HttpStatusCode)200 }  // not used
+			(request) => throw new TaskCanceledException("timeout!"),                    // retry
+			(request) => new HttpResponseMessage() { StatusCode = (HttpStatusCode)200 }  // NO retry
 			);
 		var retrySignalingOnConditionHandler = new RetrySignalingOnConditionHandler();
 		var retryAttempt = -1;
@@ -46,6 +44,79 @@ public async Task Retries_On_Signal()
 		Assert.AreEqual(3, retryAttempt);
 	}
 
+	[Test]
+	public void Retries_On_Signal_Last_Attempt_In_Exception()
+	{
+		var fixture = new Fixture().Customize(new AutoMoqCustomization());
+
+		var shortCircuitingCannedActionsHandler = new ShortCircuitingCannedActionsHandler(
+			(request) => new HttpResponseMessage() { StatusCode = (HttpStatusCode)404 }, // retry
+			(request) => new HttpResponseMessage() { StatusCode = (HttpStatusCode)200, Content = new StringContent("yawn!") }, // retry
+			(request) => throw new TaskCanceledException("timeout!"), // retry
+			(request) => throw new TaskCanceledException("timeout!"), // retry
+			(request) => throw new TaskCanceledException("timeout!"), // retry
+			(request) => throw new TaskCanceledException("timeout!")  // last attempt
+			);
+		var retrySignalingOnConditionHandler = new RetrySignalingOnConditionHandler();
+		var retryAttempt = -1;
+		var delegateHandler = new DelegateHandler(
+			(request, ct) =>
+			{
+				retryAttempt++;
+				return Task.CompletedTask;
+			});
+		var retryHandler = new ExponentialBackoffWithJitterRetryOnSignalHandler(
+			new RetrySettings
+			{
+				RetryCount = 5,
+				RetryDelayInMilliseconds = 0,
+			});
+
+		using var invoker = HttpMessageInvokerFactory.Create(
+			retryHandler, delegateHandler, retrySignalingOnConditionHandler, shortCircuitingCannedActionsHandler);
+
+		using var requestMessage = fixture.Create<HttpRequestMessage>();
+		Assert.ThrowsAsync<TaskCanceledException>(async () =>
+		{
+			using var _ = await invoker.SendAsync(requestMessage, CancellationToken.None);
+		});
+		Assert.AreEqual(5, retryAttempt);
+	}
+
+	[Test]
+	public void No_Retries_On_Signal_When_Unhandled_In_Exception()
+	{
+		var fixture = new Fixture().Customize(new AutoMoqCustomization());
+
+		var shortCircuitingCannedActionsHandler = new ShortCircuitingCannedActionsHandler(
+			(request) => throw new TurnDownForWhatException()         // unhandled
+			);
+		var retrySignalingOnConditionHandler = new RetrySignalingOnConditionHandler();
+		var retryAttempt = -1;
+		var delegateHandler = new DelegateHandler(
+			(request, ct) =>
+			{
+				retryAttempt++;
+				return Task.CompletedTask;
+			});
+		var retryHandler = new ExponentialBackoffWithJitterRetryOnSignalHandler(
+			new RetrySettings
+			{
+				RetryCount = 5,
+				RetryDelayInMilliseconds = 0,
+			});
+
+		using var invoker = HttpMessageInvokerFactory.Create(
+			retryHandler, delegateHandler, retrySignalingOnConditionHandler, shortCircuitingCannedActionsHandler);
+
+		using var requestMessage = fixture.Create<HttpRequestMessage>();
+		Assert.ThrowsAsync<TurnDownForWhatException>(async () =>
+		{
+			using var _ = await invoker.SendAsync(requestMessage, CancellationToken.None);
+		});
+		Assert.AreEqual(0, retryAttempt);
+	}
+
 	[Test]
 	public async Task Does_Not_Retry_If_No_Signal()
 	{