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

Calling purchaseProduct quickly after retrieveProductsInfo invlaidates latter's completion handler #250

Closed
3 of 9 tasks
gapl opened this issue Jul 27, 2017 · 7 comments
Closed
3 of 9 tasks
Labels
answered Questions which have accepted answers. type: bug type: question

Comments

@gapl
Copy link

gapl commented Jul 27, 2017

Platform

  • iOS
  • macOS
  • tvOS

In app purchase type

  • Consumable
  • Non-consumable
  • Auto-Renewable Subscription
  • Non-Renewing Subscription

Environment

  • Sandbox
  • Production

Version

v0.10.4

Related issues

Couldn't find any.

Report

Issue summary

Completion handler of retrieveProductsInfo never gets called if we quickly call purchaseProduct afterwards.

Very easy to reproduce:

let productId = "YOUR_PRODUCT_ID"

SwiftyStoreKit.retrieveProductsInfo([productId]) { _ in
    print("Retrieve products finished")
}

SwiftyStoreKit.purchaseProduct(productId, atomically: true) { _ in
    print("Purchase finished")
}

What did you expect to happen

That retrieveProductsInfo completion handler gets called regardless.

What happened instead

Completion handler is never invoked.

@bizz84
Copy link
Owner

bizz84 commented Aug 3, 2017

I would not recommend calling retrieveProductsInfo and purchaseProduct sequentially.

The README shows the recommended way:

https://github.com/bizz84/SwiftyStoreKit#purchase-a-product-given-a-skproduct

SwiftyStoreKit.retrieveProductsInfo(["com.musevisions.SwiftyStoreKit.Purchase1"]) { result in
    if let product = result.retrievedProducts.first {
        SwiftyStoreKit.purchaseProduct(product, quantity: 1, atomically: true) { result in
            // handle result (same as above)
        }
    }
}

@bizz84 bizz84 added type: question answered Questions which have accepted answers. labels Aug 3, 2017
@gapl
Copy link
Author

gapl commented Aug 8, 2017

I understand that is the recommended way and I have since abandoned my way of handling things.

That being said, a completion handler not getting called ever is definitely a bug which can have some sever consequences. I would not just want to sweep this one under the rug.

@bizz84
Copy link
Owner

bizz84 commented Aug 21, 2017

@gapl You are correct. This is a bug in ProductsInfoController, and it happens because once the first response is received, the request is removed and any further completion blocks will be lost.

I can see two ways of fixing this:

  • Simple implementation: Store each request and its completion with an unique identifier rather than a set of product IDs. This implies that calling retrieveProductsInfo in sequence twice with the same set of product IDs will trigger two network requests.

  • More clever implementation: Fire a request for a set of product IDs only once even if concurrent calls are made, and call all registered completion blocks once the response is received.
    This would result in only one network call to get product info even if you call retrieveProductsInfo and purchaseProduct in sequence, however it requires a more complex implementation.

I started #259 to solve this with the more efficient way. Once I'm satisfied that it works correctly and I have unit tests for this I can merge it.

If you have any feedback on this proposal, please let me know.

@gapl
Copy link
Author

gapl commented Aug 21, 2017

@bizz84 Props for tackling this issue. I've checked out your branch briefly and the approach seems solid. One possible enhancement would be to not remove the request upon successful completion as you could cache it for the duration of the session. No need to re-fire an already successful product fetch. Only thing to watch out would be to remove the request in case it errors out or is cancelled.

@bizz84
Copy link
Owner

bizz84 commented Aug 21, 2017

@gapl See related issue #212. Caching products can lead to undesired problems where a fetched product becomes outdated if the corresponding IAP is modified on iTunes Connect.

I think it's reasonable to "merge" together requests that are concurrent. However, once a response is received SwiftyStoreKit will just return it and clear all temporary state.

If clients wish to do so, they can hold to the products for a longer time.

@gapl
Copy link
Author

gapl commented Aug 21, 2017

Oh, makes sense. I was not aware of those implications. 👍

@bizz84
Copy link
Owner

bizz84 commented Aug 21, 2017

This is now fixed and available on version 0.10.7, so I'm closing this issue.

If you experience any issues with this, feel free to reopen.

@bizz84 bizz84 closed this as completed Aug 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
answered Questions which have accepted answers. type: bug type: question
Projects
None yet
Development

No branches or pull requests

2 participants