Read GitHub rate-limit headers whatever their casing [patch]

BuildMonitor.Test/GitHubRateLimitResetTests.cs

@@ -92,4 +92,53 @@ public void ARetryAfterGivenAsAnHttpDateIsHonoured()
 
 		Assert.AreEqual(retryAt, GitHub.ParseRateLimitResetTime(limited, Now));
 	}
+
+	/// <summary>
+	/// 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).
+	/// </summary>
+	[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));
+	}
+
+	/// <summary>
+	/// A 403 whose message does not mention a rate limit is told apart by its spent budget alone.
+	/// </summary>
+	[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));
+	}
 }

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;
@@ -93,7 +94,7 @@
 
 	private Owner? OwnerPendingTokenPopup { get; set; }
 
 	internal override void ShowMenu()
 	{
 		if (Hexa.NET.ImGui.ImGui.BeginMenu(Name))
 		{
@@ -204,7 +205,7 @@
 			}
 
 			// Add all organizations the user belongs to
 			IReadOnlyList<Organization> organizations = await CurrentClient!.Organization.GetAllForCurrent().ConfigureAwait(false);
 			foreach (Organization org in organizations)
 			{
 				OwnerName orgName = org.Login.As<OwnerName>();
@@ -219,7 +220,7 @@
 		}).ConfigureAwait(false);
 	}
 
 	internal override async Task UpdateRepositoriesAsync(Owner owner)
 	{
 		if (!HasValidCredentials(owner))
 		{
@@ -247,7 +248,7 @@
 				// Filter to only repos owned by this user (GetAllForCurrent returns repos from all orgs the user has access to)
 				IReadOnlyList<Octokit.Repository> currentUserRepos = await GitHubRepository.GetAllForCurrent().ConfigureAwait(false);
 				int totalCount = currentUserRepos.Count;
 				foreach (Octokit.Repository repo in currentUserRepos)
 				{
 					if (repo.Owner.Login.Equals(owner.Name.ToString(), StringComparison.OrdinalIgnoreCase))
 					{
@@ -268,7 +269,7 @@
 				catch (NotFoundException)
 				{
 					// Owner might be an org-only account, try org repos instead
 					userRepositories = [];
 				}
 
 				// Try to get organization repositories - this only works for orgs
@@ -281,7 +282,7 @@
 				catch (NotFoundException)
 				{
 					// Owner is not an organization, that's fine
 					organizationRepositories = [];
 				}
 			}
 
@@ -306,13 +307,13 @@
 
 				// Get existing repository or create new one
 				bool isNew = false;
 				Repository repository = owner.Repositories.GetOrAdd(repositoryId, _ =>
 				{
 					isNew = true;
 					return owner.CreateRepository(repositoryName, repositoryId);
 				});
 
 				if (isNew)
 				{
 					newRepos++;
 					Log.Info($"GitHub: Discovered repository: {owner.Name}/{repositoryName}");
@@ -347,7 +348,7 @@
 				}
 			}
 
 			if (newRepos > 0 || updatedRepos > 0 || archivedRepos > 0)
 			{
 				Log.Info($"GitHub: Repository update for {owner.Name}: {newRepos} new, {updatedRepos} updated, {archivedRepos} archived removed, {owner.Repositories.Count} total");
 			}
@@ -373,7 +374,7 @@
 
 					// Get existing build or create new one
 					bool isNew = false;
 					Build build = repository.Builds.GetOrAdd(buildId, _ =>
 					{
 						isNew = true;
 						return repository.CreateBuild(buildName, buildId);
@@ -879,7 +880,7 @@
 			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 @@
 			}
 		}
 
-		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 @@
 		return now + SecondaryRateLimitBackoff;
 	}
 
+	/// <summary>
+	/// Looks a response header up by name, ignoring case as HTTP does.
+	/// </summary>
+	/// <remarks>
+	/// Octokit keys <see cref="IResponse.Headers"/> case-sensitively, by each name as it arrived on
+	/// the wire. GitHub sends <c>x-ratelimit-remaining</c> in lowercase, so an exact-case lookup of
+	/// <c>X-RateLimit-Remaining</c> missed it and a spent budget was retried every minute
+	/// (ktsu-dev/BuildMonitor#389).
+	/// </remarks>
+	/// <param name="headers">The response headers.</param>
+	/// <param name="name">The header name, in any casing.</param>
+	/// <param name="value">The header's value, when it is present.</param>
+	/// <returns>Whether the header is present.</returns>
+	internal static bool TryGetHeader(IReadOnlyDictionary<string, string> headers, string name, [NotNullWhen(true)] out string? value)
+	{
+		if (headers.TryGetValue(name, out value))
+		{
+			return true;
+		}
+
+		KeyValuePair<string, string> match = headers.FirstOrDefault(header => string.Equals(header.Key, name, StringComparison.OrdinalIgnoreCase));
+		value = match.Value;
+		return match.Key is not null;
+	}
+
 	/// <summary>
 	/// How long to back off from a secondary rate limit that names no <c>Retry-After</c>. GitHub asks
 	/// for at least a minute.
@@ -918,7 +944,7 @@
 		}
 
 		// 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)
 		{