Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix "Nothing to Restore" even when transactions exist #640

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

mshibanami
Copy link

@mshibanami mshibanami commented Jun 14, 2021

Hi,

I got the same problem with #620 on my macOS app.
This PR is for fixing that.

Problem details

StoreKit calls the following delegate methods of SwiftyStoreKit's PaymentQueueController when restoring transactions is successful:

  1. paymentQueue(_:, updatedTransactions:)
  2. paymentQueueRestoreCompletedTransactionsFinished(_:)

I confirmed that these methods are called simultaneously on their own different threads. So if there are so many restored transactions, the code in method 1 takes long time and then the delegate method 2 finishes earlier than 1. In other words, the completion of restoring transactions is finished earlier than the code that processes restored transactions.

It seems that SwiftyStoreKit's PaymentQueueController doesn't handle this case correctly and nil is set to restorePurchases property of RestorePurchasesController before finishing the restoration. I think this is the cause.

I'm not sure how many transactions causes this problem. Maybe up to the machine power. This video is when 28 transactions exist:

Screen.Recording.2021-06-14.at.8.18.18.pm_converted.mov

Probably this happens on other platforms too.

Implementation

I newly defined restorationDispatchQueue in PaymentQueueController so that the code of delegate methods 1 and 2 run on the same thread in order.

This works fine on my Mac as follows:

Screen.Recording.2021-06-14.at.8.50.22.pm_converted.mov

By the way, maybe other delegate methods, such as paymentQueue(_: updatedDownloads:), also need to be run on the same thread but since I'm neither familiar with StoreKit nor this library and I'm not too sure if it unnecessarily impacts other functionalities or not, I leave them as-is. (But please let me know if it actually doesn't impact or my approach is completely wrong.🙂)


Thanks so much for maintaining this great library!

@bizz84
Copy link
Owner

bizz84 commented Jun 14, 2021

1 Error
🚫 Tests were not updated

Generated by 🚫 Danger

@mshibanami mshibanami changed the base branch from master to develop June 14, 2021 12:33
@mshibanami mshibanami force-pushed the fix/nothing_to_restore_even_when_transactions_exist branch from d374b93 to ecaddfc Compare June 14, 2021 12:35
@mshibanami
Copy link
Author

Tests were not updated

Sorry I'm not sure how I should write tests in this case but I'm happy to write it if it makes sense.

@Sam-Spencer Sam-Spencer self-requested a review June 18, 2021 04:21
@Sam-Spencer Sam-Spencer added area: purchase flows purchase processes, efficiency and failures difficulty: advanced we really need help with this one macOS directly related to Mac status: code review need to review the code type: bug labels Jun 18, 2021
@mshibanami
Copy link
Author

I noticed that sometimes paymentQueueRestoreCompletedTransactionsFinished(_:) is queued earlier than paymentQueue(_:, updatedTransactions:). It's understandable, since they're called at the same time. Therefore, I added tiny-enough delay (0.1ms) as e5bb4a5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: purchase flows purchase processes, efficiency and failures difficulty: advanced we really need help with this one macOS directly related to Mac status: code review need to review the code type: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants