Skip to content

Commit 820cbea

Browse files
Implement secondary rate limit handling in GitHub CLI
Co-authored-by: begonaguereca <[email protected]>
1 parent 4a35810 commit 820cbea

File tree

3 files changed

+116
-2
lines changed

3 files changed

+116
-2
lines changed

RELEASENOTES.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,4 @@
1+
# Release Notes
12

3+
## What's New
4+
- Added secondary rate limit handling to GitHub CLI commands to gracefully handle 403/429 responses with proper retry logic, exponential backoff, and clear error messaging. This improves reliability when running multiple concurrent migrations.

src/Octoshift/Services/GithubClient.cs

Lines changed: 85 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ public class GithubClient
2222

2323
private const string DEFAULT_RATE_LIMIT_REMAINING = "5000";
2424
private const int MILLISECONDS_PER_SECOND = 1000;
25+
private const int SECONDARY_RATE_LIMIT_MAX_RETRIES = 3;
26+
private const int SECONDARY_RATE_LIMIT_DEFAULT_DELAY = 60; // 60 seconds default delay
2527

2628
public GithubClient(OctoLogger log, HttpClient httpClient, IVersionProvider versionProvider, RetryPolicy retryPolicy, DateTimeProvider dateTimeProvider, string personalAccessToken)
2729
{
@@ -159,7 +161,8 @@ public virtual async Task<string> PatchAsync(string url, object body, Dictionary
159161
string url,
160162
object body = null,
161163
HttpStatusCode expectedStatus = HttpStatusCode.OK,
162-
Dictionary<string, string> customHeaders = null)
164+
Dictionary<string, string> customHeaders = null,
165+
int retryCount = 0)
163166
{
164167
await ApplyRetryDelayAsync();
165168
_log.LogVerbose($"HTTP {httpMethod}: {url}");
@@ -199,9 +202,15 @@ public virtual async Task<string> PatchAsync(string url, object body, Dictionary
199202
SetRetryDelay(headers);
200203
}
201204

205+
// Check for secondary rate limits before handling primary rate limits
206+
if (IsSecondaryRateLimit(response.StatusCode, content))
207+
{
208+
return await HandleSecondaryRateLimit(httpMethod, url, body, expectedStatus, customHeaders, response, content, headers, retryCount);
209+
}
210+
202211
if (response.StatusCode == HttpStatusCode.Forbidden && _retryDelay > 0)
203212
{
204-
(content, headers) = await SendAsync(httpMethod, url, body, expectedStatus, customHeaders);
213+
(content, headers) = await SendAsync(httpMethod, url, body, expectedStatus, customHeaders, retryCount);
205214
}
206215
else if (expectedStatus == HttpStatusCode.OK)
207216
{
@@ -280,4 +289,78 @@ private void EnsureSuccessGraphQLResponse(JObject response)
280289
throw new OctoshiftCliException($"{errorMessage ?? "UNKNOWN"}");
281290
}
282291
}
292+
293+
private bool IsSecondaryRateLimit(HttpStatusCode statusCode, string content)
294+
{
295+
// Secondary rate limits return 403 or 429
296+
if (statusCode is not HttpStatusCode.Forbidden and not HttpStatusCode.TooManyRequests)
297+
{
298+
return false;
299+
}
300+
301+
// Check if this is a primary rate limit (which we handle separately)
302+
if (content.ToUpper().Contains("API RATE LIMIT EXCEEDED"))
303+
{
304+
return false;
305+
}
306+
307+
// Common secondary rate limit error patterns
308+
var contentUpper = content.ToUpper();
309+
return contentUpper.Contains("SECONDARY RATE LIMIT") ||
310+
contentUpper.Contains("ABUSE DETECTION") ||
311+
contentUpper.Contains("YOU HAVE TRIGGERED AN ABUSE DETECTION MECHANISM") ||
312+
statusCode == HttpStatusCode.TooManyRequests;
313+
}
314+
315+
private async Task<(string Content, KeyValuePair<string, IEnumerable<string>>[] ResponseHeaders)> HandleSecondaryRateLimit(
316+
HttpMethod httpMethod,
317+
string url,
318+
object body,
319+
HttpStatusCode expectedStatus,
320+
Dictionary<string, string> customHeaders,
321+
HttpResponseMessage response,
322+
string content,
323+
KeyValuePair<string, IEnumerable<string>>[] headers,
324+
int retryCount = 0)
325+
{
326+
if (retryCount >= SECONDARY_RATE_LIMIT_MAX_RETRIES)
327+
{
328+
throw new OctoshiftCliException($"Secondary rate limit exceeded. Maximum retries ({SECONDARY_RATE_LIMIT_MAX_RETRIES}) reached. Please wait before retrying your request.");
329+
}
330+
331+
var delaySeconds = GetSecondaryRateLimitDelay(headers, retryCount);
332+
333+
_log.LogWarning($"Secondary rate limit detected (attempt {retryCount + 1}/{SECONDARY_RATE_LIMIT_MAX_RETRIES}). Waiting {delaySeconds} seconds before retrying...");
334+
335+
await Task.Delay(delaySeconds * MILLISECONDS_PER_SECOND);
336+
337+
return await SendAsync(httpMethod, url, body, expectedStatus, customHeaders, retryCount + 1);
338+
}
339+
340+
private int GetSecondaryRateLimitDelay(KeyValuePair<string, IEnumerable<string>>[] headers, int retryCount)
341+
{
342+
// First check for retry-after header
343+
var retryAfterHeader = ExtractHeaderValue("Retry-After", headers);
344+
if (!string.IsNullOrEmpty(retryAfterHeader) && int.TryParse(retryAfterHeader, out var retryAfterSeconds))
345+
{
346+
return retryAfterSeconds;
347+
}
348+
349+
// Then check if x-ratelimit-remaining is 0 and use x-ratelimit-reset
350+
var rateLimitRemaining = GetRateLimitRemaining(headers);
351+
if (rateLimitRemaining <= 0)
352+
{
353+
var resetUnixSeconds = GetRateLimitReset(headers);
354+
var currentUnixSeconds = _dateTimeProvider.CurrentUnixTimeSeconds();
355+
var delayFromReset = (int)(resetUnixSeconds - currentUnixSeconds);
356+
357+
if (delayFromReset > 0)
358+
{
359+
return delayFromReset;
360+
}
361+
}
362+
363+
// Otherwise use exponential backoff: 1m → 2m → 4m
364+
return SECONDARY_RATE_LIMIT_DEFAULT_DELAY * (int)Math.Pow(2, retryCount);
365+
}
283366
}

src/OctoshiftCLI.Tests/Octoshift/Services/GithubClientTests.cs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2084,6 +2084,34 @@ public void It_Only_Sets_The_Product_Name_In_User_Agent_Header_When_Version_Prov
20842084
httpClient.DefaultRequestHeaders.UserAgent.ToString().Should().Be("OctoshiftCLI");
20852085
}
20862086

2087+
[Fact]
2088+
public async Task PostAsync_Handles_Secondary_Rate_Limit_With_429_Status()
2089+
{
2090+
// Arrange
2091+
var handlerMock = new Mock<HttpMessageHandler>();
2092+
handlerMock
2093+
.Protected()
2094+
.SetupSequence<Task<HttpResponseMessage>>(
2095+
"SendAsync",
2096+
ItExpr.Is<HttpRequestMessage>(req => req.Method == HttpMethod.Post),
2097+
ItExpr.IsAny<CancellationToken>())
2098+
.ReturnsAsync(CreateHttpResponseFactory(
2099+
statusCode: HttpStatusCode.TooManyRequests,
2100+
content: "Too many requests",
2101+
headers: new[] { ("Retry-After", "1") })())
2102+
.ReturnsAsync(CreateHttpResponseFactory(content: "SUCCESS_RESPONSE")());
2103+
2104+
using var httpClient = new HttpClient(handlerMock.Object);
2105+
var githubClient = new GithubClient(_mockOctoLogger.Object, httpClient, null, _retryPolicy, _dateTimeProvider.Object, PERSONAL_ACCESS_TOKEN);
2106+
2107+
// Act
2108+
var result = await githubClient.PostAsync("http://example.com", _rawRequestBody);
2109+
2110+
// Assert
2111+
result.Should().Be("SUCCESS_RESPONSE");
2112+
_mockOctoLogger.Verify(m => m.LogWarning(It.Is<string>(s => s.Contains("Secondary rate limit detected"))), Times.Once);
2113+
}
2114+
20872115
private object CreateRepositoryMigration(string migrationId = null, string state = RepositoryMigrationStatus.Succeeded) => new
20882116
{
20892117
id = migrationId ?? Guid.NewGuid().ToString(),

0 commit comments

Comments
 (0)