Properly Map Galaxy Errors -> PurchasesError#2927
Conversation
| IAP_ERROR_CONNECT_TIMEOUT(-1011), | ||
| IAP_ERROR_NOT_EXIST_LOCAL_PRICE(-1012), | ||
| IAP_ERROR_NOT_AVAILABLE_SHOP(-1013), | ||
| IAP_ERROR_INVALID_ACCESS_TOKEN(-1015), |
There was a problem hiding this comment.
This isn't a typo, there is no error with the code -1014 in the docs: https://developer.samsung.com/iap/programming-guide/iap-helper-programming.html#ErrorVo-and-Response-code
| -> PurchasesErrorCode.StoreProblemError | ||
| GalaxyErrorCode.IAP_ERROR_NOT_AVAILABLE_SHOP -> PurchasesErrorCode.PurchaseNotAllowedError | ||
| GalaxyErrorCode.IAP_ERROR_INVALID_ACCESS_TOKEN -> PurchasesErrorCode.InvalidCredentialsError | ||
| GalaxyErrorCode.IAP_ERROR_NONE -> return null // This means that the ErrorVo isn't an error |
There was a problem hiding this comment.
It didn't feel right to return a PurchasesError with an unknown code or something like that here, since this galaxy error code explicitly means that an error didn't occur.
This code really shouldn't be executed, since most calling code will be checking our ErrorVo.isError() function, but I still added this here to exercise defensive programming.
There was a problem hiding this comment.
Hmm I guess the main problem is that this makes the return nullable, which we need to handle everywhere... Maybe it's worth returning the unknown message here but just log something before that?
There was a problem hiding this comment.
Yeah, it definitely muddies up the call sites. Updated to return an UnknownError in 132448c 👍
| GalaxyErrorCode.IAP_ERROR_INVALID_ACCESS_TOKEN to PurchasesErrorCode.InvalidCredentialsError, | ||
| ) | ||
|
|
||
| expectedMappings.forEach { (galaxyCode, purchasesErrorCode) -> |
There was a problem hiding this comment.
Instead of iterating over the hardcoding mappings here, should we iterate over all values of the GalaxyErrorcode enum, so we make sure we catch all of them? Then, we can convert expectedMappings to a map, for easier access to the expected PurchasesErrorCode.
There was a problem hiding this comment.
I love that idea! That'll make sure that we remember to add future mappings to this test :)
Made the change in 09c48d5
| -> PurchasesErrorCode.StoreProblemError | ||
| GalaxyErrorCode.IAP_ERROR_NOT_AVAILABLE_SHOP -> PurchasesErrorCode.PurchaseNotAllowedError | ||
| GalaxyErrorCode.IAP_ERROR_INVALID_ACCESS_TOKEN -> PurchasesErrorCode.InvalidCredentialsError | ||
| GalaxyErrorCode.IAP_ERROR_NONE -> return null // This means that the ErrorVo isn't an error |
There was a problem hiding this comment.
Hmm I guess the main problem is that this makes the return nullable, which we need to handle everywhere... Maybe it's worth returning the unknown message here but just log something before that?
Description
This PR extends the
Error.toPurchasesError()function to properly map all error codes that are documented to us in the Samsung SDK's documentation to aPurchaseErrorwith the most appropriatePurchaseErrorCode. This will allow us to return the most descriptive error as we can when we receive an error from the Galaxy Store.