Processing webhooks in the queue

TomTech → changed the visibility of the branch 3424966-processing-webhooks-in to hidden.

vmarchuk → changed the visibility of the branch 3424966-processing-webhooks-in to active.

Link to MR https://git.drupalcode.org/project/commerce_stripe/-/merge_requests/88/d...
@TomTech I left a few questions for MR, could you please check them? After that, I will test the patch locally.

@TomTech

I wanted to test the patch locally, but I can't even apply the code changes:

➜  kickstart ddev drush updb -y
 ------------------------------- ----------- --------------- ------------------------------------------------- 
  Module                          Update ID   Type            Description                                      
 ------------------------------- ----------- --------------- ------------------------------------------------- 
  commerce_stripe_webhook_event   9101        hook_update_n   9101 - Install the advanced queue, if possible.  
 ------------------------------- ----------- --------------- ------------------------------------------------- 


 // Do you wish to run the specified pending updates?: yes.                                                             

>  [notice] Update started: commerce_stripe_webhook_event_update_9101
>  [error]  Drupal\Core\Entity\EntityStorageBase::create(): Argument #1 ($values) must be of type array, bool given, called in /var/www/html/web/modules/contrib/commerce_stripe/modules/commerce_stripe_webhook_event/commerce_stripe_webhook_event.install on line 159 
>  [error]  Update failed: commerce_stripe_webhook_event_update_9101 
 [error]  Update aborted by: commerce_stripe_webhook_event_update_9101 
 [error]  Finished performing updates. 
Failed to run drush updb -y: exit status 1

Small commit pushed to use appropriate FileStorage class.

@TomTech

Thanks for the hook_update() related fix. I tested the patch locally, it seems to work fine.
The only thing I'm worried about is that after placing the order (and processing the queue) I see the payment_intent.succeeded event succeed. But it's a success because all the conditions we have in case Event::PAYMENT_INTENT_SUCCEEDED were skipped and the webhook did nothing.
Don't you think we should add a new condition and skip it?

@vmarchuk

The scenario you describe is not specific to the queue implementation, it seems. Can you provide details if it is?

The OOTB behavior for Event::PAYMENT_INTENT_SUCCEEDED handles payments in the 'authorization' or 'new' state, specifically,

if ($payment->getState()->getId() === 'authorization') {
    // logic here
}
if ($payment->getState()->getId() === 'authorization') {
    // logic here
}

This code could be changed to a switch statement, to log the case you describe e.g.

switch ($payment->getState()->getId()) {
    case 'authorization':
        // logic here
        break;

    case 'new':
        // logic here
        break;

    default:
        // skip logic here
}

Let's handle that in a separate issue, though, since it isn't specific to queue.

Great news! :)

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

All my webhook events are in Skipped status
But when I manually add them to the process queue and run the queue worker - they are Succeeded now

Other question:
the payment entity related to the Succeeded webhook is not "authorized"

I expect this to work like this:
- user pays
- payment entity created
- webhook processed on queue
- some visual result in the payment

Am I wrong?
Or some changes to payment entity will only be done if with webhook is something wrong?

Hi guys,

I am considering using Stripe webhooks in order to finalize orders as recommended by Stripe. Stripe also recommend to respond with a 200 code ASAP before even dealing with the webhook itself. They also recommend to use an asynchronous queue so that we can deal with multiple events being sent simultaneously.
I figured this is why you decided to give the option to queue events and use Advanced Queue instead of processing webhooks right away.
Now, I discovered that queues need to be processed either by drush or by cron.
My server is currently set to run Drupal's cron every 5 minutes. So this would mean that in the worst case scenario, if I rely only on webhooks to finalize the order, it could take up to 5 minutes before the order actually completes!?
Am I correct?

I noticed that even when webhooks are enabled, you still also rely on the code inside onReturn() to finalize the order. It is not disabled. Am I right as well?
Did you do that precisely because relying only on queued webhooks could take too long so you prefer to rely on the return url to finalize the order and webhooks are only here as a safe guard in case something goes wrong with the return url?
I still believe that 5 minutes could be too long even for the safe guard, and the webhook should be processed ASAP right after the 200 response is sent to Stripe.
ChatGPT tells me I could manually trigger the queue like this
\Drupal::service('advancedqueue.worker_manager')->processQueue('my_custom_queue');
what do you think of this? why didn't you do it?
Chat also told me about continuous workers like Supervisord and Systemd. Is that a good idea? Is it easy to use? Do you use them?

Thank you in advanced, I'm a bit lost, I thought webhooks was the way to go, I thought queueing them was the way to go, and now I don't know what to think anymore…

In the end, here is the solution I'm probably going to go for instead of using Advanced Queue:

// first, I store the webhook data in the database, then I trigger it's processing but through a cURL post request that will work asynchronously, allowing me to respond to Stripe after 1 second of waiting max, or less if processing is even faster.
$http_client = \Drupal::httpClient();
$domain = \Drupal::request()->getSchemeAndHttpHost();
try {
  $http_client->post($domain . '/process-webhook-async', ['timeout' => 1]);
} catch (ConnectException $e) {
  // timeout exception will be thrown if webhook processing takes more than 1 sec, but it's ok, we do nothing and let the rest of the code execute so that Stripe gets the 200 code response
  // webhook processing will not be affected by the 1 second limit
}
return new Response('', 200);

this would replace the Queue for me, allowing me to process the webhook ASAP but still return a 200 code to Stripe ASAP as well and even if the webhook processing would fail. And I would still be able to reprocess a failed webhook based on what's stored in the database.
What do you think? Do you see a problem with my solution? Do you still see any advantage to use AdvancedQueue ?

Hi Nicolas!

Appreciate the detailed analysis and questions.

You are touching on a lot of intertwined items, and it would require more time to provide a thorough response. I'll give this summarized response for now.

1. onReturn() is the "happy path" for the typical scenario and provides the smoothest customer flow in 99% of cases. (for card processing.)
2. onNotify() / webhooks handles additional scenarios, like:
   1. a customer who completed stripe payment on the client/stripe side, but then they lost network connection before the redirect to onReturn() has been invoked.
   2. refunded/cancelled payments that occur on the processor/bank side.
   3. delayed payment processing, e.g. ACH(bank payments) that are not guaranteed at the time of order placement. The webhook might not fire for several days to indicate the payment was a success/fail.

As of 1.2, both onReturn and onNotify handle checkout completion, and check if it has already happened, to prevent double processing. onReturn always occurs for the client facing experience, but processing may be skipped when it detects that onNotify has already handled it.

onReturn and onNotify/webhooks is NOT an either/or. They work together to handle many different scenarios.

I would recommend to

1. Enable the commerce_stripe_webhook_event submodule (We will likely promote this to the commerce_stripe module in a future version, rather than having it as a submodule.)
2. Enable queued webhook processing using advanced queue
3. Set up a scheduled job that runs every minute, executing the drush command drush advancedqueue:queue:process commerce_stripe_webhook_event

There are ways to scale up even more from this, but this is a good starting point.