klaviyo / magento2-klaviyo

37 stars 51 forks source link

\Klaviyo\Reclaim\Plugin\Api\CartSearchRepository::afterGetList Infinite Loop #133

Closed pmclain closed 2 years ago

pmclain commented 2 years ago

Steps to reproduce

All cart interactions below should be done via API

  1. Create cart
  2. Add at least one product to cart
  3. Apply coupon code to cart (must be a code)
  4. Edit the cart product in the admin panel
  5. Reload cart

Expected Result

/rest/{{store}}/V1/carts/mine should return your cart ID

Actual Result

An infinite loop is triggered when a quote has quote.coupon_code <> NULL AND quote.trigger_recollect = 1

Summarizing the call stack triggered by this combination: https://github.com/klaviyo/magento2-klaviyo/blob/4e63ccd99ce74d6f72839fcccf764553596a0d45/Plugin/Api/CartSearchRepository.php#L57

https://github.com/magento/magento2/blob/ab815b51dad2a89b120cc53b26eefaee80a336f3/app/code/Magento/Quote/Model/QuoteIdToMaskedQuoteId.php#L53

https://github.com/magento/magento2/blob/ab815b51dad2a89b120cc53b26eefaee80a336f3/app/code/Magento/Quote/Model/QuoteRepository.php#L136

https://github.com/magento/magento2/blob/ab815b51dad2a89b120cc53b26eefaee80a336f3/app/code/Magento/Quote/Model/Quote.php#L2461

☝️ Recollection is triggered here because editing a product in the quote sets quote.trigger_recollect = 1

During recollection https://github.com/magento/magento2/blob/ab815b51dad2a89b120cc53b26eefaee80a336f3/app/code/Magento/SalesRule/Observer/CouponCodeValidation.php#L66 validates the coupon code. The validation calls CartRepositoryInterface::getList which re-triggers your plugin.

$found = $this->cartRepository->getList(
  $this->criteriaBuilder->addFilter('main_table.' . CartInterface::KEY_ENTITY_ID, $quote->getId())
    ->create()
)->getItems();

At this point the request is infinitely looping until it timeouts or exhausts memory.

klaviyojad commented 2 years ago

@pmclain I am seeing that this issue has been logged by @ihor-sviziev here which leads me to think its a magento issue. I will check if the latest version 2.4.3 it is fixed

ihor-sviziev commented 2 years ago

@pmclain, please apply patch from the following PR: https://github.com/magento/magento2/pull/33324 It's not merged, so definitely not available in any release

pmclain commented 2 years ago

I'm surprised Klaviyo's stance on the issue is "not our problem" considering the critical level of impact. Pointing your merchants to a possible fix in core Magento with a TBD release while your module cripples checkout is disappointing.

ihor-sviziev commented 2 years ago

@pmclain I think klaviyo might introduce the workaround (hack against the issue in the core), but the changes should be similar to what I created - fetch the masked quote using direct queries, but not preventing of re calculation of something. Otherwise, we might see some not clear/random issues

pmclain commented 2 years ago

@ihor-sviziev I don't care what the fix looks like as long as it's fixed. We've already hotfixed this in our instances. It wound up costing a significant drop in conversion since a rolling ~10% of carts were left in a broken state after upgrading from v2 to v3.

ihor-sviziev commented 2 years ago

@klaviyojad, that's pretty strong argument for introducing a workaround for this issue inside the klaviyo extension.

PS: we had this issue on the cart requests from Klaviyo and it was causing huge cpu on web nodes and db load, but seems like you had it on the frontend, which is more critical

klaviyojad commented 2 years ago

@pmclain apologies for the back and forth on this issue. We have added the changes to version 3.0.8 to handle it