search

PeterStaev / nativescript-purchase

:moneybag: A NativeScript plugin for making in-app purchases!
Apache License 2.0
83 stars 28 forks source link

Fix null check in iOS transactions #71

Closed interrobrian closed 5 years ago

interrobrian commented 5 years ago

The iOS transaction class has its null check backwards. It only attempts to use transactionState if it is null, rather than if the transaction is not null. Related to #58

PeterStaev commented 5 years ago

Hey @interrobrian , your check is also not correct. The nativeValue is never null. Only its transactionState can sometimes be null, so only the switch has to be withing the check. All other related code will be working, even if the transactionState is null. I've fixed the code and new version is now on npm.

interrobrian commented 5 years ago

@PeterStaev As found by @p-3 in issue #58 and confirmed in development in our end, there are indeed instances where nativeValue enters the iOS Transaction constructor as null. It's possibly related to when a build has been published to TestFlight, but we're not entirely sure. If there is no null check for nativeValue in the iOS transaction class, then #58 should be re-opened to find an alternate solution to that problem, as the part of pull request #67 that fixed that problem has been undone. Perhaps the null check belongs in paymentQueueUpdatedTransactions in purchase.ios.ts?

PeterStaev commented 5 years ago

As state in this comment: https://github.com/PeterStaev/nativescript-purchase/issues/58#issuecomment-451231803 only the transactionState was getting a NULL value, not the whole nativeValue. And this was causing problems with the switch as you cannot do a switch on null.

So you are saying that there are cases where the whole nativeValue is null?

interrobrian commented 5 years ago

When we first ran into this problem, we did some testing and confirmed that the entire nativeValue was indeed coming in as null. Hence, we got the same error as #58: JS ERROR TypeError: null is not an object (evaluating 'nativeValue.transactionState'), which indicates that it failed out trying to treat nativeValue as an object when it was null, which is not an object. The switch should work just fine if transactionState is null, as it just wouldn't fall into any of the cases.

At present, we can't reproduce it. I believe it has to do with having a build on TestFlight, but not attached to your app. We are currently waiting for review from Apple, so we can't pull our build out to test this anymore.

That all being said, I realize looking into this more that this PR is probably not the best way to fix it, and the best place for the null check would be purchase.ios.ts.

PeterStaev commented 5 years ago

Ok, thanks for the details! I will see to add a null check, but here instead, as it makes more sense. https://github.com/PeterStaev/nativescript-purchase/blob/ad1f94aac4fd317373d9af0bf4f7dde1409318b1/purchase.ios.ts#L128

interrobrian commented 5 years ago

Yeah, I agree that spot makes more sense. Thank you!