search

jamesmontemagno / InAppBillingPlugin

Cross-platform In App Billing Plugin for .NET
MIT License
651 stars 152 forks source link

Not possible to purchase the same renewable subscription again after it has once expired (iOS) #487

Closed DiebBlue closed 2 years ago

DiebBlue commented 2 years ago

Hello,

I am not sure if its a bug or if I am doing something wrong. I have just updated from version 4.0.2 to version 5.4.0 (I have also tested version 6.4.0 with the same result). Since version 5.4.0 I call the FinalizePurchaseAsync-function after the validation of the iOS renewable subscription purchase. That is all, that I have changed in my code. Since version 5.4.0 I cannot purchase the renewable subscription with the same product_id after it was once expired with the sandbox tester account. In the old version 4.0.2 it works as expected. Does anyone have any idea what went wrong here?

With regards DiebBlue

Bug Information

Version Number of Plugin: 5.4.0 (same behavior with 6.4.0) Device Tested On: iPhone SE (2016) Simulator Tested On: no Version of VS: 2019 Version 16.11.18 Version of Xamarin: 16.11.000.197 Versions of other things you are using: Xamarin Forms 5.0.0.2337 Xamarin Essentials 1.7.3 Xamarin.CommunityToolkit 2.0.1 Xamarin.iOS and Xamarin.Mac SDK 15.2.0.17

Steps to reproduce the Behavior

Expected Behavior

Actual Behavior

Code snippet

//You must connect
bool connected = await billing.ConnectAsync();

if (!connected)
{
    //Couldn't connect
    return null;
}

//make purchase
purchase = await billing.PurchaseAsync(product.ProductInfo.ProductId, product.ProductType);

//possibility that a null came through.
if (!CheckPurchase(product.ProductType, purchase))
{
    //did not purchase
}
else
{
    if(Device.RuntimePlatform == Device.Android)
    {
        // Android
        await ValidatePurchase_Android(purchase, billing, product, false);
    }
    else
    {
        // iOS
        await ValidatePurchase_iOS(purchase, billing, product, false);
    }
    return purchase;
}

Screenshots

Debug breakpoint: purchase_code Contents of the purchase at the breakpoint: returned_expired_purchase

yonkahlon commented 2 years ago

Does anyone know if this is only an issue for Sandbox purchases?

adam-russell commented 2 years ago

@DiebBlue I actually encountered the same thing. Not sure why I decided to upgrade the package, since 4.0.2 had been working solidly for me for a while. When I first upgraded it, I changed the code per the documentation and tested purchasing a subscription, and thought, man that was easy, since it worked no problem. Yeah, I was wrong about that. Renewals and purchasing again are very tough in 5.x+.

I'm still trying to figure out the right flow, partially just updating this with what I've figured out. The behavior isn't always reproducible, and I can't quite tell if it's just because of my code.

I think there might be a couple things missing from the documentation. Apple's documentation says you have to call finishTransaction (which is called from FinalizePurchaseAsync) on Restored and Failed transactions, as well. (4.0.2 did this automatically) I might have missed something, but I think the plugin documentation should mention calling FinalizePurchaseAsync on those statuses, too.

If you have an unfinished transaction, PurchaseAsync ends up returning an "old" transaction that hasn't had finishTransaction called on it, and doesn't even present the payment popup.

Anyway, I also implemented OnPurchaseComplete, and made sure to call FinalizePurchaseAsync from there for Purchased and Restored, and also made sure to call FinalizePurchaseAsync for Failed from the return on PurchaseAsync It seems to mostly work, but it's not as solid as what was running on 4.0.2 (any failures when doing things with in app purchases are very concerning). Part of it, I think may have to do with the way FinalizePurchaseAsync works now for iOS -- it restores purchases, then finds the transaction based on transaction id, then finishes it. The old version 4.0.2 just finished the actual SKPaymentTransaction that came through UpdatedTransactions which seems a lot more foolproof to me, but I might be off base here.

Anyway, like I said, there's probably something I'm not doing correctly or missing, not sure if you've figured out a good flow since you added this issue?

yonkahlon commented 2 years ago

Thanks @adam-russell!

It's not working for me though - I did implement OnPurchaseCompleteand call FinalizePurchaseAsync from there but I'm still having the same issue.

I did notice something very strange though - there are tons of purchases that are the same:

var purchases = await billing.GetPurchasesAsync(ItemType.Subscription);
foreach (var inAppBillingPurchase in purchases)
{
    Log.Info($"TransactionDateUCT: {inAppBillingPurchase.TransactionDateUtc}. Id: {inAppBillingPurchase.TransactionIdentifier}. State: {inAppBillingPurchase.State}");
}

You can see there are just a ton of these (there is a lot more than in this image). The TransactionDate is the original time, not the renewal time. image

... might get a chance later to investigate this.

I might have to go back to 4.0.2, but Android won't allow that beyond November. So need to figure this out soon :P

adam-russell commented 2 years ago

@yonkahlon Wow, ok.

I believe there are a couple problems here.

GetPurchasesAsync calls RestoreAsync which, after actually doing the restore, loops through transactions on SKPaymentQueue.DefaultQueue and adds the original transactions into the transaction list. I looks like this is a problem because there isn't a dupe check, so if you have multiple transactions with the same original, that original gets added to the list multiple times. On a subscription, I think there may be a lot of transactions with that first original, too. (Which may explain why you have a ton of those in your list.)

I'm not sure what your actual SKPaymentTransactions look like, but the code in FinalizePurchaseAsync has this code: var transaction = purchases.Where(p => p.TransactionIdentifier == transactionIdentifier).FirstOrDefault(); So it'll only ever update the first one in the list with that TransactionIdentifier. Apple's documentation on transactionIdentifier https://developer.apple.com/documentation/storekit/skpaymenttransaction/1411288-transactionidentifier makes this a little scary to me, too, since it says it's only defined except with the state is purchased or restored (So what happens with failed? Do these become transactions that can never be finished with the current version of the library? Not sure.)

I actually rolled back to 4.0.2 temporarily, too, because I haven't quite wrapped my head around everything. I'm pretty sure that it needs a code fix in the InAppBillingPlugin library, though.

I think the easiest "fix" would be to add a flag to have UpdatedTransactions automatically call FinishTransaction on any SKPaymentTransactions like 4.0.2 did. It looks like some older versions of the code had this flag, too.

That might be a little hacky, though. Here's what I'm currently thinking without doing that, though (and I might be off base and missing some of the flow):

It's a slightly separate concern, but I would be tempted to associate SKPaymentTransaction more directly InAppBillingPurchase which probably defeats part of the purpose of a cross platform library. It would make finishing them more straightforward, and eliminate the need to restore transactions for so many calls. The other thing this would help is the speed. Not sure if I'm the only one, but my purchase flow works more slowly than I'd like (and definitely much slower than other apps I've seen).

yonkahlon commented 2 years ago

@adam-russell thanks for that! In my brief look through the code I saw the RestoreAsync call and suspected that loop.

So essentially FinalizePurchaseAsync ends up adding SKPaymentTransactions to the queue each call instead of removing it.

Yeah you right - there is no event called when the transaction fails (TransactionCompleted isn't publicly available, only onPurchaseSuccess is):

        public override void UpdatedTransactions(SKPaymentQueue queue, SKPaymentTransaction[] transactions)
        {
            var rt = transactions.Where(pt => pt.TransactionState == SKPaymentTransactionState.Restored);

            // Add our restored transactions to the list
            // We might still get more from the initial request so we won't raise the event until
            // RestoreCompletedTransactionsFinished is called
            if (rt?.Any() ?? false)
                restoredTransactions.AddRange(rt);

            foreach (var transaction in transactions)
            {
                if (transaction?.TransactionState == null)
                    break;

                Debug.WriteLine($"Updated Transaction | {transaction.ToStatusString()}");

                switch (transaction.TransactionState)
                {
                    case SKPaymentTransactionState.Restored:
                    case SKPaymentTransactionState.Purchased:
                        TransactionCompleted?.Invoke(transaction, true);

                        onPurchaseSuccess?.Invoke(transaction.ToIABPurchase());
                        break;
                    case SKPaymentTransactionState.Failed:
                        TransactionCompleted?.Invoke(transaction, false);
                        break;
                    default:
                        break;
                }
            }
        }

I think that not completing the purchase automatically is okay - apple suggest that you do it after you verify the receipt. This lib no longer does the validation automatically, so now we have to validate then complete the purchase.

If you validate receipts, validate them before completing the transaction, and take one of the paths described above.

jamesmontemagno commented 2 years ago

Yes, this was a big change because it actually causes a lot of issues when you finalize things with consumable items. So you much now call var ack = await CrossInAppBilling.Current.FinalizePurchaseAsync(purchase.TransactionIdentifier); directly after you call PurchaseAsync for all platforms.

As documented here: https://jamesmontemagno.github.io/InAppBillingPlugin/PurchaseSubscription.html

We had thought of trying to make things autofinalize, but again it just seems like bad practice from what is there there.

Now, I will look into the other thing which is

jamesmontemagno commented 2 years ago

Alright....

Sooooo... for now i added a static property on the InAppBIllingImplementation on iOS to flip on a way to finalize everything.

I also added an event to handle on failures for iOS as well incase it is needed.

We still should figure out what is going on though for subscriptions.

adam-russell commented 2 years ago

@jamesmontemagno Awesome, thanks James, I know that'll help at least my case out a lot! I think those changes will help a lot, though I realize the finalize flag isn't good practice.

I think the first big things on subscriptions (prior to adding you adding back the finalize flag, which I'll probably take advantage of) for me were documentation based:

I do also think RestoreAsync should have a duplicate check for original transaction (because you probably end up with a bunch of renewal periods with the same original), but again, I might be missing something.

Also, hopefully I don't sound too critical in the above comments -- I personally really appreciate this library. It's saved me a lot of time, and maybe I'm just being picky because 4.0.2 was really rock solid, at least for my use case. I never had any bugs reported in purchasing and I never saw failures in the logs from it, so thank you very much for it.

jamesmontemagno commented 2 years ago

Thanks for all the research and write-up on this. I guess that all makes sense to me on the renewals for sure.

I did make a change in this latest beta that I just pushed out.

When you call FinalizePurchaseAsync... it will find all with the transactionIdentifier:

            var transactions = purchases.Where(p => p.TransactionIdentifier == transactionIdentifier);

            if ((transactions?.Count() ?? 0) == 0)
                return false;
            try
            {
                foreach(var transaction in transactions)
                    SKPaymentQueue.DefaultQueue.FinishTransaction(transaction);
            }
            catch(Exception ex)
            {
                Debug.WriteLine("Unable to finish transaction: " + ex);
                return false;
            }

On a real flow I would: 1.) User purchases sub 2.) Finalize 3.) When you need to "renew" or you have "restored" call "GetPurchasesAsync" 4.) Loop through and finalize ALL purchases.

That should handle it to be honest. You could also do step 3/4 when the user tries to buy the purchase again.

blmiles commented 2 years ago

Hello,

See all same issues here so will try not to reiterate ALL that has been covered already.

This library/plugin was running really well until version 5.4.0 and now 6.4.0. Only updating because we have to, to be able to update app for Android and THAT deadline is looming FAST!

Update as of 915am-09/20 using the latest Beta version in iOS app, Android not tested yet.---------------

This does NOT work

                var purchase = await billing.PurchaseAsync(productId, ItemType.Subscription);

                //The Stores will do their tricks here...should see a purchase dialogue on device 

                if (purchase == null) //handle null or continue

There is never a purchase dialogue as was displayed prior to version 5.4.0 The purchase object returned is expired (an old purchase, not current) and needs to be restored. Cannot see how/where to do that, even looping through all purchases returned using GetPurchasesAsync() method.

Maybe when FinalizePurchaseAsync() is called, the purchase dialogue should be displayed and the purchase is "acknowledged" AFTER the user has confirmed purchase?

Also, after looping through all purchases and calling FinalizePurchaseAsync() on each, that SHOULD have set the IsAcknowledged flag to true on each one. The next time I get all purchases, I should only need to call FinalizePurchaseAsync() on those that have not been "acknowledged" but looping through again, none of existing purchases have IsAcknowledged = true. Not sure if the FinalizePurchaseAsync() method should be updating the purchases in the app store.

I have two auto-renewing subscriptions in the App Store. Any purchase object returned from PurchaseAsync(); or GetPurchasesAsync() have their AutoRenewing == false; Not sure if that reflects what's setup in the App Store OR if that reflects that the item is cancelled/expired.

I'm sorry, I appreciate all efforts on this as the last working version was excellent! THIS is not good. Not tested properly before release and SO MANY DEVS rely on this for their apps.

PLEASE GET THIS FIXED ASAP. PLEASE.

Tag @jamesmontemagno

yonkahlon commented 2 years ago

@jamesmontemagno thanks so much! Just tested with the flag and it works for me: InAppBillingImplementation.FinishAllTransactions = true;

I tried without the flag, and follow this flow:

On a real flow I would: 1.) User purchases sub 2.) Finalize 3.) When you need to "renew" or you have "restored" call "GetPurchasesAsync" 4.) Loop through and finalize ALL purchases.

That should handle it to be honest. You could also do step 3/4 when the user tries to buy the purchase again.

This didn't work for me with 6.5.0, but maybe I'm doing something wrong. I "Restore Purchase" on start up to check that the subscription is still valid.

Just for clarity, shouldn't you call FinalizePurchaseAsync once in step 4 since it loops through all transactions for you?

Also, just to reiterate what @adam-russell said: thanks so much for this library, it's saved so much time having to figure out to implement Android and iOS billing!

adam-russell commented 2 years ago

@blmiles

You should be able to use the new beta that James put out if you include prerelease versions -- it's 6.5.0-beta. Then, set the new flag so it works like 4.0.2 where you initialize the plugin in your AppDelegate:

InAppBillingImplementation.FinishAllTransactions = true;
InAppBillingImplementation.OnShouldAddStorePayment = OnShouldAddStorePayment;
var current = CrossInAppBilling.Current;

At least for me, this makes it work nicely like 4.0.2 on initial tests just like @yonkahlon above.

@jamesmontemagno Great, thanks for the clarifications!

So, with the FinishAllTransactions flag, everything seems to work for me as I mentioned above.

Trying to implement it without that, I'm still having issues. Because of the version of Xamarin.iOS I'm using in my build, I'm having issues running the debugger on a physical device, so I'm partially guessing below (so take with a grain of salt or whatever):

One of the problems I missed before (sorry about that) that @blmiles is referencing above is that FinalizePurchaseAsync always returns false when I try it with a just completed purchase when the transaction isn't the original. I think the problem is that it's trying to use RestoreAsync then loop through the transactions to finish the right one. (Thanks for the loop in there, just in case!) The problem looks like RestoreAsync adds originals from the current queue and restores completed transactions (https://developer.apple.com/documentation/storekit/skpaymentqueue/1506123-restorecompletedtransactions) so the one you're trying to complete isn't always there. I think it'll be on SKPaymentQueue.DefaultQueue.Transactions while there's an observer attached (https://developer.apple.com/documentation/storekit/skpaymentqueue/1506026-transactions)? I also have OnPurchaseComplete implemented in my version of the code without the FinishAllTransactions flag, and it gets partially sorted out when I kill and restart the app (maybe because I'm also verifying the latest receipt with Apple?) , but without the debugger attached, I'm a little confused about what's happening.

blmiles commented 2 years ago

@adam-russell @yonkahlon @jamesmontemagno thanks for your input on this thread!

Setting that flag, did help, thanks for pointing it out, I completely missed adding that.

When calling the PurchaseAsync(); method, the purchase dialogue does not always show on a new subscription. Not sure why but if it doesn't and simply returns the purchase object, all fails. Will keep testing.

When the app starts, I do a GetPurchasesAsync() I loop through all purchases returned (100s in the test env., shouldn't be many on real world), I validate the purchaseToken on each. I noticed that the IsAcknowledged flag is always false on the most current valid purchase (it was "acknowledged" at time of purchasing). Not sure if we should have to call FinalizePurchaseAsync() method on current purchase/subscription every time, maybe there's a reason for it still being false.

MUCH BETTER! I'm a little less stressed and hopeful this can be deployed soon.

I will pay attention to updates and will add the 6.5.0 release version when it's ready.

Now to test Android...

Thanks to ALL!

DiebBlue commented 2 years ago

@yonkahlon @adam-russell @blmiles @jamesmontemagno Thank you for your commitment to this issue, which has led quite quickly to a short-term solution.

I would also like to join the previous speakers in thanking @jamesmontemagno for this library, which has also saved me a lot of time and effort.

Vdragancea commented 2 years ago

I'm also trying to migrate from 4.02 to something that supports Google billing client V4. after tons of tests( package version 6.4 ) i can say that purchases on IOS works for me only if i use

await billing.FinalizePurchaseAsync(purchase.PurchaseToken); and not TransactionIdentifier.

but i'm not sure if this approach really acknowledges the purchase as the data that returned by GetPurchasesAsync has the isAcknowldged=false. so i'm not sure if this purchase will not be rollbacked in the future i fi use it this way

blmiles commented 2 years ago

@Vdragancea

Using the PurchaseToken returns true? I think you do need to be using the TransactionIdentifier. That should return true if the purchase has completed.

However, you're right, the IsAcknowledged flag on the purchase is never set to true. I mentioned that in a comment above. If one does a GetPurchasesAsync() call after completing a purchase, shouldn't we expect the IsAcknowledged on the latest valid purchase to be TRUE?

FinalizePurchaseAsync(purchase.TransactionIdentifier); should set the IsAcknowledged flag accordingly, no?

@jamesmontemagno

jamesmontemagno commented 2 years ago

IsAcknowledged only applies to Android, there is no way to know on iOS if a transactions has been completed. It is silly that Apple doesn't expose it.

jamesmontemagno commented 2 years ago

I will create another method that will take in an array of transaction identifiers.

On iOS it is absolutely the TransactionIdentifier:

var transactions = purchases.Where(p => p.TransactionIdentifier == transactionIdentifier);

is my code

On Android the TransactionIdentifier and PurchaseToken are the same - https://github.com/jamesmontemagno/InAppBillingPlugin/blob/master/src/Plugin.InAppBilling/Converters.android.cs#L11

The purchase token on iOS is the transcation receipt info: https://github.com/jamesmontemagno/InAppBillingPlugin/blob/master/src/Plugin.InAppBilling/InAppBilling.apple.cs#L336

jamesmontemagno commented 2 years ago

Alright, so in 6.6.0... you can now pass in a bunch of transactionids to finalize... this should help with speed so it doesn't restore every time.

Additionally... just for good measure specific to iOS i have a new way to finalize products based on IDs... so that should really finalize everything that is restored with a specific Id..... worth a try too

jamesmontemagno commented 2 years ago

Alright, so 6.6.1 introduces these new methods, but also now turns on FinishAllTransactions automatically for backwards compatibility. I think this is just easier for now. I have unlisted the older stable nugets

blmiles commented 2 years ago

Awesome @jamesmontemagno Hope to get this in and tested shortly! Thanks for your quick efforts, I was seriously concerned a few days ago...