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..d71259c 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,31 @@ 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; + } + + KeyValuePair match = headers.FirstOrDefault(header => string.Equals(header.Key, name, StringComparison.OrdinalIgnoreCase)); + value = match.Value; + return match.Key is not null; + } + /// /// How long to back off from a secondary rate limit that names no Retry-After. GitHub asks /// for at least a minute. @@ -918,7 +944,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) {