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 on only a single route #1900

Open
1 task done
milad-afkhami opened this issue Feb 27, 2024 · 10 comments
Open
1 task done

Throttle on only a single route #1900

milad-afkhami opened this issue Feb 27, 2024 · 10 comments

Comments

@milad-afkhami
Copy link

Is there an existing issue that is already proposing this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe it

My challenge is how to apply throttling to a specific route without setting up a global throttle. My concern arises from the necessity to log every single request from every single user (in memory) when establishing a global throttling system, to ascertain whether the new request necessitates throttling. How can I accomplish this?

Describe the solution you'd like

Globally config it like:

imports: [
    ...
    ThrottlerModule.forRoot()
]

and use it like this on the route:

@Throttle([{ limit: 3, ttl: 60000 }])
@Get()
findAll() {
  return "List users works with custom rate limiting.";
}

Notes:

  1. It's better to remove the default keyword on the Throttle decorator
  2. It's good to add support for arrays inside the Throttle decorator
  3. I have tested the global config and added a guard with an empty config and it doesn't work.

Teachability, documentation, adoption, migration strategy

No response

What is the motivation / use case for changing the behavior?

For more customizations and more control for throttling and saving more space.

@milad-afkhami
Copy link
Author

@Canoir it was your company's concern too, right?

@jmcdo29
Copy link
Member

jmcdo29 commented Feb 27, 2024

Okay, I think I see the request here. Instead of being forced to use the ThrottlerModule it would be nice to set the @UseGuards() and @Throttle() on a case-by-case basis, correct?

I think the ThrottlerModule should still be imported for the access to the storage service, but that may be movable to not be required in a dynamic module, or we just keep the forRoot as is and allow an empty function call (easiest as there's no changes there)

In the @Throttle(), I can probably allow it to take in the config or a record of name: config for sake of ease and transform it under the hood. If we were to take in either the record or the array, if the array had two configurations, both without name, the latter would overwrite the former and lead to unexpected behavior. Do you think this would make for a good DX?


As for the initial "How can I implement this?", so long as there's a config object passed to the forRoot() (something that has a ttl and limit, it can be ridiculous numbers too, e.g. [{ limit: 0, ttl: 0 }]) the n the guard should work with the overwrite from the @Throttle() decorator

@Canoir
Copy link

Canoir commented Feb 27, 2024

Okay, I think I see the request here. Instead of being forced to use the ThrottlerModule it would be nice to set the @UseGuards() and @Throttle() on a case-by-case basis, correct?

I think the ThrottlerModule should still be imported for the access to the storage service, but that may be movable to not be required in a dynamic module, or we just keep the forRoot as is and allow an empty function call (easiest as there's no changes there)

In the @Throttle(), I can probably allow it to take in the config or a record of name: config for sake of ease and transform it under the hood. If we were to take in either the record or the array, if the array had two configurations, both without name, the latter would overwrite the former and lead to unexpected behavior. Do you think this would make for a good DX?

As for the initial "How can I implement this?", so long as there's a config object passed to the forRoot() (something that has a ttl and limit, it can be ridiculous numbers too, e.g. [{ limit: 0, ttl: 0 }]) the n the guard should work with the overwrite from the @Throttle() decorator

I agree with you and about the name: config , I think that is great too, our need is exactly what you said, but your other answer make me a bigger question, what will happen if we pass ttl: 0 and limit 0 to global config? I mean if the guard only passes cause of 0 then there is a little bit shitty but solution to our problem even now, right?

@jmcdo29
Copy link
Member

jmcdo29 commented Feb 27, 2024

The idea I suggested was just to make sure that there was a default config so that the @Throttle() override properly works. So long as the guard is not applied to any routes without the @Throttle() then the point is moot,. If it is, you'll find out pretty quickly 😄

@mukunda-
Copy link

mukunda- commented Aug 2, 2024

It took me an hour to realize that I need to have a default config block, even if I'm not using it. I'd say it should have a default if not specified.

@dominic-schmid
Copy link

So, I'm not sure if I'm just doing something wrong but:

  • Global throttling configuration with a global ThrottlerGuard provider works
  • Global throttling configuration + overriding controller method (ignored)
  • Global throttling configuration + overriding controller method + providing ThrottlerGuard specifically for module (ignored)

How would I mark only certain routes to be throttled then?
@Throttle({ long: { limit: 5, ttl: 60000 } })

@mukunda-
Copy link

mukunda- commented Aug 7, 2024

@dominic-schmid currently it would be like this:

  1. import throttler with a default configuration provided
  2. mark your route with the @Throttle line and a @UseGuards(ThrottlerGuard)

For all routes to be affected, there is another way to apply the ThrottlerGuard globally. Otherwise, it is only going to affect specific routes marked with the ThrottlerGuard.

This issue is concerning step 1 - it would be better if there was a preconfigured "default" rather than needing to specify a dummy configuration if you are using custom limit/ttl for each route.

@jvvcn
Copy link

jvvcn commented Aug 21, 2024

Having same problem - I do use module with default configuration, however there are routes that I would like to use different rate limit that is defined in module. Currently I cannot do it without overriding default configurations... Any ideas how to workaround it?

@jmcdo29
Copy link
Member

jmcdo29 commented Aug 22, 2024

Having same problem - I do use module with default configuration, however there are routes that I would like to use different rate limit that is defined in module. Currently I cannot do it without overriding default configurations... Any ideas how to workaround it?

@jvvcn that's kind of the idea of what should be done. Is there something you'd rather be able to do?

@jameslufz
Copy link

jameslufz commented Oct 20, 2024

I was removed old messages isn't the solution

Now I think I found the solution. 🧐

However, you must set up throttling in app.module.ts globally. But your setup doesn't specify any values for ttl and limit; both are set to 0.

ThrottlerModule.forRoot([{
    ttl: 0,
    limit: 0,
}]),

And never forget to add this to providers.

{
    provide: APP_GUARD,
    useClass: ThrottlerGuard
},

And finally your app.module.ts will look like this.

import { ThrottlerGuard, ThrottlerModule } from '@nestjs/throttler';
import { APP_GUARD } from '@nestjs/core';

@Module({
    imports: [
        ThrottlerModule.forRoot([{
            ttl: 0,
            limit: 0,
        }]),
    ],
    controllers: [
        AppController,
    ],
    providers: [
        {
            provide: APP_GUARD,
            useClass: ThrottlerGuard
        },
    ],
})

Then, the target route in your controller that you need will look like this.

import { Controller, Get, Res } from '@nestjs/common';
import { Response } from 'express';
import { Throttle } from '@nestjs/throttler';

@Controller('animal')
export class AnimalController
{
    @Throttle({ default: { limit: 3, ttl: (5 * 60 * 1000) } })
    @Get("cats")
    async GetCats( @Res() res: Response )
    {
        return res.status(200).json([ ...catList ])
    }
}

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

No branches or pull requests

7 participants