type-graphql icon indicating copy to clipboard operation
type-graphql copied to clipboard

directive graphql-rate-limit does not work

Open gyl89 opened this issue 1 year ago • 6 comments

Describe the Bug @rateLimit directive does not seem to work (coming from the package graphql-rate-limit-directive)

To Reproduce Define resolverClasses (including one mutation with @rateLimit), then to build the schema, use the code

import {rateLimitDirective} from 'graphql-rate-limit-directive';
  ...
private async buildSchema(resolverClasses: any): Promise<GraphQLSchema> {
    const { rateLimitDirectiveTypeDefs, rateLimitDirectiveTransformer } = rateLimitDirective();


    const basicSchema = await buildSchema({
      authChecker: ({ context }, perms) => accessCheck(context, perms),
      resolvers: resolverClasses,
      container,
    });
    const schemaMergedWithTypes = mergeSchemas({
      schemas: [basicSchema],
      typeDefs: rateLimitDirectiveTypeDefs,
    });
    const schemaWithRateLimitDirective = rateLimitDirectiveTransformer(schemaMergedWithTypes);
    return schemaWithRateLimitDirective;
  }

...

Expected Behavior When using "@rateLimit({ limit: 3, duration: 60 })" before a mutation, it should limit the rate at which the mutation can be called.

Logs When building the schema through buildSchema: Error parsing directive definition "@rateLimit({ limit: 3, duration: 60 })"

Environment (please complete the following information):

  • OS: Mac os, docker environment
  • Node 16-alpine.
  • Package version : 2.0.0.beta3, "graphql-rate-limit-directive": "2.0.4"
  • TypeScript version : 4.9.5

gyl89 avatar Nov 10 '23 15:11 gyl89

Directives should be passed to the buildSchema function. TypeGraphQL performs schema check after build, so it does not know the definition of your directive and yells.

MichalLytek avatar Nov 11 '23 09:11 MichalLytek

Thanks for your answers.

I feel thought that the documentation is not very clear : https://typegraphql.com/docs/directives.html Part on the 'fake-rename-directives" does not mention that. Besides, the package I mentioned does not export the directive but only the transformer and the typedefs.

gyl89 avatar Nov 13 '23 10:11 gyl89

Fair point. Maybe @carlocorradini can tell you more as he wrote this chapter (based on git blame).

For now, try to use skipCheck: true option of buildSchema.

MichalLytek avatar Nov 13 '23 13:11 MichalLytek

Sorry for the late reply, I'll check it ASAP 🥳

carlocorradini avatar Nov 18 '23 12:11 carlocorradini

@gyl89 Strange... I'm using the same approach with graphql-auth-directive (see https://github.com/carlocorradini/graphql-auth-directive/blob/main/examples/typegraphql/index.ts) Can you create an example repo with the code so we can examine and test it? Thanks 😅🥳

carlocorradini avatar Nov 19 '23 13:11 carlocorradini

@gyl89 Can you provide a repository with a minimal reproducible code example, in order to debug the issue?

MichalLytek avatar Jan 06 '24 16:01 MichalLytek

FWIIW, it's working for me just like @carlocorradini describes:

const schema = await buildSchema({
  resolvers,
  authChecker,
  pubSub: serviceHelpers.getPubSub(),
  globalMiddlewares: [ErrorInterceptorMiddleware],
})

const redisClient = new Redis(options)

const limiterOptions: IRateLimiterRedisOptions = {
  storeClient: redisClient,
  keyPrefix: 'grrl', // must be unique for limiters with different purpose
}

const keyGenerator = <TContext>(
  directiveArgs: RateLimitArgs,
  source: unknown,
  args: { [key: string]: unknown },
  context: TContext,
  info: GraphQLResolveInfo,
): string => {
  const defaultKey = defaultKeyGenerator(directiveArgs, source, args, context, info)
  return `${(context as GraphQlContext).user?.id || (context as GraphQlContext).req.ip}:${defaultKey}`
}

class DebugRateLimiterRedis extends RateLimiterRedis {
  consume(
    key: string | number,
    pointsToConsume?: number,
    options?: { [key: string]: any }
  ) {
    console.log(`[CONSUME] ${key} for ${pointsToConsume}`);
    return super.consume(key, pointsToConsume, options);
  }
}

const {
  rateLimitDirectiveTypeDefs,
  rateLimitDirectiveTransformer,
} = rateLimitDirective({
  name: 'rateLimit',
  keyGenerator,
  limiterClass: DebugRateLimiterRedis,
  limiterOptions,
})

const mergedSchema = mergeSchemas({
  schemas: [schema],
  typeDefs: [rateLimitDirectiveTypeDefs]
})

const newSchema = rateLimitDirectiveTransformer(mergedSchema)

And in the resolvers, adding: @Directive('@rateLimit(limit: 30, duration: 60)') does not throw up any errors.

shadowbrush avatar Jun 03 '24 19:06 shadowbrush

Nice

carlocorradini avatar Jun 03 '24 22:06 carlocorradini

So closing as cannot reproduce 🔒

MichalLytek avatar Jun 04 '24 05:06 MichalLytek