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
314 stars 11 forks source link

[QUESTION]: Limit based on resolver response #371

Open simplecommerce opened 10 months ago

simplecommerce commented 10 months ago

Hi,

I don't think its currently possible but I was wondering if there is a way to only consume points based on resolver response?

In my use case, I have a rate limit applied on a login attempt. I want it to only count when the login fails.

For example, if I try to login 6 times and every time it fails, the rate limit should always count. But if I login successfully in between those attempts, it should not count.

ravangen commented 10 months ago

Hi, thanks for your question, an iteration on https://github.com/ravangen/graphql-rate-limit/issues/370.

The rate limiter runs before the resolver response is calculated, and hard to pass down state into the original resolve function. Could likely achieve your goals still via a callback, as described in https://github.com/ravangen/graphql-rate-limit/issues/370#issuecomment-1795040938

simplecommerce commented 10 months ago

Hi, thanks for your question, an iteration on #370.

The rate limiter runs before the resolver response is calculated, and hard to pass down state into the original resolve function. Could likely achieve your goals still via a callback, as described in #370 (comment)

Maybe I am wrong but I think I understood your callback idea.

I was thinking maybe it would be a good idea to have an argument we can pass to the @rateLimit for example manual which would be a boolean, if true, the directive would pass a callback function to the resolver instead of executing the directive automatically.

That way in the resolver, a user could handle executing the directive logic manually with a callback that you could pass?

For example:

type Mutation {
    login(user: String!, password: String!): Boolean! @rateLimit(manual: true)
}
{
   Mutation: {
      login: (obj, args, context, info) {
         // do login, if login fails
         if (false) {
              // return context.rateLimit.callback here ?
         }

         return true;
      }
   }
}

This way, you could use both this library for generic purposes, or specific purposes also since you can then handle manual scenarios with the resolve responses.

Hope I understood or made sense of it.

ravangen commented 10 months ago

If you want something under context, you could use the existing setState (docs, example). But the interface right now doesn't include a way to expose the specific limiter used or the calculated cost that was used.

We don't need a manual directive argument. If something is "manual" and so bespoke to this one field, then why use the directive at all? Why not have your custom rate limit logic in your resolve? What value do you derive by using the directive?

simplecommerce commented 10 months ago

If you want something under context, you could use the existing setState (docs, example). But the interface right now doesn't include a way to expose the specific limiter used or the calculated cost that was used.

We don't need a manual directive argument. If something is "manual" and so bespoke to this one field, then why use the directive at all? Why not have your custom rate limit logic in your resolve? What value do you derive by using the directive?

Its because the directive in my case is already setup and configured to follow certain logic for the points calculation, etc. The only feature that I am missing is to be able to control when to count/consume the points based on a mutation's response in some cases vs others where I need it to behave as it already does.

Also makes it easier for other programmers on our project to do it without having to resort to creating another rate limiter custom to the resolver itself.

But thats only on my end though.

ravangen commented 10 months ago

I think the callback style could work in this case. My point is that at some point it is valid to have a custom limiter.