Rate Limit Plugin: Issue with 'allow(1)->everySeconds(1)->sleep()' Exceeding One Request per Second #392
Description
I'm encountering an issue with the rate limit plugin in my project. Specifically, when using the resolveLimits function with the sequence Limit::allow(1)->everySeconds(1)->sleep(), the plugin doesn't consistently adhere to the specified rate limit of one request per second. Occasionally, it exceeds this limit and makes multiple requests within a single second.
Upon investigating the plugin's code, I noticed a potential issue within the handleExceededLimit function. There seems to be a scenario where the $limit->getRemainingSeconds value reaches zero, possibly leading to the delay not being properly applied. While I can't confirm if this is the root cause, I made a modification to this method within my connector, and it appears to have mitigated the issue.
protected function handleExceededLimit(Limit $limit, PendingRequest $pendingRequest): void
{
if (! $limit->getShouldSleep()) {
$this->throwLimitException($limit);
}
$existingDelay = $pendingRequest->delay()->get() ?? 0;
$remainingMilliseconds = ($limit->getRemainingSeconds() === 0 ? 1 : $limit->getRemainingSeconds()) * 1000;
$pendingRequest->delay()->set($existingDelay + $remainingMilliseconds);
}
In production, with the limit of one request per second and without changing anything in the handleExceededLimit method:
In localhost, setting the delay in handleExceededLimit as 0:
In localhost, setting the delay in handleExceededLimit to 1 if it $limit->getRemainingSeconds reaches 0:
My current connector:
<?php
namespace Connector;
use App\Exceptions\UnauthorizedSetup;
use App\Http\Middleware\Saloon\ResponseLogger;
use App\Models\Store;
use App\Traits\InteractsWithSaloonResponses;
use GuzzleHttp\RequestOptions;
use GuzzleHttp\TransferStats;
use Illuminate\Support\Facades\Cache;
use MyIntegration\Http\Pagination\Paged;
use Saloon\Http\Connector;
use Saloon\Http\PendingRequest;
use Saloon\Http\Request;
use Saloon\Http\Response;
use Saloon\PaginationPlugin\Contracts\HasPagination;
use Saloon\RateLimitPlugin\Contracts\RateLimitStore;
use Saloon\RateLimitPlugin\Limit;
use Saloon\RateLimitPlugin\Stores\LaravelCacheStore;
use Saloon\RateLimitPlugin\Stores\MemoryStore;
use Saloon\RateLimitPlugin\Traits\HasRateLimits;
use Saloon\Traits\Plugins\AcceptsJson;
use Saloon\Traits\Plugins\AlwaysThrowOnErrors;
use Saloon\Traits\Plugins\HasTimeout;
use Symfony\Component\HttpFoundation\Response as ResponseCode;
class IntegrationConnector extends Connector implements HasPagination
{
use AcceptsJson;
use AlwaysThrowOnErrors;
use HasRateLimits {
hasRateLimits::handleTooManyAttempts as protected traitHandleTooManyAttempts;
}
use HasTimeout;
use InteractsWithSaloonResponses;
public ?string $requestTime = null;
protected int $requestTimeout = 10;
public ?int $tries = 3;
public ?bool $useExponentialBackoff = true;
private const LIMITER = 'integration-connector';
public function __construct(private readonly Store $store)
{
$oAuth = $this->store->oAuth()->active()->first();
if (is_null($oAuth)) {
throw new UnauthorizedSetup('Could not find active oAuth to use.', fatal: true);
}
$this->middleware()->onRequest(
static fn (PendingRequest $pendingRequest) => $pendingRequest->headers()
->add('Authorization', 'Bearer '.$oAuth->refresh()->access_token),
);
$this->middleware()->onResponse(new ResponseLogger($store));
$this->retryInterval = app()->environment('production') ? 2000 : null;
}
public function resolveBaseUrl(): string
{
return 'integrationUrl';
}
protected function defaultConfig(): array
{
$time = &$this->requestTime;
return [
RequestOptions::VERIFY => false,
RequestOptions::ON_STATS => function (TransferStats $stats) use (&$time) {
$time = (string) $stats->getTransferTime();
},
];
}
public function paginate(Request $request): Paged
{
return new Paged($this, $request);
}
protected function resolveLimits(): array
{
return [
Limit::allow(1)->everySeconds(1)->name('second')->sleep(),
Limit::allow(60)->everyMinute()->name('minute')->sleep(),
Limit::allow(290)->everyFiveMinutes()->name('five-minutes')->sleep(),
Limit::allow(118_000)->everyDay()->name('daily'),
];
}
protected function resolveRateLimitStore(): RateLimitStore
{
if (! app()->environment('production')) {
return new MemoryStore();
}
return new LaravelCacheStore(Cache::store('redis'));
}
protected function handleTooManyAttempts(Response $response, Limit $limit): void
{
if ($response->status() === ResponseCode::HTTP_TOO_MANY_REQUESTS) {
$this->responseLogger($this->store, $response);
}
$this->traitHandleTooManyAttempts($response, $limit);
}
protected function handleExceededLimit(Limit $limit, PendingRequest $pendingRequest): void
{
if (! $limit->getShouldSleep()) {
$this->throwLimitException($limit);
}
$existingDelay = $pendingRequest->delay()->get() ?? 0;
$remainingMilliseconds = ($limit->getRemainingSeconds() === 0 ? 1 : $limit->getRemainingSeconds()) * 1000;
$pendingRequest->delay()->set($existingDelay + $remainingMilliseconds);
}
protected function getLimiterPrefix(): ?string
{
return self::LIMITER;
}
}
I'm seeking guidance on identifying the underlying cause of this inconsistency and exploring potential solutions. Could you please provide insights into what might be causing this behavior and suggest possible remedies?