We're now making great use of the webhook feature to add robustness to our checkout flow - great job!
However - we seem to be having a problem with a race condition where the webhook from Stripe (payment_intent.succeeded) - is arriving before the onReturn callback from the browser.
This then results in a "Call to a member function getState() on null" error returned to Stripe.
The onReturn route is then processed - and the order is left with two Payment items attached to the order - both with the same payment intent. Thankfully - we only see one real payment in Stripe....
I've put a small patch in place - on my test site - which prevents the Payment entity from being created if it already exists:
commerce_stripe\src\Plugin\Commerce\PaymentGateway\StripePaymentElement.php - around line 570
$existing_payment = $payment_storage->loadByRemoteId($payment_intent->id);
if ($existing_payment) {
// Payment already exists - don't create duplicate
\Drupal::logger('commerce_stripe_webhook')->error('Duplicate found; React to the payment intent returned in the onReturn callback.: Order; @order_id, Intent: @intent_id', [
'@order_id' => $order->id(),
'@intent_id' => $payment_intent->id,
]);
return;
}
Has anybody else seen repeated Payments - and does the above make sense?
| Comment | File | Size | Author |
|---|---|---|---|
| #20 | commerce_stripe_webhook_events-3536293-MR-196-20.patch | 3.18 KB | anybody |
| #12 | 3536293_webhook_racing_on_return.patch | 2.13 KB | ahmed eldesoky |
| #7 | Screenshot 2025-07-17 173329.jpg | 134.69 KB | newaytech |
| #5 | Screenshot 2025-07-16 155645.jpg | 61.35 KB | newaytech |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
jsacksick commentedThis is probably a Commerce Stripe issue and I don't think it is still relevant as the webhook processing is queued? Are you running the latest version?
Comment #3
newaytech commentedHi @jsacksick - thanks for checking in. Yes - Commerce Version: 3.1.0 & Commerce stripe: Version: 2.0.1
Was not aware that the webhooks could be diverted to a queue - is that a setting somewhere?
Comment #4
jsacksick commentedYes, you need to turn on the commerce_stripe_webhook_event module. And we support processing the jobs with advancedqueue as well if it's installed.
Comment #5
newaytech commentedWow - a tick box fix. nice one - sorted now...
Comment #6
tomtech commented@newaytech,
Thanks for the report.
Receiving the webhook before the return is a valid and expected scenario.
The most important scenario, is if the customer completes the payment, the payment is processed on stripe, but the onReturn does NOT occur at all. e.g. If the customer loses their internet connection before onReturn is triggered from their browser.
The payment completed on the stripe side, and the webhook is there to complete the placement of the order, even if onReturn does not. Otherwise, we would have a stripe payment, but the order is not marked as placed/paid.
We have tested this extensively and built the module to handle race conditions between onReturn and onNotify(webhook). No matter which occurs first, both should work, and duplicate payment entities should not be created.
Sounds like you have encountered a scenario we are not accounting for, or something new causing an issue.
Can you provide more details on the "Call to a member function getState() on null" error you encountered?
There should hopefully be a full stack trace or at least a more detailed error message in the log messages so we can look into this.
(Queueing Webhook events is primarily helpful when your site is under load and can't process all the messages in real time. e.g. a Black FridayGiving Tuesday type of event, or a ticket drop for a large concert. It also provides other benefits, but queuing should solely be relied on for handling the race condition, as a slow internet connection could still cause onReturn to occur after onNotify.)
Comment #7
newaytech commentedHi @tomtech,
Thanks for looking into this - and also all the work around making the module more robust with the webhooks.
I've turned off the queue feature - and placed a test order (using a 3DS card) - I then see the below stack trace on the initial webhook (I changed the code to see the full stack being returned to Stripe) : - This
I'm using Fulfilment, with validation - and have a custom flow plugin defined (but I also went back to the default flow - and got the same error)
Comment #8
tomtech commented@newaytech,
Thanks for providing this.
Some observations:
1. The StripePaymentElement line numbers indicated in the stack trace don't seem to line up with commerce_stripe 2.0.1. Can you confirm you are using 2.0.1 with no patches/modifications?
2. The order workflow shouldn't be relevant here. The checkout flow could be if you have a custom checkout flow.
3. While the stack trace is here in full, there is usually one more line that reports the error in the error log. It appears the the "Call to a member function getState() on null" is occurring when isVisible() is called on a particular checkout pane. Identifying that pane, and the line this occurs on would be helpful in determining this issue. This is possibly an issue with that particular checkout pane.
Comment #9
newaytech commentedHi @tomtech
Thanks for the comments - in response:
1. Yes - I added various extra logging to understand better what was happening - but base module is 100% 2.0.1
2. I have reverted to the default workflow - and the checkout flow got me thinking...
3. Yes - I saw that too - and used the error we saw before I changed the call to show a full trace. The issue does point toward a problem when rendering the panes (why do we do this for a server to server call?)
Soo... Thinking about your comment regarding the panes - and also along with my own debugging - decided to turn off the "Email registration" module - and then hey presto - the first webhook from Stripe succeeded. I also saw the chain of events in watchdog - and the thread belonging to the Stripe IP processes the order to the point where it drops into my queue for onward warehouse processing.
Do we now have to transfer this over to the Email Reg (email_registration - Version: 2.0.0-rc8) team?
Comment #10
jonathanshawProbably not
because this should never happen even if there are bugs in other modules, it seems like our defences against it are not strong enough.
Comment #11
anybody@newaytech could you maybe post a screenshot of your checkout steps configuration so we better understand how it's configured? Totally agree with #10 here.
Comment #12
ahmed eldesoky commentedWe encountered the same issue using Commerce Stripe 2.1.0 with Commerce Core 3.1.0.
In our case, the Stripe webhook was triggered before the onReturn() function executed, which resulted in duplicate payments being created for the order. Stripe then received the following error response:
Call to a member function getState() on null
For debugging, we temporarily added the following line after line 979:
\Drupal\Core\Utility\Error::logException($this->logger, $throwable);This produced the following log entry:
Tracing the stack shows that this
line in StripePaymentElement
when using the Multistep (default) checkout flow and having the Guest registration after checkout pane enabled, leads to isVisible() being invoked before a current order is set, which violates Commerce Core’s expectations (see: https://git.drupalcode.org/project/commerce/-/blob/3.x/modules/checkout/src/Plugin/Commerce/CheckoutPane/CheckoutPaneBase.php?ref_type=heads#L58
).
The attached patch prevents the premature call and resolves the issue.
Comment #13
anybody@ahmed eldesoky could you then please provide the patch as MR instead to be reviewed?
Comment #14
vmarchuk@tomtech
We can use the same solution I used here https://git.drupalcode.org/project/commerce_authnet/-/blob/8.x-1.x/src/P... as I also faced the same issue when I tried to reuse the code from the commerce_stripe module.
Comment #15
anybody@vmarchuk maybe also check commerce_paypal which is widely used, if it has the same flaw?
Comment #17
alanhdev commentedThere is a CheckoutOrderManager service (commerce_checkout.checkout_order_manager) that provides a method getCheckoutFlow(OrderInterface $order). This gets the checkout flow, sets the order property and then returns it.
Maybe this service could be used instead of explicitly setting the order on the checkout flow?
Comment #18
jonathanshaw#17 is also proposed in #14 by @vmarchuk from the commerce team
Comment #19
anybodyI can now confirm I'm also getting the same error
as in #12 in 2.1.0 without any patch.
Furthermore some payments are saved twice, so the order balance is negative!
This definitely happens since enabling the Commerce Stripe Webhook Events submodule and configuring the payment_intent.* webhooks on the Stripe side.
So I'm confirming this is a major issue with the webhooks submodule!
Comment #20
anybodyStatic patch attached from MR!196 until this is fixed.
Comment #21
anybodyAs of the change after #18
Comment #22
anybodyThe error
is gone now, with MR!196 / #20 applied!
Sadly the payments are still created twice now!
And even worse:
Now the URL which I think is visible to the user:
https://www.example.com/checkout/50034/review/return?payment_intent=xxxx&payment_intent_client_secret=xxxxxxxxxxxxxxx&redirect_status=succeededthrows an Exception:Comment #23
jonathanshawI made 3 changes:
(1) Added an order lock, to handle the race condition where onReturn and processWebhook were working exactly simultaneously
(2) Invoke placeOrder() in onReturn() only if the payment is recorded there; if processWebhook() has already recorded the payment, defer to how it has left the order.
(3) Remove the change to loading the checkout flow plugin in placeOrder(). It's a valid change, but it's no longer relevant to this issue now that I've done (2), and it's better handled in #3571373: Refine the StripePaymentElement::placeOrder() logic
Comment #24
jonathanshawComment #25
tomtech commentedHmm...this change is calling loadForUpdate() in onReturn() here: https://git.drupalcode.org/project/commerce_stripe/-/merge_requests/196/...
But, loadForUpdate() is already called by commerce core returnPage(), here: https://git.drupalcode.org/project/commerce/-/blob/3.x/modules/payment/s...
just before it invokes onReturn() on this plugin here: https://git.drupalcode.org/project/commerce/-/blob/3.x/modules/payment/s...
The calls to loadForUpdate() in returnPage() and in the webhook were done precisely to handle this scenario and tested pretty extensively.
If you are experiencing this issue, something else seems be at play, causing the semaphore pattern from preventing this from occurring.
Here are some possibilities:
1. If you have any custom code at play in here, make sure that you do NOT call $order->save(), as that releases the lock. (And also causes other issues, as Drupal does not handle entities well when save() is called multiple times on a single request. See: https://www.drupal.org/project/drupal/issues/3563047)
2. If you have multiple web servers, ensure that you still have one shared lock backend. If not, then two requests handled by different servers AND have two different lock backend instances will not know about each other, and therefore won't block each other from proceeding.
3. Might you have custom code invoking onReturn() that did not come from returnPage()? In that case, that calling code would need to handle the locking, similar to returnPage().
4. Do you have a custom step in the checkout workflow between review->payment->complete? If so, you are likely preventing the order from being transitioned out of draft. The behavior at this point is undefined. If an order is not moved out of draft, after the payment is recorded in the checkout(or webhook) flow, then there are many possible side effects and undefined behavior.
Some other notes:
The call to releaseLock() in the MR is not needed. see: https://www.drupal.org/project/drupal/issues/3563047
The order lock is released when the order is saved. see: https://www.drupal.org/project/drupal/issues/3563047
(You really should only ever need to invoke releaseLock() if you called loadForUpdate(), then decide you will not be saving the order.)
IRT the comment:
We shouldn't even end up in onReturn(). If the order is handled in the webhook, the code in returnPage(), after acquiring the lock, checks to see if we are on the correct step..and if not, progresses us forward. (validateStepId() right after loadForUpdate() handles this.)
Also, setting the component on this to Payment Element, since this seems to be specific to it, and not applicable to Card Element.
Comment #26
jonathanshawOuch, apologies for the noise, you're completely right my changes in #23 are ill-conceived and failed to properly consider what returnPage() was doing.
Comment #27
tomtech commented@jonathanshaw, your thought process makes sense! Since we are already doing the lockForUpdate(), though, seems this has a different root.
I am able to reproduce the issue when the next step (almost always completed) has a checkout pane whose isVisible() references the order.
The approach by @ahmed-eldesoky and @vmarchuk makes sense. We aren't setting the order in placeOrder(). While this isn't an issue in onReturn(), since onReturn() actually has an order in the request parameters, if the webhook fires first, it will not have the order.
The fix proposed in 14 and 17 makes sense. I've updated the MR to reflect this, with a couple tweaks.
The tests just passed, so adding it to the merge train.
Comment #29
tomtech commentedMerged. Thanks all!
(If you have a race condition that is NOT resolved by this, please open a new issue with the details, as this resolves the specific case identified multiple times in this issue.)
Comment #32
anybodyWe just tried re-enabling the webhook after this is fixed now, but sadly #19 still happens despite the fix, using Express Checkout.
I now created a new issue for that: #3590889: Using the Express Checkout with Webhook enabled payments are added twice and order balance becomes negative
I also created a further META issue because it seems there are still other race conditions for things that happen in both methods: #3590914: [META] Ensure Webhooks and onReturn don't race, do the same things twice or overwrite each other (especially with Express Checkout)