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

Using rate limiting with graphQL loadSchemaSync #354

Closed joaogsousa closed 1 year ago

joaogsousa commented 1 year ago

We have a graphQL federation setup, in which the main graphQL schema is composed of sub-schemas defined by several microservices.

For this to work each microservice needs to call loadSchemaSync(), then there will be some magic that will merge the provided schema with the schemas provided by other microservices.

But, when I use the rateLimit directive, the call to loadSchemaSync() fails. With the following error:

[14:35:45.466] ERROR (21608): An Error in starting Jigsaw, Unknown directive "@rateLimit". stack: "Error: Unknown directive \"@rateLimit\".\n at assertValidSDL (D:\jh-trabalho\findmypast\workspace\jigsaw\node_modules\graphql\validation\validate.js:135:11)\n at buildASTSchema (D:\jh-trabalho\findmypast\workspace\jigsaw\node_modules\graphql\utilities\buildASTSchema.js:44:34)\n at makeExecutableSchema (D:\jh-trabalho\findmypast\workspace\jigsaw\node_modules\@graphql-tools\schema\cjs\makeExecutableSchema.js:73:47)\n at mergeSchemas (D:\jh-trabalho\findmypast\workspace\jigsaw\node_modules\@graphql-tools\schema\cjs\merge-schemas.js:32:63)\n at getSchemaFromSources (D:\jh-trabalho\findmypast\workspace\jigsaw\node_modules\@graphql-tools\load\cjs\schema.js:56:46)\n at loadSchemaSync (D:\jh-trabalho\findmypast\workspace\jigsaw\node_modules\@graphql-tools\load\cjs\schema.js:32:12)\n at createApolloServer (D:\jh-trabalho\findmypast\workspace\jigsaw\src\graphql\apollo-server.ts:31:30)\n at server (D:\jh-trabalho\findmypast\workspace\jigsaw\src\server.ts:117:42)\n at async startServer (D:\jh-trabalho\findmypast\workspace\jigsaw\src\main.ts:12:5)"

I think I did the rateLimit setup right: image image

Does this lib support adding the rateLimitDirectiveTypeDefs through the graphQL tool loadSchemaSync() ?

ravangen commented 1 year ago

A reproduction repository/code would be appreciated instead of images. I haven't personally used federation/async loading.

I suspect we have all the necessary data, that it would be more a matter of calling graphql-tools functions with the right options and order of operations. This library's code conforms to the suggest approach according to their docs. I'm a bit short on time currently to deep dive into another library to assist, sorry.

ravangen commented 1 year ago

Not sure about the exact way to achieve this, but what if you make an basic schema with the directive (and transformer applied), and have your additional schemas merged together. In isolation the partial schemas may be invalid, but together once loaded they would be known? Have a rateLimiter schema exported from a module as a GraphQLSchema object, and use the ModuleLoader? Does loadSchemaSync support merging? Again, haven't tried it, just theorizing. The Guild would know best.

joaogsousa commented 1 year ago

Hummm will try, thanks for the help!

joaogsousa commented 1 year ago

An update that I could get rid of the error by adding the directive directly on the graphQL schema instead of using typeDefs: image

The directive seems now to be loaded in the schema OK, but the rate limiting is not working. I cant find any keys related to the rate limit being added to redis and I do tons of requests without being rate limited.

ravangen commented 1 year ago

The rateLimitDirectiveTransformer is what wraps resolver functions with a rate limit check on sites where the directive is applied on object types and fields, it isn't enough to just have the directive added to the schema.

To help you debug locally, you could use the in-memory limiter, and have a log or breakpoint (example)

joaogsousa commented 1 year ago

I added the directive definition in the schema, used the rateLimitDirectiveTransformer, and added the @rateLimit directive to desired fields.

ravangen commented 1 year ago

Sounds like it is working, happy to hear! 😄