commerce-stripe icon indicating copy to clipboard operation
commerce-stripe copied to clipboard

Missing webhook for payment / charge success?

Open samhibberd opened this issue 11 months ago • 2 comments

Description

We use a custom stripe payment element integration to handle payments, with the final redirect from stripe to us hitting the commerce payments/complete-payment controller to complete everything / trigger after paid events / custom logic and then display our confirmation page etc etc.

There is the know limitation that crops up where something fails (either user behavior or network issues) that causes the payment to be successful in stripe but where the craft controller is never reached.

In this case should there be (and we thought there was) a fallback to complete the order via the Stripe webhook. I've had a good look around the codebase and can't see any logic for this, or handling any other webhooks in standard Gateway (not SubscriptionGateway).

Could be wrong but feels like it would work well and address this rare (but frustrating) issue, event if the webhook arrived first and completed the order then the confirmation page would be shown, as craft protects against duplicate completions and the transaction hash mutex would prevent any database issues.

May be over simplifying this but feels like a win?

Also is this kinda related?

Additional info

  • Craft CMS version: 4.x
  • Stripe for Craft Commerce version: 4.x
  • PHP version: 8.2
  • Database driver & version: mariabdb
  • Plugins & versions:

samhibberd avatar Mar 07 '25 09:03 samhibberd

@samhibberd I would have thought that

https://github.com/craftcms/commerce-stripe/blob/5de45b9773fe8c3cb237f59c257d24e07607c016/src/base/SubscriptionGateway.php#L528

would have been handling the successful payment intents? There was a bug we fixed recently in that method to ensure that checkout sessions (which used a different transaction reference) would be completed.

This line in that method should be marking the order as complete: https://github.com/craftcms/commerce-stripe/blob/5de45b9773fe8c3cb237f59c257d24e07607c016/src/base/SubscriptionGateway.php#L588

Hitting the complete payment controller action after that should just take you to the success page. Can you confirm the handlePaymentIntentSucceeded() method is not being called?

lukeholder avatar Apr 07 '25 06:04 lukeholder

I will run some testing and confirm, but I assumed (when reviewing the code initially) that this logic is only run for subscriptions? I am referencing the single payment logic?

samhibberd avatar Apr 07 '25 18:04 samhibberd

@lukeholder i've run some local testing using the stripe cli and (contrary to what I understood) the webhook handlers in the Subscription gateway are triggered.

I'm finding it hard to get my head around the flow and where the webhook fits in, as I understand it:

  1. Payment Intent created when we load / display the payment form.
  2. Payment form completed and submitted.
  3. Stripe redirects to the craft pay controller, which completes the payment and shows the success page.

In my testing the handlePaymentIntentSucceeded() webhook handler was fired, but before the stripe redirect happened and no transaction was updated. I have subsequently retested, forcing the webhook logic to be run before the complete payment action, and no transaction is updated.

In the case that the redirect is prevented from happening, by lost network request or some other unexpected behavior, surely the handlePaymentIntentSucceeded handler should update the transaction (completing the order) even if this happens before the stripe redirect happens?

I can't get this to happen with the latest version of stripe commerce! Any thoughts?

samhibberd avatar Apr 08 '25 11:04 samhibberd

Hi @lukeholder any thoughts on my feedback?

samhibberd avatar Apr 23 '25 12:04 samhibberd

Hi @samhibberd

Sorry for delay on this. I am kinda confused about what you are seeing.

The payment intent success webhook handler code is here:

https://github.com/craftcms/commerce-stripe/blob/5.x/src/base/SubscriptionGateway.php#L528-L591

It will save the transaction as successful and update the order as completed with updateOrderPaidInformation().

Are you saying that this did not work? What did the method do when it was called by the webhook?

lukeholder avatar Apr 23 '25 12:04 lukeholder

So in my testing the handlePaymentIntentSucceeded() webhook handler was fired, but no transaction was updated and the order was not completed. The order only completes when the redirect successfully hits the complete payment action.

To clarify, we are seeing this behavior on 4.x we have not updated the site yet and are unable to test the 5.x code.

samhibberd avatar Apr 23 '25 13:04 samhibberd

Hi @lukeholder have you had a chance to review my feedback?

samhibberd avatar May 13 '25 11:05 samhibberd

Please can someone @lukeholder @brandonkelly @nfourtythree give this a look over, we are having to regularly hunt through Stripe logs to identify the craft complete payment url and trigger it manually. And as I understand it, addressing this would fix.

samhibberd avatar May 21 '25 10:05 samhibberd

@samhibberd Sorry, I was looking into this yesterday and had not been able to reproduce.

You said that if you manually trigger the complete-payment controller action, it will mark the order as paid and completed, but if you trigger the payment intent success webhook the code here:

https://github.com/craftcms/commerce-stripe/blob/4.x/src/base/SubscriptionGateway.php#L531-L594

Which handles the webhook is not running, or just not completing the order?

I will give you a branch momentarily to test with some logging in that code. Give me an hour and I will send update you with the branch to try.

Thanks

lukeholder avatar May 21 '25 11:05 lukeholder

The code in 4.x is the same as 5.x for the webhook successful payment intent.

I have just made a branch off 4.x and put logging in various places in the method: https://github.com/craftcms/commerce-stripe/pull/348

To try the branch, change your craftcms/commerce-stripe requirement in composer.json to:

"require": {
  "craftcms/commerce-stripe": "dev-bugfix/log-webhook as 4.1.7",
  "...": "..."
}

Then run composer update.

Can you then clear your logs (back them up), and then re-trigger a successful webhook for an affected order and then send the logs to [email protected] so I can take a look at what parts of the method it is not hitting.

Thanks

lukeholder avatar May 21 '25 11:05 lukeholder

@samhibberd

I will run some testing and confirm, but I assumed (when reviewing the code initially) that this logic is only run for subscriptions? I am referencing the single payment logic?

The payment intent success handler is for single payments.

lukeholder avatar May 21 '25 11:05 lukeholder

Hi @lukeholder, thanks for coming back to me:

Which handles the webhook is not running, or just not completing the order?

From my testing the webhook is running, just not completing the order.

By adding a delay to the actionCompletePayment, to ensure the webhook logic is run before the controller logic, my debugging sees the webhook code running, but the $updateTransaction being null at the point that I would expect it to have a value:

Image

NB: screenshot from ray shows the lines that the output was added.

Will run again with the debugging branch and provide the output.

samhibberd avatar May 21 '25 13:05 samhibberd

I think I have found the issue.

We are expecting the transactions that are marked as complete via a webhook to be in a processing status (not a redirect status) since processing status is used when we have a successful payment but there could be additional async actions that the customer must do to complete the payment at a later time (or a bank payment to clear etc).

Thinking about it, we should probably still allow redirect transactions to be completed by the webhook though.

I have made the fix on the same logging branch as of commit: d287a148e3b267bef1fd21a87d5212ae27b4a8d0

change your branch to:

"craftcms/commerce-stripe": "dev-bugfix/log-webhook#d287a148e3b267bef1fd21a87d5212ae27b4a8d0 as 4.1.7",

And try re-running the webhook from the stripe console for the affected order. (I think it was pi_3RRZ9fGQVThKnTiZ0PpqnOtr in the transaction dump you sent me)

Will update here once confirmed.

lukeholder avatar May 28 '25 23:05 lukeholder

Brilliant, this has fixed the issue in my scenario 🎉

samhibberd avatar May 29 '25 07:05 samhibberd

@lukeholder Are we looking at an upcoming release for this, be great to get this fix out in the wild 😆 ?

samhibberd avatar Jun 05 '25 08:06 samhibberd

Stripe for Commerce 4.1.8 is now out with a fix for this.

I will update this ticket once 5.0.8 is released with the same fix tonight.

lukeholder avatar Jul 09 '25 04:07 lukeholder

This is now fixed in 5.1.0 Thanks!

lukeholder avatar Jul 09 '25 13:07 lukeholder