Skip to content

Rate Limiter Service#97

Merged
Rick-Anderson merged 3 commits into
dotnet:mainfrom
fiyazbinhasan:rate-limiter-service
Oct 13, 2022
Merged

Rate Limiter Service#97
Rick-Anderson merged 3 commits into
dotnet:mainfrom
fiyazbinhasan:rate-limiter-service

Conversation

@fiyazbinhasan
Copy link
Copy Markdown
Contributor

Fixes #14

@Rick-Anderson please review. I found that naming HomeController as Home2Controller is a bit weird. I renamed it back to HomeController. Let me know if the name Home2 was intentional or not. The project was giving me some errors when I started working on it. I tried to fix the issues. Please run the app with different rate limiter configurations. If I miss any configuration. let me know. Thanks 😸

@Rick-Anderson
Copy link
Copy Markdown
Contributor

HomeController as Home2Controller is a bit weird.

I did that because when using Razor Pages and Home controller you have two defaults for the home page.

Thanks for doing this, I'll check out the code.

@Rick-Anderson
Copy link
Copy Markdown
Contributor

@maartenba @wtgodbe all the rate limiters stopped working. I'm testing with localhost and hitting refresh quickly. The timestamp changes but I never hit the limit.

using Microsoft.AspNetCore.RateLimiting;
using System.Threading.RateLimiting;

var builder = WebApplication.CreateBuilder(args);

builder.Services.AddRateLimiter(_ => _
    .AddFixedWindowLimiter(policyName: "fixed", options =>
    {
        options.PermitLimit = 4;
        options.Window = TimeSpan.FromSeconds(120);
        options.QueueProcessingOrder = QueueProcessingOrder.OldestFirst;
        options.QueueLimit = 2;
    }));

var app = builder.Build();

static string GetTicks() => (DateTime.Now.Ticks & 0x11111).ToString("00000");

app.MapGet("/", () => Results.Ok($"FixedWindowLimiter {GetTicks()}"))
                           .RequireRateLimiting("fixed");

app.Run();

@maartenba
Copy link
Copy Markdown
Contributor

Haven't tested yet, but looks from that snippet a app.UseRateLimiter(); is missing.

@fiyazbinhasan
Copy link
Copy Markdown
Contributor Author

@maartenba @Rick-Anderson, I guess it should have both AddRateLimiter and UseRateLimiter? Although, I'm a bit confused about the configurations. In AddRateLimiter, an Action<RateLimiterOptions> can be configured. Then again in UseRateLimiter(), an instance of RateLimiterOptions can also be set. However, it throws an exception when running the app,

InvalidOperationException: This endpoint requires a rate limiting policy with name fixed, but no such policy exists.

Please take a look at the following configuration for the scenario I'm talking about

using Microsoft.AspNetCore.RateLimiting;
using System.Threading.RateLimiting;

var builder = WebApplication.CreateBuilder(args);

builder.Services.AddRateLimiter(_ =>
{
    _.AddFixedWindowLimiter(policyName: "fixed", options =>
        {
            options.PermitLimit = 4;
            options.Window = TimeSpan.FromSeconds(12);
            options.QueueProcessingOrder = QueueProcessingOrder.OldestFirst;
            options.QueueLimit = 2;
        });

    _.RejectionStatusCode = StatusCodes.Status429TooManyRequests;
});

var app = builder.Build();

app.UseRateLimiter(new RateLimiterOptions
{
    RejectionStatusCode = StatusCodes.Status429TooManyRequests
});

static string GetTicks() => (DateTime.Now.Ticks & 0x11111).ToString("00000");

app.MapGet("/", () => Results.Ok($"Hello {GetTicks()}"))
                           .RequireRateLimiting("fixed");

app.Run();

Any reason behind the overload of UseRateLimiter that also takes the RateLimiterOptions?

@maartenba
Copy link
Copy Markdown
Contributor

First one is registered as options in the service container and lets you add multiple policies etc.
Latter one is "fixed" and initialized immediately.

@fiyazbinhasan
Copy link
Copy Markdown
Contributor Author

@maartenba got it. thanks 😺

@Rick-Anderson it would be great if you can come up with a scenario where a rate limiter needs to be initialized immediately using app.UseRateLimiter(new RateLimiterOptions()). As of right now, every sample has a simple call to `app.UseRateLimiter()``

@wtgodbe
Copy link
Copy Markdown
Member

wtgodbe commented Oct 13, 2022

Any reason behind the overload of UseRateLimiter that also takes the RateLimiterOptions?

This was the only way to add rate limiting & configure options in the original design. We kept it around after adding the ServiceCollection extension to not break old apps, and to allow users to set up their app in either way. They're not really meant to be used in tandem.

@fiyazbinhasan
Copy link
Copy Markdown
Contributor Author

So it comes down to this: you always need the app.UseRateLimiter() middleware in the pipeline without any parameters along with the ServiceCollection extension. And if you want to initialize the rate limiter immediately without any specific policy or other configurations, no need to call the app.AddRateLimiter(Action<RateLimiterOptions>); simply go with just the app.UseRateLimiter(new RateLimiterOptions())! Correct me if I'm wrong @wtgodbe

@wtgodbe
Copy link
Copy Markdown
Member

wtgodbe commented Oct 13, 2022

So it comes down to this: you always need the app.UseRateLimiter() middleware in the pipeline without any parameters along with the ServiceCollection extension. And if you want to initialize the rate limiter immediately without any specific policy or other configurations, no need to call the app.AddRateLimiter(Action); simply go with just the app.UseRateLimiter(new RateLimiterOptions())! Correct me if I'm wrong @wtgodbe

Yep, you got it!

@Rick-Anderson Rick-Anderson merged commit a3d1c88 into dotnet:main Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

write sample using service for rate limit

4 participants