ravangen / graphql-rate-limit

🚦 Fixed window rate limiting middleware for GraphQL. Use to limit repeated requests to queries and mutations.
https://www.npmjs.com/package/graphql-rate-limit-directive
MIT License
317 stars 12 forks source link

Directive not applying to GQL mutation within Apollo Server 4 #399

Closed sdee3 closed 7 months ago

sdee3 commented 9 months ago

Hi, I've been having problems setting this up within my fullstack app where I'm serving the BE through Apollo Server version 4.10.0.

Given the simple setup here:

import { GraphQLSchema } from 'graphql'
import { makeExecutableSchema } from '@graphql-tools/schema'
import { mergeTypeDefs } from '@graphql-tools/merge'
import { rateLimitDirective } from 'graphql-rate-limit-directive'

import { types } from './type-defs'
import { resolvers } from './resolvers'

const { rateLimitDirectiveTransformer, rateLimitDirectiveTypeDefs } =
  rateLimitDirective()

const typeDefs = mergeTypeDefs([rateLimitDirectiveTypeDefs, ...types])

const schema: GraphQLSchema = rateLimitDirectiveTransformer(
  makeExecutableSchema({ typeDefs, resolvers })
)

export { schema, typeDefs }

I was expecting that the directive would automatically work for one of my mutations that implement this directive:

submitData: Boolean @rateLimit(limit: 2, duration: 60)

But triggering this mutation from my FE over 20x within 5 seconds does not lead to anything other than my mutation running completely - as if there's no directive applied to it.

What could be wrong?

This is part of what I have installed in my BE package.json:

"@apollo/server": "^4.10.0",
"@graphql-tools/merge": "^9.0.1",
"@graphql-tools/schema": "^10.0.2",
"@graphql-tools/utils": "^10.0.11",
"express": "^4.18.2",
"graphql": "^16.8.1",
"graphql-rate-limit-directive": "^2.0.5",
"graphql-scalars": "^1.22.4",
"graphql-subscriptions": "^2.0.0",
"graphql-tag": "^2.12.6",
"graphql-ws": "^5.14.2",
"rate-limiter-flexible": "^4.0.1",
 ...
ravangen commented 8 months ago

I went through Apollo's getting started guide and experienced no issues with the limiter, see example branch.

I didn't leverage mergeTypeDefs as it was not necessary. The example has a query, but mutation also limits.

Maybe use a custom limiter class to see how many times things are being "consumed":

// This is not necessary, it exists to demonstrate when we check the rate limit usage
class DebugRateLimiterMemory extends RateLimiterMemory {
  consume(key, pointsToConsume, options) {
    console.log(`[CONSUME] ${key} for ${pointsToConsume}`);
    return super.consume(key, pointsToConsume, options);
  }
}

const {
  rateLimitDirectiveTypeDefs,
  rateLimitDirectiveTransformer
} = rateLimitDirective({
  limiterClass: DebugRateLimiterMemory
});

For example, in your setup, I am unclear what limiter class you are using and any custom key generator.

ravangen commented 7 months ago

Happy to reopen if necessary