bitpay / bitpay-checkout-for-woocommerce

BitPay Checkout for WooCommerce
MIT License
14 stars 22 forks source link

Two BitPay invoices associated to one Woocommerce order, causing incorrect order cancellation #81

Closed shaunek-hero closed 3 months ago

shaunek-hero commented 4 months ago

Hi BitPay devs, we have had this plugin turned on in prod for a few weeks and we have been getting a successful orders 👍 But we found one odd case that I wanted to report as I think this would be considered a bug. I'm going to describe two separate issues relating to this order, the second being the most severe in my opinion.

  1. We got a new Woo order a few days ago and the associated BitPay invoice was KahkySByARKYCv4GQP6kiB. I have viewed the logs and I can see that our server relatively quickly got IPN messages for paid, confirmed, and complete which is great. However, as you can see from the order note history on this order (screenshot below) there is an order note for when we got "paid" and "complete" but not for "confirmed." This is an issue for us because we have the plugin settings configured to change the order status when we get the "confirmed" IPN message - and in this case it didn't change the order status as we wanted it to. I've also checked my wp-content/uploads/wc-logs/fatal-errors-2024-03-12-ce767fecd51525d2cc90aee13a0f1e2b.log file to make sure that Wordpress didn't have any fatal errors at that time, and I don't see anything.
  2. Another issue with this same order is that a second BitPay invoice was associated, AxWG4QnduzcaoyqjmwieiY. I can see in the logs that this second invoice was created AFTER the final IPN message for KahkySByARKYCv4GQP6kiB was received (ie, the "complete" message). It is a bit of a mystery to me how this second invoice was created in the first place. The real damage this caused was that this second invoice expired a while later and then our server got the IPN for expired and this caused the Woo order to get cancelled. This ideally wouldn't have happened.

Screenshot 2024-03-14 at 19 06 18

I have the log file you can review, but I'm a bit hesitant to share here on public Github since I think there might be a secret that is logged in these log files (specifically the "token" field on the "NEW BITPAY INVOICE" log message). If you want to view the log file let me know how I can securely send to you.

We are using version 5.3.2 of the plugin. PHP 8.2.16. Wordpress 6.4.3. Woocommerce 8.6.1.

bobbrodie commented 4 months ago

Hey @shaunek-hero thanks for reporting -- we'll take a look at this and keep you posted. It would be helpful to have the logs, so I'll follow up with you tomorrow with more information about that after I fill a few folks in on this. Thanks again!

bobbrodie commented 4 months ago

Hi @shaunek-hero! When you have a moment, could you drop your logs over to integrations@bitpay.com? We'll collaborate and look at two things to check the originating reason for duplicate invoice and can work with you on action from there.

One question I have mean time is if could you check the _bitpay_checkout_transactions table to see if you have multiple entries for the same order_id matching this order and let us know?

shaunek-hero commented 4 months ago

Thanks @bobbrodie, I sent an email to integrations@bitpay.com with the log file. I also queried the table you requested with this query:

select * from _bitpay_checkout_transactions where order_id = 142590;

Results:

29  142590  KahkySByARKYCv4GQP6kiB  complete    2024-03-12 18:54:48
30  142590  AxWG4QnduzcaoyqjmwieiY  expired 2024-03-12 20:31:06

Out of curiosity I ran this query to see if this has happened for other orders:

select * from _bitpay_checkout_transactions where order_id in (
  select order_id from _bitpay_checkout_transactions group by order_id having count(*) > 1
);

Results:

9   141615  H6CYmshEKoNCXn2MF7TQ4w  expired 2024-03-02 03:21:20
10  141615  EaLUrhiyvSDbCWbbmC1KRp  expired 2024-03-02 03:26:07
11  141615  YZo7VRW9Lpar9fmQ7FAwgv  expired 2024-03-02 03:27:13
16  141717  Y6ek5vvzXG1yrTibkz2aqo  expired 2024-03-03 13:35:39
17  141717  DnB57Q8T8K41hPUNmF1jUn  expired 2024-03-03 13:39:41
27  142510  XhdCABdSbBVE4An6DTjjys  expired 2024-03-11 23:04:26
28  142510  D8SfmqJ5qrt9ZUpBEBDMos  expired 2024-03-11 23:08:52
29  142590  KahkySByARKYCv4GQP6kiB  complete    2024-03-12 18:54:48
30  142590  AxWG4QnduzcaoyqjmwieiY  expired 2024-03-12 20:31:06

So besides this order I've reported (order id 142590) there were three other cases where multiple invoices were created, but all of those invoices weren't paid, so in the end no harm done to us. I don't know your plugin well enough to know for sure but I was somewhat surprised that there were multiple invoices for these orders, my initial thought is that the plugin should try to prevent that from happening. I unfortunately don't have any solid ideas as to how this could have happened but my gut tells me the user/customer might be doing some special sequence of clicks, maybe with the back button (???).

Please let me know if I can provide any additional information to help you. For example, if you want additional log files I can send them along, just let me know.

bobbrodie commented 4 months ago

Thanks so much @shaunek-hero -- we're looking into it. When you choose to pay with BitPay, the order is created and marked pending payment. An invoice is created at BitPay (because it's technically an offline payment, unlike a credit card).

We have run a few tests and found that if you go back to your WooCommerce cart, make no changes, go to checkout, and place the order again, WooCommerce does not create a new order, but rather re-initiates the payment cycle for the existing order, adding an additional record to the transactions table.

In some ways, WooCommerce not creating a new order makes sense, but in a timed offline payment with an expiration window, it can cause issues.

Note: If you add or remove items from your cart, WooCommerce will create a new order instead.

We've got two thoughts on this.

Idea 1 - Simple We think the simplest solution here is to clear the cart when the order is placed. We expect this to have the least amount of friction and implementation time.

Idea 2 - Complex This is a little more of a stretch, but does come with the convenience of having the ability to easily re-initiate the payment flow, although I'm not sure that's needed. I think we could do a feature enhancement for this later, where maybe we show a link in the user's order list based on status == wc-payment and transaction status != expired.

WooCommerce Order Flow drawio

We'd love to hear your thoughts on this, and are feel like idea 1 is the best solution for now.

shaunek-hero commented 4 months ago

Thanks for asking my opinion on this and for detailing out how to repro this. I've learned something new as well, interesting how Woo handles the case of going back after the order is created.

In my opinion both solutions you have outlined are viable. The first idea of clearing the cart makes a lot of sense to me and honestly I think it is probably slightly superior even though it is so simple. Also because it is so simple. My primary rationale is based on the fact that, as you stated, this BitPay payment is very similar to an offline payment, and the behavior of Woocommerce's built-in offline payments is to clear cart when the order is created. Try out the "Check payments" (cheque) and "Direct bank transfer" (bacs) payment methods in the checkout and then hit the back button and you'll confirm that the cart is empty. Once you place an order it is somewhat understood that your cart will be cleared. That being said I don't have a lot of experience with other payment methods like this, such as Paypal, etc, maybe other 3rd party payments don't clear the cart?

The other more complicated option could certainly work as well. It is nice that it could handle this case where customers hit the back button and checks out again, but as a Woocomerce merchant I wouldn't really expect this BitPay payment method to do what you diagramed out in idea 2. If you implemented this and I found out about it I would be surprised. I would be happy you handled it. I do worry it might be more complicated than necessary though.

One small detail that might be coloring my preference is that we did do small a customization to the Customer Pending email that is delivered to the customer when the order is placed. We've inserted link to the invoice right there in the email, the reasoning we had was "what if a customer accidentally closes their browser or otherwise have some sort of brief internet outage as they are redirected to the BitPay invoice - in this rare case it might be helpful to them to have the link in the email instead of them having no way to access the invoice". Whether or not our customers will use this is not known to us, I'm just pointing out because if the cart gets cleared when the order is placed, our customer still have a way to get to the invoice in their email regardless. I believe we implemented this by grabbing the _transaction_id entry from the postmeta table and the HTML we add to the email looks something like this:

<strong>If you haven't already completed the cryptocurrency transaction online, please pay the invoice <a href="https://test.bitpay.com/invoice?id={bitpay_transaction_id}&v=4">here</a>.</strong>

The invoice is valid for 15 minutes only. You will be notified by email once we have received your crypto funds.

I know your plugin already uses the woocommerce_email_before_order_table hook to send some info via email, but we didn't really like how that displayed on all the email so we overrode/suppressed that callback from being invoked (via remove_action() call) and replaced it with the above bit (only for the first email).

Sorry, I've rambled on a bit, but the summary is I think we would be happy with either solution, with a slight preference for idea 1.

bobbrodie commented 4 months ago

Hey @shaunek-hero we've got some updates for this in the master branch which is where 5.4.0 sits. We've tested 5.4.0 in the test environment with dozens of invoices and scenarios. We have a few options here that I wanted to run by you:

  1. If you want to give 5.4.0 a spin, you could download master, unpack it, remove -master from the directory name, and run composer install in the plugin directory (PHP 8 or higher), and drop it into your store
  2. We can prep a zip for you if you let us know which version of PHP you're on
  3. If you prefer to keep 5.3.x, we could check out the 5.3.2 tag, push a 5.3.3 branch, tag it, and push to the marketplace

We plan on releasing 5.4.0 early next week, but are happy to do this today for you to help in any way we can. You've been an awesome contributor and we love the engagement.

Thanks!

shaunek-hero commented 4 months ago

@bobbrodie Great news. Thanks for reaching out.

I would love to give it a test, and option 1 is probably what I'll go with for testing out the new code. Only issue I have is that I probably won't be able to get to it until sometime next week, maybe after you publish the new version in the Wordpress repository. So if you were hoping I could test it out before you release I won't be able to do that. But great you guys have done some extensive tests.

For the very short term we'll plan on not upgrading when you publish since we did apply a couple minor patches to 5.3.2. But since I want to keep current with your changes I'll grab from your github master branch and then if everything is perfect I'll consider updating my prod store with the 5.4 version you publish. But if I need more changes I might consider applying my changes in a branch on my fork, and deploying with that code.

shaunek-hero commented 3 months ago

Oh my, my last reply I said I would do some testing and provide some feedback which I forgot to do (ended up being a wild week, so fell off my list of priorities)... let me see if I can do that by tomorrow. I noticed you guys haven't published version 5.4.0 to the Wordpress repository yet, were you waiting on my feedback before you published? If so my appologies!

bobbrodie commented 3 months ago

@shaunek-hero no worries at all! We've been doing a lot of testing on our side as well, we're planning to release on Monday to the marketplace. If you do use it from GitHub, just remember to run composer install since we don't keep the vendor dir in the repo but do distribute it with the plugin on the marketplace :)