Address reviewer feedback: use Transaction.unfinished and return error for non-existent transactions

Changes 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

Author: ANONYMOUSZED-beep

packages/in_app_purchase/in_app_purchase_storekit/CHANGELOG.md

@@ -1,8 +1,8 @@
 ## 0.4.6+3
 
-* Fixes consumable products not being repurchasable after the first purchase in StoreKit2.
-* Ensures `finish()` always completes even when the transaction is not found in the transaction history.
-* Improves transaction lookup to also check `Transaction.unfinished` for consumables.
+* Fixes `finish()` method to always call its completion handler in StoreKit2.
+* `finish()` now returns an error when the transaction is not found in unfinished transactions.
+* Adds `fetchUnfinishedTransaction()` helper for looking up transactions that need to be completed.
 
 ## 0.4.6+2
 

packages/in_app_purchase/in_app_purchase_storekit/darwin/in_app_purchase_storekit/Sources/in_app_purchase_storekit/StoreKit2/InAppPurchasePlugin+StoreKit2.swift

@@ -264,15 +264,18 @@ extension InAppPurchasePlugin: InAppPurchase2API {
   func finish(id: Int64, completion: @escaping (Result<Void, Error>) -> Void) {
     Task {
       do {
-        let transaction = try await fetchTransaction(by: UInt64(id))
+        let transaction = try await fetchUnfinishedTransaction(by: UInt64(id))
         if let transaction = transaction {
           await transaction.finish()
           completion(.success(Void()))
         } else {
-          // Transaction not found - this can happen for consumables that have
-          // already been finished or are no longer in the transaction history.
-          // We still return success as the transaction is effectively complete.
-          completion(.success(Void()))
+          // Transaction not found in unfinished transactions
+          completion(
+            .failure(
+              PigeonError(
+                code: "storekit2_transaction_not_found",
+                message: "Transaction not found in unfinished transactions.",
+                details: "Transaction ID: \(id)")))
         }
       } catch {
         completion(
@@ -376,12 +379,10 @@ extension InAppPurchasePlugin: InAppPurchase2API {
     return transactions
   }
 
-  /// Helper function to fetch specific transaction by ID.
-  /// First checks Transaction.all, then falls back to unfinished transactions
-  /// to ensure consumable transactions can be found and finished.
-  func fetchTransaction(by id: UInt64) async throws -> Transaction? {
-    // First, try to find in Transaction.all
-    for await result in Transaction.all {
+  /// Helper function to fetch specific transaction by ID from unfinished transactions.
+  /// Used by finish() to find transactions that need to be completed.
+  func fetchUnfinishedTransaction(by id: UInt64) async throws -> Transaction? {
+    for await result in Transaction.unfinished {
       switch result {
       case .verified(let transaction):
         if transaction.id == id {
@@ -391,11 +392,13 @@ extension InAppPurchasePlugin: InAppPurchase2API {
         continue
       }
     }
+    return nil
+  }
 
-    // If not found in Transaction.all, check unfinished transactions
-    // This is important for consumables that may have been purchased
-    // but not yet iterated through Transaction.all
-    for await result in Transaction.unfinished {
+  /// Helper function to fetch specific transaction by ID from all transactions.
+  /// Used for general transaction lookups.
+  func fetchTransaction(by id: UInt64) async throws -> Transaction? {
+    for await result in Transaction.all {
       switch result {
       case .verified(let transaction):
         if transaction.id == id {
@@ -405,7 +408,6 @@ extension InAppPurchasePlugin: InAppPurchase2API {
         continue
       }
     }
-
     return nil
   }
 }

packages/in_app_purchase/in_app_purchase_storekit/example/shared/RunnerTests/InAppPurchaseStoreKit2PluginTests.swift

@@ -415,19 +415,19 @@ final class InAppPurchase2PluginTests: XCTestCase {
     await fulfillment(of: [finishExpectation], timeout: 5)
   }
 
-  func testFinishNonExistentTransactionSucceeds() async throws {
-    // Test that finishing a non-existent transaction returns success
-    // This is important for consumables that may have already been finished
-    // or are no longer in the transaction history
+  func testFinishNonExistentTransactionReturnsError() async throws {
+    // Test that finishing a non-existent transaction returns an error
     let finishExpectation = self.expectation(
-      description: "Finishing non-existent transaction should succeed")
+      description: "Finishing non-existent transaction should return error")
 
     plugin.finish(id: 999999) { result in
       switch result {
       case .success():
-        finishExpectation.fulfill()
+        XCTFail("Finish should fail for non-existent transaction")
       case .failure(let error):
-        XCTFail("Finish should NOT fail for non-existent transaction. Failed with \(error)")
+        let pigeonError = error as! PigeonError
+        XCTAssertEqual(pigeonError.code, "storekit2_transaction_not_found")
+        finishExpectation.fulfill()
       }
     }