throttler
throttler copied to clipboard
feat: `skip` option for skipping by logic.
PR Checklist
Please check if your PR fulfills the following requirements:
- [x] The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
- [x] Tests for the changes have been added (for bug fixes / features)
- [x] Docs have been added / updated (for bug fixes / features)
PR Type
What kind of change does this PR introduce?
- [ ] Bugfix
- [x] Feature
- [ ] Code style update (formatting, local variables)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] CI related changes
- [ ] Other... Please describe:
What is the current behavior?
There is no option to skip by logic. ignoreUserAgents
is not safe because the user agent string can be spoofed.
What is the new behavior?
Add the option skip
to skip by logic:
@Module({
imports: [
ThrottlerModule.forRoot({
ttl: 60,
limit: 10,
skip(context, req, res) {
return req.ip === 'google-bot-ip';
}
}),
],
})
Override by @SkipThrottle
@Controller()
@SkipThrottle((context, req, res) => req.ip === 'another-ip')
class Controller {
@Get() // skip when req.ip === 'another-ip'
async index() {
return '';
}
@Get('root-setting')
@SkipThrottle(false) // skip when req.ip === 'google-bot-ip'
async useDefault() {
return '';
}
@Get('method-setting')
@SkipThrottle((context, req, res) => req.ip === 'another-ip-1') // skip when req.ip === 'another-ip-1'
async useMethod() {
return '';
}
@Get('dont-skip-at-all')
@SkipThrottle(null) // don't skip at all
async dontSkipAtAll() {
return '';
}
}
Does this PR introduce a breaking change?
- [ ] Yes
- [x] No
Other information
I think rather than making this an option we pass in configuration it should be a method inside the guard that can be overwritten. Do you think you could update the code to be like that?
I think rather than making this an option we pass in configuration it should be a method inside the guard that can be overwritten. Do you think you could update the code to be like that?
I understand but I don't find that better.
With the current Guard we were able to override the method handleRequest
and put the skipping logic there.
I think making it an optional option helps developer don't have to create a new guard.
@jmcdo29 @micalevisk when can we have this feature?
Are there any other way to use @SkipThrottle() conditionally?
@jmcdo29 @micalevisk when can we have this feature?
Are there any other way to use @SkipThrottle() conditionally?
I still feel like a method for the class and using a new class is a cleaner approach and that I'd prefer to take that route. Maybe some method like shouldThrottle(context: ExecutionContext): boolean
, though that could cause issues when each route has its own condition, so maybe an option via decorator is a better approach here.
I'm working on the next major version of this package where a few things will be changed regarding the @Throttle()
decorator and the configuration passed to it, so I think I'll probably update this as well to align with that
Implemented in #1565