[Proposal] Response Header with X-Rate-Limit-Count

[alabama]

Hey,

what do you guys think to add header information to the client response class? With that information you can improve the limit process. The API returns header information described here: https://developer.riotgames.com/docs/rate-limiting

The real problem i run into was this: https://developer.riotgames.com/discussion/community-discussion/show/VeazJAgi RiotSchmick (NA) describes, that you can get a 429 but without the "Retry-After" header information, which means that the league of legends service itself had a problem serving the requested data. For me this issue occurred very often in the matchlist and specific match request.

So what do you think about adding header information into the Response class and handling this "429 Response Error" different?

[phelion]

I favor this idea, have the same problem here :wink:

[dnlbauer]

i'd like to have that too, but im dont have enough time to do that right now :/ If you want to make this, pull requests are welcome as always :)

[m1so]

handling this "429 Response Error" different?

What do you have in mind @alabama @danijoo? Some endpoint won't return X-Rate-Limit-Type or Retry-After header as mentioned in Rate limiting docs, which means that when you hit that endpoint's rate limit there is no way to determine how much time you should wait for the next request.

One option is to add a condition and new exception when checking for request errors in AbstractApi

if ($response->getCode() === 429 && !$response->hasHeader('Retry-After')) {
    throw new UnderlyingServiceRateLimitReached('...');
}

class UnderlyingServiceRateLimitReached extends HttpServerError {} 

but that would require adding header information to Client and ClientInterface classes.

Then you should be able to handle this special case in your code by for example creating auto-retry method

class RequestUtils
{
    public static function autoRetry(callable $f)
    {
        try { 
            $f();
        } catch (UnderlyingServiceRateLimitReached $e) { 
            sleep(2);
            static::autoRetry($f);
        } catch (...) { ... }
    }
}

[dnlbauer]

I merged your pull request. Maybe you can even extend this so UnderlyingServiceRateLimitReached includes a field holding header value of Retry-After:

public static function autoRetry(callable $f)
    {
        try { 
            $f();
        } catch (UnderlyingServiceRateLimitReached $e) { 
            sleep($e->getRetryAfter());
            static::autoRetry($f);
        } catch (...) { ... }
    }

[m1so]

You mean Http429? Because UnderlyingServiceRateLimitReached is thrown only when Retry-After header is not present in the response.

[dnlbauer]

yes :)

[dnlbauer]

yes :)

Michal Baumgartner [email protected] schrieb am Do., 20. Okt. 2016 um 21:08 Uhr:

You mean Http429? Because UnderlyingServiceRateLimitReached is thrown only when Retry-After header is not present in the response.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/paquettg/leaguewrap/issues/119#issuecomment-255199255, or mute the thread https://github.com/notifications/unsubscribe-auth/AEeQ8tSJi1jLBZ8U5nxWvO6ijQnNGZ6Zks5q17wtgaJpZM4Jus1g .