lighthouse icon indicating copy to clipboard operation
lighthouse copied to clipboard

When using throttle directive how can the client be informed when they can retry again etc.

Open LiamKarlMitchell opened this issue 1 year ago • 3 comments

What problem does this feature proposal attempt to solve?

When the @throttle directive is applied and hit, the client side has no way to know when they can re-try.

Which possible solutions should be considered?

How could the problem be solved? What alternative solutions exist? Which is best?

Status on the response is 200 OK not a 429 Too many requests http status which is ok I suppose as there is the errors array.

json
{
    "errors": [
        {
            "message": "Rate limit for Query.user exceeded. Try again later.",
            "extensions": {
                "category": "rate-limit"
            },

But no information is included with the error for when client can re-try their request. It may not be configured to decay at a rate of 1 minute after-all.

Graphql Spec does not mention how to error specifically for rate limit information presumably details could be added to the extended data. https://spec.graphql.org/October2021/#sec-Errors

Reading into this elsewhere on githubs api docs for example.

https://docs.github.com/en/graphql/overview/resource-limitations#rate-limit Suggests a rateLimit object.

Where this issue for githubs own graphql mentions response headers.

https://github.com/github/docs/issues/22607

Suggests headers.

< x-ratelimit-limit: 5000
< x-ratelimit-remaining: 0
< x-ratelimit-reset: 1670985544
< x-ratelimit-used: 5000
< x-ratelimit-resource: graphql

LiamKarlMitchell avatar Sep 21 '23 11:09 LiamKarlMitchell

Lighthouse leverages Laravel's rate limiting, which is rather simplistic in its implementation.

I could see adding additional structured data to rate limit errors as described in https://docs.github.com/en/graphql/overview/resource-limitations#rate-limit, but am unsure how much information the Laravel rate limiter provides.

A solution involving HTTP headers does not belong with @throttle or even Lighthouse at all, since that would mix the responsibilites of the GraphQL and HTTP layer. @throttle provides per-field rate limits, not per-request.

spawnia avatar Sep 21 '23 11:09 spawnia

Right, that makes sense. Laravel throttle does have an availableIn which returns the seconds https://laravel.com/docs/10.x/rate-limiting#determining-limiter-availability

I see if name is not set on the throttle directive, then the key includes the IP address of the request.

'key' => sha1($this->directiveArgValue('prefix') . $this->request->ip()),

Looks possible to get attempts, availableIn, remaining.

    protected function handleLimit(string $key, int $maxAttempts, float $decayMinutes, string $fieldReference): void
    {
        if ($this->limiter->tooManyAttempts($key, $maxAttempts)) {
            $this->limiter->attempts($key);
            $this->limiter->availableIn($key);
            $this->limiter->remaining($key, $maxAttempts);
            throw new RateLimitException($fieldReference);
        }

        $this->limiter->hit($key, (int) ($decayMinutes * 60));
    }

So it should be possible to extend the RateLimitException with ProvidesExtensions and provide more information from the limiter.

Question would be if doing so, what would the property names look like and what data to return back.

LiamKarlMitchell avatar Sep 21 '23 13:09 LiamKarlMitchell

I am open for pull requests that improve how @throttle works, but am not planning to work on it myself.

spawnia avatar Sep 21 '23 13:09 spawnia