From 9f2913668ef1efac1c8867cfc36dc34bac9167fb Mon Sep 17 00:00:00 2001 From: matt-edmondson Date: Thu, 29 Oct 2026 18:29:55 +0000 Subject: [PATCH] Honour Retry-After on a GitHub rate limit [patch] Only X-RateLimit-Reset was read, which on a secondary limit is the end of the primary hourly window, so a limit that asked for 60 s paused polling for the 10-minute cap. Use Retry-After when it is sent, X-RateLimit-Reset only when the primary budget is spent, and otherwise back off a minute. Part of ktsu-dev/BuildMonitor#385 (the GitHub half). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW2nrCvY4DyoEF72Nk1 --- .../GitHubRateLimitResetTests.cs | 95 +++++++++++++++++++ BuildMonitor/Providers/GitHub.cs | 48 +++++++++- 2 files changed, 139 insertions(+), 4 deletions(-) create mode 100644 BuildMonitor.Test/GitHubRateLimitResetTests.cs diff --git a/BuildMonitor.Test/GitHubRateLimitResetTests.cs b/BuildMonitor.Test/GitHubRateLimitResetTests.cs new file mode 100644 index 0000000..485b8e7 --- /dev/null +++ b/BuildMonitor.Test/GitHubRateLimitResetTests.cs @@ -0,0 +1,95 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.BuildMonitor.Test; + +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Net; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +using Octokit; + +/// +/// Covers when a rate-limited GitHub request may be retried. Only X-RateLimit-Reset was read, +/// which on a secondary limit is the end of the primary hourly window, so a limit that asked for a +/// minute paused polling for the 10-minute cap (ktsu-dev/BuildMonitor#385). +/// +[TestClass] +public sealed class GitHubRateLimitResetTests +{ + private static readonly DateTimeOffset Now = new(2026, 10, 29, 12, 0, 0, TimeSpan.Zero); + + private sealed class FakeResponse(HttpStatusCode statusCode, IReadOnlyDictionary headers) : IResponse + { + public object Body => "{}"; + public IReadOnlyDictionary Headers { get; } = headers; + public ApiInfo ApiInfo => null!; + public HttpStatusCode StatusCode { get; } = statusCode; + public string ContentType => "application/json"; + } + + private static ApiException ApiFailure(HttpStatusCode statusCode, Dictionary headers) => + new(new FakeResponse(statusCode, headers)); + + private static string UnixSeconds(DateTimeOffset time) => + time.ToUnixTimeSeconds().ToString(CultureInfo.InvariantCulture); + + /// + /// The case from the issue: a secondary limit with most of the hourly budget left. + /// + [TestMethod] + public void ASecondaryLimitWaitsForItsRetryAfterRatherThanThePrimaryReset() + { + ApiException secondary = ApiFailure(HttpStatusCode.Forbidden, new() + { + ["X-RateLimit-Remaining"] = "4000", + ["X-RateLimit-Reset"] = UnixSeconds(Now.AddMinutes(45)), + ["Retry-After"] = "60", + }); + + Assert.AreEqual(Now.AddSeconds(60), GitHub.ParseRateLimitResetTime(secondary, Now)); + } + + [TestMethod] + public void ASpentPrimaryBudgetWaitsForTheReset() + { + DateTimeOffset reset = Now.AddMinutes(45); + ApiException primary = ApiFailure(HttpStatusCode.Forbidden, new() + { + ["X-RateLimit-Remaining"] = "0", + ["X-RateLimit-Reset"] = UnixSeconds(reset), + }); + + Assert.AreEqual(reset, GitHub.ParseRateLimitResetTime(primary, Now)); + } + + /// + /// GitHub's guidance for a secondary limit that names no wait is to back off for at least a + /// minute, not until the primary window resets. + /// + [TestMethod] + public void ASecondaryLimitWithNoRetryAfterBacksOffForAMinute() + { + ApiException secondary = ApiFailure(HttpStatusCode.Forbidden, new() + { + ["X-RateLimit-Remaining"] = "4000", + ["X-RateLimit-Reset"] = UnixSeconds(Now.AddMinutes(45)), + }); + + Assert.AreEqual(Now + GitHub.SecondaryRateLimitBackoff, GitHub.ParseRateLimitResetTime(secondary, Now)); + } + + [TestMethod] + public void ARetryAfterGivenAsAnHttpDateIsHonoured() + { + DateTimeOffset retryAt = Now.AddSeconds(90); + ApiException limited = ApiFailure(HttpStatusCode.TooManyRequests, new() + { + ["Retry-After"] = retryAt.ToString("r", CultureInfo.InvariantCulture), + }); + + Assert.AreEqual(retryAt, GitHub.ParseRateLimitResetTime(limited, Now)); + } +} diff --git a/BuildMonitor/Providers/GitHub.cs b/BuildMonitor/Providers/GitHub.cs index cb82b1a..2df6278 100644 --- a/BuildMonitor/Providers/GitHub.cs +++ b/BuildMonitor/Providers/GitHub.cs @@ -854,22 +854,62 @@ private void UpdateRateLimitFromApiInfo() } } - private static DateTimeOffset? ParseRateLimitResetTime(ApiException exception) + private static DateTimeOffset? ParseRateLimitResetTime(ApiException exception) => + ParseRateLimitResetTime(exception, DateTimeOffset.UtcNow); + + /// + /// Works out when a rate-limited request may be retried, following GitHub's guidance: honour + /// Retry-After when it is sent, wait for X-RateLimit-Reset only when the primary + /// budget is spent, and otherwise back off for a minute. + /// + /// + /// A secondary limit can be hit with most of the hourly budget left, and its + /// X-RateLimit-Reset is still the end of the primary window, often most of an hour away. + /// Reading only that header paused polling for the 10-minute cap rather than the minute or so + /// GitHub asked for (ktsu-dev/BuildMonitor#385). + /// + /// The rate-limited response. + /// The current time, which a relative Retry-After counts from. + /// When to retry, or when the response carried no headers. + internal static DateTimeOffset? ParseRateLimitResetTime(ApiException exception, DateTimeOffset now) { - if (exception.HttpResponse?.Headers == null) + IReadOnlyDictionary? headers = exception.HttpResponse?.Headers; + if (headers == null) { return null; } - if (exception.HttpResponse.Headers.TryGetValue("X-RateLimit-Reset", out string? resetValue) && + if (headers.TryGetValue("Retry-After", out string? retryAfterValue)) + { + if (int.TryParse(retryAfterValue, NumberStyles.None, CultureInfo.InvariantCulture, out int retryAfterSeconds)) + { + return now + TimeSpan.FromSeconds(retryAfterSeconds); + } + + if (DateTimeOffset.TryParseExact(retryAfterValue, "r", CultureInfo.InvariantCulture, DateTimeStyles.AssumeUniversal, out DateTimeOffset retryAt)) + { + return retryAt; + } + } + + if (headers.TryGetValue("X-RateLimit-Remaining", out string? remainingValue) && + int.TryParse(remainingValue, CultureInfo.InvariantCulture, out int remaining) && + remaining == 0 && + headers.TryGetValue("X-RateLimit-Reset", out string? resetValue) && long.TryParse(resetValue, CultureInfo.InvariantCulture, out long unixTimestamp)) { return DateTimeOffset.FromUnixTimeSeconds(unixTimestamp); } - return null; + return now + SecondaryRateLimitBackoff; } + /// + /// How long to back off from a secondary rate limit that names no Retry-After. GitHub asks + /// for at least a minute. + /// + internal static TimeSpan SecondaryRateLimitBackoff { get; } = TimeSpan.FromMinutes(1); + private static bool IsRateLimitResponse(ApiException exception) { if (exception.HttpResponse?.Headers == null)