handle rate limit in httpClientService#450
Conversation
Signed-off-by: Stefan Bischof <[email protected]>
…rvice.java Co-authored-by: Hannes Wellmann <[email protected]>
| continue; | ||
| } | ||
|
|
||
| if (response.statusCode() == 429 && tries++ < maxRatelimitRequestsPerExecure) { |
There was a problem hiding this comment.
I'm thinking that reusing tries could result in odd-to-explain behaviour. Five 502s, followed by a 429 means that we stop trying. I think that it makes sense, but is that the behaviour that we want to have?
| Long resetTime = oResetTime.map(Long::valueOf).orElseGet(System::currentTimeMillis); | ||
| Long sleepTime = resetTime - System.currentTimeMillis() + saftyMargin; | ||
| logger.info("x-ratelimit-reset needs a sleep for: " + sleepTime + " ms"); | ||
| Thread.sleep(sleepTime); |
There was a problem hiding this comment.
I'm already uncomfortable with possibly waiting 10 seconds to bail after repeated 502s. This could -- under a very extreme set of circumstances -- block for 5 minutes. It's unlikely, but it is possible.
IMHO, we need to be able to let the user decide how to handle this. We could, for example, add a -D parameter to configure how many times to retry. With this first implementation, the default should be 0.
| if (response.statusCode() == 429 && tries++ < maxRatelimitRequestsPerExecure) { | ||
| long saftyMargin = 100l; | ||
| logger.info("HTTP response 429 (ratelimit)"); | ||
| Optional<String> oResetTime = response.headers().firstValue("x-ratelimit-reset"); |
There was a problem hiding this comment.
Nit: I would inline this line with the next to avoid the quasi Hungarian Notation. IMHO, it would read better and isn't a violation of the Law of Demeter.
| } | ||
|
|
||
| if (response.statusCode() == 429 && tries++ < maxRatelimitRequestsPerExecure) { | ||
| long saftyMargin = 100l; |
3 participants
this max helps with #448