saloonphp / saloon

🤠 Build beautiful API integrations and SDKs with Saloon
https://docs.saloon.dev
MIT License
2.09k stars 107 forks source link

Rate Limit Handler Seems to Be Unreliable With Multiple Workers #346

Closed pikant closed 11 months ago

pikant commented 11 months ago

Problem

I have been tinkering around with the Rate Limit Handler.

With only one worker, it works perfectly fine.

With multiple works, Saloon starts doing great. However, after a minute or so, having 2-5 works running, Saloon throws many RateLimitReachedException, although I have advised it to sleep if the limit is reached. This leads to all workers failing:

https://flareapp.io/share/Lm8qv2kP

Screenshot 2023-12-07 at 12 41 03

My Question: Is there a way to make the Rate Limit Handler ready for a multi-worker setup? Or am I missing something?

My Setup

For my Saloon app, I am using a fresh Laravel 10 installation, created today. Here are the classes in use:

Connector:

<?php

namespace App\Http\Integrations\RateLimitTestConnector;

use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Str;
use Saloon\Http\Connector;
use Saloon\Http\Response;
use Saloon\RateLimitPlugin\Contracts\RateLimitStore;
use Saloon\RateLimitPlugin\Limit;
use Saloon\RateLimitPlugin\Stores\LaravelCacheStore;
use Saloon\RateLimitPlugin\Stores\RedisStore;
use Saloon\RateLimitPlugin\Traits\HasRateLimits;
use Saloon\Traits\Plugins\AcceptsJson;

class RateLimitTestConnector extends Connector
{
    use AcceptsJson;
    use HasRateLimits;

    protected bool $shouldSleep = true;

    public function __construct()
    {
        $workerId = Str::random(6);
        $this->middleware()->onRequest(function () use ($workerId) {
            ray()->count('request worker '.$workerId)->gray();
        });
        $this->middleware()->onResponse(function (Response $response) use ($workerId) {
            ray()->count('response worker '.$workerId)->green();
        });
    }

    public function resolveBaseUrl(): string
    {
        return 'http://laravel-test-api.test/api';
    }

    protected function defaultHeaders(): array
    {
        return [
            'Content-Type' => 'application/json',
            'Accept' => 'application/json',
        ];
    }

    protected function defaultConfig(): array
    {
        return [];
    }

    /*
    | rate limiting
    */

    protected function resolveLimits(): array
    {
        return [
            Limit::allow(10)->everySeconds(10)->sleep(),
        ];
    }

    protected function resolveRateLimitStore(): RateLimitStore
    {
        $redis = new \Redis();
        $redis->connect('127.0.0.1');

        return new RedisStore($redis);

        // also tried this, but it doesn't work either

        return new LaravelCacheStore(Cache::store('redis'));
    }
}

Request:

<?php

namespace App\Http\Integrations\RateLimitTestConnector\Requests;

use Saloon\Enums\Method;
use Saloon\Http\Request;

class GetTest extends Request
{
    protected Method $method = Method::GET;

    public function resolveEndpoint(): string
    {
        // simple hello works endpoint without any rate limiting
        return '/test';
    }
}

Test Command:

<?php

namespace App\Console\Commands;

use App\Http\Integrations\RateLimitTestConnector\RateLimitTestConnector;
use App\Http\Integrations\RateLimitTestConnector\Requests\GetTest;
use Illuminate\Console\Command;

class TestSaloonCommand extends Command
{
    protected $signature = 'test-saloon';

    protected $description = 'Tests saloon';

    public function handle()
    {
        $shopifyConnector = new RateLimitTestConnector();

        while (true) {
            $shopifyConnector->send(new GetTest());

            $this->output->write('.');

            // sleep between 50 and 250 ms
            usleep(50 * random_int(1, 5));
        }
    }
}

To simulate multiple works, I am just spinning up multiple command line tabs. For the Cache, I am using Redis locally.

For the API, I am using also a fresh installation of Laravel that has no rate limits and returns just "Hello Word" as JSON.

pikant commented 11 months ago

In fact, my local API had rate limiting enabled. Saloon automatically picked up this information and threw the exception. I was able to fix the problem simply by enabling retries on the connector.

Sammyjo20 commented 11 months ago

Glad you were able to sort the issue out! 🤠