MichalLytek / type-graphql

Create GraphQL schema and resolvers with TypeScript, using classes and decorators!
https://typegraphql.com
MIT License
8.04k stars 678 forks source link

directive graphql-rate-limit does not work #1569

Closed gyl89 closed 5 months ago

gyl89 commented 1 year ago

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):

MichalLytek commented 1 year ago

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.

gyl89 commented 1 year ago

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.

MichalLytek commented 1 year ago

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.

carlocorradini commented 12 months ago

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

carlocorradini commented 12 months ago

@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 😅🥳

MichalLytek commented 10 months ago

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

shadowbrush commented 5 months ago

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.

carlocorradini commented 5 months ago

Nice

MichalLytek commented 5 months ago

So closing as cannot reproduce 🔒