Add specific type and min supported amount in error response for the payments below min supported amount #10112
Conversation
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
|
Size Change: +1.12 kB (0%) Total Size: 1.36 MB
ℹ️ View Unchanged
|
…rted-amount-in-error-response-for-the-payments-below-min-supported-amount
mgascam
changed the title
…rted-amount-in-error-response-for-the-payments-below-min-supported-amount
…rted-amount-in-error-response-for-the-payments-below-min-supported-amount
| const errorMessages: Record< string, string > = { | ||
| const getAmountTooSmallError = ( error: WCPayError ): string => { | ||
| const currency = | ||
| error.data?.extra_details?.minimum_amount_currency ?? 'USD'; |
There was a problem hiding this comment.
I feel rather than falling back to these defaults, maybe we could omit the message in case the server doesn't provide the extra_details. Or do we see any benefits of this approach?
There was a problem hiding this comment.
Good point. Instead of using defaults, I'm falling back to this message 'The payment amount is too small to be processed.' when the server doesn't provide the extra_details 7cf9528. WDYT?
| return sprintf( | ||
| /* translators: %1$s: minimum amount, %2$s: currency code */ | ||
| __( | ||
| 'The minimum amount to capture is %1$s %2$s.', |
There was a problem hiding this comment.
capture seems like an internal term, to the end shopper, we shouldn't expose this directly IMO. 🤔
There was a problem hiding this comment.
You're right—capture does feel like an internal term. I've switched to using process in this commit 01795d0.
| return new WP_Error( | ||
| 'wcpay_capture_error', | ||
| 'amount_too_small' === $error_code ? 'wcpay_capture_error_amount_too_small' : 'wcpay_capture_error', |
There was a problem hiding this comment.
Not sure how I feel about having a separate error code just for this scenario, maybe there can be some other meta field that indicates this to the clients and the error code remains the same for all? Open to thoughts here since I don't have a strong opinion either ways.
There was a problem hiding this comment.
maybe there can be some other meta field that indicates this to the clients and the error code remains the same for all?
This is a valid point @deepakpathania . Instead of creating a new error code, we could use metadata to handle this specific case:
{
"code": "wcpay_capture_error",
"message": "Payment capture failed to complete with the following message: Minimum required amount was not reached. The minimum amount to capture is 0.5 USD.",
"data": {
"status": 400,
"error_type": "amount_too_small",
"extra_details": {
"minimum_amount": 50,
"minimum_amount_currency": "usd"
}
}
}While I implemented the specific wcpay_capture_error_amount_too_small code as requested in the issue, I like your suggestion of using metadata. It would reduce the number of error codes we need to maintain and provide more flexibility for future error types.
Before making this change though, I'd like to check with @kidinov (who reported the issue) to ensure this approach would meet their needs.
There was a problem hiding this comment.
@mgascam @deepakpathania 👋 Thanks for working on that!
I am still validating this with the iOS folks so that I will give a definite answer on Monday.
A quick question here. minimum_amount are those cents? What will it be in other currency subunit systems, e.g., when there are no subdivisions or it's 1/1000 currency?
Also, minimum_amount_currency is this ISO 4217 or something else? It looks like that, but usually, it's in upper case, not lower case therefore confirming
There was a problem hiding this comment.
Thank you @kidinov for your quick response!
I am still validating this with the iOS folks so that I will give a definite answer on Monday.
Great 👍 Looking forward to the confirmation.
minimum_amount are those cents? What will it be in other currency subunit systems, e.g., when there are no subdivisions or it's 1/1000 currency?
Yes, the amounts are in the currency's minor unit, following Stripe's convention. For example, in USD, the minimum_amount is expressed in cents, so it should be converted to the main unit (by dividing by 100) if needed.
For currencies with no subdivisions, such as JPY, the amount is already expressed in the main unit, so no conversion is necessary. Regarding currencies with a 1/1000 subdivision, I don't believe any are currently supported, but please correct me if I'm mistaken.
Additionally, if it simplifies your workflow, we can provide the amounts in the currency's main unit to avoid conversion on your end. We have a utility function, WC_Payments_Utils::interpret_stripe_amount, that handles this. Just let me know your preference.
Also, minimum_amount_currency is this ISO 4217 or something else? It looks like that, but usually, it's in upper case, not lower case therefore confirming
Yes, the value follows ISO 4217, and you're right—it should be in uppercase. I'll ensure the necessary updates are made to include this.
There was a problem hiding this comment.
Thanks for the clarification!
iOS folks confirmed that the API is fine
The only point that came up is that the same changes needed to be done for the Stripe plugin; otherwise, it'll be a regression
There was a problem hiding this comment.
Thanks for sharing more context @kidinov. Let me clarify a few things, based on my own understanding:
-
You're right that both WooPayments and Stripe plugins need to handle this case consistently for the best app experience. This change is specifically for WooPayments, and a similar change would need to be made in the Stripe plugin (woocommerce-gateway-stripe).
-
Regarding potential regressions:
- This change adds additional error details (minimum_amount and currency) while maintaining backwards compatibility
- Older versions of WooPayments will continue to return the basic error response. So I'd suggest that
the apps can implement progressive enhancement - use the additional details when available, fall back to basic error handling when not. Please correct me if I'm wrong, but this aproach ensures no regression for stores using either plugin or different versions. WDYT?
There was a problem hiding this comment.
That's slightly more complicated than that, unfortunately. Android app was handling it using an error that comes from Stripe's SDK processing payment step until that was broken by a change, when for some reason for Woo payments account:
we enabled a special gate on the WooPayments account that would allow for very small charges under the 50cent limit documented
p1736206944981919/1735228071.598689-slack-C01G168NFC2
And the error started to come without any details from the "capture" step. So the current state of the android app is that the error handled in a general way, and users don't have a clear picture why the error happens. Missing implementation of this error in the Stripe plugin won't cause a regression, it will be just as right now for the Stripe plugin users.
iOS app handled it differently. They hardcoded the values from different countries and checked them locally. So, for them, it will be a regression, as they will have to remove their locally built logic for this case and switch to the handling of the error based on the error provided by the backend. So for them, missing change in Stripe plugin will be regression
In any case, if the change for stripe will be released, ideally at a similar timing with WooPayments, this will be ideal 🙏
There was a problem hiding this comment.
@kidinov thanks for the additional context. I created woocommerce/woocommerce-gateway-stripe#3728 to tackle the Stripe plugin change. I'll try to apply the same changes we have in this PR and request a review of the team that maintains the Stripe plugin. EDIT: The codebases are somewhat similar, which initially led me to overestimate how quickly I could introduce the changes. After taking a closer look, I believe it would be better for someone more familiar with the codebase to implement the changes.
About the release of the changes, I'd say we should first release the change in the Stripe plugin, to avoid the iOS app regression. What do you think? But one thing is not clear: how this change would affect previous versions of the mobile apps? Would they break?
cc: @deepakpathania
There was a problem hiding this comment.
What do you think? But one thing is not clear: how this change would affect previous versions of the mobile apps? Would they break?
The old versions will behave the same way they behave now, as the API change is not breaking
The issue lies with the new versions of the iOS app, which will expect the changed API but lack client-side handling for the case. If they operate against Stripe or WooPayments, which have not yet implemented the change, so as a result, users will only see a generic error. We will make the necessary changes to the iOS app with some delay to minimize the number of instances like this. Considering that the issue is relatively minor and will eventually resolve with updates to the plugins, I believe this approach is acceptable
There was a problem hiding this comment.
The stripe gateway isn't managed by us directly so someone from @Automattic/quark would have to port these changes to that plugin as well in case there is preference to release them at the same time.
However I do agree that there should be a progressive approach followed by apps in case this additional information is provided from our end since the change doesn't break backward compatibility.
…rted-amount-in-error-response-for-the-payments-below-min-supported-amount
…rted-amount-in-error-response-for-the-payments-below-min-supported-amount
…rted-amount-in-error-response-for-the-payments-below-min-supported-amount
…rted-amount-in-error-response-for-the-payments-below-min-supported-amount
…rted-amount-in-error-response-for-the-payments-below-min-supported-amount
…rted-amount-in-error-response-for-the-payments-below-min-supported-amount
…rted-amount-in-error-response-for-the-payments-below-min-supported-amount
…rted-amount-in-error-response-for-the-payments-below-min-supported-amount
…rted-amount-in-error-response-for-the-payments-below-min-supported-amount
…rted-amount-in-error-response-for-the-payments-below-min-supported-amount
…rted-amount-in-error-response-for-the-payments-below-min-supported-amount
Fixes #10092
Changes proposed in this Pull Request
This pull request enhances error handling for payment captures in the WooCommerce Payments plugin. The primary changes include adding support for a new error type related to capturing amounts that are too small, updating the associated error messages, and adding relevant tests.
Key changes:
Error Handling Enhancements
wcpay_capture_error_amount_too_smallfor cases where the capture amount is too small.getErrorMessagefunction to handle the new error type and format the error message with the minimum amount and currency.Documentation Updates
These changes provide clearer error messages and ensure that the system handles edge cases more gracefully.
Testing instructions
Pre-requisites
Capturing an authorization
Capturing a terminal payment
npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge