Skip to content

Commit 7857ce6

Browse files
coj337coj337
committed
Reduce gist rate limit usage and add better messaging when hitting gist rate limits
1 parent 0d04c2f commit 7857ce6

2 files changed

Lines changed: 99 additions & 18 deletions

File tree

‎GitRekt.Tests/SourcesTests.cs‎

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,10 @@ public async Task SearchCodePagesAsync_DeserializesRepositoryResultsWithReposito
138138
public async Task SearchGistPagesAsync_UsesLazyAppTokenForGistApiFetches()
139139
{
140140
var accessTokenProvider = new CountingAccessTokenProvider();
141-
var handler = new StubGithubHandler();
141+
var handler = new StubGithubHandler
142+
{
143+
ExhaustAnonymousGistSearchRateLimit = true
144+
};
142145
using var httpClient = new HttpClient(handler)
143146
{
144147
BaseAddress = new Uri("https://api.github.com/")
@@ -197,6 +200,31 @@ public async Task SearchGistPagesAsync_AllowsQualifierOnlyQueries()
197200
Assert.NotEmpty(result.TextMatches!);
198201
}
199202

203+
[Fact]
204+
public async Task GetGistFilesAsync_ReusesGistFetchedDuringSearch()
205+
{
206+
var handler = new StubGithubHandler();
207+
using var httpClient = new HttpClient(handler)
208+
{
209+
BaseAddress = new Uri("https://api.github.com/")
210+
};
211+
using var client = new GithubClient(httpClient: httpClient);
212+
213+
GithubSearchResult? result = null;
214+
215+
await foreach (var page in client.SearchGistPagesAsync("Password1"))
216+
{
217+
result = Assert.Single(page.Items);
218+
}
219+
220+
Assert.NotNull(result?.Gist);
221+
var files = await client.GetGistFilesAsync(result.Gist.Id);
222+
223+
Assert.Single(files);
224+
Assert.Equal(1, handler.Requests.Count(request =>
225+
request.PathAndQuery.StartsWith("/gists/abc123abc123abc123abc123abc12312", StringComparison.Ordinal)));
226+
}
227+
200228
private sealed class CountingAccessTokenProvider : IGithubAccessTokenProvider
201229
{
202230
public int CallCount { get; private set; }
@@ -211,6 +239,7 @@ private sealed class CountingAccessTokenProvider : IGithubAccessTokenProvider
211239
private sealed class StubGithubHandler : HttpMessageHandler
212240
{
213241
public List<GithubRequestSnapshot> Requests { get; } = [];
242+
public bool ExhaustAnonymousGistSearchRateLimit { get; init; }
214243

215244
protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
216245
{
@@ -249,8 +278,8 @@ protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage reques
249278
: pathAndQuery.StartsWith("/gists/abc123", StringComparison.Ordinal)
250279
? """
251280
{
252-
"id": "abc123",
253-
"html_url": "https://gist.github.com/octo/abc123",
281+
"id": "abc123abc123abc123abc123abc12312",
282+
"html_url": "https://gist.github.com/octo/abc123abc123abc123abc123abc12312",
254283
"description": "test gist",
255284
"owner": { "login": "octo" },
256285
"files": {
@@ -268,7 +297,7 @@ protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage reques
268297
if (string.Equals(host, "gist.github.com", StringComparison.OrdinalIgnoreCase)
269298
&& pathAndQuery.StartsWith("/search", StringComparison.Ordinal))
270299
{
271-
return Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK)
300+
var response = new HttpResponseMessage(HttpStatusCode.OK)
272301
{
273302
Content = new StringContent(
274303
"""
@@ -285,15 +314,23 @@ protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage reques
285314
""",
286315
Encoding.UTF8,
287316
"text/html")
288-
});
317+
};
318+
if (ExhaustAnonymousGistSearchRateLimit)
319+
{
320+
response.Headers.TryAddWithoutValidation("X-RateLimit-Resource", "core");
321+
response.Headers.TryAddWithoutValidation("X-RateLimit-Remaining", "0");
322+
response.Headers.TryAddWithoutValidation("X-RateLimit-Reset", DateTimeOffset.UtcNow.AddHours(1).ToUnixTimeSeconds().ToString(System.Globalization.CultureInfo.InvariantCulture));
323+
}
324+
325+
return Task.FromResult(response);
289326
}
290327

291328
if (pathAndQuery.StartsWith("/gists/abc123abc123abc123abc123abc12312", StringComparison.Ordinal))
292329
{
293330
json = """
294331
{
295-
"id": "abc123",
296-
"html_url": "https://gist.github.com/octo/abc123",
332+
"id": "abc123abc123abc123abc123abc12312",
333+
"html_url": "https://gist.github.com/octo/abc123abc123abc123abc123abc12312",
297334
"description": "test gist",
298335
"owner": { "login": "octo" },
299336
"files": {

‎GitRekt/GithubClient.cs‎

Lines changed: 55 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,12 @@ internal sealed class GithubClient : IDisposable
2929
private readonly Action? _clearStatusMessage;
3030
private readonly Dictionary<string, GithubRateLimitState> _rateLimitStates = new(StringComparer.OrdinalIgnoreCase);
3131
private readonly Dictionary<string, string> _fileContentCache = new(StringComparer.OrdinalIgnoreCase);
32+
private readonly Dictionary<string, GithubGistResponse> _gistCache = new(StringComparer.OrdinalIgnoreCase);
3233
private readonly Dictionary<string, string> _gistFileContentCache = new(StringComparer.OrdinalIgnoreCase);
3334
private readonly Dictionary<string, GithubRepositoryTreeResponse> _repositoryTreeCache = new(StringComparer.OrdinalIgnoreCase);
3435
private readonly object _rateLimitLock = new();
3536
private readonly object _fileContentCacheLock = new();
37+
private readonly object _gistCacheLock = new();
3638
private readonly object _gistFileContentCacheLock = new();
3739
private readonly object _repositoryTreeCacheLock = new();
3840

@@ -534,14 +536,34 @@ private async Task<GithubGistSearchPage> GetGistSearchPageAsync(string query, in
534536

535537
private async Task<GithubGistResponse> GetGistAsync(string gistId, CancellationToken cancellationToken)
536538
{
539+
lock (_gistCacheLock)
540+
{
541+
if (_gistCache.TryGetValue(gistId, out var cachedGist))
542+
{
543+
return cachedGist;
544+
}
545+
}
546+
537547
var requestUri = $"gists/{Uri.EscapeDataString(gistId)}";
538-
return await GetJsonAsync(
548+
var gist = await GetJsonAsync(
539549
requestUri,
540550
"core",
541551
"GitHub gist fetch failed",
542552
GithubJsonSerializerContext.Default.GithubGistResponse,
543553
cancellationToken,
544554
allowAnonymousRetry: true);
555+
556+
lock (_gistCacheLock)
557+
{
558+
_gistCache.TryAdd(gistId, gist);
559+
560+
if (!string.IsNullOrWhiteSpace(gist.Id))
561+
{
562+
_gistCache.TryAdd(gist.Id, gist);
563+
}
564+
}
565+
566+
return gist;
545567
}
546568

547569
private async Task<IReadOnlyList<GithubSearchResult>> SearchGistAsync(GithubGistResponse gist, IReadOnlyList<string> searchTerms, CancellationToken cancellationToken)
@@ -852,14 +874,15 @@ private async Task<HttpResponseMessage> SendGetAsync(
852874
bool allowAnonymousRetry,
853875
bool useAuthentication = true)
854876
{
855-
await WaitForKnownRateLimitAsync(rateLimitResource, cancellationToken);
856-
857877
if (useAuthentication)
858878
{
859879
await EnsureFreshAccessTokenAsync(forceRefresh: false, cancellationToken);
860880
}
861881

862882
var authorization = _httpClient.DefaultRequestHeaders.Authorization;
883+
var useAuthenticatedRateLimit = useAuthentication && authorization is not null;
884+
885+
await WaitForKnownRateLimitAsync(rateLimitResource, useAuthenticatedRateLimit, cancellationToken);
863886

864887
if (!useAuthentication)
865888
{
@@ -880,7 +903,7 @@ private async Task<HttpResponseMessage> SendGetAsync(
880903
}
881904
}
882905

883-
UpdateRateLimitState(response, rateLimitResource);
906+
UpdateRateLimitState(response, rateLimitResource, useAuthenticatedRateLimit);
884907

885908
if (!allowAnonymousRetry || response.IsSuccessStatusCode || authorization is null || !useAuthentication)
886909
{
@@ -897,8 +920,9 @@ private async Task<HttpResponseMessage> SendGetAsync(
897920
try
898921
{
899922
_httpClient.DefaultRequestHeaders.Authorization = null;
923+
await WaitForKnownRateLimitAsync(rateLimitResource, useAuthentication: false, cancellationToken);
900924
var anonymousResponse = await _httpClient.GetAsync(requestUri, cancellationToken);
901-
UpdateRateLimitState(anonymousResponse, rateLimitResource);
925+
UpdateRateLimitState(anonymousResponse, rateLimitResource, useAuthentication: false);
902926
return anonymousResponse;
903927
}
904928
finally
@@ -934,9 +958,11 @@ private async Task<bool> TryRefreshAccessTokenAsync(CancellationToken cancellati
934958
return _httpClient.DefaultRequestHeaders.Authorization is not null;
935959
}
936960

937-
private async Task WaitForKnownRateLimitAsync(string rateLimitResource, CancellationToken cancellationToken)
961+
private async Task WaitForKnownRateLimitAsync(string rateLimitResource, bool useAuthentication, CancellationToken cancellationToken)
938962
{
939-
while (TryGetKnownRateLimitDelay(rateLimitResource, out var retryDelay))
963+
var rateLimitStateKey = CreateRateLimitStateKey(rateLimitResource, useAuthentication);
964+
965+
while (TryGetKnownRateLimitDelay(rateLimitStateKey, out var retryDelay))
940966
{
941967
if (retryDelay > MaxAutomaticRateLimitDelay)
942968
{
@@ -974,7 +1000,7 @@ private bool TryGetKnownRateLimitDelay(string rateLimitResource, out TimeSpan re
9741000
return false;
9751001
}
9761002

977-
private void UpdateRateLimitState(HttpResponseMessage response, string fallbackResource)
1003+
private void UpdateRateLimitState(HttpResponseMessage response, string fallbackResource, bool useAuthentication)
9781004
{
9791005
var resource = TryGetHeaderValue(response.Headers, "X-RateLimit-Resource", out var resourceValue)
9801006
? resourceValue!
@@ -995,12 +1021,19 @@ private void UpdateRateLimitState(HttpResponseMessage response, string fallbackR
9951021
return;
9961022
}
9971023

1024+
var rateLimitStateKey = CreateRateLimitStateKey(resource, useAuthentication);
1025+
9981026
lock (_rateLimitLock)
9991027
{
1000-
_rateLimitStates[resource] = new GithubRateLimitState(remaining, resetAt);
1028+
_rateLimitStates[rateLimitStateKey] = new GithubRateLimitState(remaining, resetAt);
10011029
}
10021030
}
10031031

1032+
private static string CreateRateLimitStateKey(string resource, bool useAuthentication)
1033+
{
1034+
return $"{(useAuthentication ? "auth" : "anonymous")}:{resource}";
1035+
}
1036+
10041037
private bool TryGetRateLimitDelay(
10051038
HttpResponseMessage response,
10061039
GithubErrorResponse? errorResponse,
@@ -1115,9 +1148,9 @@ private HttpRequestException CreateRateLimitException(HttpStatusCode statusCode,
11151148
builder.Append($". Retry after {FormatDelay(retryDelay)}");
11161149
}
11171150

1118-
if (!_hasAuthentication)
1151+
if (ShouldSuggestPatForRateLimit(rateLimitMessage))
11191152
{
1120-
builder.Append(". Set GITHUB_ACCESS_TOKEN or pass --token to increase rate limits");
1153+
builder.Append(". For gist scans, use a GitHub PAT with --token or GITHUB_ACCESS_TOKEN to increase this limit; GitHub App installation tokens do not raise limits for arbitrary public gist reads");
11211154
}
11221155

11231156
if (!string.IsNullOrWhiteSpace(rateLimitMessage))
@@ -1128,6 +1161,17 @@ private HttpRequestException CreateRateLimitException(HttpStatusCode statusCode,
11281161
return new HttpRequestException(builder.ToString(), null, statusCode);
11291162
}
11301163

1164+
private bool ShouldSuggestPatForRateLimit(string? rateLimitMessage)
1165+
{
1166+
if (!_hasAuthentication)
1167+
{
1168+
return true;
1169+
}
1170+
1171+
return !string.IsNullOrWhiteSpace(rateLimitMessage)
1172+
&& rateLimitMessage.Contains("Authenticated requests get a higher rate limit", StringComparison.OrdinalIgnoreCase);
1173+
}
1174+
11311175
private string FormatGithubFailureMessage(string failurePrefix, HttpStatusCode statusCode, string? errorMessage)
11321176
{
11331177
var message = string.IsNullOrWhiteSpace(errorMessage)

0 commit comments

Comments
 (0)