Cashier-stripe: New swapAndInvoice() always set to past_due for SCA cards

[ipalaus]

• Cashier Version: 10.1.1
• Laravel Version: 5.8.19

Description:

Our application has a "switch plan" feature which we used swap() to charge the user and change their plan. With the new Cashier version, we need to use swapAndInvoice() in order to change the plan and also collect the money. While the documentation suggests that we catch the exception and redirect the user, this doesn't happen with any of Stripe's testing cards and all subscriptions are marked as past_due status.

Steps To Reproduce:

1. Subscribe a user to a Stripe plan with any of the testing cards: 4000002500003155, 4000002760003184, 4000003800000446.
2. Swap the user to a new plan using swapAndInvoice().

All testing cards will fail at https://github.com/laravel/cashier/blob/10.0/src/Billable.php#L234 and return false. The error Stripe returns is Nothing to invoice for subscription which is weird because I see that Stripe is creating an invoice and the payments need confirmation:

This will also leave your subscriptions in the database in a past_due statues:

... which means that the user doesn't have a valid subscription anymore, even that he paid for another plan.

Knowing that the payment is missing a verification, I'd have expected that Cashier would have thrown the exception it suggest on the documentation in order to show the confirmation page. I've dig through the code and haven't found a culprit yet.

That said, I've just tested activating the cashier.payment_notification to sent out a notification when a payment is required. The notification is sent over email, with the correct link to be clicked and confirm the payment. Confirming the payment works, and it gets marked as paid in the Stripe dashboard and sync in the database. I still think we should be redirecting if we know a payment is required, and also we should put a subscription on past_due making it a non-active subscription and loosing access even when the user is actually subscribed to a valid plan.

[driesvints]

I'm not sure what's going wrong with your app since we have specific tests for this case here:

https://github.com/laravel/cashier/blob/10.0/tests/Integration/SubscriptionsTest.php#L239-L289

These verify that the correct exception is thrown.

[ipalaus]

I think the test you're pointing out is testing something different. I'm using a card that requires SCA, the card is valid. I think it should be tested with one of the pm_card_authenticationRequired... tokens as it's a different case?

[driesvints]

@ipalaus seems like Stripe added some new cards in the meantime. These weren't there a month ago. I'll check in on this.

[ipalaus]

@driesvints I gave it some more thought and are mainly 2 issues here:

1. The exception wasn't thrown thus the user didn't get redirected to the payment page and has to wait for an email to complete the process.

2. A user that has a monthly plan (and let's say, it's on day 10) and decides to switch to a yearly subscription and it's payment fails, the subscription will be marked as past_due. Will this is not strictly related to how you decided to handle #768 (and I agree with the behavior you described there)... I still think the user should have an active subscription for what he paid for (the month), with our without a past_due invoice.

[driesvints]

I still think the user should have an active subscription for what he paid for (the month), with our without a past_due invoice.

I don't agree with this because technically they're already on a different plan which could offer different features. If you haven't fulfilled that payment then you shouldn't be able to use that plan. But like I said: opinions can greatly differ on this and it's subjective at least.

[driesvints]

I'll need some more time to think this over on what the best solution is. I can't promise anything right now in terms of when that'll be but I'll try to look into this soon.

[netpok]

I believe it's the correct behavior, because the subscription became past_due.

If it stays active and let's say the swap happened in the start of the cycle, then suddenly the subscription will be cancelled in the middle of a cycle. By setting it past due the application has a chance of notifying the user about it.

[driesvints]

I've been looking into this again and I can reproduce this. The reason why the exception isn't thrown is because Stripe apparently can't invoice the user for the subscription right after. It gives me the following error back:

{
  "error": {
    "code": "invoice_no_subscription_line_items",
    "doc_url": "https://stripe.com/docs/error-codes/invoice-no-subscription-line-items",
    "message": "Nothing to invoice for subscription",
    "param": "subscription",
    "type": "invalid_request_error"
  }
}

I've sent a message to support asking why this is because the scenario that I'm following to invoice the user right after the subscription change is described in their API docs here: https://stripe.com/docs/api/subscriptions/update

If you'd like to charge for an upgrade immediately, just pass prorate as true (as usual), and then invoice the customer as soon as you make the subscription change. That will collect the proration adjustments into a new invoice, and Stripe will automatically attempt to collect payment on the invoice.

I'll keep you updated.

[driesvints]

Stripe got back to me:

Looks like an invoice was created immediately when you passed trial_end: now!

Which sort of makes sense. For this case (and I also guess for yours @ipalaus) I had trial days set on the plans in Stripe. But because I pass in "now" for the trial_end parameter it doesn't enters the trial period of the plan and immediately starts the subscription and thus a new cycle and invoice is created immediately. Therefor the invoice call in the swapAndInvoice method is redundant and it's normal that we get the error from above.

I'm trying to see what the best way is to fix this and I already have an idea but I'm atm a bit stuck trying to recreate this with a test. I've written the test below (for the following test file: https://github.com/laravel/cashier/blob/10.0/tests/Integration/SubscriptionsTest.php) but it passes atm. It seems to me that this should exactly recreate the failure. It would atm, need to fail, saying that the expected exception wasn't thrown.

    public function test_required_sca_auth_with_no_trial_during_plan_swap_results_in_an_exception()
    {
        $user = $this->createCustomer('test_required_sca_auth_with_no_trial_during_plan_swap_results_in_an_exception');

        $subscription = $user->newSubscription('main', static::$planId)->create('pm_card_visa');

        // Set a card that requires a next action as the customer's default payment method.
        $user->updateDefaultPaymentMethod('pm_card_authenticationRequired');

        try {
            // Attempt to swap and pay with the SCA card.
            $subscription = $subscription->swapAndInvoice(static::$premiumPlanId);

            $this->fail('Expected exception '.PaymentActionRequired::class.' was not thrown.');
        } catch (PaymentActionRequired $e) {
            // Assert that the payment needs an extra action.
            $this->assertTrue($e->payment->requiresAction());

            // Assert that the plan was swapped anyway.
            $this->assertEquals(static::$premiumPlanId, $subscription->refresh()->stripe_plan);

            // Assert subscription is past due.
            $this->assertTrue($subscription->pastDue());
        }
    }

I'll need to figure out first how to reproduce the problem with a test before I can fix it.

[ipalaus]

Thank you for looking into this!

When you say you had trial days set in the Stripe plan, you mean when you created the plan? In my case, all the plans I created have 0 trial days. I don't think that makes a difference.

In order to fix our particular case and offer a switching plan option for users, this is the code I had to come up with. It's not ideal the extra call to get the latest payment again as it does add some extra time to the request, but at least I was able to get it working for our customers.

try {
    $subscription = \Auth::user()->subscription();
    $subscription->swapAndInvoice($plan->id, ['off_session' => true]);
} catch (IncompletePayment $exception) {
    // Never thrown
    $redirect = route('cashier.payment', [$exception->payment->id, 'redirect' => route('settings.subscription')]);

    return response()->json(['status' => 'requires_action', 'location' => $redirect]);
} catch (SubscriptionUpdateFailure $exception) {
    throw $exception;
}

$payment = $subscription->latestPayment();

if ($payment->isSucceeded()) {
    return response()->json(['status' => 'success', 'newPlan' => $request->get('plan')]);
} elseif ($payment->requiresAction()) {
    $redirect = route('cashier.payment', [$payment->id, 'redirect' => route('settings.subscription')]);

    return response()->json(['status' => 'requires_action', 'location' => $redirect]);
} elseif ($payment->requiresPaymentMethod()) {
    return response()->json(['status' => 'requires_payment_method']);
}

throw new \Exception("Couldn't complete the payment correctly: {$payment->id}");

[bayview]

Thanks ipalaus for posting this work-around.
It's not as pretty as the version that catches the IncompletePayment exception due to the extra screen where the user has to retype the card number, but it works for now.

Much appreciated.
Hopefully a future version of Cashier will get the exception working again.

[driesvints]

I still plan on taking a new look at this soon. Currently working on Tax Rates for Cashier v11. I'll try to get this one tackled as well in the upcoming month.

[html5maker]

This is a real problem for upgrading a subscription with SCA.
For example user is on basic plan and he decided to upgrade to next plan.
SwapAndInvoice will modify the subscription and make the subscription status past_due.
BUT how is it possible to rollback to basic if user wasn't able to pay for upgrade?

[driesvints]

@html5maker I realize that this is a problem but like I said I'm totally stuck on this. I need to have the above test pass before I can implement a fix.

[html5maker]

You should pass "payment_behavior":"error_if_incomplete" option to fix the test:
https://stripe.com/docs/api/subscriptions/update

[driesvints]

Ah that might be it. I'll try to look at that later on. Thanks

[html5maker]

Contacted Stripe about this issue. Here is their reply:

I would like to take a moment to thank you for reaching out to us about the issue you've encountered with subscription behaviour when an upgrade fails.

I looked into things and you're right that feature was lost but it is in the pipeline to come back.

As things stand at the moment upgrading a subscription creates a new subscription on our end and when that fails it won't automatically revert to a previous state.

I don't have a time frame for the update to the Billing system to give you at this stage, but it'll come through on our monthly billing updates and will be mentioned on the Stripe blog and the developers digest once its fixed.

[driesvints]

@html5maker thanks for that. I'm gonna park this for now then until that happened.

[driesvints]

@html5maker did you ever hear back about this?

[driesvints]

Can anyone here please try out v12 to see if the problem still persists? I have a feeling this is resolved in the latest release.

[bayview]

I upgraded to v12 and tried to test my code as is. It still redirected to an intermediate screen, and also sent an email to the user that more info was needed. (It wasn't because of the intermediate screen.)

However, it will take me a little time to get my head around the issue again. I will do so in the next few days and give you a real answer of whether it is solved.

Thanks for your efforts.

[bayview]

OK, I was testing the wrong thing.

The swapAndInvoice issue with Secure 3D cards is indeed fixed with Cashier v12.
The IncompletePayment exception is triggered now exactly as it should be, allowing us to redirect the user to cashier.payment.

Thanks again!

[driesvints]

Thanks I'm gonna close this then unless there are other objections.