[in_app_purchase] Removed maintaining own cache of transactions #2911
Conversation
…SKPaymentQueue.transactions where needed - Removed unnecessary and broken payment validation from addPayment - Refactored finishTransaction for finishing transactions properly - Fixed: restoreTransactions did not call result(nil) causing the call never complete
cc @LHLL |
…ifier, not productIdentifier
Just wondering whether this PR is expected to ship with next release or you recommend applying it manually for now? |
Thanks for the fix, looks like this removed all restrictions for checking transactions. Here are some questions:
|
|
|
|
"Cannot connect to iTunes Store" normally happens when a user is using the same test account in both sandbox environment and prod environment. But other scenarios such as Apple service is unavailable could also lead to this error. Having multiple transactions of the same SKProduct is allowed by the StoreKit and that's why we need to prevent it from happening since it's super hard to debug due to fact that Apple will not expose the Apple ID to the third-party apps. The cached transaction array is used to ensure there will be only one transaction of a given product ID at any given time. |
What was actually the reason trying to maintain your own cached transaction array instead of using My end users do not use Sandbox environment and it is very hard to believe that Apple service is unavailable every day. So it is some other issue which happens occasionally. My experience from StoreKit behavior is very different. If you have already added a Therefore since the purchase dialog is not shown and user has to always confirm a purchase using that dialog, user cannot make a double purchase. What I don't know for sure is that will you get some related error for the second purchase attempt via observer (e.g. "Cannot connect to iTunes Store") or will it just fail silently. And after you have called |
My experience from StoreKit behavior is very different. If you have already added a SKPayment to the queue i.e. the Apple purchase dialog has been shown and user has confirmed the purchase, but you have failed to call finishTransaction for any reason, another attempt to add a SKPayment to the queue will fail and the purchase dialog is NOT shown. What was actually the reason trying to maintain your own cached transaction array instead of using SKPaymentQueue.transactions? |
Based on my tests (I use Non-Consumables and auto-renewing Subscriptions in my app) and also what is reported in several GitHub issues and StackOverflow cases the behavior seems to be the same for all product types, even for Consumables.
So I think the account related stuff you are talking about is completely out of the scope of this plugin. That's something the app developer must take care of, not this plugin. I don't even understand what this plugin could do for it. In my app I am already associating a subscription to a single account. I do this by uploading the subscription receipt to my backend where it is saved to a database with the UID of the authenticated user. Then if another account tries to upload the same receipt, I can reject it and user gets a response "Not subscribed". But user is allowed also to change his account by first deleting the current account and then signing in with a different account in which case the subscription gets transferred to the new account. I think I have provided enough information related to this PR. Feel free to cancel and close this if you think it is too much against the design idea of your implementation, I understand that too. This is not a problem to me as my app is now working and I can hang in my fork as long as I see it is necessary. It may be very challenging to get that cached transactions array to work properly. But of course, as always, everything is possible :) |
I totally agree with kinex research. It makes complete sense to remove the cache. I believe the cache has been the source of much the trouble we have had with this plug-in. I also believe it doesn't even work in some case. I spent countless hours on Browserstack testing with real-life devices and the behavior of this integration wasn't clear to me. For instance, you may be signed-in to your Apple ID on your iPhone. The plug-in then prompts another user login and you can use another Apple ID. In some cases, I have used an Apple ID different from the iPhone one (sandbox user) but still it returned the subscription of the main user rather than the new account. Kindly would you consider my suggestion to make the transaction cache optional? We can leave it on by default. We can then get real-life feedback and see which implementation would be less troublesome. But my theory is that indeed the cache is not required, given that some devs moved to another package and seem very happy. Another suggestion I could make is to use my Apple developer code-level ticket to have Apple take a look at this although I don't know if that will be very fruitful. |
A receipt that contains enough information or a transaction identifier is only available when a transaction enters purchased/restored state. What can cause issue transactions from the previous app running session which there is guarantee the same account will be logged in. If your only need is to be able to clear failed transactions, this can be a valid feature that some apps might not handle it properly in the previous version and the only way to handle that is to have a new API. |
All above examples are Apple's sandbox issues, you can easily fix them by resetting your test device. You are not supposed to test with different Apple IDs on the same device in the TestFlight environment. What I think would be useful is to expose a new API that kills all failed transactions from previous sessions. |
I get that but what happens in production? Couldn't they be signed-in to one Apple ID on their phone and use another log-in for IAP? What would happen in such case? |
iOS does allow you to use one for iCloud and one for App Store. But thing could become tricky when a user has two Apple ID for the App Store(Imaging one for US App Store, and the other for the Mexico App Store). This leads to a lot of in-app purchase bugs based on my experience and cannot be debugged purely on the app side since Apple does not expose which Apple ID the user is using. I personally use storefront to mitigate some of those bugs, but this API will not be called if two Apple IDs are used in the same region. An example is, let's say your app provides a subscription with multiple tiers. User purchased tier one with Apple ID A, and later upgraded to tier two with Apple ID B. This transaction will go through and the user will be charged on both Apple ID A and Apple ID B. But regardless, I don't think this is related to this plugin and PR-467870043 should fix the issue discussed in this PR. And do let me know if you think a new API to remove all failed transactions is what you need. |
It is still totally unclear to me why do you need the cached transactions. Could you please clarify these questions again:
Here is also a related blog post, which lists caching transactions as one of the mistakes you should avoid. |
Let's assume you support login with Google, then you easily verify this your first question by adding a transaction of product A for Google Account A, wait for it to become success. Don't finish the transaction, switch to Google Account B, purchase the same product again. Now your backend can also validate the receipt with Apple and deliver the product to Google Account B. At this time, we should block Google Account B from purchasing until the transaction being finished. I am not the original author of the cached transaction, but the issue you are facing is not related to the cache itself but zombie transactions in the SKPaymentQueue. And based on my testing, the only edge case that cache transaction will become an issue is actually restoring transactions and this can be fixed by PR-467870043. I am open to remove the cache if you can prevent the above example from happening. |
I am repeating myself again, but based on all my current knowledge and tests this (trying to purchase the same product again before finishing the transaction) is simply not possible, Apple (StoreKit) already prevents it. That's why I asked if you can point any related GitHub issue, StackOverflow case or anything which a kind of "proves" this could happen if you don't explicitly prevent it. Considering previous "fact", this is not important, but I want to also mention that nowhere is said that purchases should be connected to a single app account? App can allow using any number of accounts if it decides so. Also, verifying a receipt does not require signing in to your app account so you don't need to wait for user to sign in to be able to finish a transaction. My point here is that all app account related stuff is out of the context of this plugin. This plugin is for making purchases. It is app's responsibility to decide how it uses those purchased products. I am confident removing the transactions cache improves the stability of this plugin, but I am not saying it fixes all the issues. For some reason I still have to clear the transactions from |
You can easily reproduce it by:
As for The rule of thumb should be avoid creating duplicate transactions of the same product in the transaction queue except for restored transactions. I am ok to accept or make any new changes, but the change should at least cover all known edge cases instead of part of them. |
You really have spent time on thinking edge cases :) What is the expected end result? Not sure if anyone can have a correct answer to that. But if you want to keep the check in - (BOOL)addPayment:(SKPayment *)payment {
for (SKPaymentTransaction *transaction in self.queue.transactions) {
if ([transaction.payment.productIdentifier isEqualToString:payment.productIdentifier]) {
return NO;
}
}
[self.queue addPayment:payment];
return YES;
} This LGTM. Feel free the add this change to this PR. |
…s for the same product
…pp_purchase_fixes # Conflicts: # packages/in_app_purchase/CHANGELOG.md # packages/in_app_purchase/pubspec.yaml
Suggested fixes done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks.
Looks like some tests failed on CI. @kinex Do you know what's going on? |
…urchase_fixes # Conflicts: # packages/in_app_purchase/CHANGELOG.md # packages/in_app_purchase/ios/Classes/FIAPaymentQueueHandler.h # packages/in_app_purchase/ios/Classes/FIAPaymentQueueHandler.m # packages/in_app_purchase/ios/Classes/InAppPurchasePlugin.m
@cyanglaz lint_darwin_plugins PLUGIN_SHARDING / testAddPaymentWithSameProductIDWillFail: it seems this test works only if the old transaction cache is there. Not sure if we can just remove the whole test. Or could someone who knows this this test better fix it. test CHANNEL:master/stable: I have difficulties to see from the log why this fails. Could this be related to the failing lint_darwin_plugins PLUGIN_SHARDING test? Btw is there some doc on how to run those tests locally? |
@LHLL would know more about the To run the failed tests locally, just run |
@kinex You can just remove that test since you removed the cache. |
- fixed sk_methodchannel_apis_test
I guess Anyway, now all tests pass. I fixed one test and removed two tests. I had to remove also |
…ter#2911) * - Removed maintaining own cache of transactions, it is better to use SKPaymentQueue.transactions where needed - Removed unnecessary and broken payment validation from addPayment - Refactored finishTransaction for finishing transactions properly - Fixed: restoreTransactions did not call result(nil) causing the call never complete * - Updated changelog * - Fixed call to finishTransaction: parameter must be transactionIdentifier, not productIdentifier * - review fixes: verify in addPayment there are no pending transactions for the same product * - reverted accidental change * - fixed formatting issues * - fixed formatting issues * - fixed test (removed obsolete references to old transactions cache) * - removed obsolete test testAddPaymentWithSameProductIDWillFail - fixed sk_methodchannel_apis_test * - removed testDuplicateTransactionsWillTriggerAnError Co-authored-by: LHLL <yijiexu@google.com>
…ter#2911) * - Removed maintaining own cache of transactions, it is better to use SKPaymentQueue.transactions where needed - Removed unnecessary and broken payment validation from addPayment - Refactored finishTransaction for finishing transactions properly - Fixed: restoreTransactions did not call result(nil) causing the call never complete * - Updated changelog * - Fixed call to finishTransaction: parameter must be transactionIdentifier, not productIdentifier * - review fixes: verify in addPayment there are no pending transactions for the same product * - reverted accidental change * - fixed formatting issues * - fixed formatting issues * - fixed test (removed obsolete references to old transactions cache) * - removed obsolete test testAddPaymentWithSameProductIDWillFail - fixed sk_methodchannel_apis_test * - removed testDuplicateTransactionsWillTriggerAnError Co-authored-by: LHLL <yijiexu@google.com>
This PR made things even worse, since the same problem now occurs because of restored transactions... |
@ziggycrane I doubt that. What do you mean by "same problem"? Caching the transactions was the source of all problems. My app is in AppStore, with very detailed error reporting to Crashlytics and I don't see any related issues. I guess iOS 14 caused some new issues again, but those have already been resolved in the latest version of this plugin. So make sure you are using the latest version of the plugin. I suggest you add a new issue where you describe the issue you are facing with source code. "Things are worse" comments do not help a lot here. |
Well, it's as simple as - with a new user purchase subscription. Cancel it later and then attempt to resubscribe and you will fail with the same error as before, because you now check the queue transactions and restored transactions are in queue so it will fail every time. |
You can see from the comments above why the check is in Do you receive those restored purchases from |
I checked code in From my understanding I don't face this issue in my app as I am clearing the whole transaction queue before I call |
Yes, you are right I have gotten that far too. For me, the problem is that I for the life of me can't finish these transactions even after queryPastPurchases. They appear every time as "restored" later even when I complete them. Could you please show me the example of your workaround? I am desperate to make it work. |
Calling In my opinion this should be fixed so that also restored purchases are passed to Other possible fix would be to call This is my helper function: Future<void> clearTransactionsIos() async {
if (Platform.isIOS) {
final transactions = await SKPaymentQueueWrapper().transactions();
for (final transaction in transactions) {
try {
await SKPaymentQueueWrapper().finishTransaction(transaction);
} catch (e) {
print(e);
}
}
// magic delay, not sure if needed
await Future.delayed(const Duration(seconds: 1));
}
} I call it just before before calling |
…ter#2911) * - Removed maintaining own cache of transactions, it is better to use SKPaymentQueue.transactions where needed - Removed unnecessary and broken payment validation from addPayment - Refactored finishTransaction for finishing transactions properly - Fixed: restoreTransactions did not call result(nil) causing the call never complete * - Updated changelog * - Fixed call to finishTransaction: parameter must be transactionIdentifier, not productIdentifier * - review fixes: verify in addPayment there are no pending transactions for the same product * - reverted accidental change * - fixed formatting issues * - fixed formatting issues * - fixed test (removed obsolete references to old transactions cache) * - removed obsolete test testAddPaymentWithSameProductIDWillFail - fixed sk_methodchannel_apis_test * - removed testDuplicateTransactionsWillTriggerAnError Co-authored-by: LHLL <yijiexu@google.com>
Description
Removed maintaining own cache of transactions as it is not working reliably. It is better to use
SKPaymentQueue.transactions
where needed. The old logic trying to maintain list of transactions did not work properly causing severe issues when trying to make a purchase and when trying to finish a transaction.Related changes:
addPayment
(AppStore won't accept purchasing same product twice, no need to check it here). Changed also the return value ofaddPayment
frombool
tovoid
.finishTransaction
to finish transactions properly.This pull request fixes also an issue in the
restoreTransactions
. It did not callresult(nil)
so the call never completed.Related Issues
flutter/flutter#53534
flutter/flutter#57903
flutter/flutter#57356
It is possible this fixes also other reported issues in this plugin as the behavior in the old version is very unstable.
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]
). This will ensure a smooth and quick review process.///
).flutter analyze
) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?