Checkout /pay/token leads to 404

[solverat]

Q	A
Bug report?	yes
Feature request?	no
BC Break report?	no
RFC?	no

We currently struggle with a serious issue. On a P11 Installation, we receive a lot of failed orders without any error logs. After checking a lot of access logs, I found a coherence.

All these failed orders are coming from a 404 when entering /_locale/shop/pay/{token}}:

A status 404 can be thrown here:

CoreShop/src/CoreShop/Bundle/PayumBundle/Controller/PaymentController.php

Lines 47 to 64 in f1f118c

	public function prepareCaptureAction(Request $request): RedirectResponse
	{
	if ($request->attributes->has('token')) {
	$property = 'token';
	$identifier = $request->attributes->get('token');
	} else {
	$property = 'id';
	$identifier = $request->attributes->get('order');
	}
	/**
	* @var OrderInterface|null $order
	*/
	$order = $this->getOrderRepository()->findOneBy([$property => $identifier], true);
	if (null === $order) {
	throw new NotFoundHttpException(sprintf('Order with %s "%s" does not exist.', $property, $identifier));
	}

After checking the order versions:

On exact 12:25:21, the order changes its state from cart to order. If the time is right, orderRepository()->findOneBy() won't find any order because of… I'm not sure? Runtime-Cache? Cache generally? Locks?

Any help would be appreciated!

[jdreesen]

Maybe related to #2511 / #2515?

[dpfaffenbauer]

for sure related to that, but the token is generated when the cart is created. so quite interesting... can we somehow further dbeug?

[dpfaffenbauer]

It also cannot be locks or runtime cache. the listing always queries the database and since said before, the token is generated on cart creation.

[solverat]

We also have non existing token issues (users with old cart before there was the token feature) but this is not the issue here, the token is definitely available. I'm quite sure, that it's about the order state (as you can see, the state changed in the same second as the the user gets redirected). It's also not happening all the time, which is also a cache related indication to me.

[dpfaffenbauer]

It doesn't really matter what state it is, as long as the order has a token, it has to be found.

[dpfaffenbauer]

What if we add a specific function to the repository to get Order by Token and use that? Maybe the findOneBy does something weird there?

[solverat]

The token is available, but the order has to be in state "order", which is most probably not the case since the state changed at the same time.

[dpfaffenbauer]

no it hasn't, the Payment Controller only gets the Order, creates the Payment and redirects to payum.

[dpfaffenbauer]

What if you visit the link later? Like if you thing it is a race-condition, it should work like a second later or so

[solverat]

Not sure. I'll check it on monday. The only thing i know (and as you can see in the screenshot), the state (see version screenshot) changed in the same second then the access log recorded the 404 (see log screenshot).

[solverat]

Btw! Maybe its a route issue? Coreshop delivers two routes with an almost same endpoint: /pay/{token} and /pay/{order}.
The order route has a numeric validation on {order}. Don't know, I have to test it.

[dpfaffenbauer]

Yes, also sounds reasonable. Maybe delete the old one? or change priorities?

[solverat]

Jop, found it. The issue starts here:

CoreShop/src/CoreShop/Bundle/FrontendBundle/Resources/install/pimcore/staticroutes.yml

Lines 138 to 139 in f1f118c

	coreshop_payment:
	pattern: '/(\w+)\/shop\/pay\/([0-9]+)$/'

All tokens of our failed orders are starting with a numeric character (See example above): 4TqEHjxRIp. This will match with the route coreshop_payment (pay/([0-9]+)$) first, which leads to the wrong identifier here:

CoreShop/src/CoreShop/Bundle/PayumBundle/Controller/PaymentController.php

Lines 53 to 54 in f1f118c

	$property = 'id';
	$identifier = $request->attributes->get('order');

Removing the route coreshop_payment fixes this. This is quite a thing, because this increases bounce rates (since you may just think, the customer just didn't want to complete his order).

[dpfaffenbauer]

Increasing the priority of the coreshop_payment_token helps?

[solverat]

Yes, the prio should be switched also (the legacy route prio is currently higher than the new one).

[dpfaffenbauer]

Ok, I will create a PR and do a release monday/tuesday

[solverat]

Btw. with #2511, all existing carts can't be used for a valid checkout, because the token is missing.

[dpfaffenbauer]

@solverat token will be generated here: https://github.com/coreshop/CoreShop/blob/4.0/src/CoreShop/Component/Order/Factory/OrderFactory.php#L43, that is in there since #2265

Should we still add a migration for missing tokens?

[dpfaffenbauer]

or a fallback that if no token is there, create one first in the checkout controller

[dpfaffenbauer]

 if (!$order->getToken()) {
     //Fallback for Orders/Carts without token (eg. legacy carts)
    $tokenGenerator = new UniqueTokenGenerator();
    $order->setToken($tokenGenerator->generate(10));
    $order->save();
}

In the checkout-controller before generating the payment route

[solverat]

I'll open another issue for that!

[solverat]

To confirm: It's still better to remove the coreshop_payment if you're not using it in a custom override or something similar, right?

[dpfaffenbauer]

yes.

[dpfaffenbauer]

we kept it for BC reasons, in case somebody uses it in custom overrides