Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

During the invoice.paid event, maybe set the cancel_at_end_of_period on the WCPay Subscription if this is the last payment #2860

Open
mattallan opened this issue Sep 7, 2021 · 1 comment
Labels
category: projects For any issues which are part of any project, including bugs, enhancements, etc. component: wcpay subscriptions Issues related to Stripe Billing Subscriptions focus: subscriptions needs feedback The issue/PR needs a response from any of the parties involved in the issue.

Comments

@mattallan
Copy link
Contributor

mattallan commented Sep 7, 2021

Description

At the moment, we handle cancelling the WCPay Subscription in two ways:

  1. When the Woo Subscription status is updated to cancelled/expired, we cancel it in Stripe
  2. During the invoice.upcoming webhook, if there's no next payment date we cancel/pause the subscription in Stripe as a last precaution

There are a few risks we're taking with this approach. Firstly, we're relying on the invoice.upcoming being handled successfully on every store, but that's not always the case (i.e. the store might be fataling at the time we need to process this incoming webhook). Stripe should retry these webhooks but the store could remain down for any amount of time.
The second risk we're taking is we're relying on the Woo subscription to be set to cancelled/expired before Stripe processes the invoice payment which is not always the case.

The main thing we want to avoid here is customers being charged by Stripe when their subscription is cancelled in Woo.

One extra thing we could be doing is during when we process the invoice.paid event, after we have called $order->payment_complete() check if there's a next payment date on the subscription.

If there's no next payment we should be able to safely mark the WCPay subscription as cancel_at_end_of_period.
This will mean no more payments will be taken and we reduce the risks from webhooks not being handled/delivered properly.

Example

I haven't reproduced this in my local environment but here's how I imagine an issue could happen:

  1. Purchase a daily subscription that expires after 3 days, i.e. $10/daily for 3 days
  2. 24hrs later - second Payment:
    1. Stripe sends an invoice.upcoming webhook event (we check if the subscription has a next payment date or isn't expiring)
    2. 1hr after that, Stripe processes the payment and sends an invoice.paid event (we create the renewal in Woo)
  3. 24hrs later - final Payment:
    1. Stripe sends an invoice.upcoming
    2. 1hr later Stripe process payment and we create the renewal order in Woo on the invoice.paid webhook.
    3. Subscription is set to expire in 24hrs in WooCommerce (but we don't tell Stripe that this is the final payment)
  4. 24hrs later - Stripe prepares another invoice
    1. Stripe sends the invoice.upcoming. This is where our code notices the subscription in Woo doesn't have a next payment date and cancel the subscription in stripe before it processes payment
    2. During the invoice.paid, if we didn't cancel the subscription in Stripe, another payment will be processed ❌

Because the Subscription in Woo is set to expire/cancel, there is a chance that the subscription status in Woo gets updated to cancelled/expired before step 4 happens, which will cancel the subscription before stripe processes an incorrect payment. With that said, because Stripe controls the renewal cycle we can't rely on the Subscription in Woo always being cancelled before Stripe.

Therefore, one solution to this problem is to address it in step 3 ii., after calling payment_completed(), check if the subscription doesn't have a next payment date and then send a request to Stripe to set the cancel_at_end_of_period param on the subscription.

@mattallan mattallan added the component: wcpay subscriptions Issues related to Stripe Billing Subscriptions label Sep 7, 2021
@a-danae a-danae self-assigned this Oct 18, 2021
@haszari
Copy link
Contributor

haszari commented Nov 1, 2021

Is this still important to improve @mattallan ?

If so, can you add info to the description to demonstrate the bug/gap in a merchant or shopper flow please. Then we can determine impact and priority.

@haszari haszari added the needs feedback The issue/PR needs a response from any of the parties involved in the issue. label Nov 8, 2021
@htdat htdat added the category: projects For any issues which are part of any project, including bugs, enhancements, etc. label Nov 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: projects For any issues which are part of any project, including bugs, enhancements, etc. component: wcpay subscriptions Issues related to Stripe Billing Subscriptions focus: subscriptions needs feedback The issue/PR needs a response from any of the parties involved in the issue.
Projects
None yet
Development

No branches or pull requests

4 participants