jamesmontemagno / InAppBillingPlugin

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

Double call PurchaseAsync may be incorrect (Droid) #125

Closed VIZORG closed 6 years ago

VIZORG commented 6 years ago

Hi there, If in the first call of PurchaseAsync we get response which is not equal 0 we throw exception (for example UserCancelled), but tcsPurchase was already created. Other call PurchaseAsync always returns null, because tcsPurchase is not null and is not completed.

May be it's better to replace create of tcsPurchase after switch ( responce )?

prashantvc commented 6 years ago

Could you fill out the following details, it will help us to troubleshoot the issue

Bug Information

Version Number of Plugin: Device Tested On: Simulator Tested On: Version of VS: Version of Xamarin: Versions of other things you are using:

Steps to reproduce the Behavior

Expected Behavior

Actual Behavior

Code snippet

Screenshotst

VIZORG commented 6 years ago

Bug Information

Version Number of Plugin: 1.2.4 Device Tested On: FLY IQ446 Simulator Tested On: Version of VS: Community 2017, 15.5.6 Version of Xamarin: 23.3.0 (Android) Versions of other things you are using: Xamarin Forms 2.3.4.247

Steps to reproduce the Behavior

1) We make product purchase from a test purchase account by call PurchaseAsync. As a result we have successfull purchase. 2) In next step we call purchase this product again and get exception PurchaseError.AlreadyOwned. After that we process this exception. 3) Call purchase this product again by call PurchaseAsync.

Expected Behavior

Get exception PurchaseError.AlreadyOwned.

Actual Behavior

PurchaseAsync return null.

AndreiMisiukevich commented 6 years ago

@VIZORG Hi)

It seems like https://github.com/jamesmontemagno/InAppBillingPlugin/issues/121

Try to download and add the last PRE-release version of this Plugin)

VIZORG commented 6 years ago

@AndreiMisiukevich Hi)

I saw #121 and I think that bug is steel exist.

@jason2211 wrote:

Another option would be to set the tcsPurchase to null whenever the purchase is returned without invoking the BuyIntent.

I think it's right way.

Call PurchaseAsync and getting exception PurchaseError.AlreadyOwned are used for simple reproduction. Ок, let's say that response in PurchaseAsync returns not 7 (PurchaseError.AlreadyOwned), but 2 (PurchaseError.ServiceUnavailable)... First call PurchaseAsync will throw exception InAppBillingPurchaseException(PurchaseError.ServiceUnavailable). Second call PurchaseAsync will return null and we'll not know what happened...

Why can we not replace line

tcsPurchase = new TaskCompletionSource<PurchaseResponse>();

after

switch(response)
{
......
}

?

AndreiMisiukevich commented 6 years ago

Do you think, that https://github.com/jamesmontemagno/InAppBillingPlugin/blob/master/src/Plugin.InAppBilling.Android/InAppBillingImplementation.cs#L500 won't manage your case (ServiceUnavailable) ?

Check this method with attention) https://github.com/jamesmontemagno/InAppBillingPlugin/blob/master/src/Plugin.InAppBilling.Android/InAppBillingImplementation.cs#L537-L539

Did you try last pre-release version?)

VIZORG commented 6 years ago

I used last pre-release version.

Yes, https://github.com/jamesmontemagno/InAppBillingPlugin/blob/master/src/Plugin.InAppBilling.Android/InAppBillingImplementation.cs#L500 manage case (ServiceUnavailable) and other cases, but only when tcsPurchase.Task is over.

@jason2211 in #121 wrote about it:

Since the HandleActivityResult will never be called due to the BuyIntent never being sent to Android, all future purchase attempts will return null due to the tcsPurchase.Task.IsCompleted check at the beginning of PurchaseAsync method (line 290).

If we throw exception in PurchaseAsync before call Context.StartIntentSenderForResult than, of course, it will not manage with HandleActivityResult. We catch this exception in code where we call PurchaseAsync and this is right.

However, in this time tcsPurchase is not null and is not Completed. If we call PurchaseAsync again we return null by first statement:

if (tcsPurchase != null && !tcsPurchase.Task.IsCompleted)
                return null;
AndreiMisiukevich commented 6 years ago

@VIZORG Sorry, you are right infinitely.. I should have looked through code, before arguing with you)