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

Throttle middleware does not always reset after defined duration #51779

Closed
kapir opened this issue Jun 12, 2024 · 9 comments
Closed

Throttle middleware does not always reset after defined duration #51779

kapir opened this issue Jun 12, 2024 · 9 comments
Assignees

Comments

@kapir
Copy link

kapir commented Jun 12, 2024

Laravel Version

10.44.0

PHP Version

8.2.20

Database Driver & Version

No response

Description

We are experiencing 429 error codes from our API, which is unexpected. Inspecting our Sentry issues, it seems that the RateLimiting levels are not reset over time. This occurs randomly. Initially, we used throttle:60,5, and this happened for 2% of our users. We are now using RateLimiter::for and seeing it for 0.02%.

Setup:

RateLimiter::for('api', function (Request $request) {
    return Limit::perMinutes(10, 600)->by($request->user()?->id ?: $request->ip());
   });

    protected $middlewareGroups = [
        'web' => [
...
        ],

        'api' => [
            'throttle:api',
            'bindings',
            'emptyStringsToNull',
        ],
    ];

// config/cache.php
return [    'default' => 'file' 
...
];

Logging Details:
For IP address X and user A:

  • At Jun 9, 2024, 9:55:43 AM UTC, the header x-ratelimit-remaining = 44
  • At Jun 9, 2024, 12:12:51 PM UTC, the header x-ratelimit-remaining = 43
    There are no other logged interactions for either user A or IP address X in this time range.

Speculation
One possible cause could be concurrent requests to the API. This might affect the rate limiting behavior

Steps To Reproduce

We have a hard time reproducing this ourselves, but i am open to provide access to our application if that could help you fix this issue.

@timacdonald
Copy link
Member

@kapir, did you happen to change your rate limits when you noticed this, i.e., users were allowed to make more or less requests to the endpoint?

Also, are you noticing this issue for authenticated users or guests? Because you are using an "IP" fallback, it is possible if you have a proxy, like Cloudflare, that the clients actual IP address is not being used to rate limit but instead the proxy's IP address.

Lastly, I would recommend not using the file cache in production. It never clears itself. I would recommend Redis, Memcache, or the database driver.

@kapir
Copy link
Author

kapir commented Jun 18, 2024

  1. Rate limiting was not changed in this time range. The error still occurs today for our users
  2. 99,9% of our traffic is for authenticated user, so we have currently only seen this issue for authenticated users
  3. So this indicates that the issue is the file cache? I will follow up on your recommendations.

Thank you for replying

@timacdonald
Copy link
Member

I see that 99.9% of users are authenticated but this is only impacting 0.02% of traffic, which does seem to feel like it could be the IP issue I mentioned. I know that isn't perfect math, but it does feel like it fits the story.

I would certainly check you have client IP forwarding configured property anywhere it is needed.

@driesvints
Copy link
Member

Closing this issue because it's inactive, already solved, old or not relevant anymore. Feel to open up a new issue if you're still experiencing this.

@kapir
Copy link
Author

kapir commented Jul 22, 2024

I see that 99.9% of users are authenticated but this is only impacting 0.02% of traffic, which does seem to feel like it could be the IP issue I mentioned. I know that isn't perfect math, but it does feel like it fits the story.

I would certainly check you have client IP forwarding configured property anywhere it is needed.

Apologies for the late response.
We can see that its the authenticated endpoints that are getting 429 errors, therefore it cannot be the client IP forwarding.

@timacdonald
Copy link
Member

I'd recommend putting some logging in place so you can determine what is going on here.

Personally, I would recommend putting the following middleware in place globally in your application.

<?php

namespace App\Http\Middleware;

use Illuminate\Support\Facades\Log;

class LoggingMiddleware
{
    public function __invoke($request, $next)
    {
        return $next($request);
    }

    public function terminate($request, $response)
    {
        if ($response->getStatusCode() === 429) {
            Log::debug('Request hit rate limit.',[
                'user_id' => $request->user()->id,
                'ip' => $request->ip(),
                'route' => $request->route()?->uri,
            ]);
        }
    }
}

We likely need to be able reproduce the issue to be able to address it.

Is it possible this is because users are actually hitting the rate limit?

@kapir
Copy link
Author

kapir commented Aug 2, 2024

Hi, as mentioned earlier we have confirmation that users are not hitting the limit within the specified timerange we are logging the received x-ratelimit-remaining headers, that is how we notices that the limit is not reset even with hours inbetween.

We have now added logging as you recommended. This confirms that its authenticated users that are receiving throttling, they are also on different ip-addresses.

Request hit rate limit. {"user_id":6284,"ip":"ip a","route":"api.messages"}
Request hit rate limit. {"user_id":11375,"ip":"ip b","route":"api.profile"}

As mentioned above the only case i can see happening, is that somehow parallel requests does something to how the throttle reads/handles its values.

Do note that we have not moved of the file cache yet.

Thank you again for your replies.

@timacdonald
Copy link
Member

Circling back to this, I'm unsure why this would be happening. I can't replicate it on my end and I don't think we have had any other reports of this issue.

If you can provide some more steps for us to reproduce the issue on our end, I would be happy to continue digging in.

Sorry for the troubles on this one.

@kapir
Copy link
Author

kapir commented Oct 30, 2024

Hi, thanks for following up on this. I was actually planning on following as well.

As of two weeks ago we moved from Laravel 10 to 11.
The day after that we also moved from using the File Cache to using Redis cache driver instead.

After doing this we have not experienced the issue at all. It seems that either or both of the steps above have fixed it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants