diff --git a/src/Octoshift/RetryPolicy.cs b/src/Octoshift/RetryPolicy.cs index 48c94ec8e..ea09e08c2 100644 --- a/src/Octoshift/RetryPolicy.cs +++ b/src/Octoshift/RetryPolicy.cs @@ -1,4 +1,7 @@ using System; +using System.IO; +using System.Linq; +using System.Net; using System.Net.Http; using System.Threading.Tasks; using OctoshiftCLI.Services; @@ -18,6 +21,108 @@ public RetryPolicy(OctoLogger log) _log = log; } + public async Task HttpRetry(Func> func) + { + var policy = Policy + .HandleResult(r => + { + var sc = (int)r.StatusCode; + if (sc is not 403 and not 429) + { + return false; + } + + // Treat Retry-After or X-RateLimit-Remaining: 0 as secondary-rate limiting signals + if (r.Headers.RetryAfter != null) + { + return true; + } + + if (r.Headers.TryGetValues("X-RateLimit-Remaining", out var remain) && + remain?.FirstOrDefault() == "0") + { + return true; + } + + // Fallback: any 403/429 without headers still gets backoff per docs + return true; + }) + .WaitAndRetryAsync( + retryCount: 5, + sleepDurationProvider: (attempt, outcome, ctx) => + { + var r = outcome.Result; + + // 1) Honor Retry-After header + var ra = r.Headers.RetryAfter?.Delta; + if (ra.HasValue) + { + return ra.Value; + } + + // 2) If remaining == 0, wait until reset + if (r.Headers.TryGetValues("X-RateLimit-Remaining", out var remainVals) && + remainVals?.FirstOrDefault() == "0" && + r.Headers.TryGetValues("X-RateLimit-Reset", out var resetVals) && + long.TryParse(resetVals?.FirstOrDefault(), out var resetEpoch)) + { + var resetAt = DateTimeOffset.FromUnixTimeSeconds(resetEpoch); + var wait = resetAt - DateTimeOffset.UtcNow; + if (wait > TimeSpan.Zero) + { + return wait; + } + } + + // 3) Otherwise: at least 1 minute, exponential thereafter (1,2,4,8,...) + var minutes = Math.Max(1, Math.Pow(2, attempt - 1)); + return TimeSpan.FromMinutes(minutes); + }, + onRetryAsync: async (outcome, wait, attempt, ctx) => + { + var r = outcome.Result; + string body = null; + try + { + body = r.Content != null ? await r.Content.ReadAsStringAsync() : null; + } + catch (IOException) + { + // ignored + } + catch (ObjectDisposedException) + { + // ignored + } + catch (InvalidOperationException) + { + // ignored + } + + _log?.LogVerbose( + $"Secondary rate limit (HTTP {(int)r.StatusCode}). " + + $"Retrying in {wait.TotalSeconds:N0}s (attempt {attempt}). " + + $"Retry-After={r.Headers.RetryAfter?.Delta?.TotalSeconds}, " + + $"X-RateLimit-Remaining={TryHeader(r, "X-RateLimit-Remaining")}, " + + $"X-RateLimit-Reset={TryHeader(r, "X-RateLimit-Reset")}. " + + $"Body={(body?.Length > 200 ? body[..200] + "…" : body)}"); + }); + + var resp = await policy.ExecuteAsync(func); + + if (!resp.IsSuccessStatusCode) + { + var finalBody = (resp.Content != null) ? await resp.Content.ReadAsStringAsync() : null; + throw new OctoshiftCliException($"HTTP {(int)resp.StatusCode} after retries. Body={finalBody}"); + } + + return resp; + + static string TryHeader(HttpResponseMessage r, string name) + => r.Headers.TryGetValues(name, out var vals) ? vals?.FirstOrDefault() : null; + } + + // Existing overload preserved (exception-based) public async Task HttpRetry(Func> func, Func filter) { var policy = Policy.Handle(filter) @@ -45,23 +150,23 @@ public async Task> RetryOnResult(Func> func, Func Retry(Func> func) => await CreateRetryPolicyForException().ExecuteAsync(func); private AsyncRetryPolicy CreateRetryPolicyForException() where TException : Exception => Policy - .Handle() - .WaitAndRetryAsync(5, retry => retry * TimeSpan.FromMilliseconds(_retryInterval), (Exception ex, TimeSpan ts, Context ctx) => + .Handle() + .WaitAndRetryAsync(5, retry => retry * TimeSpan.FromMilliseconds(_retryInterval), (Exception ex, TimeSpan ts, Context ctx) => + { + if (ex is HttpRequestException httpEx) { - if (ex is HttpRequestException httpEx) + _log?.LogVerbose($"[HTTP ERROR {(int?)httpEx.StatusCode}] {ex}"); + if (httpEx.StatusCode == HttpStatusCode.Unauthorized) { - _log?.LogVerbose($"[HTTP ERROR {(int?)httpEx.StatusCode}] {ex}"); - if (httpEx.StatusCode == System.Net.HttpStatusCode.Unauthorized) - { - // We should not retry on an unathorized error; instead, log and bubble up the error - throw new OctoshiftCliException($"Unauthorized. Please check your token and try again", ex); - }; - } - else - { - _log?.LogVerbose(ex.ToString()); + // We should not retry on an unauthorized error; instead, log and bubble up the error + throw new OctoshiftCliException("Unauthorized. Please check your token and try again", ex); } - _log?.LogVerbose("Retrying..."); - }); + } + else + { + _log?.LogVerbose(ex.ToString()); + } + _log?.LogVerbose("Retrying..."); + }); } } diff --git a/src/Octoshift/Services/GithubClient.cs b/src/Octoshift/Services/GithubClient.cs index 573fd796a..f4a73f50d 100644 --- a/src/Octoshift/Services/GithubClient.cs +++ b/src/Octoshift/Services/GithubClient.cs @@ -181,7 +181,7 @@ public virtual async Task PatchAsync(string url, object body, Dictionary } } - using var response = await _httpClient.SendAsync(request); + using var response = await _retryPolicy.HttpRetry(() => _httpClient.SendAsync(request)); _log.LogVerbose($"GITHUB REQUEST ID: {ExtractHeaderValue("X-GitHub-Request-Id", response.Headers)}"); var content = await response.Content.ReadAsStringAsync();