From 53ff5c16bd7c4bd7a1b1b064c875fc1167d24b6b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 04:24:39 +0000 Subject: [PATCH 1/3] Read GitHub rate-limit headers whatever their casing [patch] Octokit keys response headers case-sensitively, as they arrived on the wire, and GitHub sends x-ratelimit-remaining and x-ratelimit-reset in lowercase. The exact-case X-RateLimit-* lookups missed them, so a spent primary budget fell through to the one-minute secondary backoff and was retried every minute until it reset. Look Retry-After and the X-RateLimit-* headers up through a helper that ignores case, as HTTP does. Fixes #389. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW4T6sUBiRNom98NuYi --- .../GitHubRateLimitResetTests.cs | 49 +++++++++++++++++++ BuildMonitor/Providers/GitHub.cs | 42 ++++++++++++++-- 2 files changed, 87 insertions(+), 4 deletions(-) diff --git a/BuildMonitor.Test/GitHubRateLimitResetTests.cs b/BuildMonitor.Test/GitHubRateLimitResetTests.cs index 485b8e7..222d33f 100644 --- a/BuildMonitor.Test/GitHubRateLimitResetTests.cs +++ b/BuildMonitor.Test/GitHubRateLimitResetTests.cs @@ -92,4 +92,53 @@ public void ARetryAfterGivenAsAnHttpDateIsHonoured() Assert.AreEqual(retryAt, GitHub.ParseRateLimitResetTime(limited, Now)); } + + /// + /// GitHub sends the rate-limit headers in lowercase and proxies re-case them, but Octokit keys + /// them case-sensitively as they arrived, so an exact-case lookup missed a spent budget and + /// retried it every minute (ktsu-dev/BuildMonitor#389). + /// + [TestMethod] + [DataRow("x-ratelimit-remaining", "x-ratelimit-reset")] + [DataRow("X-Ratelimit-Remaining", "X-Ratelimit-Reset")] + public void ASpentPrimaryBudgetWaitsForTheResetWhateverTheHeaderCasing(string remainingName, string resetName) + { + DateTimeOffset reset = Now.AddMinutes(45); + ApiException primary = ApiFailure(HttpStatusCode.Forbidden, new() + { + [remainingName] = "0", + ["x-ratelimit-limit"] = "5000", + [resetName] = UnixSeconds(reset), + }); + + Assert.AreEqual(reset, GitHub.ParseRateLimitResetTime(primary, Now)); + } + + [TestMethod] + public void ALowercaseRetryAfterIsHonoured() + { + ApiException limited = ApiFailure(HttpStatusCode.TooManyRequests, new() + { + ["retry-after"] = "30", + }); + + Assert.AreEqual(Now.AddSeconds(30), GitHub.ParseRateLimitResetTime(limited, Now)); + } + + /// + /// A 403 whose message does not mention a rate limit is told apart by its spent budget alone. + /// + [TestMethod] + [DataRow("x-ratelimit-remaining")] + [DataRow("X-Ratelimit-Remaining")] + public void AForbiddenResponseWithASpentBudgetIsARateLimitWhateverTheHeaderCasing(string remainingName) + { + ApiException forbidden = ApiFailure(HttpStatusCode.Forbidden, new() + { + [remainingName] = "0", + }); + + Assert.DoesNotContain("rate limit", forbidden.Message, StringComparison.OrdinalIgnoreCase); + Assert.IsTrue(GitHub.IsRateLimitException(forbidden)); + } } diff --git a/BuildMonitor/Providers/GitHub.cs b/BuildMonitor/Providers/GitHub.cs index 2df6278..9ab5a0c 100644 --- a/BuildMonitor/Providers/GitHub.cs +++ b/BuildMonitor/Providers/GitHub.cs @@ -3,6 +3,7 @@ namespace ktsu.BuildMonitor; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Text.RegularExpressions; using System.Threading.Tasks; @@ -879,7 +880,7 @@ private void UpdateRateLimitFromApiInfo() return null; } - if (headers.TryGetValue("Retry-After", out string? retryAfterValue)) + if (TryGetHeader(headers, "Retry-After", out string? retryAfterValue)) { if (int.TryParse(retryAfterValue, NumberStyles.None, CultureInfo.InvariantCulture, out int retryAfterSeconds)) { @@ -892,10 +893,10 @@ private void UpdateRateLimitFromApiInfo() } } - if (headers.TryGetValue("X-RateLimit-Remaining", out string? remainingValue) && + if (TryGetHeader(headers, "X-RateLimit-Remaining", out string? remainingValue) && int.TryParse(remainingValue, CultureInfo.InvariantCulture, out int remaining) && remaining == 0 && - headers.TryGetValue("X-RateLimit-Reset", out string? resetValue) && + TryGetHeader(headers, "X-RateLimit-Reset", out string? resetValue) && long.TryParse(resetValue, CultureInfo.InvariantCulture, out long unixTimestamp)) { return DateTimeOffset.FromUnixTimeSeconds(unixTimestamp); @@ -904,6 +905,39 @@ private void UpdateRateLimitFromApiInfo() return now + SecondaryRateLimitBackoff; } + /// + /// Looks a response header up by name, ignoring case as HTTP does. + /// + /// + /// Octokit keys case-sensitively, by each name as it arrived on + /// the wire. GitHub sends x-ratelimit-remaining in lowercase, so an exact-case lookup of + /// X-RateLimit-Remaining missed it and a spent budget was retried every minute + /// (ktsu-dev/BuildMonitor#389). + /// + /// The response headers. + /// The header name, in any casing. + /// The header's value, when it is present. + /// Whether the header is present. + internal static bool TryGetHeader(IReadOnlyDictionary headers, string name, [NotNullWhen(true)] out string? value) + { + if (headers.TryGetValue(name, out value)) + { + return true; + } + + foreach (KeyValuePair header in headers) + { + if (string.Equals(header.Key, name, StringComparison.OrdinalIgnoreCase)) + { + value = header.Value; + return true; + } + } + + value = null; + return false; + } + /// /// How long to back off from a secondary rate limit that names no Retry-After. GitHub asks /// for at least a minute. @@ -918,7 +952,7 @@ private static bool IsRateLimitResponse(ApiException exception) } // Check if X-RateLimit-Remaining is 0, indicating rate limit exceeded - if (exception.HttpResponse.Headers.TryGetValue("X-RateLimit-Remaining", out string? remainingValue) && + if (TryGetHeader(exception.HttpResponse.Headers, "X-RateLimit-Remaining", out string? remainingValue) && int.TryParse(remainingValue, CultureInfo.InvariantCulture, out int remaining) && remaining == 0) { From 5699c3d1dfdf2934b0f82321e30b87f0221bd1b5 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 04:34:58 +0000 Subject: [PATCH 2/3] Filter the header scan with Where, per SonarCloud Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW4T6sUBiRNom98NuYi --- BuildMonitor/Providers/GitHub.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/BuildMonitor/Providers/GitHub.cs b/BuildMonitor/Providers/GitHub.cs index 9ab5a0c..f913661 100644 --- a/BuildMonitor/Providers/GitHub.cs +++ b/BuildMonitor/Providers/GitHub.cs @@ -925,13 +925,10 @@ internal static bool TryGetHeader(IReadOnlyDictionary headers, s return true; } - foreach (KeyValuePair header in headers) + foreach (KeyValuePair header in headers.Where(header => string.Equals(header.Key, name, StringComparison.OrdinalIgnoreCase))) { - if (string.Equals(header.Key, name, StringComparison.OrdinalIgnoreCase)) - { - value = header.Value; - return true; - } + value = header.Value; + return true; } value = null; From 0cc8c8d0444d8735957f3f52d74c8ab0e3ba30e3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 04:44:42 +0000 Subject: [PATCH 3/3] Find a case-variant header with FirstOrDefault rather than a loop that always exits SonarCloud S1751 rated the Where loop, which returned on its first iteration, as a reliability bug. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW4T6sUBiRNom98NuYi --- BuildMonitor/Providers/GitHub.cs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/BuildMonitor/Providers/GitHub.cs b/BuildMonitor/Providers/GitHub.cs index f913661..d71259c 100644 --- a/BuildMonitor/Providers/GitHub.cs +++ b/BuildMonitor/Providers/GitHub.cs @@ -925,14 +925,9 @@ internal static bool TryGetHeader(IReadOnlyDictionary headers, s return true; } - foreach (KeyValuePair header in headers.Where(header => string.Equals(header.Key, name, StringComparison.OrdinalIgnoreCase))) - { - value = header.Value; - return true; - } - - value = null; - return false; + KeyValuePair match = headers.FirstOrDefault(header => string.Equals(header.Key, name, StringComparison.OrdinalIgnoreCase)); + value = match.Value; + return match.Key is not null; } ///