Skip to content

Handle GitHub API rate limit errors appropriately #4299

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
HebaruSan merged 1 commit into from
Feb 6, 2025
Merged

Handle GitHub API rate limit errors appropriately #4299

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cmdline/Action/Install.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,9 @@ public int RunCommand(CKAN.GameInstance instance, object raw_options)
user.RaiseError("{0}", kraken.ToString());
return Exit.ERROR;
}
catch (DownloadThrottledKraken kraken)
catch (RequestThrottledKraken kraken)
{
user.RaiseError("{0}", kraken.ToString());
user.RaiseError("{0}", kraken.Message);
user.RaiseMessage(Properties.Resources.InstallTryAuthToken, kraken.infoUrl);
return Exit.ERROR;
}
Expand Down
4 changes: 2 additions & 2 deletions ConsoleUI/InstallScreen.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,8 @@ public override void Run(Action? process = null)
RaiseError("{0}", ex.ToString());
} catch (ModuleDownloadErrorsKraken ex) {
RaiseError("{0}", ex.ToString());
} catch (DownloadThrottledKraken ex) {
if (RaiseYesNoDialog(string.Format(Properties.Resources.InstallAuthTokenPrompt, ex.ToString()))) {
} catch (RequestThrottledKraken ex) {
if (RaiseYesNoDialog(string.Format(Properties.Resources.InstallAuthTokenPrompt, ex.Message))) {
if (ex.infoUrl != null) {
ModInfoScreen.LaunchURL(theme, ex.infoUrl);
}
Expand Down
4 changes: 2 additions & 2 deletions Core/Net/NetAsyncDownloader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,9 @@ public void DownloadAndWait(ICollection<DownloadTarget> targets)
&& url.IsAbsoluteUri
&& Net.ThrottledHosts.TryGetValue(url.Host, out Uri? infoUrl)
&& infoUrl is not null
? new DownloadThrottledKraken(url, infoUrl)
? new RequestThrottledKraken(url, infoUrl, wex)
: null)
.OfType<DownloadThrottledKraken>()
.OfType<RequestThrottledKraken>()
.FirstOrDefault();
if (throttled is not null)
{
Expand Down
3 changes: 1 addition & 2 deletions Core/Properties/Resources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,7 @@ If the game is still running, close it and try again. Otherwise check the permis
Consult this page for help:
{0}</value></data>
<data name="KrakenMissingCertificateNotUnix" xml:space="preserve"><value>Oh no! Our download failed with a certificate error!</value></data>
<data name="KrakenDownloadThrottled" xml:space="preserve"><value>Download from {0} was throttled.
Consider adding an authentication token to increase the throttling limit.</value></data>
<data name="KrakenDownloadThrottled" xml:space="preserve"><value>Request to {0} throttled</value></data>
<data name="KrakenAlreadyRunning" xml:space="preserve"><value>Lock file with live process ID found at:
{0}

Expand Down
47 changes: 39 additions & 8 deletions Core/Types/Kraken.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.IO;
using System.Linq;
using System.Collections.Generic;
using System.Net;

using log4net;

Expand Down Expand Up @@ -452,21 +453,51 @@ public override string ToString()
}
}

public class DownloadThrottledKraken : Kraken
public class RequestThrottledKraken : Kraken
{
public readonly Uri throttledUrl;
public readonly Uri infoUrl;
public readonly Uri throttledUrl;
public readonly Uri infoUrl;
public readonly DateTime? retryTime;

public DownloadThrottledKraken(Uri url, Uri info) : base()
public RequestThrottledKraken(Uri url, Uri info, WebException exc, string? reason = null)
: this(url, info, ExceptionRetryTimes(exc).Max(), reason)
{
throttledUrl = url;
infoUrl = info;
}

public override string ToString()
public RequestThrottledKraken(Uri url, Uri info,
DateTime? retryTime, string? reason = null)
: base(reason
?? string.Format(Properties.Resources.KrakenDownloadThrottled,
url.Host))
{
throttledUrl = url;
infoUrl = info;
this.retryTime = retryTime;
}

// https://docs.github.com/en/rest/using-the-rest-api/best-practices-for-using-the-rest-api?apiVersion=2022-11-28#handle-rate-limit-errors-appropriately
private static IEnumerable<DateTime> ExceptionRetryTimes(WebException exc)
{
return string.Format(Properties.Resources.KrakenDownloadThrottled, throttledUrl.Host);
// If the retry-after response header is present, you should not retry your request
// until after that many seconds has elapsed.
if (exc.Response?.Headers["Retry-After"] is string waitString
&& int.TryParse(waitString, out int waitSeconds))
{
yield return DateTime.UtcNow + TimeSpan.FromSeconds(waitSeconds);
}
// If the x-ratelimit-remaining header is 0, you should not make another request
// until after the time specified by the x-ratelimit-reset header.
// The x-ratelimit-reset header is in UTC epoch seconds.
if (exc.Response?.Headers["X-RateLimit-Reset"] is string epochString
&& int.TryParse(epochString, out int epochSeconds))
{
yield return UnixEpoch + TimeSpan.FromSeconds(epochSeconds);
}
// Otherwise, wait for at least one minute before retrying.
yield return DateTime.UtcNow + TimeSpan.FromMinutes(1);
}

private static readonly DateTime UnixEpoch = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc);
}

public class RegistryInUseKraken : Kraken
Expand Down
4 changes: 2 additions & 2 deletions GUI/Main/MainInstall.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,8 +489,8 @@ private void PostInstallMods(object? sender, RunWorkerCompletedEventArgs? e)
currentUser.RaiseMessage("{0}", exc.ToString());
break;

case DownloadThrottledKraken exc:
string msg = exc.ToString();
case RequestThrottledKraken exc:
string msg = exc.Message;
currentUser.RaiseMessage("{0}", msg);
if (configuration != null && CurrentInstance != null
&& YesNoDialog(string.Format(Properties.Resources.MainInstallOpenSettingsPrompt, msg),
Expand Down
8 changes: 8 additions & 0 deletions Netkan/Processors/QueueHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.IO;
using System.Linq;
using System.Text;
using System.Threading;

using Amazon.SQS;
using Amazon.SQS.Model;
Expand Down Expand Up @@ -181,6 +182,13 @@ private IEnumerable<SendMessageBatchRequestEntry> Inflate(Message msg)
// error CS1631: Cannot yield a value in the body of a catch clause
caught = true;
caughtMessage = e.Message;
if (e is RequestThrottledKraken k && k.retryTime is DateTime dt)
{
// Let the API credits recharge
var span = dt.Subtract(DateTime.UtcNow);
caughtMessage += $"; sleeping {span}";
Thread.Sleep(span);
}
}
if (caught)
{
Expand Down
54 changes: 26 additions & 28 deletions Netkan/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,13 @@ public static int Main(string[] args)
if (!string.IsNullOrEmpty(Options.ValidateCkan))
{
var ckan = new Metadata(JObject.Parse(File.ReadAllText(Options.ValidateCkan)));
var inf = new Inflator(
Options.CacheDir,
Options.OverwriteCache,
Options.GitHubToken,
Options.GitLabToken,
Options.NetUserAgent,
Options.PreRelease,
game);
var inf = new Inflator(Options.CacheDir,
Options.OverwriteCache,
Options.GitHubToken,
Options.GitLabToken,
Options.NetUserAgent,
Options.PreRelease,
game);
inf.ValidateCkan(ckan);
Console.WriteLine(QueueHandler.serializeCkan(
PropertySortTransformer.SortProperties(ckan)));
Expand All @@ -72,17 +71,16 @@ is string[] array
&& array[0] is var input
&& array[1] is var output)
{
var qh = new QueueHandler(
input,
output,
Options.CacheDir,
Options.OutputDir,
Options.OverwriteCache,
Options.GitHubToken,
Options.GitLabToken,
Options.NetUserAgent,
Options.PreRelease,
game);
var qh = new QueueHandler(input,
output,
Options.CacheDir,
Options.OutputDir,
Options.OverwriteCache,
Options.GitHubToken,
Options.GitLabToken,
Options.NetUserAgent,
Options.PreRelease,
game);
qh.Process();
return ExitOk;
}
Expand All @@ -94,14 +92,13 @@ is string[] array
var netkans = ReadNetkans(Options);
Log.Info("Finished reading input");

var inf = new Inflator(
Options.CacheDir,
Options.OverwriteCache,
Options.GitHubToken,
Options.GitLabToken,
Options.NetUserAgent,
Options.PreRelease,
game);
var inf = new Inflator(Options.CacheDir,
Options.OverwriteCache,
Options.GitHubToken,
Options.GitLabToken,
Options.NetUserAgent,
Options.PreRelease,
game);
var ckans = inf.Inflate(
Options.File,
netkans,
Expand Down Expand Up @@ -177,7 +174,8 @@ private static CmdLineOptions ProcessArgs(string[] args)

private static Metadata[] ReadNetkans(CmdLineOptions Options)
{
if (!Options.File?.EndsWith(".netkan") ?? false)
if (!Options.File?.EndsWith(".netkan", StringComparison.OrdinalIgnoreCase)
?? false)
{
Log.WarnFormat("Input is not a .netkan file");
}
Expand Down
7 changes: 5 additions & 2 deletions Netkan/Sources/Github/GithubApi.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,12 @@ private static bool TryGetGitHubPath(Uri url,
catch (WebException k)
{
if (((HttpWebResponse?)k.Response)?.StatusCode == HttpStatusCode.Forbidden
&& k.Response.Headers["X-RateLimit-Remaining"] == "0")
&& k.Response.Headers["X-RateLimit-Remaining"] == "0"
&& Net.ThrottledHosts.TryGetValue(url.Host, out Uri? infoUrl)
&& infoUrl is not null)
{
throw new Kraken($"GitHub API rate limit exceeded: {path}");
throw new RequestThrottledKraken(url, infoUrl, k,
$"GitHub API rate limit exceeded: {path}");
}
throw;
}
Expand Down
7 changes: 6 additions & 1 deletion Netkan/Transformers/SpacedockTransformer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,14 @@ private Metadata TransformOne(Metadata metadata, JObject json, SpacedockMod sdMo
}
}
}
catch (RequestThrottledKraken)
{
// Treat rate limiting as a real error to avoid temporary metadata degradation
throw;
}
catch
{
// Just give up, it's fine
// Just give up, invalid URLs are fine
}
}
TryAddResourceURL(metadata.Identifier, resourcesJson, "repository", sdMod.source_code);
Expand Down