Payment entity created after webhook

Created on 20 February 2024, 9 months ago
Updated 9 September 2024, 2 months ago

For some reason when the webhook is triggered the payment entity does not exist

The checkout flow is set up according to instructions Stripe panes are on review step, payment process (the one creating payment entity) is in the payment step, but the webhook is fired first and it does no find the payment, and right after that the payment with correct remote_id is created, but it is too late

Not sure if this is my setup issue only, or someone else is facing it also?

๐Ÿ› Bug report
Status

Fixed

Version

1.0

Component

Code

Created by

๐Ÿ‡บ๐Ÿ‡ฆUkraine gilmord ๐Ÿ‡บ๐Ÿ‡ฆUkraine

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

Merge Requests

Comments & Activities

  • Issue created by @gilmord
  • ๐Ÿ‡บ๐Ÿ‡ฆUkraine gilmord ๐Ÿ‡บ๐Ÿ‡ฆUkraine
  • ๐Ÿ‡บ๐Ÿ‡ฆUkraine gilmord ๐Ÿ‡บ๐Ÿ‡ฆUkraine

    I use commerce core 2.38.0 and commerce_stripe dev-1.x, and I face this issue on every payment

  • ๐Ÿ‡บ๐Ÿ‡ฆUkraine gilmord ๐Ÿ‡บ๐Ÿ‡ฆUkraine

    I have added sleep(1) in StripePaymentElement::processWebHook which is a horrible solution but it works

    checking on how the other payment gateways are dealing with the same issue

  • ๐Ÿ‡บ๐Ÿ‡ฆUkraine gilmord ๐Ÿ‡บ๐Ÿ‡ฆUkraine

    Here is my checkout flow:

  • ๐Ÿ‡บ๐Ÿ‡ฆUkraine gilmord ๐Ÿ‡บ๐Ÿ‡ฆUkraine

    I checked the commerce paypal module, they create payment in the onReturn method of the payment gateway plugins:

    Example 1

    Example 2

    Example 3

    Looks like this part is missing for the Commerce Stripe and payment is created in "Payment process" pane and not on the onReturn of the gateway plugin

  • ๐Ÿ‡บ๐Ÿ‡ฆUkraine gilmord ๐Ÿ‡บ๐Ÿ‡ฆUkraine

    By the way visually all looks good - order is paid, user is charged, user sees no errors

    I have noticed the issue when enabled commerce_stripe_webhook_event and checked the webhook logs at /admin/commerce/config/stripe-webhook-events

  • ๐Ÿ‡บ๐Ÿ‡ฆUkraine gilmord ๐Ÿ‡บ๐Ÿ‡ฆUkraine


  • ๐Ÿ‡ฆ๐Ÿ‡นAustria agoradesign

    Thanks for pointing this out! I wasn't aware of this. imho this is really a huge problem! And I'll raise the priority of this issue, due to the fact that handling payment info is a very sensitive task, and the current implementation might lead to a bunch of problems, and I'll explain why:

    First, as described in the issue summary, it's really quite common, that the notification callbacks are called before the customer returns to the site. So, we'll miss a lot of succesful payments due to this. And also as described, you can never rely on desperate workaround like waiting for a second. Because this may help you in let's say 70% of the cases, but sometimes you might need 2 or 3 seconds or even more. so that's never the answer (I have tried similiar workarounds for a different problems with payment notifications on another project)

    Second, the main purpose of such webhooks is that a store can receive a reliable feedback on payments. It does happen quite regularly that a user never returns from an offsite payment gateway. Sometimes the user may have closed the browser window too early (some do that deliberately, some by accident). Sometimes the internet connection may hang. Sometimes the Drupal website may have a short down, just in the moment, where the user comes back, or any other problem might occur (eg a database serialization error, etc)

    So, it's nice to receive webhooks for status updates, but it is even more important to receive those webhooks to guarantee that we never miss a successful payment, just because the user haven't returned properly to the website!

    So, for a payment gateway, that reliably sends webhook notifications, it's far more important to process the payment information in the onNotify() hook than in the onReturn() hook - in other words, it'll better in doubt to skip processing there (only do a short verification of the passed data there) and leave the processing completely to onNotify() than relying on a payment entity created in onReturn()... I can't find the text passage right now, but Stripe even recommends not processing the payment informatin on return, instead rely on the notification callbacks only.

    Given my experience with other payment gateways, we might introduce another problem, as soon as we process payment both in onReturn() and onNotify(), because the overlapping requests can cause double-placements and related problems. It's hard to find the perfect solution for this problem then. I've tried different approaches, which all work quite good for us. The latest one was to process payment info in onReturn() directly, and in onNotify() create an advancedqueue job item, which will be processed on cron then. This way, you get rid of 99% of conurrent request problems, can send an immediate response to the payment gateway, which is also important. And in most of the cases, there's no need to do any processing anymore, as the order is already placed in Drupal, when cron runs. But for the small amount of use cases, where the user never returned, the order is placed on cron then

  • ๐Ÿ‡ฆ๐Ÿ‡นAustria agoradesign

    fyi, opened discussion in Slack regarding this: https://drupal.slack.com/archives/C1TLCCF9B/p1709025789941939

  • ๐Ÿ‡บ๐Ÿ‡ฆUkraine marchuk.vitaliy Rivne, UA

    Given my experience with other payment gateways, we might introduce another problem, as soon as we process payment both in onReturn() and onNotify(), because the overlapping requests can cause double-placements and related problems. It's hard to find the perfect solution for this problem then. I've tried different approaches, which all work quite good for us. The latest one was to process payment info in onReturn() directly, and in onNotify() create an advancedqueue job item, which will be processed on cron then. This way, you get rid of 99% of conurrent request problems, can send an immediate response to the payment gateway, which is also important. And in most of the cases, there's no need to do any processing anymore, as the order is already placed in Drupal, when cron runs. But for the small amount of use cases, where the user never returned, the order is placed on cron then

    This is correct and we planned to work on it in the future. There is no other way but to queue the webhook to be processed by a cron job.
    From the file StripePaymentElement.php

            $queue_webhook = FALSE;
            if ($queue_webhook) {
              // Soon, we will support queueing webhook processing.
              return new Response('', 200);
            }
    
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States gcb

    Any updates on this? It seems the approach is generally agreed upon. This issue is causing major pain for us on a high-volume commerce site.

  • First commit to issue fork.
  • Assigned to TomTech
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States TomTech
    • TomTech โ†’ committed bc940faa on 8.x-1.x
      Issue #3422570 by TomTech, gilmord, agoradesign, vmarchuk, gcb: Payment...
  • Status changed to Fixed 3 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States TomTech

    This MR does the following:

    Creates a payment when a payment intent success webhook event is received, if the payment has not yet been created.
    For advanced queue webhook event processing, webhooks are deferred 5s to minimize race conditions with a return url.
    Race conditions are improved via $order_storage->loadForUpdate($order_id) (including an enhancement in commerce core 2.40) See: ๐Ÿ“Œ Order Storage Lock Release Fixed

    Payment will be created by whichever process gets it first...whether the return url from the user browser, the other will await the other to complete, then proceed accordingly.

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

Production build 0.71.5 2024