Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quote Attribute trigger_recollect causes a timeout #9580

Closed
bh-ref opened this issue May 10, 2017 · 11 comments · Fixed by #14812
Closed

Quote Attribute trigger_recollect causes a timeout #9580

bh-ref opened this issue May 10, 2017 · 11 comments · Fixed by #14812
Labels
Fixed in 2.1.x The issue has been fixed in 2.1 release line Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release

Comments

@bh-ref
Copy link
Contributor

bh-ref commented May 10, 2017

Preconditions

  1. Magento CE 2.1.5
  2. MariaDB 10.1.20
  3. PHP 7.0.14

Steps to reproduce

  1. We created a custom shipping carrier. This shipping carrier implements the method
    public function collectRates(RateRequest $request).
  2. Log in as a customer.
  3. Add some products to your quote.
  4. Do not go to checkout. Log off.
  5. Change some product and/or catalog price rules which affect the products in the quote from step 2. This should set the attribute trigger_recollect to 1 (integer) for that quote because of
    \Magento\Sales\Observer\Backend\CatalogProductSaveAfterObserver::execute() and
    \Magento\Sales\Observer\Backend\CatalogPriceRule::execute().
  6. If you now log back in as a customer, your previous quote will be loaded.
  7. If you then add a product, the shipping rate from our custom shipping carrier will be collected. We use \Magento\Checkout\Model\Cart::getQuote() to do some checks. \Magento\Quote\Model\Quote::_afterLoad() will eventually be triggered, which calls \Magento\Quote\Model\Quote::collectTotals() if the attribute trigger_recollect is set to 1.
  8. Now, things begin to repeat because the rates for our custom shipping carrier are collected again.
  9. A timeout appears because of this loop.
  10. The customer cannot open any other page on the website, because this cycle continues for that customer. After waiting for the page to finish loading, all the customer will see in the end is a HTTP error 503.

Expected result

  1. The attribute trigger_recollect in a quote should be set to 0 (integer) at some time.

Actual result

  1. In all the magento code, I could only see the attribute trigger_recollect in a quote being set to 1, but never back to 0.

My patch, which seems to work fine, is:
In \Magento\Quote\Model\Quote::_afterLoad() add a reset of the attribute trigger_recollect after collecting the totals:

    protected function _afterLoad()
    {
        // collect totals and save me, if required
        if (1 == $this->getData('trigger_recollect')) {
            $this
                ->collectTotals()
                ->setTriggerRecollect(0) // patch: reset the attribute trigger_recollect
                ->save();
        }
        return parent::_afterLoad();
    }
@robbanl
Copy link

robbanl commented May 10, 2017

We have the EXACT same issue with Magento 2.1.4 that started a couple of days ago, probably due to a composer update! I'm about to try this fix right now!

@robbanl
Copy link

robbanl commented May 10, 2017

This solved our issue! Oh my god!

@robbanl
Copy link

robbanl commented May 11, 2017

@bh-ref Did you create module to fix this issue (I don't have any hopes that Magento will solve this within a year) or do you want me to create one?

@bh-ref
Copy link
Contributor Author

bh-ref commented May 11, 2017

@robbanl I'm glad it helped you!

Thanks for the offer, but we have a process were we directly patch the magento core files and note the github issues we open for these patches. If the patches are no longer applicable after a magento update, we either check if the problem was solved or update our github issues to let the magento team know that it still exists in the current version.
We also tried that approach of creating a module, but I think we could miss a fix because the plugin we would write would maybe work even after magento actually fixed the problem. So for us it is more a question of how you want to track if the bugs you discovered were fixed at some point...

@dankocherga
Copy link

This issue also causes problems with coupon codes:

  1. Add 2 products to the shopping cart
  2. Go to admin panel and disable one of the products
  3. Try applying invalid coupon code in the shopping cart

Expected result: error message is displayed.
Actual result: success message is displayed.

Disabling a product sets a trigger_recollect flag to all quotes containing this product.
Then, when you try to apply a coupon code this happens:

  1. Quote is loaded, and totals are collected in afterLoad, since trigger_recollect is set. This action also sets totals_collected_flag = true to avoid repeated collecting.
  2. Coupon code is set to quote and Magento tries to collect totals, that should validate the coupon inside. But totals are already collected in afterLoad, so totals_collected_flag is set and collectTotals with coupon code does not happen.

@magento-engcom-team magento-engcom-team added Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed and removed G1 Passed labels Sep 11, 2017
@plastiqman
Copy link

We have the same problem in a Magento 2.1.9

We have detected 125 carts with this problem. But we have installed the MSP_CashOnDelivery.

Do you know if you have fixed this error in 2.1.9?

Thanks.

@magento-engcom-team magento-engcom-team added the Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed label Nov 30, 2017
@magento-engcom-team
Copy link
Contributor

@bh-ref, thank you for your report.
The issue is already fixed in 2.2.0

@magento-engcom-team magento-engcom-team added 2.1.x Fixed in 2.2.x The issue has been fixed in 2.2 release line Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release labels Nov 30, 2017
@magento-engcom-team
Copy link
Contributor

Hi @bh-ref. Thank you for your report.
The issue has been fixed in #14719 by @philippsander in 2.2-develop branch
Related commit(s):

The fix will be available with the upcoming 2.2.5 release.

@magento-engcom-team
Copy link
Contributor

Hi @bh-ref. Thank you for your report.
The issue has been fixed in #14812 by @ihor-sviziev in 2.3-develop branch
Related commit(s):

The fix will be available with the upcoming 2.3.0 release.

@LucScu
Copy link

LucScu commented May 31, 2018

@plastiqman Same issue here with 2.2.2.
After i applied the fix magento commit the issue remains. So i go deep in the core and with your hint i find that MSP_CashOnDelivery create this loop

Magento\Quote\Model\QuoteRepository->getForCustomer() exec loadByCustomer()
Magento\Quote\Model\Quote->loadByCustomer() exec _afterLoad()
_afterLoad() exec collectTotals()
collectTotals() trigs some events that call MSP\CashOnDelivery\Model\Total\Quote->collect()
MSP\CashOnDelivery\Model\Total\Quote\Cashondelivery->collect() exec _canApplyTotal()
MSP\CashOnDelivery\Model\Total\Quote\AbstractTotal->_canApplyTotal() exec $this->paymentMethodManagement->getList()

finally we come back to
Magento\Quote\Model\QuoteRepository->get() that exec Magento\Quote\Model\Quote->loadByIdWithoutStore() where _afterLoad() is recalled and it create the loop

For now i fixed it with this code
if ($quote->getTriggerRecollect() == 1) return $this;
in MSP\CashOnDelivery\Model\Total\Quote\Cashondelivery and MSP\CashOnDelivery\Model\Total\Quote\CashondeliveryTax as first check in collect().

@magento/magespecialist Are you aware of this issue?

@ishakhsuvarov
Copy link
Contributor

Hi @bh-ref. Thank you for your report.
The issue has been fixed in #15522 by @rogyar in 2.1-develop branch
Related commit(s):

The fix will be available with the upcoming 2.1.15 release.

@ishakhsuvarov ishakhsuvarov added the Fixed in 2.1.x The issue has been fixed in 2.1 release line label May 31, 2018
JohnMTrimbleIII added a commit to JohnMTrimbleIII/taxjar-magento2-extension that referenced this issue Aug 20, 2018
…llect is never unset, at least taxjar won't continuously request the api endpoints
magento-devops-reposync-svc pushed a commit that referenced this issue Feb 8, 2025
…loyment

[Bengals] Functional Tests Mainline Deployment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fixed in 2.1.x The issue has been fixed in 2.1 release line Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants