- Notifications
You must be signed in to change notification settings - Fork3.6k
[in_app_purchase_storekit] Fix consumable repurchase issue in StoreKit2#10521
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
[in_app_purchase_storekit] Fix consumable repurchase issue in StoreKit2#10521
Conversation
Fixesflutter/flutter#179111## Changes- Fixed inish() method to always call the completion handler, even when the transaction is not found- Added fallback to Transaction.unfinished when looking up transactions- Added proper error handling with PigeonError for transaction finish failures- Added tests for non-existent transaction finish and consumable repurchase scenarios## ProblemWith StoreKit2, consumable products could only be purchased once because:1. The inish() method would never call its completion handler if the transaction wasn't found in Transaction.all2. This caused the Dart side to hang indefinitely3. Consumable transactions needed to be found in Transaction.unfinished as well## Solution- inish() now returns success even if the transaction is not found (it's effectively already complete)- etchTransaction() now checks both Transaction.all and Transaction.unfinished- Proper error handling added for unexpected failures
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.
Code Review
This pull request effectively addresses a critical issue with repurchasing consumable products in StoreKit2. The changes to thefinish andfetchTransaction methods are logical and well-implemented, ensuring that the completion handler is always called and that unfinished transactions are correctly handled. The addition of error handling and corresponding tests significantly improves the robustness of the implementation. I have a couple of suggestions to further enhance code maintainability and test reliability.
| // Finish the transaction | ||
| plugin.finish(id:0){ result in | ||
| switchresult{ | ||
| case.success(): | ||
| finishExpectation.fulfill() | ||
| case.failure(let error): | ||
| XCTFail("Finish should NOT fail. Failed with\(error)") | ||
| } | ||
| } | ||
| await fulfillment(of:[finishExpectation], timeout:5) |
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.
Using a hardcoded transaction ID0 makes this test fragile and dependent on the specific behavior of the test environment. A more robust approach would be to dynamically fetch the transaction created by the first purchase and use its actual ID. This ensures the test is more reliable and less likely to break with changes to the test session implementation.
lettransactionsExpectation=self.expectation(description:"Get transactions")vartransactionId:Int64? plugin.transactions{ resultinswitch result{case.success(let transactions): transactionId= transactions.first?.id transactionsExpectation.fulfill()case.failure(let error):XCTFail("Getting transactions should NOT fail. Failed with\(error)") transactionsExpectation.fulfill()}}awaitfulfillment(of:[transactionsExpectation], timeout:5)guardlet idToFinish= transactionIdelse{XCTFail("Could not get transaction ID to finish")return} // Finish the transaction plugin.finish(id: idToFinish){ result in switchresult{ case.success(): finishExpectation.fulfill() case.failure(let error):XCTFail("Finish should NOT fail. Failed with\(error)")}}await fulfillment(of:[finishExpectation], timeout:5)
| } | ||
| } | ||
| // If not found in Transaction.all, check unfinished transactions |
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.
The documentation saysall should include unfinished consumables:https://developer.apple.com/documentation/storekit/transaction/all#Discussion. But if this method is only used byfinish, shouldn't we useunfinished in first place instead ofall?
| await transaction.finish() | ||
| completion(.success(Void())) | ||
| }else{ | ||
| // Transaction not found - this can happen for consumables that have |
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.
This feels like a case that should be treated as an error since the programmer is trying to finish a non-existent transaction and we should not fail silently? Unless there're legitimate cases where developers can't tell if a transaction is finished or not.
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.
Thanks for the review! I've addressed both points:
finish() now uses a new fetchUnfinishedTransaction() helper that only checks Transaction.unfinished
finish() now returns an error (storekit2_transaction_not_found) when the transaction is not found, instead of silently succeeding
…r for non-existent transactionsChanges based on@LongCatIsLooong's review:- Use Transaction.unfinished instead of Transaction.all for finish() since we're looking for transactions that need to be finished- Return an error when attempting to finish a non-existent transaction instead of silently succeeding- Added fetchUnfinishedTransaction() helper specifically for finish()- Updated tests to expect error for non-existent transaction
hellohejinyu commentedDec 5, 2025
 Even using |
If a transaction is not found in Transaction.unfinished, it means the transaction has already been finished. This should be treated as a success case rather than an error, since the desired outcome (transaction completed) has been achieved.This ensures that calling completePurchase() multiple times or on already-finished transactions works correctly, allowing consumable products to be repurchased.
ANONYMOUSZED-beep commentedDec 5, 2025
Thanks for the feedback! After further investigation (including a report from@hellohejinyu that the error-returning approach didn't work), I've updated the implementation to return success instead of error when the transaction is not found. Rationale: If a transaction is not in Transaction.unfinished, it means the transaction has already been finished. This is actually a success case — the desired outcome (transaction completed) has been achieved. Returning an error would break user flows because: SK2PurchaseDetails.pendingCompletePurchase returns true for all purchased items This should now properly allow consumable products to be repurchased after finishing. |
LouiseHsu left a comment
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! the updated finish() logic looks good to me.
Fixesflutter/flutter#179111
Changes
Problem
With StoreKit2, consumable products could only be purchased once because:
Solution
Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.
List which issues are fixed by this PR. You must list at least one issue.
Pre-Review Checklist
[shared_preferences]pubspec.yamlwith an appropriate new version according to thepub versioning philosophy, or I have commented below to indicate whichversion change exemption this PR falls under1.CHANGELOG.mdto add a description of the change,following repository CHANGELOG style, or I have commented below to indicate whichCHANGELOG exemption this PR falls under1.///).If you need help, consider asking for advice on the #hackers-new channel onDiscord.
Note: The Flutter team is currently trialing the use ofGemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.↩↩2↩3