feat: skip option for skipping by logic.

[linhx]

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• Bugfix
• 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
• No

Other information

[micalevisk]

I guess we can improve the ...param: any part somehow

[linhx]

@micalevisk at first, I defined the Skip method like this:

export type SkipMethod = (
  context: ExecutionContext,
  req: Record<string, any>,
  res: Record<string, any>,
) => boolean | Promise<boolean>;

but the problem is when someone want to override the handleRequest method (for example Working with Websockets), the param now is not request and response.
So, I define the method params as ...param: any so others can use their custom params.

[jmcdo29]

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?

[linhx]

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.

[harshmehta813]

@jmcdo29 @micalevisk when can we have this feature?

Are there any other way to use @SkipThrottle() conditionally?

[jmcdo29]

@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

[jmcdo29]

Implemented in #1565