MONEI / Shopify-api-node

Node Shopify connector sponsored by MONEI
https://monei.com/shopify-payment-gateway/
MIT License
945 stars 278 forks source link

429 Response: Exceeded 2 calls per second for api client #518

Closed nicmesan2 closed 2 years ago

nicmesan2 commented 2 years ago

I am having a big issue on my Shopify App that I listed a couple of days ago. The core functionality of my app is an endpoint that hits the Shopify API endpoint to edit draft_orders several multiple times. I am fully aware of the rate limits of 40 requests per minute and the refresh rate of 2 requests per second.

The strange thing is the timestamp of these failed requests:

PUT https://****.myshopify.com/admin/draft_orders/124016722005.json [429] 09:09:16
PUT https://****.myshopify.com/admin/draft_orders/124016722005.json [429] 09:09:18
PUT https://****.myshopify.com/admin/draft_orders/124016722005.json [429] 09:09:23
PUT https://****.myshopify.com/admin/draft_orders/124016722005.json [429] 09:09:31
PUT https://****.myshopify.com/admin/draft_orders/124016722005.json [429] 09:09:47
PUT https://****.myshopify.com/admin/draft_orders/124016722005.json [429] 09:10:19

All of these are returning:

Error code: 429 (Too many request)
Error message: REASON: Exceeded 2 calls per second for api client. Reduce request rates to resume uninterrupted service.
Copy

As you can see there is more than 1 minute of time between the first failed request and the latest. So I am completely confused of what could be causing the issue.

Please if anyone can help me I will be really appreciate it.

Thank you!

lpinca commented 2 years ago

Hard to say without knowing the timestamps of successful requests. Some requests might succeed, then the limit is hit, then no more requests come through, then the limit is hit again and so on. Are you using the autoLimit option? If so, note that the limit is applied per Shopify instance.

nicmesan2 commented 2 years ago

Hello! I think I may found the problem. I was (wrongly) assuming that the user would make just one request and wait for that request to finish before sending another one. Thus this was my code in the endpoint:

const handleGetRequest = async (req, res) => {
  const {shop, accessToken} = await Shopify.Utils.loadCurrentSession(
    req,
    res
  );

  const client = getShopifyClient(shop, accessToken);

  // do stuff with Shopify client

}

Where getShopifyClient is:

const getShopifyClient = (shopName, accessToken) => {
  const shopify = new Shopify({
    autoLimit: { calls: 1, interval: 1100, bucketSize: 30 },
    shopName: shopName,
    accessToken: accessToken,
  });

  shopify.on("callLimits", (limits) =>
    console.log(
      `CURRENT API LIMIT STATE: ${util.inspect(
        limits,
        false,
        null,
        true /* enable colors */
      )}`
    )
  );

  shopify.on("callGraphqlLimits", (limits) =>
    console.log(
      `CURRENT API LIMIT STATE: ${util.inspect(
        limits,
        false,
        null,
        true /* enable colors */
      )}`
    )
  );

  return shopify;
};

Now I realize that this code will create a new instance of the Shopify class every-time I receive a new request (despite being from the same user, with the same shop name and access token).

If this is the case, would it be a solution to memoize the getShopifyClient return the same instance in case I send the same shop name? Something like this:

let memoizedClient = {}
const getShopifyClient = (shopName, accessToken) => {
  if (memoizedClient[shopName]) {
     return memoizedClient[shopName]
  }
  const shopify = new Shopify({
    autoLimit: { calls: 1, interval: 1100, bucketSize: 30 },
    shopName: shopName,
    accessToken: accessToken,
  });

  shopify.on("callLimits", (limits) =>
    console.log(
      `CURRENT API LIMIT STATE: ${util.inspect(
        limits,
        false,
        null,
        true /* enable colors */
      )}`
    )
  );

  shopify.on("callGraphqlLimits", (limits) =>
    console.log(
      `CURRENT API LIMIT STATE: ${util.inspect(
        limits,
        false,
        null,
        true /* enable colors */
      )}`
    )
  );

  memoizedClient[shopName] = shopify

  return shopify;
};

What do you think?

Thanks in advance for your help! :)

lpinca commented 2 years ago

Yes it make perfect sense.

lpinca commented 2 years ago

I'm closing this. Discussion can continue if needed.