Fix Stripe duplicate sub canceling.

[ro-tex]

PULL REQUEST

Overview

This PR fixes a problem with our ability to cancel subscriptions for users. We want to be able to do that, so if a user manages to create two subscriptions in parallel, we can cancel the earlier one in order to not charge them twice.
This code has been in there forever but apparently didn't work properly. It's an old peculiarity of Stripe's service.

The PR is a bit longer than it needs to because touching this part of the code allowed me to clean it up a little bit. I'll mark the relevant section.

Example for Visual Changes

Checklist

• All git commits are signed. (REQUIRED)
• All new methods or updated methods have clear docstrings.
• Testing added or updated for new methods.
• Verified if any changes impact the WebPortal Health Checks.
• Appropriate documentation updated.
• Changelog file created.

Issues Closed

Closes SKY-960

[linear]

SKY-960 Investigate duplicated subscriptions

Accounts' Stipe webhook code should handle duplicated subscriptions. But it doesn't.

Test account with dup sub:

dev3 ( https://account.dev3.siasky.dev/ ) email: rurtorekki@vusra.com / pass: asdasd

Discord discussion: https://discord.com/channels/542938080349519882/777881716240941086/984755898079649852

[meeh0w]

Tested it on dev3 and it does indeed cancel the previous subscription, so yay 🎉

That said, right after a success message in the logs:

Successfully cancelled sub with id 'sub_1LAqNgIzjULiPWN6yzlQWoGh' for user '62a985ddf4de246a1bf21adf' with Stripe customer id 'cus_LsbaAY5TJTolGc'.

I'm also seeing a failure message:

Failed to cancel sub with id '' for user '62a985ddf4de246a1bf21adf' with Stripe customer id 'cus_LsbaAY5TJTolGc'.

Error: '{\"code\":\"resource_missing\",\"doc_url\":\"https://stripe.com/docs/error-codes/resource-missing\",\"status\":404,\"message\":\"No such subscription: 'sub_1LAqNgIzjULiPWN6yzlQWoGh'\",\"param\":\"id\",\"request_id\":\"req_s10PgGfWowzGMv\",\"type\":\"invalid_request_error\"}'

This is kind of confusing, because the 2nd log says sub id is blank, but if you inspect the Stripe error you'll notice we're trying to cancel the same subscription for the 2nd time (id sub_1LAqNgIzjULiPWN6yzlQWoGh).

Functionality itself seems to be working, but there's something about this error I don't quite understand - if subsc.ID is blank, then why does sub.Cancel(subsc.ID, &p) try to cancel a sub with ID = sub_1LAqNgIzjULiPWN6yzlQWoGh...

Personally I'd rather wait for @ro-tex to come and investigate further, but maybe someone else with more Go expertise understands what's going on.

[ro-tex]

Good catch, @meeh0w! The issue is that the sub.Cancel() method on Stripe's API returns a sub object which I store in the same var I use to iterate over the list of active subs. If the operation fails, the response is an empty sub and the log output is useless. This is a good example of the advantages of immutable vars over mutable ones.

I fixed the overwrite and added a bit more debug output, so we can get a better understanding of the underlying issue. That issue, I suspect, is that we somehow get duplicate entries in the list of active subscriptions of the user that we get from Stripe.

This is the original code:

	for _, subsc := range subs {
		if subsc == nil || (mostRecentSub != nil && subsc.ID == mostRecentSub.ID) {
			continue
		}
		if subsc.ID == "" {
			api.staticLogger.Warnf("Empty subscription ID! User ID '%s', Stripe ID '%s', subscription object '%+v'", u.ID.Hex(), u.StripeID, subs)
			continue
		}
		subsc, err = sub.Cancel(subsc.ID, &p)
		if err != nil {
			api.staticLogger.Warnf("Failed to cancel sub with id '%s' for user '%s' with Stripe customer id '%s'. Error: '%s'", subsc.ID, u.ID.Hex(), s.Customer.ID, err.Error())
		} else {
			api.staticLogger.Tracef("Successfully cancelled sub with id '%s' for user '%s' with Stripe customer id '%s'.", subsc.ID, u.ID.Hex(), s.Customer.ID)
		}
	}

I don't see how else we can hit both the Failed and Successfully log lines in this code with the same sub id unless it's duplicated. And we know that it's the same sub id from the example above where it's sub_1LAqNgIzjULiPWN6yzlQWoGh.

Let's deploy this on dev and see how it goes. I believe the logic is already fine and we only need to make sure that we're getting useful log output.