saloonphp / rate-limit-plugin

🤠 Handle rate limits beautifully in your Saloon API integrations or SDKs
https://docs.saloon.dev
MIT License
15 stars 0 forks source link

Misleading documentation - detectTooManyAttempts #7

Closed xscasnya closed 1 year ago

xscasnya commented 1 year ago

In documentation when we are reading about the disabling of the "too many attempts" detection (429 code), the docs states that we are able to disable it like this:

Additionally, may choose to disable this functionality by adding the protected bool $detectTooManyAttempts = false property to your connector or request which defines the trait.

use Saloon\Http\Connector;
use Saloon\RateLimitPlugin\Stores\MemoryStore;
use Saloon\RateLimitPlugin\Traits\HasRateLimits;

class SpotifyConnector extends Connector
{
    use HasRateLimits;

    protected bool $detectTooManyAttempts = false;
}

However if done, this directly opposes to the php design principles for traits which states the following:

If a trait defines a property then a class can not define a property with the same name unless it is compatible (same visibility and type, readonly modifier, and initial value), otherwise a fatal error is issued.

Source here.

This can be bypassed by setting the value of this in a constructor:

public function __construct()
    {
        $this->detectTooManyAttempts = false;
    }

I'd suggest to change the documentation here. Also I think a better way to handle this would be to make the property private and create a protected getter which could be overriden in the child class (connectors / requests).

Sammyjo20 commented 1 year ago

Hey @xscasnya thank you very much for pointing this out. I forgot that you aren't able to redeclare properties from traits when the class is not abstract - I will update the documentation accordingly. Again, many thanks!