jmcdo29 / nest-lab

A repository to hold "experimental" packages for Nest. Honestly, I'm just tired of not having a good scope to put packages under :smile_cat:
87 stars 7 forks source link

using @nest-lab/throttler-storage-redis instead of deprecated library nestjs-throttler-storage-redis is not working #51

Open nahlaFathy opened 2 months ago

nahlaFathy commented 2 months ago

i'm using nest js throttler @nestjs/throttler ^6.2.1 and when i'm trying to use @nest-lab/throttler-storage-redis instead of deprecated library nestjs-throttler-storage-redis is not working here is a code sample :

import { ConfigService } from '@nestjs/config';
import { ThrottlerModuleOptions } from '@nestjs/throttler';
import { ThrottlerStorageRedisService } from '@nest-lab/throttler-storage-redis';

export function buildThrottlingOptions(config: ConfigService): ThrottlerModuleOptions {
  return {
    storage: new ThrottlerStorageRedisService(config.getOrThrow('REDIS_URL')),
    throttlers: [
      {
        limit: config.getOrThrow('THROTTLING_REQUESTS_LIMIT_PER_TTL'),
        ttl: config.getOrThrow('THROTTLING_TTL_IN_SECONDS'),
      },
    ],
  };
}
@Module({
  imports: [
    ThrottlerModule.forRootAsync({
      imports: [],
      inject: [ConfigService],
      useFactory: (config: ConfigService) => buildThrottlingOptions(config),
    })
    ]})
 @Injectable()
export class BaseThrottlerGuard extends ThrottlerGuard {
  protected generateKey(context: ExecutionContext, suffix: string): string {
    const request = context.switchToHttp().getRequest();
    const prefix = (request.user?.mobileNumber || request.body?.mobileNumber).replace('+', '');

    return `${prefix}-${suffix}`;
  }

  async checkLimit(
    requestProps: ThrottlerRequest,
    options?: { exceptionMessage?: string; suffix?: string },
  ): Promise<boolean> {
    const { context, limit, ttl, throttler, blockDuration } = requestProps;
    const suffix = options?.suffix || 'default-rate-limit';
    const exceptionMessage = options?.exceptionMessage || 'TOO_MANY_REQUESTS';
    const key = options?.suffix || this.generateKey(context, suffix);
    const { totalHits } = await this.storageService.increment(key, ttl, limit, blockDuration, throttler.name || key);

    if (totalHits > limit) {
      throw new ThrottlerException(exceptionMessage);
    }

    return true;
  }

  handleRequest(requestProps: ThrottlerRequest, options?: CustomThrottlerOptions): Promise<boolean> {
    const { exceptionMessage, suffix } = options || {};

    return this.checkLimit(requestProps, { suffix, exceptionMessage });
  }
}

everything is working except this.storageService.increment it's not adding keys to redis so totalHits will always be 1 but when i'm using nestjs-throttler-storage-redis but old version it works correctly with adding keys to redis and increment correctly

jmcdo29 commented 2 months ago

I'm pretty sure that the version of this library is meant to work on throttler v6 and up, and that you should use the deprecated library, due to breaking changes of v6

nahlaFathy commented 2 months ago

I'm pretty sure that the version of this library is meant to work on throttler v6 and up, and that you should use the deprecated library, due to breaking changes of v6

I know it's meant to be used with v6 so i'm trying to configure it now but it's not working no keys added to redis but when i replace it with the deprecated library old version it works so i'm trying to figure out what i'm missing here

i'm using throttler "@nestjs/throttler": "^6.2.1" and ioredis "ioredis": "^5.3.2"

jmcdo29 commented 2 months ago

Please provide a minimum reproduction repository (Git repository/StackBlitz/CodeSandbox project).

why reproductions are required