Pass additional data to the intent authentication exception #7873
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: 0 B Total Size: 1.27 MB ℹ️ View Unchanged
|
hsingyuc
force-pushed
the
fix/pass-additional-data-to-intent-authentication-exception
branch
from
79cf635 to
fb61488
Compare
hsingyuc
force-pushed
the
fix/pass-additional-data-to-intent-authentication-exception
branch
from
4f5510a to
bd654f7
Compare
hsingyuc
force-pushed
the
fix/pass-additional-data-to-intent-authentication-exception
branch
from
337f1f5 to
859d908
Compare
hsingyuc
force-pushed
the
fix/pass-additional-data-to-intent-authentication-exception
branch
from
c416d1d to
e70d499
Compare
There was a problem hiding this comment.
@hsingyuc I haven't yet tested the changes, but I have a couple of questions that I wanted to ask.
-
I see a few other instances of the
We're not able to process this payment. Please try again later.exception in theupdate_order_statusfunction. Do we know if these can be reached in the WooPay flow? If yes, should they be updated as well? -
As far as I can see, WooPay doesn't display this error to the shopper. What do you think about changing the error message to better indicate the error and include the order_id and meta_order_id in the message itself? This would make it easier to get this onto production as we wouldn't need the WC dependency.
I did process of elimination, we are not going through the other exception while testing this error. Let me know if you found an edge case otherwise I think we don't need to worry about it.
I agree with more clear error message but we do display the message to the shopper on the WooPay checkout page(See below). Since you mentioned, it seems weird we show this message to shoppers.
|
…tication-exception
@hsingyuc Ah thanks, you're right. WDYT of removing the WCPay error message from WooPay and showing only the generic "Merchant store order creation failed" part. If the merchant order fails, I don't think there'll be any actionable input for the shopper in the complete error message. We will still log the complete error in WooPay logs. I am suggesting this because, this way, we can include |
For a quick fix, that's a good approach. Do we need to get any input from designer, @pierorocca? |
|
@malithsen After thinking about it more, does that mean we also change all the error messages to a generic message like |
…tication-exception
Nonce is invalid is not actionable to a shopper. "Coupon usage limit..." can be useful though.
Edit: We seem to have failed orders that go against my initial hypothesis that they are failing because the woopay order id is equal to the merchant order id. So adding logs closer to the check would still be useful. P2: pdpOw2-45Y-p2#comment-3865 |
|
If I'm following along correctly there seem to be different categories of errors - some not actionable, some too obscure for shopper comprehension, some that would be useful. Could you please summarize all the use cases that need feedback? |
That's what I was initially thinking but to avoid a review of error messages that would increase the scope of this PR, perhaps we can handle the |
That's correct. We have three types of error messages below showing to shoppers. Initially, Malith mentioned updating the three messages below to one generic message and showing it to shoppers. But now we might not do that because
|
Are we confirming doing it this way? Do you think we should do both or close this PR? If we confirm doing this way, I can open PRs. |
@hsingyuc If you don't have any concerns about the new approach, I think we should do that as it would allow us to get these logging out faster. |
|
PR created #7951 |
|
Thanks @hsingyuc and @malithsen. Besides preventing errors as the first line of defense:
@gpressutto5 recently updated many notifications to a new component. That work should be a good inventory of existing errors and error messages. Would there be value in taking that inventory of those error codes and come up with a revised list of front-end error messages? |
|
Close the PR because this PR fixed the issue. |
Slack discussion p1702070481324349/1702053239.912509-slack-C022WMN88KG
Currently, we only throw an exception message
We're not able to process this payment. Please try again later.and don't have further information for debugging.Changes proposed in this Pull Request
Throw additional data
intent_meta_dataandorder_idto the intent authentication exception so we have more information for the failed order.Testing instructions
trueerror_log(print_r($e,true));behind the catch to see the additional dataIf test with WooPay
See the test instruction https://github.com/Automattic/woopay/pull/2394
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