shopify-api-js leaky bucket rate limiting does not keep order of calls

Overview/summary

Shopify API rate limiting is based on leaky bucket. However library implementation relies on very naive ‘retry’ strategy without caring about leaky bucket and requests queuing so in edge cases it can end up with serious problem, even requests that will never be called. Two cases described below : Here is most important part of https://github.com/Shopify/shopify-node-api/blob/main/src/clients/http_client/http_client.ts#L128-L141

RestAPI As leaky bucket is built based on number of request this is almost ok implementation but only ‘almost’. As there is no queuing just ‘setTimeout’ it is still possible that if request A fails and is waiting till for retry there is another request B that will be made just before retry of A and therefore retry of B will fail again, in edge case it can be never called. Solution is simple : there should be queue of requests, when request fails because of rate limit then it is added at the end of queue of waiting requests, and library then makes request one by one from queue.
GraphQL Actual implementation ignores leaky bucket mechanism and, if i didn't miss anything just retries after one second. Solution would be as above queue but this time bases on the throttle status returned by graphQL API.

Motivation

What inspired this enhancement?

...

Area

[x] Add any relevant Area: <area> labels to this issue

Checklist

[x] I have described this enhancement in a way that is actionable (if possible)

Jun 04 '21 08:06 bogdanrn

Hi, thx @bogdanrn for posting this. This is issue i wanted to post but for some reason it wasnt possible, so I asked my friend @bogdan to post the issue. Can someone explain me btw is there is some limit of issues per user or am i blacklisted for some reason?

Jun 04 '21 09:06 pociej

Hi @bogdanrn, @pociej thanks for posting about this! These are valid points, we are currently looking into it. @pociej, I'm not quite sure why you wouldn't be allowed to post 🤔 we definitely don't have a limit or blacklist. Let me know if this is still an issue for you!

Jun 07 '21 15:06 mllemango

Hi @mllemango any update on this?

Jun 29 '21 14:06 pociej

Thanks Shopify for providing this SDK. really a life changer for someone like me who built the implementations by hand. No is there any updates about this matter ? thanks !

Nov 09 '21 16:11 mgara

Hi @mllemango any update?

Nov 16 '21 13:11 pociej

Hey everyone, we haven't had a chance to get back to this one yet. We'll keep this issue open for others to report the same problem and that will help us prioritize.

Nov 16 '21 16:11 mllemango

Any update on this?