strangerstudios / paid-memberships-pro

The Trusted Membership Platform That Grows with You: Restrict access to content and charge recurring subscriptions using Stripe, PayPal, and more. Fully open source. 100% GPL.
https://www.paidmembershipspro.com
Other
468 stars 360 forks source link

PayPal Express: duplicate API requests to PayPal #1802

Closed mircobabini closed 3 years ago

mircobabini commented 3 years ago

I get "A successful Billing Agreement has already been created for this token" on first ?review=... request, from PayPal Express.

Check below the full investigation process.

mircobabini commented 3 years ago

This is what I get:

[L_SHORTMESSAGE0] => Transaction refused because of an invalid argument. See additional error messages for details.
[L_LONGMESSAGE0] => A successful Billing Agreement has already been created for this token.
[L_SEVERITYCODE0] => Error

Still can't say why. Probably the user reloaded the "?review=..." page because it was taking too long.

This is happening with logged in users as well.

mircobabini commented 3 years ago

I think this is the point:

The page ?review=... is loaded, the Billing Agreement is created, but the CreateRecurringPaymentsProfile request fails (ACK != SUCCESS && ACK != SUCCESSWITHWARNING):

Then this is triggered: https://github.com/strangerstudios/paid-memberships-pro/blob/dev/classes/gateways/class.pmprogateway_paypalexpress.php#L789-L798

Sad story short:

In fact it triggers PayPal error code 11456.

Probably don't worth to find a solution... better move on and switch to PayPal Checkout soon.

mircobabini commented 3 years ago

Can't say for sure the above is correct yet (still investigating); but I've also found that the fatal error is thrown when a user is consuming a gift ($morder = ..., not empty, but not even an order... ??).

Btw we should check if $pmpro_requirebilling here https://github.com/strangerstudios/paid-memberships-pro/blob/dev/preheaders/checkout.php#L805-L816. If not, it's a free order. Doesn't have to ->cancel().

mircobabini commented 3 years ago

Super weird here: I've logged everything and no, the CreateRecurringPaymentsProfile is just called once. The first time it's called I get back the error code 11456.

Doesn't make any sense to me, just opened an issue on PayPal Message Center.

ideadude commented 3 years ago

We should handle this use case better, but am I correct that it seems to be happening only when user creation fails?

This might be because of a security plugin or something hooking into user_register and not allowing us to register the user in code.

Or in the past, it happened because our code to check if the username/email was available somehow didn't check well enough. When it came time to create the user, WP complained about the username or email/etc.

mircobabini commented 3 years ago

I've had issues with BlogVault in the past but I can confirm my patch is still alive (unhook their loginInit checks during checkout).

With all the logs I've enabled I would expect to have at least something logged since seems that two requests are thrown. PayPal just replied to my case and they can confirm the request is thrown twice (twice in just 1 second), I really can't say why I'm unable to see it in the logs.

The current setup is:

Not a super easy setup to debug issues but we can't live without it; the traffic is huge.

Just a note for the future: we can use idempotency on requests which support it, adding just 1 header: PayPal-Request-Id.

...but am I correct that it seems to be happening only when user creation fails?

Nope. I have had many cases:

Btw 80% of the orders are from returning users, not so helpful then. Also note that in my case I've disabled the username field during checkout; it's automatically populated by the email address (not changed at all, just identical to the email address).

I don't think this has to do with our checks, but can't exclude a security measure from some other component yet. The only thing I've noticed just last night is that the returning users having issues were old ones... they were still having user_login != user_email.

I've forced user_login = user_email update this morning; since that moment I've had no issues anymore.

If the next coming issues are only on new users, this would be a +1 for "it seems to be happening only when user creation fails".

mircobabini commented 3 years ago

Update:

Just found one interesting log related to one of these: [05-Oct-2021 08:20:01 UTC] YVwKrBRax@oYV1WFsO-kuQAAAA8: PPHttpPost: CreateRecurringPaymentsProfile methodName_ failed: cURL error 61: Unrecognized content encoding type. libcurl understands deflate, gzip, br content encodings.

This is for the logged in user. Sent over to PayPal Seller support to see what they think about it. I've logged a few other similar error on other PPHttpPost cURL request: usually re-running the cURL request works... which is weird.

mircobabini commented 3 years ago

After a few more hours... I was googling about that weird cURL error 61 and rethinking about this:

Of course I've checked what I've done on 10-15 Aug first. 1) updated some plugins => BlogVault is one of these 2) wp-rocket had broken my site without a specific reason... Siteground support told me it was because of an incompatibility with their managed "PHP Ultrafast" => they suggested to switch back to normal PHP 7.4 3) then, switched to PHP 7.4

I've just switched back to PHP Ultrafast now; I don't really know what's different here but that cURL issue must be related to PHP somehow... at least based on my experience.

If the issue persists, I will disable BlogVault for a while...

🤮

mircobabini commented 3 years ago

Nope, php was not the cause. Just turned off BlogVault, let's see.

mircobabini commented 3 years ago

It's not BlogVault as well. Damn it!

Just got a reply from PayPal Seller Support:

The duplicate error does not occur on every CreateRecurringPayment request, but 10% of the total request experienced this issue. Like you explained, there might be an asynchronous call lacking await or cancelation token in your integration, causing the duplicate request. Adding the PayPal request Id to the header of the request will work for the API. The reason it is not outlined in the link provided is that this integration is part of our legacy products and the PayPal request Id is an enhancement to resolve duplicate API requests for recent integration. The content-type error would not relate to the duplicate request issue as we will not accept an invalid request on our end.

I think this issues is not useful anymore, gonna investigate with Siteground now.

There are just two things tracked down here on which we can work on:

I will investigate these further later on and eventually open new issues if needed.