dh: Fix RetryOnSignalHandler for handled ex on last attempt

rmandvikar · Sep 26, 2023 · a0922a8 · a0922a8
1 parent 5e1fde4
commit a0922a8
Show file tree

Hide file tree

Showing 2 changed files with 127 additions and 5 deletions.
diff --git a/src/rm.DelegatingHandlers/ExponentialBackoffWithJitterRetryOnSignalHandler.cs b/src/rm.DelegatingHandlers/ExponentialBackoffWithJitterRetryOnSignalHandler.cs
@@ -51,6 +51,7 @@ protected override async Task<HttpResponseMessage> SendAsync(
 		HttpRequestMessage request,
 		CancellationToken cancellationToken)
 	{
+		Exception exception = null;
 		var tuple = await retryPolicy.ExecuteAsync(
 			action: async (context, ct) =>
 			{
@@ -61,20 +62,27 @@ protected override async Task<HttpResponseMessage> SendAsync(
 				request.Properties.Remove(RequestProperties.RetrySignal);
 				try
 				{
+					exception = null;
 					var response = await base.SendAsync(request, ct)
 						.ConfigureAwait(false);
 					return (request, response);
 				}
-				catch (Exception)
+				catch (Exception ex)
 					when (request.Properties.TryGetValue(RequestProperties.RetrySignal, out var retrySignaledObj) && (bool)retrySignaledObj)
 				{
+					// capture the exception to rethrow on last attempt
+					exception = ex;
 					// swallow if retry signaled on ex
 					return (request, null);
 				}
 			},
 			context: new Context(),
 			cancellationToken: cancellationToken)
 				.ConfigureAwait(false);
+		if (exception != null)
+		{
+			throw exception;
+		}
 		return tuple.response;
 	}
 }
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,122 @@ public async Task Retries_On_Signal()
 		Assert.AreEqual(3, retryAttempt);
 	}
 
+	[Test]
+	[TestCase(0)]
+	[TestCase(5)]
+	public void Retries_On_Signal_All_Attempts_In_Exception(int retryCount)
+	{
+		var fixture = new Fixture().Customize(new AutoMoqCustomization());
+
+		var shortCircuitingCannedActionsHandler = new ShortCircuitingCannedActionsHandler(
+			(request) => throw new TaskCanceledException("timeout!"), // retry
+			(request) => throw new TaskCanceledException("timeout!"), // retry
+			(request) => throw new TaskCanceledException("timeout!"), // retry
+			(request) => throw new TaskCanceledException("timeout!"), // retry
+			(request) => throw new TaskCanceledException("timeout!"), // retry
+			(request) => throw new TaskCanceledException("timeout!")  // retry
+			);
+		var retrySignalingOnConditionHandler = new RetrySignalingOnConditionHandler();
+		var retryAttempt = -1;
+		var delegateHandler = new DelegateHandler(
+			(request, ct) =>
+			{
+				retryAttempt++;
+				return Task.CompletedTask;
+			});
+		var retryHandler = new ExponentialBackoffWithJitterRetryOnSignalHandler(
+			new RetrySettings
+			{
+				RetryCount = retryCount,
+				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(retryCount, 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]
+	[TestCase(0)]
+	[TestCase(5)]
+	public void No_Retries_On_Signal_When_Unhandled_In_Exception(int retryCount)
+	{
+		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 = retryCount,
+				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()
 	{