Automattic / woocommerce-subscriptions-core

Subscriptions core package for WooCommerce
Other
86 stars 32 forks source link

Use try/catch when calling process_payment to avoid fatals. #341

Closed tommyshellberg closed 1 year ago

tommyshellberg commented 1 year ago

Fixes #330, please see for details.

Please also see discussion in p1670014386600359-slack-C029ME0FYRZ for how this affects processing subscription payment method changes with WooCommerce Payments in particular.

Description

See #330 for a description.

How to test this PR

Use the Change Payment Method instructions:

https://woocommerce.com/document/subscriptions/customers-view/#section-11

To simulate a failure, you could either block the network request w/ Developer Tools or use a test card that will decline. Instead of a fatal error because of a failure, you should instead see a notice with the error.

Product impact

shendy-a8c commented 1 year ago

I have tested by throwing an Exception from WCPay's process_payment() and confirm I got a fatal error with base branch and with this PR's head branch, I got the exception message displayed and not a fatal error.

I have a minor concern that the exception message got exposed and almost suggested to pass a generic error message to wc_add_notice() and log the thrown exception message but then I though with current situation, the exception message is already exposed anyway along with a fatal error, so I refrain myself from suggesting anything more.