Open leonardyrj opened 1 year ago
Hey @leonardyrj thank you for opening this issue - I'm very sorry for the slow reply on this one, been very busy lately!
Could this be throwing an exception because of the automatic 429 detection? Try disabling that and see if it works. You have to configure the limit to be slightly lower than the real API limit.
https://docs.saloon.dev/plugins/handling-rate-limits#429-too-many-attempts-detection
Confirmed this is happening to me too. The stack trace shows that it is ignoring the sleep here. However if I dump and die the Limit at an earlier stage, it shows that sleep is set to true.
I don't know if I have time to work it out but I will need to figure this out before the xmas rush as its causing downtime for us when it gets busy.
Note: our threshold is set to 0.8, and the rate limit is 1000 per minute, so there should be about a 200 request buffer for us
Ahhh so it is the fallback "too many attempts" limiter kicking in. I have no idea how its leaking an additional 200 requests and going over the defined limit, but for now this is my work around for making the sleep work on the custom limiter. Hopefully there is no knock-on issues from the change but for now this gets me out of the hole while I work out where the extra requests are going.
On the connector:
/**
* Get the limits for the rate limiter store
*
* @return array<Limit>
* @throws LimitException
*/
public function getLimits(): array
{
$tooManyAttemptsHandler = $this->detectTooManyAttempts === true ? $this->handleTooManyAttempts(...) : null;
$limits = LimitHelper::configureLimits($this->resolveLimits(), $this->getLimiterPrefix(), $tooManyAttemptsHandler);
foreach ($limits as $limit) {
if (!$limit->getShouldSleep()) {
$limit->sleep();
}
}
return $limits;
}
It happens me too.
@ClaraLeigh bit late, but could the leak be because the requests are concurrent? 200 requests hitting an endpoint at exactly the same time would make them all read the same rate limit right?
But then again, we hit a 429 too and we're not doing anything concurrent on our end, just a command with a loop doing requests.
I tested it, it only works when hitting the manual limits, not when the 429 is detected. See #https://github.com/Sammyjo20/saloon-docs/issues/72
I added sleep according to the manual so it doesn't throw an exception and it's still throwing it. It's falling into this if.
protected function resolveLimits(): array { return [ Limit::allow(60)->everyMinute()->sleep() ]; }
` protected function handleExceededLimit(Limit $limit, PendingRequest $pendingRequest): void { if (! $limit->getShouldSleep()) { $this->throwLimitException($limit); }
`