Fall back to X-RateLimit-Reset for a GitHub 429 or secondary limit with no Retry-After [patch]

CLAUDE.md

@@ -466,8 +466,7 @@ same 403 to `GitHostingAuthenticationException`. `RateLimitExceededException`,
 **arm order is load-bearing**: the three specific ones must precede it. Only `AbuseException` carries
 a relative delay (`RetryAfterSeconds`) rather than an absolute instant, so it is anchored to
 `DateTimeOffset.UtcNow`. `SecondaryRateLimitExceededException` has no reset property of its own, so
-its reset time comes from the response's `Retry-After` header when GitHub sends one, read the same
-way as the bare-429 arm below, and is `null` otherwise.
+its reset time is read the same way as the bare-429 arm below.
 
 **A bare `429` needs a fourth arm, matched on status rather than on type.** Octokit has no dedicated
 exception for it: a `429` surfaces as a plain `ApiException`, so it reached
@@ -478,8 +477,12 @@ specific claim and a 429 carried by one of those subtypes should still be answer
 Its reset time comes from the response's `Retry-After` header, read case-insensitively rather than by
 key: HTTP header names are case-insensitive and Octokit's header dictionary compares ordinally, so a
 keyed lookup would work only because `HttpResponseMessage` happens to canonicalise this header's
-casing. A `Retry-After` carrying an HTTP date rather than seconds yields `null`, since an unset
-`ResetsAt` is better than an invented one.
+casing. A `Retry-After` carrying an HTTP date rather than seconds is not used, since an unset
+`ResetsAt` is better than an invented one. Without a usable `Retry-After`, a response reporting its
+quota spent (`X-RateLimit-Remaining: 0`) gives `X-RateLimit-Reset`, GitHub's documented fallback, read
+case-insensitively and range-checked as Azure DevOps reads it. That is how a primary limit answered
+with a `429` arrives, because Octokit builds `RateLimitExceededException` only for a `403`. With
+neither header usable, `ResetsAt` is `null`.
 
 **A repository from a hosting provider carries no `LocalPath`, and how it is addressed is the
 provider's own decision.** `GitRepository.LocalPath` is nullable because a repository a provider enumerated has never been

GitIntegration.Test/Hosting/GitHubProviderTests.cs

@@ -899,6 +899,97 @@
 		Assert.IsNull(exception.ResetsAt);
 	}
 
+	[TestMethod]
+	public async Task ReadsResetsAtFromXRateLimitResetOnATooManyRequestsResponseWithASpentQuotaAsync()
+	{
+		// GitHub may answer a spent primary quota with a 429 rather than a 403. Octokit builds
+		// RateLimitExceededException, which carries Reset, only for a 403, so this arrives as a plain
+		// ApiException with no Retry-After. The reset instant is still in the headers.
+		using FakeHttpMessageHandler handler = new FakeHttpMessageHandler()
+			.Respond(
+				HttpStatusCode.TooManyRequests,
+				"{\"message\":\"You have exceeded a rate limit\"}",
+				("Content-Type", "application/json"),
+				("X-RateLimit-Remaining", "0"),
+				("X-RateLimit-Reset", "1798800000"));
+		GitHubProvider provider = new() { Owner = "contoso".As<GitProviderOwner>(), Handler = handler };
+
+		GitHostingRateLimitException exception = await Assert.ThrowsExactlyAsync<GitHostingRateLimitException>(
+			async () => await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false))
+			.ConfigureAwait(false);
+
+		Assert.AreEqual(HttpStatusCode.TooManyRequests, exception.StatusCode);
+		Assert.AreEqual(DateTimeOffset.FromUnixTimeSeconds(1798800000), exception.ResetsAt);
+	}
+
+	[TestMethod]
+	public async Task ReadsResetsAtFromXRateLimitResetOnASecondaryRateLimitResponseWithASpentQuotaAsync()
+	{
+		// GitHub's documented order for a secondary limit is Retry-After, then X-RateLimit-Reset when
+		// X-RateLimit-Remaining is 0.
+		using FakeHttpMessageHandler handler = new FakeHttpMessageHandler()
+			.Respond(
+				(HttpStatusCode)403,
+				"{\"message\":\"You have exceeded a secondary rate limit\"}",
+				("Content-Type", "application/json"),
+				("x-ratelimit-remaining", "0"),
+				("x-ratelimit-reset", "1798800000"));
+		GitHubProvider provider = new() { Owner = "contoso".As<GitProviderOwner>(), Handler = handler };
+
+		GitHostingRateLimitException exception = await Assert.ThrowsExactlyAsync<GitHostingRateLimitException>(
+			async () => await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false))
+			.ConfigureAwait(false);
+
+		StringAssert.Contains(exception.ResponseBody, "secondary rate limit");
+		Assert.AreEqual(DateTimeOffset.FromUnixTimeSeconds(1798800000), exception.ResetsAt);
+	}
+
+	[TestMethod]
+	public async Task PrefersRetryAfterOverXRateLimitResetAsync()
+	{
+		using FakeHttpMessageHandler handler = new FakeHttpMessageHandler()
+			.Respond(
+				HttpStatusCode.TooManyRequests,
+				"{\"message\":\"You have exceeded a rate limit\"}",
+				("Content-Type", "application/json"),
+				("Retry-After", "90"),
+				("X-RateLimit-Remaining", "0"),
+				("X-RateLimit-Reset", "1798800000"));
+		GitHubProvider provider = new() { Owner = "contoso".As<GitProviderOwner>(), Handler = handler };
+
+		DateTimeOffset before = DateTimeOffset.UtcNow;
+
+		GitHostingRateLimitException exception = await Assert.ThrowsExactlyAsync<GitHostingRateLimitException>(
+			async () => await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false))
+			.ConfigureAwait(false);
+
+		DateTimeOffset after = DateTimeOffset.UtcNow;
+
+		Assert.IsNotNull(exception.ResetsAt);
+		Assert.IsInRange(before.AddSeconds(90), after.AddSeconds(90), exception.ResetsAt.Value);
+	}
+
+	[TestMethod]
+	public async Task LeavesResetsAtNullWhenTheQuotaIsNotReportedSpentAsync()
+	{
+		// GitHub sends X-RateLimit-Reset on every response. The reset of a quota that still has
+		// requests left says nothing about when this one may be retried.
+		using FakeHttpMessageHandler handler = new FakeHttpMessageHandler()
+			.Respond(
+				HttpStatusCode.TooManyRequests,
+				"{\"message\":\"You have exceeded a rate limit\"}",
+				("Content-Type", "application/json"),
+				("X-RateLimit-Remaining", "4999"),
+				("X-RateLimit-Reset", "1798800000"));
+		GitHubProvider provider = new() { Owner = "contoso".As<GitProviderOwner>(), Handler = handler };
+
+		GitHostingRateLimitException exception = await Assert.ThrowsExactlyAsync<GitHostingRateLimitException>(
+			async () => await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false))
+			.ConfigureAwait(false);
+
+		Assert.IsNull(exception.ResetsAt);
+	}
+
 	[TestMethod]
 	public async Task LeavesResetsAtNullWhenRetryAfterCarriesAnHttpDateAsync()
 	{
@@ -915,7 +1006,7 @@
 		GitHubProvider provider = new() { Owner = "contoso".As<GitProviderOwner>(), Handler = handler };
 
 		GitHostingRateLimitException exception = await Assert.ThrowsExactlyAsync<GitHostingRateLimitException>(
 			async () => await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false))
 			.ConfigureAwait(false);
 
 		Assert.AreEqual(HttpStatusCode.TooManyRequests, exception.StatusCode);
@@ -937,7 +1028,7 @@
 		GitHubProvider provider = new() { Owner = "contoso".As<GitProviderOwner>(), Handler = handler };
 
 		GitHostingAuthenticationException exception = await Assert.ThrowsExactlyAsync<GitHostingAuthenticationException>(
 			async () => await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false))
 			.ConfigureAwait(false);
 
 		Assert.IsInstanceOfType<ApiException>(exception.InnerException);

GitIntegration/GitHubProvider.cs

@@ -589,9 +589,9 @@
 		return exception switch
 		{
 			RateLimitExceededException rateLimit => new GitHostingRateLimitException(exception.Message, Name, exception.StatusCode, responseBody, rateLimit.Reset, exception),
-			SecondaryRateLimitExceededException => new GitHostingRateLimitException(exception.Message, Name, exception.StatusCode, responseBody, ToResetTime(TryGetRetryAfterSeconds(exception)), exception),
+			SecondaryRateLimitExceededException => new GitHostingRateLimitException(exception.Message, Name, exception.StatusCode, responseBody, TryGetResetTime(exception), exception),
 			AbuseException abuse => new GitHostingRateLimitException(exception.Message, Name, exception.StatusCode, responseBody, ToResetTime(abuse.RetryAfterSeconds), exception),
-			{ StatusCode: HttpStatusCode.TooManyRequests } => new GitHostingRateLimitException(exception.Message, Name, exception.StatusCode, responseBody, ToResetTime(TryGetRetryAfterSeconds(exception)), exception),
+			{ StatusCode: HttpStatusCode.TooManyRequests } => new GitHostingRateLimitException(exception.Message, Name, exception.StatusCode, responseBody, TryGetResetTime(exception), exception),
 			AuthorizationException => new GitHostingAuthenticationException(authenticationMessage, Name, exception.StatusCode, responseBody, exception),
 			ForbiddenException => new GitHostingAuthenticationException(authenticationMessage, Name, exception.StatusCode, responseBody, exception),
 			NotFoundException => new GitHostingNotFoundException(exception.Message, Name, exception.StatusCode, responseBody, exception),
@@ -618,6 +618,58 @@
 	private static DateTimeOffset? ToResetTime(int? retryAfterSeconds) =>
 		retryAfterSeconds is int seconds ? DateTimeOffset.UtcNow.AddSeconds(seconds) : null;
 
+	/// <summary>
+	/// Reads when a rate-limited response says the limit lifts, for the arms whose exception carries
+	/// no reset of its own.
+	/// </summary>
+	/// <remarks>
+	/// The order is GitHub's documented one. A <c>Retry-After</c> delay comes first. Failing that, a
+	/// response that reports its quota spent (<c>X-RateLimit-Remaining: 0</c>) carries the instant it
+	/// refills in <c>X-RateLimit-Reset</c>. That is how a primary limit answered with a <c>429</c>
+	/// arrives: Octokit builds <see cref="RateLimitExceededException"/>, with its
+	/// <see cref="RateLimitExceededException.Reset"/>, only for a <c>403</c>, so without the fallback the
+	/// same exhausted quota reported a reset time at one status and none at the other.
+	/// </remarks>
+	/// <param name="exception">The failure Octokit reported.</param>
+	/// <returns>The absolute reset time, or <see langword="null"/> when neither header is usable.</returns>
+	private static DateTimeOffset? TryGetResetTime(ApiException exception) =>
+		ToResetTime(TryGetRetryAfterSeconds(exception)) ?? TryGetRateLimitReset(exception);
+
+	/// <summary>
+	/// Reads the instant a spent quota refills from a failed response's <c>X-RateLimit-Reset</c> header.
+	/// </summary>
+	/// <remarks>
+	/// Read only when <c>X-RateLimit-Remaining</c> is <c>0</c>, because GitHub sends both headers on
+	/// every response and the reset time of a quota that is not spent says nothing about when this
+	/// request may be retried. Scanned case-insensitively for the reason
+	/// <see cref="TryGetRetryAfterSeconds"/> gives, and range-checked as Azure DevOps's reading of the
+	/// same header is, so a value <see cref="DateTimeOffset"/> cannot hold yields <see langword="null"/>
+	/// rather than an exception thrown out of the translation of another one.
+	/// </remarks>
+	/// <param name="exception">The failure Octokit reported.</param>
+	/// <returns>The reset instant, or <see langword="null"/> when the quota is not reported spent or the header is absent or unusable.</returns>
+	private static DateTimeOffset? TryGetRateLimitReset(ApiException exception)
+	{
+		if (exception.HttpResponse?.Headers is not IReadOnlyDictionary<string, string> headers)
+		{
+			return null;
+		}
+
+		string? HeaderValue(string name) => headers
+			.FirstOrDefault(candidate => candidate.Key.Equals(name, StringComparison.OrdinalIgnoreCase))
+			.Value;
+
+		bool spent = long.TryParse(HeaderValue("X-RateLimit-Remaining"), NumberStyles.Integer, CultureInfo.InvariantCulture, out long remaining)
+			&& remaining == 0;
+
+		return spent
+			&& long.TryParse(HeaderValue("X-RateLimit-Reset"), NumberStyles.Integer, CultureInfo.InvariantCulture, out long epochSeconds)
+			&& epochSeconds >= DateTimeOffset.MinValue.ToUnixTimeSeconds()
+			&& epochSeconds <= DateTimeOffset.MaxValue.ToUnixTimeSeconds()
+			? DateTimeOffset.FromUnixTimeSeconds(epochSeconds)
+			: null;
+	}
+
 	/// <summary>
 	/// Reads a failed response's <c>Retry-After</c> delay, in seconds.
 	/// </summary>
@@ -683,7 +735,7 @@
 		foreach (KeyValuePair<string, string> header in headers.Where(
 			candidate => candidate.Key.Equals(headerName, StringComparison.OrdinalIgnoreCase)))
 		{
 			foreach (string trimmed in header.Value.Split(';').Select(static parameter => parameter.Trim()))
 			{
 				if (trimmed.StartsWith(urlParameter, StringComparison.OrdinalIgnoreCase))
 				{