Rate Limiter is not based off the return headers from Riot. 429 Errors are not handled.

[toothlessG22]

If you run multiple instances of RiotSharp at once or use the same API Key in another application the rate limiter does not prevent exceeding the rate limit. This leads to 429 errors. However, HandleRequestFailure fails to handle 429 errors, so instead the JSON parser ends up being unable to parse the returned data. This is super unintuitive because the error thrown is about the JSON parser, but the real problem is the rate limiter.

Potential fixes: Change the rate limiter to use the values provided in the return header by Riot. Catch 429 errors. <-- Probably should do this no matter what.

[BenFradet]

These two ideas seem pretty good to me.

Regarding the first, does Riot provide useful values in the headers? iirc, there were pretty useless a while back

[BenFradet]

Also this might be two different issues as 429 need to be handled anyway

[toothlessG22]

I think the values are pretty useful. They talk about the values here. There seem to be two ways of doing this. Either don't make the request that will put you over the limit. Or make the request and use the 429 header Retry-After to set a delay until the next call.

The first option is probably the right thing to do, but the second one might be easier to implement.

[toothlessG22]

Implementing this, especially for async, is harder than I thought. The problem with checking the X-App-Rate-Limit header is that you need to wait for the response from the server. The point of async is to avoid waiting, so that option does not work.

Using the Retry-After header might be able to be done by having a static variable in the RateLimiter that stores the latest Retry-Time given by the header. Then on a 429 Error, the requester could update the retry-time and then delay until the try time. However, I don't think that will work because the program will shoot as many requests as it can, and it won't know to stop until the first rate limit exceeding case recieved a response. So, it looks like this is not a great idea to add.

With the new 429 Error, it's much easier to see that you are going over the rate limit. This allows the user to know that they are being rate limited. The clarity on this error will allow them to be able to find out what is causing it on their end.

[MingweiSamuel]

I originally included the SetRetryAfter method to be used for this situation, but there wasn't/isn't easy access to the RateLimiter from the point where http errors were handled so I left it unused. I believe it basically does what is described above (method 2) and it does have the problem that an extra async request (maybe multiple? - not sure) will go through (kinda noted in the doc). Also, relevant java code

Another non-fix fix for the extra async request issue is to retry requests (#298)

If you are running multiple instances of RiotSharp on one API key, it is probably best to manually set the rate limit and divide by the number of concurrent instances, regardless of the 429 issue.

[MingweiSamuel]

Going to take a look at fixing this along with #298