Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
LiamKarlMitchell opened this issue Sep 21, 2023 · 3 comments
Labels
enhancement A feature or improvement

Comments

@LiamKarlMitchell
Copy link

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.

github/docs#22607

Suggests headers.

< x-ratelimit-limit: 5000
< x-ratelimit-remaining: 0
< x-ratelimit-reset: 1670985544
< x-ratelimit-used: 5000
< x-ratelimit-resource: graphql
@spawnia
Copy link
Collaborator

spawnia commented Sep 21, 2023

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 spawnia added the enhancement A feature or improvement label Sep 21, 2023
@LiamKarlMitchell
Copy link
Author

LiamKarlMitchell commented Sep 21, 2023

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.

@spawnia
Copy link
Collaborator

spawnia commented Sep 21, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A feature or improvement
Projects
None yet
Development

No branches or pull requests

2 participants