Fix crash when StoreKit returns nil original transaction

[ekurutepe]

While testing restore functionality with a sandbox user I ran into a crash where the app would persistently crash due to StoreKit returning nil for transaction.original. The crash repeats infinitely since the unfinished transaction stays in the queue and replays at next app launch.

Nit: do we really need original here at all? completeRestorePurchase only seems to need the product ID. Is it possible for the product ID to change for a transaction?

[benjaminmayo]

Are you sure that was the bug? I've never seen that return nil before. It's definitely not supposed to work like that. The StoreKit property is documented to be valid when the transactionState is restored, which is why I force unwrap it in that part of the switch.

[benjaminmayo]

In the future, MerchantKit may want to exposes properties from these transactions, and properties like the transactionIdentifier do differ. I'd like to see if we can work out why it is has been set to nil when the state is restored. If it is just a weird StoreKit bug then I guess we have no choice here but to do the ?? dance.

[benjaminmayo]

On the sandbox environment, did you delete an In-App Purchase entry through App Store Connect? I saw something on the web that StoreKit cannot provide a value for originalTransaction for restored transactions in that case.

[ekurutepe]

I did not change any IAP through App Store Connect. I deleted and reinstalled the app and then tapped Restore Purchases on my paywall and it crashed on this line because transaction.original was nil. I can try to reproduce it tomorrow. I know this should not be happening but it did. I blame the sandbox env because it's known to have bunch of issues.

[benjaminmayo]

Yeah, StoreKit has thorny edges everywhere — hence making a framework in the first place. What I’m trying to work out is if it is more semantically accurate to nil-coalesce here or catch transactions with no originalTransaction and finish the transaction without propagating it through the rest of the Merchant infrastructure, effectively silencing events.

[benjaminmayo]

I have tentatively edited the branch with my preferred 'fix' here. The ?? feels too implicit. I opted to rely on the base transaction and add a comment as a reminder that further investigation will be needed. Assuming you are happy, I will merge it.

[ekurutepe]

Looks great to me. I agree this is a better solution.

[benjaminmayo]

Thanks for bringing this PR to fruition. I incorporated this fix (and a couple of other minor changes) into the 0.11.1 release.