Skip to content
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.

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

More gracefully handle exceeding the API limit. #985

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 7 additions & 1 deletion POEApi.Infrastructure/Events/ThottledEventArgs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,15 @@ namespace POEApi.Infrastructure.Events
public class ThottledEventArgs : EventArgs
{
public TimeSpan WaitTime { get; private set; }
public ThottledEventArgs(TimeSpan waitTime)
/// <summary>
/// Whether the throttling event was expected. If it was not expected, there might be other agents or
/// untracked actions using up resources towards the limit.
/// </summary>
public bool Expected { get; private set; }
public ThottledEventArgs(TimeSpan waitTime, bool expected = true)
{
WaitTime = waitTime;
Expected = expected;
}
}
}
21 changes: 18 additions & 3 deletions POEApi.Transport/HttpTransport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -206,11 +206,26 @@ protected HttpWebResponse BuildHttpRequestAndGetResponse(HttpMethod method, stri
protected MemoryStream PerformHttpRequest(HttpMethod method, string url, bool? allowAutoRedirects = null,
string requestData = null)
{
using (var response = BuildHttpRequestAndGetResponse(method, url, allowAutoRedirects, requestData))
HttpWebResponse response = null;
// TODO: Don't retry an infinite number of times.
bool retry = true;
while (retry)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we at least make this a controlled number of attempts, rather than an infinite loop.

{
MemoryStream responseStream = GetMemoryStreamFromResponse(response);
return responseStream;
try
{
response = BuildHttpRequestAndGetResponse(method, url, allowAutoRedirects, requestData);
retry = false;
}
catch (System.Net.WebException ex) when (
!string.IsNullOrWhiteSpace(ex.Message) && ex.Message.Contains("(429) Too Many Requests."))
{
Logger.Log("Exceeded API limit while performing HTTP request: " + ex.ToString());
_taskThrottle.HandleUnexpectedOverload();
}
}

MemoryStream responseStream = GetMemoryStreamFromResponse(response);
return responseStream;
}

// The refresh parameter in this ITransport implementation is ignored.
Expand Down
17 changes: 17 additions & 0 deletions POEApi.Transport/TaskThrottle.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,5 +88,22 @@ protected void RemvoeExpiredTasks()
CurrentTasks.Dequeue();
}
}

public void HandleUnexpectedOverload()
{
lock (_lockObject)
{
// We've unexpectedly gone over the number of allowed requests in a time period. Fill up the set of
// current tasks as if we had started tasks.
while (CurrentTasks.Count < WindowLimit)
CurrentTasks.Enqueue(DateTime.Now);

// Wait at least as long as the WindowSize before trying another task.
var timeUntilNextTask = CurrentTasks.Peek() - DateTime.Now;
TimeSpan waitTime = timeUntilNextTask > WindowSize ? timeUntilNextTask : WindowSize;
Throttled?.Invoke(this, new ThottledEventArgs(waitTime, false)); // Not an expected throttle event.
System.Threading.Thread.Sleep(waitTime);
}
}
}
}
9 changes: 7 additions & 2 deletions Procurement/View/RefreshView.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,14 @@ private void model_StashLoading(POEApi.Model.POEModel sender, POEApi.Model.Event

void model_Throttled(object sender, ThottledEventArgs e)
{
if (e.WaitTime.TotalSeconds > 4)
if (!e.Expected)
update(string.Format("Exceeded GGG Server request limit; throttling activated. Waiting {0} " +
"seconds. Ensure you do not have other instances of Procurement running or other apps using " +
"the GGG API with your account.", Convert.ToInt32(e.WaitTime.TotalSeconds)),
new POEEventArgs(POEEventState.BeforeEvent));
else if (e.WaitTime.TotalSeconds > 4)
update(string.Format("GGG Server request limit hit, throttling activated. Please wait {0} seconds",
e.WaitTime.Seconds), new POEEventArgs(POEEventState.BeforeEvent));
Convert.ToInt32(e.WaitTime.TotalSeconds)), new POEEventArgs(POEEventState.BeforeEvent));
}

private void update(string message, POEEventArgs e)
Expand Down
10 changes: 8 additions & 2 deletions Procurement/ViewModel/LoginWindowViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -345,8 +345,14 @@ void Model_Authenticating(POEModel sender, AuthenticateEventArgs e)

void Model_Throttled(object sender, ThottledEventArgs e)
{
if (e.WaitTime.TotalSeconds > 4)
Update(string.Format("GGG Server request limit hit, throttling activated. Please wait {0} seconds", e.WaitTime.Seconds), new POEEventArgs(POEEventState.BeforeEvent));
if (!e.Expected)
Update(string.Format("Exceeded GGG Server request limit; throttling activated. Waiting {0} " +
"seconds. Ensure you do not have other instances of Procurement running or other apps using " +
"the GGG API with your account.", Convert.ToInt32(e.WaitTime.TotalSeconds)),
new POEEventArgs(POEEventState.BeforeEvent));
else if (e.WaitTime.TotalSeconds > 4)
Update(string.Format("GGG Server request limit hit, throttling activated. Please wait {0} seconds",
Convert.ToInt32(e.WaitTime.TotalSeconds)), new POEEventArgs(POEEventState.BeforeEvent));
}

private void Update(string message, POEEventArgs e)
Expand Down