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.

Sign up for GitHub

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

Jump to bottom

Fix OfferingsParser exceptions being swallowed #1228

Merged
merged 3 commits into from
Sep 7, 2023
Merged

Fix OfferingsParser exceptions being swallowed #1228

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f4cdc00
adds custom dispatcher
vegaro Sep 6, 2023
8bc250d
service
vegaro Sep 7, 2023
f246e86
fix compilation
vegaro Sep 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions purchases/src/main/kotlin/com/revenuecat/purchases/PurchasesFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ internal class PurchasesFactory(
val sharedPreferencesForETags = ETagManager.initializeSharedPreferences(context)
val eTagManager = ETagManager(sharedPreferencesForETags)

val dispatcher = Dispatcher(service ?: createDefaultExecutor(), runningIntegrationTests)
val dispatcher = Dispatcher(createDefaultExecutor(), runningIntegrationTests)
val backendDispatcher = Dispatcher(service ?: createDefaultExecutor(), runningIntegrationTests)
val diagnosticsDispatcher = Dispatcher(createDiagnosticsExecutor(), runningIntegrationTests)

var diagnosticsFileHelper: DiagnosticsFileHelper? = null
Expand All @@ -101,10 +102,10 @@ internal class PurchasesFactory(
val signingManager = SigningManager(signatureVerificationMode, appConfig, apiKey)

val httpClient = HTTPClient(appConfig, eTagManager, diagnosticsTracker, signingManager)
val backendHelper = BackendHelper(apiKey, dispatcher, appConfig, httpClient)
val backendHelper = BackendHelper(apiKey, backendDispatcher, appConfig, httpClient)
val backend = Backend(
appConfig,
dispatcher,
backendDispatcher,
diagnosticsDispatcher,
httpClient,
backendHelper,
Expand All @@ -123,7 +124,7 @@ internal class PurchasesFactory(

val subscriberAttributesPoster = SubscriberAttributesPoster(backendHelper)

val attributionFetcher = AttributionFetcherFactory.createAttributionFetcher(store, dispatcher)
val attributionFetcher = AttributionFetcherFactory.createAttributionFetcher(store, backendDispatcher)

val subscriberAttributesCache = SubscriberAttributesCache(cache)

Expand Down Expand Up @@ -182,7 +183,7 @@ internal class PurchasesFactory(
appConfig,
cache,
billing,
dispatcher,
backendDispatcher,
identityManager,
postTransactionWithProductDetailsHelper,
)
Expand Down Expand Up @@ -217,7 +218,7 @@ internal class PurchasesFactory(
val offeringsManager = OfferingsManager(
offeringsCache,
backend,
OfferingsFactory(billing, offeringParser),
OfferingsFactory(billing, offeringParser, dispatcher),
)

log(LogIntent.DEBUG, ConfigureStrings.DEBUG_ENABLED)
Expand Down
87 changes: 56 additions & 31 deletions purchases/src/main/kotlin/com/revenuecat/purchases/common/offerings/OfferingsFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,23 @@ import com.revenuecat.purchases.ProductType
import com.revenuecat.purchases.PurchasesError
import com.revenuecat.purchases.PurchasesErrorCode
import com.revenuecat.purchases.common.BillingAbstract
import com.revenuecat.purchases.common.Dispatcher
import com.revenuecat.purchases.common.LogIntent
import com.revenuecat.purchases.common.OfferingParser
import com.revenuecat.purchases.common.log
import com.revenuecat.purchases.models.StoreProduct
import com.revenuecat.purchases.strings.OfferingStrings
import kotlinx.serialization.SerializationException
import org.json.JSONException
import org.json.JSONObject

internal class OfferingsFactory(
private val billing: BillingAbstract,
private val offeringParser: OfferingParser,
private val dispatcher: Dispatcher,
) {

@SuppressWarnings("TooGenericExceptionCaught")
fun createOfferings(
offeringsJSON: JSONObject,
onError: (PurchasesError) -> Unit,
Expand All @@ -34,18 +38,35 @@ internal class OfferingsFactory(
)
} else {
getStoreProductsById(allRequestedProductIdentifiers, { productsById ->
logMissingProducts(allRequestedProductIdentifiers, productsById)
try {
logMissingProducts(allRequestedProductIdentifiers, productsById)

val offerings = offeringParser.createOfferings(offeringsJSON, productsById)
if (offerings.all.isEmpty()) {
onError(
PurchasesError(
PurchasesErrorCode.ConfigurationError,
OfferingStrings.CONFIGURATION_ERROR_PRODUCTS_NOT_FOUND,
),
)
} else {
onSuccess(offerings)
val offerings = offeringParser.createOfferings(offeringsJSON, productsById)
if (offerings.all.isEmpty()) {
onError(
PurchasesError(
PurchasesErrorCode.ConfigurationError,
OfferingStrings.CONFIGURATION_ERROR_PRODUCTS_NOT_FOUND,
),
)
} else {
onSuccess(offerings)
}
} catch (error: Exception) {
when (error) {
is JSONException, is SerializationException -> {
log(
LogIntent.RC_ERROR,
OfferingStrings.JSON_EXCEPTION_ERROR.format(error.localizedMessage),
)
onError(
PurchasesError(
PurchasesErrorCode.UnexpectedBackendResponseError,
error.localizedMessage,
),
)
} else -> throw error
}
}
}, { error ->
onError(error)
Expand Down Expand Up @@ -87,27 +108,31 @@ internal class OfferingsFactory(
ProductType.SUBS,
productIds,
{ subscriptionProducts ->
val productsById = subscriptionProducts
.groupBy { subProduct -> subProduct.purchasingData.productId }
.toMutableMap()
val subscriptionIds = productsById.keys
dispatcher.enqueue(command = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we should do this inside the BillingWrapper.queryProductDetailsAsync so it fixes possible problems in other places that call it... But I'm ok doing this for now and rethinking it later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true... But that might involve more testing since it might break other parts of the code. I think I am ok with doing this for now, then fixing all calls in BillingWrapper to return in our own threads

val productsById = subscriptionProducts
.groupBy { subProduct -> subProduct.purchasingData.productId }
.toMutableMap()
val subscriptionIds = productsById.keys

val inAppProductIds = productIds - subscriptionIds
if (inAppProductIds.isNotEmpty()) {
billing.queryProductDetailsAsync(
ProductType.INAPP,
inAppProductIds,
{ inAppProducts ->
productsById.putAll(inAppProducts.map { it.purchasingData.productId to listOf(it) })
onCompleted(productsById)
},
{
onError(it)
},
)
} else {
onCompleted(productsById)
}
val inAppProductIds = productIds - subscriptionIds
if (inAppProductIds.isNotEmpty()) {
billing.queryProductDetailsAsync(
ProductType.INAPP,
inAppProductIds,
{ inAppProducts ->
dispatcher.enqueue(command = {
productsById.putAll(inAppProducts.map { it.purchasingData.productId to listOf(it) })
onCompleted(productsById)
})
},
{
onError(it)
},
)
} else {
onCompleted(productsById)
}
})
},
{
onError(it)
Expand Down
11 changes: 7 additions & 4 deletions purchases/src/test/java/com/revenuecat/purchases/common/offerings/OfferingsFactoryTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import com.revenuecat.purchases.ProductType
import com.revenuecat.purchases.PurchasesError
import com.revenuecat.purchases.PurchasesErrorCode
import com.revenuecat.purchases.common.BillingAbstract
import com.revenuecat.purchases.common.Dispatcher
import com.revenuecat.purchases.common.GoogleOfferingParser
import com.revenuecat.purchases.common.OfferingParser
import com.revenuecat.purchases.models.StoreProduct
Expand All @@ -14,6 +15,7 @@ import com.revenuecat.purchases.utils.ONE_OFFERINGS_INAPP_PRODUCT_RESPONSE
import com.revenuecat.purchases.utils.ONE_OFFERINGS_RESPONSE
import com.revenuecat.purchases.utils.STUB_OFFERING_IDENTIFIER
import com.revenuecat.purchases.utils.STUB_PRODUCT_IDENTIFIER
import com.revenuecat.purchases.utils.SyncDispatcher
import com.revenuecat.purchases.utils.stubINAPPStoreProduct
import com.revenuecat.purchases.utils.stubStoreProduct
import com.revenuecat.purchases.utils.stubSubscriptionOption
Expand Down Expand Up @@ -43,17 +45,18 @@ class OfferingsFactoryTest {

private lateinit var billing: BillingAbstract
private lateinit var offeringParser: OfferingParser

private lateinit var dispatcher: Dispatcher
private lateinit var offeringsFactory: OfferingsFactory

@Before
fun setUp() {
billing = mockk()
offeringParser = GoogleOfferingParser()

dispatcher = SyncDispatcher()
offeringsFactory = OfferingsFactory(
billing = billing,
offeringParser = offeringParser
billing,
offeringParser,
dispatcher
)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not entirely sure what else to test here. The issue was due to billing.queryProductDetailsAsync( returning in a different thread, but we can't really test that 🤔


Expand Down