mollie / Shopware6

MIT License
51 stars 54 forks source link

Bug: duplicate transition to payment status cancelled #286

Open niklaswolf opened 2 years ago

niklaswolf commented 2 years ago

When using a async payment method (e.g. klarnapaylater) and choosing payment status "cancelled" in the mollie sandbox, this leads to a duplicate payment state-transition to "cancelled". This causes for example duplicate sending of the cancellation mail etc.

Bildschirmfoto 2022-02-09 um 18 16 36

I quickly had a look and I strongly think this happens because the finalize-endpoint receives the cancelled status (www/vendor/shopware/core/Checkout/Payment/PaymentService.php:182) as well as a webhook is received, which triggers another state-change (www/vendor/store.shopware.com/molliepayments/src/Facade/Notifications/NotificationFacade.php:139).

Here you can see the log output ``` [2022-02-09T17:13:42.407610+00:00] Mollie.INFO: Webhook for order 10085 and Mollie ID: ord_3z672a has been received (Session: o7n3...) {"session":"o7n3snjp3fmqpkfp8mpq63i6pm","processors":{"uid":{"uid":"4bc49c0"},"web":{"url":"/mollie/webhook/cf43624e97144d75af1704c380f7b1ab","ip":"xxxx","http_method":"POST","server":"xxxx","referrer":null}}} [] [2022-02-09T17:13:44.719943+00:00] Mollie.INFO: Finalizing Mollie payment for order 10085 with payment: klarnapaylater and Mollie IDord_3z672a (Session: ...) {"saleschannel":"Aignermunich","session":"","processors":{"uid":{"uid":"78d991a"},"web":{"url":"/payment/finalize-transaction?_sw_payment_token=eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1","ip":"xxxx","http_method":"GET","server":"xxxx","referrer":"https://www.mollie.com/"}}} [] [2022-02-09T17:13:44.813168+00:00] Mollie.ERROR: Error when finalizing order 10085, Mollie ID: ord_3z672a, The customer canceled the external payment process. Payment for order 10085 (ord_3z672a) was cancelled by the customer. (Session: ...) {"session":"","processors":{"uid":{"uid":"78d991a"},"web":{"url":"/payment/finalize-transaction?_sw_payment_token=eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1","ip":"xxxx","http_method":"GET","server":"xxxx","referrer":"https://www.mollie.com/"},"introspection":{"file":null,"line":null,"class":"Kiener\\MolliePayments\\Handler\\PaymentHandler","function":"finalize"}}} [] ```

I think this could be avoided, if the webhook endpoint would first check the current transaction-status befo re triggering a new one. If it is the current transaction status that it wants to trigger, this step could be skipped.

boxblinkracer commented 2 years ago

Hi

thanks for this yep, that is already planned...actually even a refactoring and restructuring of the transitions stuff in general, because it's sometimes confusing indeed

boxblinkracer commented 2 years ago

so new answer on that its indeed a bit hard...to keep it short...problems with race conditions and what shopware does....thats why its duplicated... but we're aware of it and have it at least on the roadmap

niklaswolf commented 2 years ago

Any news on this? :)

niklaswolf commented 2 years ago

Sorry for being pushy, but are there any news on this issue? Maybe it was already fixed in one of the last releases and simply forgotten to close this? :)

boxblinkracer commented 2 years ago

hi @niklaswolf not yet, problem is this is not really related to mollie.

here is the full explanation

the mollie transition service already skips on duplicates. this one is e.g. used in the webhooks. the shopware one does not

so what is happening

in the storefront you pay, then the payment is cancelled...this leads to 2 things 1 webhook is coming in, and in sopware as you might know, we have to throw a custom CustomerCancelledException so that the rest of the UX is working as expected.

the problem is that the webhook is coming in too fast, so the mollie service transitions to "cancelled", THEN shopware is doing the same but not checking for duplicates. the only thing is, to "delay" the webhook which is a total must-not and also technically not possible.

so bottom line...i cannot do anything to still have a reliable system, i would rather see shopware skipping duplicate transitions.

i would create a ticket there and reach out, but havent had time on this yet.

niklaswolf commented 2 years ago

@boxblinkracer Thanks for the very detailed explanation! When you've created the ticket, please let us now, than we can also back it :)