Skip to content

Implement retry logic for HTTP requests with support for Retry-After header #4636

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
TingluoHuang wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Implement retry logic for HTTP requests with support for Retry-After header #4636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/Runner.Sdk/Util/UrlUtil.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,15 @@ public static string GetVssRequestId(HttpResponseHeaders headers)
}
return string.Empty;
}

public static string GetRetryAfter(HttpResponseHeaders headers)
{
if (headers != null &&
headers.TryGetValues("retry-after", out var headerValues))
Comment thread
TingluoHuang marked this conversation as resolved.
{
Comment thread
TingluoHuang marked this conversation as resolved.
return headerValues.FirstOrDefault();
}
return string.Empty;
}
}
}
14 changes: 14 additions & 0 deletions src/Runner.Worker/ActionManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1653,6 +1653,7 @@ private async Task DownloadRepositoryArchive(IExecutionContext executionContext,
while (retryCount < 3)
{
string requestId = string.Empty;
TimeSpan? retryAfter = null;
using (var actionDownloadTimeout = new CancellationTokenSource(TimeSpan.FromSeconds(timeoutSeconds)))
using (var actionDownloadCancellation = CancellationTokenSource.CreateLinkedTokenSource(actionDownloadTimeout.Token, executionContext.CancellationToken))
{
Expand Down Expand Up @@ -1697,6 +1698,14 @@ private async Task DownloadRepositoryArchive(IExecutionContext executionContext,
}
else
{
if (response.StatusCode == HttpStatusCode.TooManyRequests)
{
// We are being throttled, use the Retry-After header (if provided) to decide backoff time.
// We will back off between 10s and 10min when Retry-After is provided.
var retryAfterHeader = UrlUtil.GetRetryAfter(response.Headers);
retryAfter = VssNetworkHelper.ConvertRetryAfterToTimeSpan(retryAfterHeader, TimeSpan.FromSeconds(10), TimeSpan.FromMinutes(10));
}
Comment on lines +1703 to +1707

// Something else bad happened, let's go to our retry logic
response.EnsureSuccessStatusCode();
}
Expand Down Expand Up @@ -1743,6 +1752,11 @@ private async Task DownloadRepositoryArchive(IExecutionContext executionContext,
if (String.IsNullOrEmpty(Environment.GetEnvironmentVariable("_GITHUB_ACTION_DOWNLOAD_NO_BACKOFF")))
{
var backOff = BackoffTimerHelper.GetRandomBackoff(TimeSpan.FromSeconds(10), TimeSpan.FromSeconds(30));
if (retryAfter.HasValue)
{
// prefer the Retry-After header.
backOff = retryAfter.Value;
}
executionContext.Warning($"Back off {backOff.TotalSeconds} seconds before retry.");
await Task.Delay(backOff);
}
Expand Down
2 changes: 2 additions & 0 deletions src/Sdk/Common/Common/Utility/HttpHeaders.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,7 @@ public static class HttpHeaders
public const string WwwAuthenticate = "WWW-Authenticate";

public const string AfdResponseRef = "X-MSEdge-Ref";

public const string RetryAfter = "Retry-After";
}
}
18 changes: 15 additions & 3 deletions src/Sdk/Common/Common/VssHttpRetryMessageHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ protected override async Task<HttpResponseMessage> SendAsync(
WinHttpErrorCode? winHttpErrorCode = null;
CurlErrorCode? curlErrorCode = null;
string afdRefInfo = null;
string retryAfterHeader = null;
try
{
if (attempt == 1)
Expand All @@ -114,7 +115,8 @@ protected override async Task<HttpResponseMessage> SendAsync(
else
{
statusCode = response.StatusCode;
afdRefInfo = response.Headers.TryGetValues(HttpHeaders.AfdResponseRef, out var headers) ? headers.First() : null;
afdRefInfo = response.Headers.TryGetValues(Internal.HttpHeaders.AfdResponseRef, out var headers) ? headers.First() : null;
retryAfterHeader = response.Headers.TryGetValues(Internal.HttpHeaders.RetryAfter, out var retryAfterValues) ? retryAfterValues.First() : null;
canRetry = m_retryOptions.IsRetryableResponse(response);
Comment thread
TingluoHuang marked this conversation as resolved.
}
}
Expand All @@ -130,7 +132,17 @@ protected override async Task<HttpResponseMessage> SendAsync(

if (attempt < maxAttempts && canRetry)
{
backoff = BackoffTimerHelper.GetExponentialBackoff(attempt, minBackoff, m_retryOptions.MaxBackoff, m_retryOptions.BackoffCoefficient);
// Honor the Retry-After header (delay in seconds or an absolute date/time) when present,
// otherwise fall back to the standard exponential backoff.
var retryAfterDelay = VssNetworkHelper.ConvertRetryAfterToTimeSpan(retryAfterHeader, minBackoff, m_retryOptions.MaxBackoff);
if (retryAfterDelay.HasValue)
{
backoff = retryAfterDelay.Value;
}
else
{
backoff = BackoffTimerHelper.GetExponentialBackoff(attempt, minBackoff, m_retryOptions.MaxBackoff, m_retryOptions.BackoffCoefficient);
}
retryInfo?.Retry(backoff);
TraceHttpRequestRetrying(traceActivity, request, attempt, backoff, statusCode, webExceptionStatus, socketError, winHttpErrorCode, curlErrorCode, afdRefInfo);
}
Expand Down Expand Up @@ -215,7 +227,7 @@ private static bool IsLowPriority(HttpRequestMessage request)

IEnumerable<string> headers;

if (request.Headers.TryGetValues(HttpHeaders.VssRequestPriority, out headers) && headers != null)
if (request.Headers.TryGetValues(Internal.HttpHeaders.VssRequestPriority, out headers) && headers != null)
{
string header = headers.FirstOrDefault();
isLowPriority = string.Equals(header, "Low", StringComparison.OrdinalIgnoreCase);
Expand Down
1 change: 1 addition & 0 deletions src/Sdk/Common/Common/VssHttpRetryOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public VssHttpRetryOptions(IEnumerable<VssHttpRetryableStatusCodeFilter> filters
HttpStatusCode.BadGateway,
HttpStatusCode.GatewayTimeout,
HttpStatusCode.ServiceUnavailable,
HttpStatusCode.TooManyRequests,
};

this.m_retryFilters = new HashSet<VssHttpRetryableStatusCodeFilter>(filters);
Expand Down
47 changes: 43 additions & 4 deletions src/Sdk/Common/Common/VssNetworkHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.ComponentModel;
using System.IO;
using System.Net;
using System.Net.Http.Headers;
using System.Net.Sockets;

namespace GitHub.Services.Common
Expand Down Expand Up @@ -221,9 +222,47 @@ private static bool IsTransientNetworkExceptionHelper(
return false;
}

/// <summary>
/// Gets the HttpStatusCode which represents a throttling error.
/// </summary>
public const HttpStatusCode TooManyRequests = (HttpStatusCode)429;

public static TimeSpan? ConvertRetryAfterToTimeSpan(string retryAfter, TimeSpan minBackoff, TimeSpan maxBackoff)
{
if (string.IsNullOrEmpty(retryAfter))
{
return null;
}

// Retry-After can be either a delay in seconds or a date/time after which to retry.
if (RetryConditionHeaderValue.TryParse(retryAfter, out var retryAfterValue))
{
TimeSpan? retryAfterDelay = null;
if (retryAfterValue.Delta.HasValue)
{
retryAfterDelay = retryAfterValue.Delta;
}
else if (retryAfterValue.Date.HasValue)
{
// Convert the absolute date/time into a delay relative to now. Guard against
// clock skew or a date in the past, in which case we ignore the header.
var delta = retryAfterValue.Date.Value - DateTimeOffset.UtcNow;
if (delta > TimeSpan.Zero)
{
retryAfterDelay = delta;
}
}

if (retryAfterDelay?.TotalSeconds < minBackoff.TotalSeconds)
{
return BackoffTimerHelper.GetRandomBackoff(minBackoff, minBackoff.Add(TimeSpan.FromSeconds(30)));
}

if (retryAfterDelay?.TotalSeconds > maxBackoff.TotalSeconds)
{
return BackoffTimerHelper.GetRandomBackoff(maxBackoff, maxBackoff.Add(TimeSpan.FromSeconds(30)));
}
Comment thread
TingluoHuang marked this conversation as resolved.

return retryAfterDelay;
}

return null;
}
}
}
10 changes: 2 additions & 8 deletions src/Sdk/WebApi/WebApi/OAuth/VssOAuthTokenHttpClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,14 +103,8 @@ private static Boolean IsValidTokenResponse(HttpResponseMessage response)

private static HttpMessageHandler CreateMessageHandler(Uri requestUri)
{
var retryOptions = new VssHttpRetryOptions()
{
RetryableStatusCodes =
{
HttpStatusCode.InternalServerError,
VssNetworkHelper.TooManyRequests,
},
};
var retryOptions = new VssHttpRetryOptions();
retryOptions.RetryableStatusCodes.Add(HttpStatusCode.InternalServerError);

HttpClientHandler messageHandler = new HttpClientHandler()
{
Expand Down
Loading