Contrib.social

combineCarts() can timeout ungracefully

Open on Drupal.org →
Created on 20 December 2021, almost 3 years ago
Updated 30 April 2024, 7 months ago

Problem/Motivation

The combineCarts() method currently loops over each item in $other_cart and adds it to the $main_cart through $this->cartManager->addOrderItem($main_cart, $item, $combine). As is, this has the effect of calling ->save() on the main cart for each order item, before we finally call it explicitly one more time after the loop.

On a clients site there are folks building up carts with 60+ items, the merge of which is then timing out when they try to log in. Because the last few lines (where the other cart is deleted) don't run, the user still has multiple carts, just now with corrupt data. Specifically, the $other_cart still believes it owns the order items that have been moved over to the $main_cart. When this happens lots of times (as when a user repeatedly tries to log in because there's a sale on) this has the slightly amusing effect that on subsequent log-in attempts, for the line items that get reached before timing out, the quantities get doubled every time. So we had users racking up baskets totalling £16 million, including 4,194,304 of some items (2^22)!

Steps to reproduce

I forced reproduction of this locally by putting an infinite while loop into the body of addOrderItem that would fire after a few iterations, and then lowered my max_execution_time to save we waiting around.

Then, as an anonymous user with items in a basket, try to log in as a user who already has items in their basket. Observe a timeout, and inspect the state.

Proposed resolution

I suggest 2 improvements:

  1. Wrap the business of combineCarts in a transaction, so that carts either get merged successfully, or are left untouched.
  2. To improve the chances of them getting merged successfully, we should also pass FALSE as the fourth argument to addOrderItem to prevent a cart save for every item, as it only needs to happen once and we're doing that already after the loop.

Remaining tasks

Upload a patch with these changes.

User interface changes

None.

API changes

None.

Data model changes

None.

🐛 Bug report
Status

Fixed

Version

1.0

Component

Code

Created by

🇬🇧United Kingdom stevetweeddale

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024