russell-archer / StoreHelper

Implementing In-App Purchases with StoreKit2 in Xcode 13 - 15 using SwiftUI, Swift 5.7 - 5.9, iOS 15 - 17 and macOS 12 - 14. Also supports tvOS and visionOS.
MIT License
422 stars 49 forks source link

Added await transaction.finish() inside Transaction.updates #65

Closed josipbernat closed 9 months ago

josipbernat commented 9 months ago

Thanks for sharing this great library with us! Unfortunately I have experienced some problems with auto-renewable subscriptions when calling product.purchase(). The function would return immediately with success but the product wasn't purchased! I found that I wasn't the only one with this problem when using StoreKit2 as you can see in this thread.

Couple of members suggested that the app should always call transaction.finish() inside Transaction.updates which I did and after that it seems that this problem is gone. If you take a closer look inside Source Code from Apple you can see how they say: "Always finish a transaction.".

russell-archer commented 9 months ago

But I already do call finish on the transaction. I do this in two places:

In StoreHelper purchase(_:options:). See line 538:

await validatedTransaction.finish()  // Tell the App Store we delivered the purchased content to the user

And in the StoreHelper handleTransactions() method. See line 895 in handleTransactions():

/// This is an infinite async sequence (loop). It will continue waiting for transactions until it is explicitly
/// canceled by calling the Task.cancel() method. See `transactionListener`.
/// - Returns: Returns a task for the transaction handling loop task.
@MainActor private func handleTransactions() -> Task<Void, Error> { 
    return Task.detached { [self] in        
        for await verificationResult in Transaction.updates {
            :
            :
            await transaction.finish()
        }
    }
}
josipbernat commented 9 months ago

But not in all cases because i.e. inside

if let expirationDate = transaction.expirationDate, expirationDate < Date() {
    // The user's subscription has expired
    StoreLog.transaction(.transactionExpired, productId: transaction.productID, transactionId: String(transaction.id))
    await self.updatePurchasedProducts(for: transaction.productID, purchased: false)
    if let handler = transactionNotification { handler(.transactionExpired, transaction.productID, String(transaction.id)) }
    await transaction.finish() // I added this.
    return
}

There is a return at the end and because of that .finish() never gets called. Maybe this seems like there is no need to call it but after adding .finish() things started working.

russell-archer commented 9 months ago

Ah yes, I see now. Sorry, for some reason I couldn't see your changes as I was viewing the PR via the Tower client. On GitHub I can see your suggested changes.

That's interesting because I remember when I first wrote this code I did research into whether or not I should call finish on the transaction in those cases where access to the product has been revoked, the subscription has expired or when the user has upgraded the subscription to a higher-value subscription. And I must have decided that it was NOT the correct thing to do :-)

Anyway, before merging your PR I'll do some testing to see if making those changes impacts anything else!!

josipbernat commented 9 months ago

No worries, the most important is that now we are on the same page. Please do some testing if you have time. I encountered this error couple of times and now I feel relieved when I finally found forum discussion with the same problem as mine. LMK if this worked.

russell-archer commented 9 months ago

I've done some testing and, for me, it seems that having the extra transaction.finish() calls inside Transaction.updates for all cases (expired, revoked, upgraded, etc.) doesn't make any difference. I've tried with and without and everything seems to work correctly. However, having looked at that Apple developer support thread I'm convinced that you're correct. So, I'm going to add the extra transaction.finish() calls.

I'm going to close this PR rather than merge it because I'm going to add the transaction.finish() call higher up the method to avoid code repeatition. Thanks!

russell-archer commented 9 months ago

All done!