woocommerce-gateway-stripe icon indicating copy to clipboard operation
woocommerce-gateway-stripe copied to clipboard

Investigate `process_response()` being called with an non charge object or an empty charge object.

Open mattallan opened this issue 1 year ago • 1 comments

Zendesk: 8778680-zd-a8c

Describe the bug

Currently our process_response() expects a charge object (ch_xxxx) to be passed to it. Inside process_response() we use the charge object to get the captured parameter and if this is empty or false we mark the order as on-hold with an unhelpful order note beginning with "Stripe charge authorized".

We're getting a number of issue reports from merchants that indicate we're calling process_response() with an invalid $response object. We add the charge ID to the order note, however, from the screenshots sent from by merchants, we can see invalid charge IDs/objects are being passed to the function:

  1. With an intent object Image
  2. With null (screenshot is of an order note that's been translated) Image

Merchants are complaining about the order note because it appears on stores that are set to capture automatically, however the real issue is that process_response() is getting passed invalid Charge IDs.

Why don't we have a charge object?

Whenever we call process_response() we fetch the latest charge using the Payment Intent.

In some cases we check the charge exists first:

// Use the last charge within the intent to proceed.
$charge = $this->get_latest_charge_from_intent( $payment_intent );

// Only process the response if it contains a charge object. Intents with no charge require further action like 3DS and will be processed later.
if ( $charge ) {
	$this->process_response( $charge, $order );
}

However, in other cases, we don't do this:

$this->process_response( end( $intent->charges->data ), $order );
$this->process_response( $this->get_latest_charge_from_intent( $response ), $order );

We should be more defensive with fetching the latest charge, but we should also investigate why the intent doesn't have a latest charge.

mattallan avatar Oct 06 '24 22:10 mattallan

8834348-zd-a8c

imodouglas avatar Oct 11 '24 13:10 imodouglas

I dug into the original issue reported in 8778680-zd-a8c a bit more today and found that this specific problem was being caused by their Subscriptio extension. See more details in slack: p1729045390912989/1728030982.720319-slack-C7U3Y3VMY

This extension was incorrectly calling $payment_gateway->process_response(end($response->charges->data), $order); inside their renewal payment process handler.

Why is this an issue?

Since v8.5.0 (in PR https://github.com/woocommerce/woocommerce-gateway-stripe/pull/2980), we updated our Stripe API version from 2019-09-09 to 2024-06-20 and in Stripe API version 2022-11-15, they remove the charges property from PaymentIntent properties (docs ref):

Image

We can't do much within the Stripe extension to fix this issue for them, so this specific bug will need to fix in their extension.

One thing we can do from our side is improve our process_response() function to better handle cases where 3rd party code is sending null or an empty $response object.

When an empty/invalid charge object is passed we are incorrectly falling into this condition which is marking the order as on-hold and setting a confusing order note:

Image

Changes we could make to prevent this behaviour:

  • Validate the $response object by checking if it contains a charge ID before continuing
  • Only run the $captured logic if the $charge->captured property is set

mattallan avatar Oct 16 '24 05:10 mattallan

Also reported in 8789458-zen. Order note shows the same message then order status changed from On hold to Failed.

thuautp avatar Oct 16 '24 09:10 thuautp

Thanks @thuautp I've been able to replicate the bug that is affecting the merchant in 8789458-zen and potentially others. I've opened a separate issue for this: https://github.com/woocommerce/woocommerce-gateway-stripe/issues/3517

mattallan avatar Oct 17 '24 04:10 mattallan

Hi, This issue has gone 150 days (5 months) without any activity. This means it is time for a check-in to make sure it is still relevant. If you are still experiencing this issue with the latest version, you can help the project by responding to confirm the problem and by providing any updated reproduction steps. Thanks for helping out.

github-actions[bot] avatar Mar 17 '25 00:03 github-actions[bot]

This issue has gone 180 days (6 months) without any activity.

github-actions[bot] avatar Apr 17 '25 00:04 github-actions[bot]