dh: Teach Retry handler to honor retry-after header

src/rm.DelegatingHandlers/ExponentialBackoffWithJitterRetryHandler.cs

@@ -1,49 +1,62 @@
 using System;
+using System.Linq;
 using System.Net.Http;
 using System.Threading;
 using System.Threading.Tasks;
 using Polly;
 using Polly.Contrib.WaitAndRetry;
 using Polly.Retry;
+using rm.Clock;
+using rm.Extensions;
 
 namespace rm.DelegatingHandlers;
 
 /// <summary>
 /// Retries on certain conditions with exponential backoff jitter (DecorrelatedJitterBackoffV2).
 /// <para></para>
 /// Retry conditions:
-///   HttpRequestException, 5xx.
+///   HttpRequestException, 5xx, 429 (see retry-after header below).
+///   <br/>
+/// retry-after header:
+///   <br/>
+///   For 503: retry honoring header if present, else retry as usual.
+///   <br/>
+///   For 429: retry honoring header only if present, else do not retry.
 /// </summary>
 /// <remarks>
 /// <see href="https://github.com/App-vNext/Polly/wiki/Retry-with-jitter">source</see>
 /// </remarks>
 public class ExponentialBackoffWithJitterRetryHandler : DelegatingHandler
 {
-	private readonly AsyncRetryPolicy<HttpResponseMessage> retryPolicy;
+	private readonly AsyncRetryPolicy<(HttpResponseMessage response, Context Context)> retryPolicy;
+	private readonly IRetrySettings retrySettings;
+	private readonly ISystemClock clock;
 
 	/// <inheritdoc cref="ExponentialBackoffWithJitterRetryHandler" />
 	public ExponentialBackoffWithJitterRetryHandler(
-		IRetrySettings retrySettings)
+		IRetrySettings retrySettings,
+		ISystemClock clock)
 	{
-		_ = retrySettings
+		this.retrySettings = retrySettings
 			?? throw new ArgumentNullException(nameof(retrySettings));
-
-		var sleepDurationsWithJitter = Backoff.DecorrelatedJitterBackoffV2(
-			medianFirstRetryDelay: TimeSpan.FromMilliseconds(retrySettings.RetryDelayInMilliseconds),
-			retryCount: retrySettings.RetryCount);
+		this.clock = clock
+			?? throw new ArgumentNullException(nameof(clock));
 
 		// note: response can't be null
 		// ref: https://github.com/dotnet/runtime/issues/19925#issuecomment-272664671
 		retryPolicy = Policy
 			.Handle<HttpRequestException>()
 			.Or<TimeoutExpiredException>()
-			.OrResult<HttpResponseMessage>(response => response.Is5xx())
+			.OrResult<(HttpResponseMessage response, Context context)>(
+				tuple => CanRetry(tuple.response, tuple.context))
 			.WaitAndRetryAsync(
-				sleepDurations: sleepDurationsWithJitter,
+				retryCount: retrySettings.RetryCount,
+				sleepDurationProvider: (retryAttempt, responseResult, context) =>
+					((TimeSpan[])context[ContextKey.SleepDurations])[retryAttempt - 1],
 				onRetry: (responseResult, delay, retryAttempt, context) =>
 				{
 					// note: response can be null in case of handled exception
-					responseResult.Result?.Dispose();
+					responseResult.Result.response?.Dispose();
 					context[ContextKey.RetryAttempt] = retryAttempt;
 				});
 	}
@@ -52,19 +65,112 @@ protected override async Task<HttpResponseMessage> SendAsync(
 		HttpRequestMessage request,
 		CancellationToken cancellationToken)
 	{
-		return await retryPolicy.ExecuteAsync(
+		// read the retry delays upfront
+		var sleepDurationsWithJitter = Backoff.DecorrelatedJitterBackoffV2(
+			medianFirstRetryDelay: TimeSpan.FromMilliseconds(retrySettings.RetryDelayInMilliseconds),
+			retryCount: retrySettings.RetryCount).ToArray();
+		var context = new Context();
+		context[ContextKey.SleepDurations] = sleepDurationsWithJitter;
+
+		var tuple = await retryPolicy.ExecuteAsync(
 			action: async (context, ct) =>
 			{
 				if (context.TryGetValue(ContextKey.RetryAttempt, out var retryAttempt))
 				{
 					request.Properties[RequestProperties.PollyRetryAttempt] = retryAttempt;
 				}
-				return await base.SendAsync(request, ct)
+				var response = await base.SendAsync(request, ct)
 					.ConfigureAwait(false);
+				return (response, context);
 			},
-			context: new Context(),
+			context: context,
 			cancellationToken: cancellationToken)
 				.ConfigureAwait(false);
+		return tuple.response;
+	}
+
+	/// <summary>
+	/// Returns true if the response can be retried considering things as,
+	/// retry attempt, and retry-after header (if present).
+	/// </summary>
+	private bool CanRetry(
+		HttpResponseMessage response,
+		Context context)
+	{
+		var sleepDurationsWithJitter = (TimeSpan[])context[ContextKey.SleepDurations];
+		if (sleepDurationsWithJitter.IsEmpty())
+		{
+			return false;
+		}
+		// retryAttempt is 0-based
+		var retryAttempt = context.TryGetValue(ContextKey.RetryAttempt, out object retryAttemptObj) ? (int)retryAttemptObj : 0;
+		if (retryAttempt == sleepDurationsWithJitter.Count())
+		{
+			return false;
+		}
+		var sleepDurationWithJitter = sleepDurationsWithJitter[retryAttempt];
+
+		var statusCode = (int)response.StatusCode;
+
+		// here be dragons
+		var retry = false;
+		// retry on 5xx, 429 only
+		if (response.Is5xx() || statusCode == 429)
+		{
+			// retry on 503, 429 looking at retry-after value
+			if (statusCode == 503 || statusCode == 429)
+			{
+				// note: look at retry-after value but don't use it to avoid surges at same time;
+				// use it to determine whether to retry or not
+				var isRetryAfterPresent = response.Headers.TryGetValue(ResponseHeaders.RetryAfter, out var retryAfterValue)
+					&& retryAfterValue != null;
+
+#if DEBUG
+				Console.WriteLine($"retryAfterValue: {retryAfterValue}");
+#endif
+
+				// retry on 503, 429 only on valid retry-after value
+				if (isRetryAfterPresent)
+				{
+					TimeSpan retryAfter;
+					retry =
+						((double.TryParse(retryAfterValue, out double retryAfterDelay)
+							// ignore network latency; delay could be 0
+							&& Math.Max((retryAfter = TimeSpan.FromSeconds(retryAfterDelay)).TotalSeconds, 0) >= 0)
+						||
+						(DateTimeOffset.TryParse(retryAfterValue, out DateTimeOffset retryAfterDate)
+							// date could be in the past due to network latency
+							&& Math.Max((retryAfter = retryAfterDate - clock.UtcNow).TotalSeconds, 0) >= 0))
+						// only retry if delay is at or above retry-after value
+						&& retryAfter <= sleepDurationWithJitter;
+				}
+				else
+				{
+					// retry on 503 if retry-after not present as typical
+					if (statusCode == 503)
+					{
+						retry = true;
+					}
+					// do NOT retry on 429 if retry-after not present as typical
+					else if (statusCode == 429)
+					{
+						retry = false;
+					}
+				}
+			}
+			else
+			{
+				// retry on 5xx (other than 503) as typical
+				retry = true;
+			}
+		}
+
+#if DEBUG
+		Console.WriteLine($"sleepDurationWithJitter: {sleepDurationWithJitter}");
+		Console.WriteLine($"retry: {retry}");
+#endif
+
+		return retry;
 	}
 }
 
@@ -83,4 +189,5 @@ public record class RetrySettings : IRetrySettings
 internal static class ContextKey
 {
 	internal const string RetryAttempt = nameof(RetryAttempt);
+	internal const string SleepDurations = nameof(SleepDurations);
 }

src/rm.DelegatingHandlers/misc/AsyncRetryTResultSyntax.cs

@@ -0,0 +1,41 @@
+using System;
+using Polly.Retry;
+
+namespace Polly;
+
+public static class AsyncRetryTResultSyntax
+{
+	/// <summary>
+	///     Builds an <see cref="AsyncRetryPolicy{TResult}" /> that will wait and retry <paramref name="retryCount" /> times
+	///     calling <paramref name="onRetry" /> on each retry with the handled exception or result, the current sleep duration, retry count, and context data.
+	///     On each retry, the duration to wait is calculated by calling <paramref name="sleepDurationProvider" /> with
+	///     the current retry number (1 for first retry, 2 for second etc), result of previous execution, and execution context.
+	/// </summary>
+	/// <param name="policyBuilder">The policy builder.</param>
+	/// <param name="retryCount">The retry count.</param>
+	/// <param name="sleepDurationProvider">The function that provides the duration to wait for for a particular retry attempt.</param>
+	/// <param name="onRetry">The action to call on each retry.</param>
+	/// <returns>The policy instance.</returns>
+	/// <exception cref="ArgumentOutOfRangeException">retryCount;Value must be greater than or equal to zero.</exception>
+	/// <exception cref="ArgumentNullException">
+	///     sleepDurationProvider
+	///     or
+	///     onRetryAsync
+	/// </exception>
+	/// <note>
+	/// issue: https://github.com/App-vNext/Polly/issues/908
+	/// </note>
+	public static AsyncRetryPolicy<TResult> WaitAndRetryAsync<TResult>(this PolicyBuilder<TResult> policyBuilder, int retryCount,
+		Func<int, DelegateResult<TResult>, Context, TimeSpan> sleepDurationProvider, Action<DelegateResult<TResult>, TimeSpan, int, Context> onRetry)
+	{
+		if (onRetry == null) throw new ArgumentNullException(nameof(onRetry));
+
+		return policyBuilder.WaitAndRetryAsync(
+			retryCount,
+			sleepDurationProvider,
+#pragma warning disable 1998 // async method has no awaits, will run synchronously
+			onRetryAsync: async (outcome, timespan, i, ctx) => onRetry(outcome, timespan, i, ctx)
+#pragma warning restore 1998
+		);
+	}
+}

src/rm.DelegatingHandlers/misc/HttpStatusCodeIntExtensions.cs

@@ -0,0 +1,55 @@
+using System.Net;
+
+namespace rm.DelegatingHandlers;
+
+public static class HttpStatusCodeIntExtensions
+{
+	public static bool Is1xx(this int statusCode)
+	{
+		return ((HttpStatusCode)statusCode).Is1xx();
+	}
+
+	public static bool Is2xx(this int statusCode)
+	{
+		return ((HttpStatusCode)statusCode).Is2xx();
+	}
+
+	public static bool Is3xx(this int statusCode)
+	{
+		return ((HttpStatusCode)statusCode).Is3xx();
+	}
+
+	public static bool Is4xx(this int statusCode)
+	{
+		return ((HttpStatusCode)statusCode).Is4xx();
+	}
+
+	public static bool Is5xx(this int statusCode)
+	{
+		return ((HttpStatusCode)statusCode).Is5xx();
+	}
+
+	/// <summary>
+	/// Returns true if status code is a client error status code (4xx).
+	/// </summary>
+	public static bool IsClientErrorStatusCode(this int statusCode)
+	{
+		return statusCode.Is4xx();
+	}
+
+	/// <summary>
+	/// Returns true if status code is a server error status code (5xx).
+	/// </summary>
+	public static bool IsServerErrorStatusCode(this int statusCode)
+	{
+		return statusCode.Is5xx();
+	}
+
+	/// <summary>
+	/// Returns true if status code is an error status code (4xx, 5xx).
+	/// </summary>
+	public static bool IsErrorStatusCode(this int statusCode)
+	{
+		return statusCode.Is4xx() || statusCode.Is5xx();
+	}
+}

src/rm.DelegatingHandlers/rm.DelegatingHandlers.csproj

@@ -23,6 +23,7 @@
     <PackageReference Include="Microsoft.Extensions.Primitives" Version="2.1.6" />
     <PackageReference Include="Polly" Version="7.2.2" />
     <PackageReference Include="Polly.Contrib.WaitAndRetry" Version="1.1.1" />
+    <PackageReference Include="rm.Clock" Version="1.0.1" />
     <PackageReference Include="rm.Extensions" Version="2.8.2" />
     <PackageReference Include="rm.FeatureToggle" Version="1.1.2" />
     <PackageReference Include="rm.Random2" Version="3.0.1" />