nuwave / lighthouse

A framework for serving GraphQL from Laravel
https://lighthouse-php.com
MIT License
3.36k stars 438 forks source link

Throttle directive sets wrong decay time #2569

Closed kompotov closed 3 months ago

kompotov commented 3 months ago

Hello there. I'm trying to use @throttle directive on my Laravel 11 app and instead of 1 minute decay I'm getting 2.5 days decay.

There's my Rate limiter

RateLimiter::for('global', function (Request $request) {
    return Limit::perMinute(5)->by($request->ip());
});

The problem might be related to this string where we multiplying our 60 seconds by 60 https://github.com/nuwave/lighthouse/blob/7f9a6d844ab8ce83f50805973a6bb066e449f953/src/Schema/Directives/ThrottleDirective.php#L87 And then we multiplying our 3600 seconds by another 60 here https://github.com/nuwave/lighthouse/blob/7f9a6d844ab8ce83f50805973a6bb066e449f953/src/Schema/Directives/ThrottleDirective.php#L121

As I can guess there should be divide operator instead of multiply on line 87

spawnia commented 3 months ago

Can you add a pull request that fixes this? Please add a failing test to either https://github.com/nuwave/lighthouse/blob/master/tests/Unit/Schema/Directives/ThrottleDirectiveTest.php or https://github.com/nuwave/lighthouse/blob/master/tests/Integration/Schema/Directives/ThrottleDirectiveTest.php first.

kompotov commented 3 months ago

Hey, I can give it a try in a few days

travisricks commented 3 months ago

Hey @kompotov, I didn't mean to step on your toes, but this bug is affecting my app so I submitted a PR to fix it.

spawnia commented 3 months ago

Thank you @travisricks for the fix, released with https://github.com/nuwave/lighthouse/releases/tag/v6.38.1.