Describe your bug or feature request.

Seeing this error when rolling back migrations.

PHP Fatal error: Uncaught TypeError: Drupal\commerce_payment\PaymentOrderUpdater::updateOrder(): Argument #1 ($order) must be of type Drupal\commerce_order\Entity\OrderInterface, null given, called in /var/www/html/web/modules/contrib/commerce/modules/payment/src/PaymentOrderUpdater.php on line 59 and defined in /var/www/html/web/modules/contrib/commerce/modules/payment/src/PaymentOrderUpdater.php:67

Here is the offending function:

  /**
   * {@inheritdoc}
   */
  public function updateOrders() {
    if (!empty($this->updateList)) {
      /** @var \Drupal\commerce_order\OrderStorage $order_storage */
      $order_storage = $this->entityTypeManager->getStorage('commerce_order');
      foreach ($this->updateList as $order_id) {
        $order = $order_storage->loadForUpdate($order_id);
        $this->updateOrder($order, TRUE);
      }
    }
  }

Maybe in the foreach loop, before we call $this->updateOrder($order, TRUE); we should check

if (!$order) {
  continue;
}
CommentFileSizeAuthor
#3 3383333-3.patch639 bytestonytheferg

Comments

tonytheferg created an issue. See original summary.

tonytheferg’s picture

Issue summary: View changes
tonytheferg’s picture

Status: Active » Needs review
StatusFileSize
new639 bytes
jsacksick’s picture

@tonytheferg: Maybe adding this check doesn't hurt... However I think there's a problem with your migration as it shouldn't trigger this logic...

tonytheferg’s picture

Totally Agree. It's just the timing of the rollback.

but maybe there is a small chance somehow programatically creating or deleting payments/orders might trigger the error.

I needed the patch for my hackery anyhow. 🤣

jose reyero’s picture

Status: Needs review » Reviewed & tested by the community

We've had the very same issue unrelated with migration and it looks like there's something wrong with how PaymentOrderUpdater
works.

The thing is you may only see this error on the console when using cron as it is triggered from PaymentOrderUpdater::destruct(), maybe it doesn't even get into the logs as it is produced when the request is being destroyed.

It happens when you delete a commerce order from some cron process. Then PaymentOrderUpdated keeps the order id cached, but fails to reload the order, that was deleted, when doing the clean up operations...

And yes, the patch fixes the issue, or at least prevents the error, which I'd say is a very bad one as it can go hidden for a long time and have very unexpected consequences, since it prevents the request from being properly terminated...

jsacksick’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.