REST API Rate Limit Issues - handleRest() function - AWS Lambda

[roberttolton]

Hi,

Thanks for your work on the package by the way.

I've trying to use it in a serverless environment - AWS Lambda with Laravel Vapor.

I have created a class that implements StateStorage in order to put the rate limits, timings etc into a shared cache instance:

<?php

namespace CustomClasses;

use Illuminate\Support\Facades\Cache;
use Osiset\BasicShopifyAPI\Contracts\StateStorage;
use Osiset\BasicShopifyAPI\Session;

class BasicShopifyApiLaravelStorage implements StateStorage
{
    protected $cacheKey = 'shopify-api-rate-limit-storage';
    protected $ttl = 3600;

    /**
     * {@inheritdoc}
     */
    public function all(): array
    {
        return Cache::get($this->cacheKey, []);
    }

    /**
     * {@inheritdoc}
     */
    public function get(Session $session): array
    {
        $shop = $session->getShop();
        $cachedData = Cache::get($this->cacheKey, []);

        return $cachedData[$shop] ?? [];
    }

    /**
     * {@inheritdoc}
     */
    public function set(array $values, Session $session): void
    {
        $cachedData = Cache::get($this->cacheKey, []);

        $cachedData[$session->getShop()] = $values;
        Cache::put($this->cacheKey, $cachedData, $this->ttl);
    }

    /**
     * {@inheritdoc}
     */
    public function push($value, Session $session): void
    {
        $shop = $session->getShop();
        $cachedData = Cache::get($this->cacheKey, []);

        if (!isset($cachedData[$shop])) {
            $this->reset($session);
            $cachedData = Cache::get($this->cacheKey, []);
        }

        array_unshift($cachedData[$shop], $value);
        Cache::put($this->cacheKey, $cachedData, $this->ttl);
    }

    /**
     * {@inheritdoc}
     */
    public function reset(Session $session): void
    {
        $cachedData = Cache::get($this->cacheKey, []);
        $cachedData[$session->getShop()] = [];
        Cache::put($this->cacheKey, $cachedData, $this->ttl);
    }
}

I can see the cache data getting set correctly, as such:

>>> Cache::get('shopify-api-rate-limit-storage');
=> [
     "example.myshopify.com" => [
       [
         "left" => 37,
         "made" => 3,
         "limit" => 40,
       ],
       1.630509591706E+15,
       [
         "left" => 38,
         "made" => 2,
         "limit" => 40,
       ],
       1.6305095913914E+15,
       [
         "left" => 38,
         "made" => 2,
         "limit" => 40,
       ],
       1.6305095910853E+15,
       [
         "left" => 38,
         "made" => 2,
         "limit" => 40,
       ],
       1.6305095906846E+15,
       [
         "left" => 39,
         "made" => 1,
         "limit" => 40,
       ],
       1.630509590353E+15,
       [
         "left" => 39,
         "made" => 1,
         "limit" => 40,
       ],
       1.6305095899897E+15,
       [
         "left" => 39,
         "made" => 1,
         "limit" => 40,
       ],
       1.630509589654E+15,
       [
         "left" => 39,
         "made" => 1,
         "limit" => 40,
       ],
       1.6305095892665E+15,
       [
         "left" => 39,
         "made" => 1,
         "limit" => 40,
       ],
       1.6305095889041E+15,
       [
         "left" => 39,
         "made" => 1,
         "limit" => 40,
       ],
       1.6305095885244E+15,
       [
         "left" => 39,
         "made" => 1,
         "limit" => 40,
       ],
       1.6305095872599E+15,
       [
         "left" => 39,
         "made" => 1,
         "limit" => 40,
       ],
       1.6305091743179E+15,
       [
         "left" => 39,
         "made" => 1,
         "limit" => 40,
       ],
       1.6305091738396E+15,
       [
         "left" => 39,
         "made" => 1,
         "limit" => 40,
       ],
       1.6305091734432E+15,
       [
         "left" => 38,
         "made" => 2,
         "limit" => 40,
       ],
       1.6305091730654E+15,
       [
         "left" => 39,
         "made" => 1,
         "limit" => 40,
       ],
       1.6305091727318E+15,
       [
         "left" => 39,
         "made" => 1,
         "limit" => 40,
       ],
       1.6305091724052E+15,
       [
         "left" => 38,
         "made" => 2,
         "limit" => 40,
       ],
       1.6305091720891E+15,
       [
         "left" => 39,
         "made" => 1,
         "limit" => 40,
       ],
       1.6305091717451E+15,
       [
         "left" => 39,
         "made" => 1,
         "limit" => 40,
       ],
       1.6305091713195E+15,
       [
         "left" => 39,
         "made" => 1,
         "limit" => 40,
       ],
       1.6305091709396E+15,
       [
         "left" => 39,
         "made" => 1,
         "limit" => 40,
       ],
       1.6305091698909E+15,
     ],
   ]

However, when I'm running this, it constantly hits Shopify API Limits, and it seems like it doesn't even try to slow down.

Looking at the source files, I see the handleRest() function counts the number of elements in the above array, indexed by session / shop name. But why the !== check and not less-than-or-equal <=?

/**
 * Handle REST checks.
 *
 * @param BasicShopifyAPI $api
 *
 * @return bool
 */
protected function handleRest(BasicShopifyAPI $api): bool
{
    // Get the client
    $client = $api->getRestClient();
    $td = $client->getTimeDeferrer();
    $ts = $client->getTimeStore();

    $times = $ts->get($api->getSession());
    if (count($times) !== $api->getOptions()->getRestLimit()) {   ///<<<---This Line
        // Not at our limit yet, allow through without limiting
        return false;
    }

    // Determine if this call has passed the window
    $firstTime = end($times);
    $windowTime = $firstTime + 1000000;
    $currentTime = $td->getCurrentTime();

    if ($currentTime > $windowTime) {
        // Call is passed the window, reset and allow through without limiting
        $ts->reset($api->getSession());

        return false;
    }

    // Call is inside the window and not at the call limit, sleep until window can be reset
    $sleepTime = $windowTime - $currentTime;
    $td->sleep($sleepTime < 0 ? 0 : $sleepTime);
    $ts->reset($api->getSession());

    return true;
}

Considering that the elements in the cached array fill-up rapidly from multiple Lambda functions, I quickly get more elements than the getRestLimit(), which is 2, so the function exits on the return false because 10 !== 2 and never triggers the sleep.

@osiset Am I missing something here? Apologies if so.

[gnikyt]

I believe it is a bug. I believe it needs to be a less than.

[roberttolton]

@osiset Wouldn't less-than-or-equal be fine? Because having 2 requests per second is fine, just not more than that. Right?

[gnikyt]

Yes it should be fine that way. You're welcome to try it out and see if it fixes your issue. If so you can submit a PR. I'm unfortunately tied up this week :(

[roberttolton]

@osiset No worries - I'll work on this tomorrow and send up a PR.