Closed alex-dorokhov closed 8 years ago
I would generally think we should not do architectural changes unless it makes things either easier for the end-user (developer's that use gdx-pay) or we identify a problem that cannot be solved otherwise (bad architecture).
I have to admit I haven't looked much at the iOS backend.
But the problem is SKPaymentQueue restores unfinished transactions before we have any information about products (like price and currency), which is required to fill gdxpay.Transaction object (see PurchaseManageriOSApple.transaction(SKPaymentTransaction)). So, seems it is kind of a dead loop.
Seems to fix this bug, we need an architectural change. Or do you have any other ideas?
Could we just cache the unfinished transactions that are returned via SKPaymentQueue whenever our product information such as price & currencies are not available (yet)? Once we have the information available that's when we build our transaction object and finally notify about it/confirm delivery. I believe that should work?
Unfortunately no, because iOS requires to finish transactions right only inside the observer's method, so still before we have any products information.
We could however try to cache product info before the potential crash on the disk. Right after we got it from Apple. That will not save us from all possible cases, like: no space on the device, or user reinstalled the app. But those are more unusual. In case no information is cached, we could just pass 0.0 as a price and empty string as a currency, because anyway if user reinstalled an app, we can't be sure which price was used to buy a product.
Not all transactions return price/currency information as documented in Transaction.java. For some backends it wasn't possible. In my opinion it is fine to return -1 (!) if the price is not known and 'null' if the currency is not know.
In my opinion that's fine. I store transactions on my server in a database. Although it's nice to have price/currency available where this is supported, leaving both -1/null is not a problem either. What's your take on that?
I send price of transaction to analytics services, so I can analyze users which spend money.
May be it is not so critical case then. So, I would just not provide real price information (-1 and null only) in case of restore of unfinished transactions, because it is unusual in general. Or what do you think?
I would think it will be quite unusual to happen. Also, you need to consider that the network might just go down when you send data to the analysis server, or the analysis server might be done. Also, there could be problems on the analysis server preventing a transaction to be processed. Even transactions with real price information might not make it to the analysis server database all the time.
In any case, we could add functionality to iOS gdx-pay which would query pricing information and only notify of the initially failed transaction once that pricing information is fully available. Still, this doesn't mean that every transaction will be recorded on the analysis server if problems there occur.
Up to you if you want to provide a workaround for the missing pricing information, i.e. query/notify only when pricing is available. I would think though it's somewhat difficult to reproduce/test and wouldn't solve all the problems.
In any case, we could add functionality to iOS gdx-pay which would query pricing information and only notify of the initially failed transaction once that pricing information is fully available.
That would require us to store transaction on the disk, because if we call SKPurchaseQueue.finishTransaction on transaction which is not known to the app yet, then gdx-pay is responsible to deliver it later - that is bad :)
So, I would still prefer to store price information before the purchase (e.g. in PurchaseManageriOSApple.purchase() method) and then use it for unfinished transaction.
But as you wrote, analytics services do not guarantee delivery of all events, so I would just pass -1 as a price in case of unfinished transaction for now. There are more important tasks than perfect fix for this issue!
And I will fix it soon.
Sounds good. Thinking about it, how you describe it, it sounds more of a shortfall of the iOS payment system not being able to obtain pricing information for purchase restores!? Maybe there is a way to still get the pricing information we are just not aware of it? There isn't much we can do on the gdx-pay side even if we were to change the gdx-pay architecture.
In any case, I agree, passing -1/null is fine for now. Not an urgent matter in my opinion.
Maybe there is a way to still get the pricing information we are just not aware of it?
No, I have double checked. But it could be there is a way to query unfinished transactions manually after we have pricing information. Sorry, my fault, I didn't mention this before, I will check it.
So, the fix was much easier than the whole conversation about it! :-D Sorry for your time and thanks for your doubts.
Great fixed - changes merged :-D
If anything bad is occurred during handling transaction in observer (like NPE, out of memory, shutdown of the device, or any other unexpected exit from the app), then transaction is not marked as finished. The consequences are:
It is written in the Apple docs, SQPaymentQueue should be called right at the app start, before any other action is performed with the queue. In the current version the transaction observer is set up after product information is fetched. I have tried to change gdx-pay to setup observer right inside PurchaseManager.install method and this helped to finish the transaction and get my gems, which I have paid for.
But the problem is SKPaymentQueue restores unfinished transactions before we have any information about products (like price and currency), which is required to fill gdxpay.Transaction object (see PurchaseManageriOSApple.transaction(SKPaymentTransaction)). So, seems it is kind of a dead loop.
Seems to fix this bug, we need an architectural change. Or do you have any other ideas?