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

Add error reporting to GooglePayRepository #8218

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add error reporting to GooglePayRepository #8218

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
eb92959
Add error reporting to `GooglePayRepository`
samer-stripe Apr 4, 2024
65b00d5
Remove constructor invocations of `RealErrorReporter`
samer-stripe Apr 5, 2024
e59d532
Remove constructor changes to `GooglePayLauncher`
samer-stripe Apr 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 22 additions & 5 deletions payments-core/src/main/java/com/stripe/android/googlepaylauncher/GooglePayLauncher.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import com.stripe.android.model.PaymentIntent
import com.stripe.android.model.SetupIntent
import com.stripe.android.networking.PaymentAnalyticsEvent
import com.stripe.android.networking.PaymentAnalyticsRequestFactory
import com.stripe.android.payments.core.analytics.ErrorReporter
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.launch
Expand Down Expand Up @@ -71,12 +72,18 @@ class GooglePayLauncher internal constructor(
resultCallback.onResult(it)
},
googlePayRepositoryFactory = {
val context = activity.application

DefaultGooglePayRepository(
context = activity.application,
context = context,
environment = config.environment,
billingAddressParameters = config.billingAddressConfig.convert(),
existingPaymentMethodRequired = config.existingPaymentMethodRequired,
allowCreditCards = config.allowCreditCards
allowCreditCards = config.allowCreditCards,
errorReporter = ErrorReporter.createFallbackInstance(
context = context,
productUsage = setOf(PRODUCT_USAGE),
)
)
},
PaymentAnalyticsRequestFactory(
Expand Down Expand Up @@ -112,12 +119,18 @@ class GooglePayLauncher internal constructor(
resultCallback::onResult,
),
googlePayRepositoryFactory = {
val context = fragment.requireActivity().application

DefaultGooglePayRepository(
context = fragment.requireActivity().application,
context = context,
environment = config.environment,
billingAddressParameters = config.billingAddressConfig.convert(),
existingPaymentMethodRequired = config.existingPaymentMethodRequired,
allowCreditCards = config.allowCreditCards
allowCreditCards = config.allowCreditCards,
errorReporter = ErrorReporter.createFallbackInstance(
context = context,
productUsage = setOf(PRODUCT_USAGE)
)
)
},
paymentAnalyticsRequestFactory = PaymentAnalyticsRequestFactory(
Expand Down Expand Up @@ -369,7 +382,11 @@ fun rememberGooglePayLauncher(
environment = config.environment,
billingAddressParameters = config.billingAddressConfig.convert(),
existingPaymentMethodRequired = config.existingPaymentMethodRequired,
allowCreditCards = config.allowCreditCards
allowCreditCards = config.allowCreditCards,
errorReporter = ErrorReporter.createFallbackInstance(
context = context,
productUsage = setOf(GooglePayLauncher.PRODUCT_USAGE)
)
)
},
PaymentAnalyticsRequestFactory(
Expand Down
7 changes: 7 additions & 0 deletions ...nts-core/src/main/java/com/stripe/android/googlepaylauncher/GooglePayLauncherViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import com.stripe.android.model.StripeIntent
import com.stripe.android.networking.PaymentAnalyticsRequestFactory
import com.stripe.android.networking.StripeApiRepository
import com.stripe.android.networking.StripeRepository
import com.stripe.android.payments.core.analytics.ErrorReporter
import com.stripe.android.utils.mapResult
import com.stripe.android.view.AuthActivityStarterHost
import kotlinx.coroutines.Dispatchers
Expand Down Expand Up @@ -261,12 +262,18 @@ internal class GooglePayLauncherViewModel(
paymentAnalyticsRequestFactory = analyticsRequestFactory
)

val errorReporter = ErrorReporter.createFallbackInstance(
context = application,
productUsage = productUsageTokens
)

val googlePayRepository = DefaultGooglePayRepository(
context = application,
environment = args.config.environment,
billingAddressParameters = args.config.billingAddressConfig.convert(),
existingPaymentMethodRequired = args.config.existingPaymentMethodRequired,
allowCreditCards = args.config.allowCreditCards,
errorReporter = errorReporter,
logger = logger
)

Expand Down
7 changes: 6 additions & 1 deletion ...core/src/main/java/com/stripe/android/googlepaylauncher/GooglePayPaymentMethodLauncher.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import com.stripe.android.googlepaylauncher.GooglePayPaymentMethodLauncher.Resul
import com.stripe.android.model.PaymentMethod
import com.stripe.android.networking.PaymentAnalyticsEvent
import com.stripe.android.networking.PaymentAnalyticsRequestFactory
import com.stripe.android.payments.core.analytics.ErrorReporter
import dagger.assisted.Assisted
import dagger.assisted.AssistedInject
import kotlinx.coroutines.CoroutineScope
Expand Down Expand Up @@ -130,7 +131,11 @@ class GooglePayPaymentMethodLauncher @AssistedInject internal constructor(
environment = config.environment,
billingAddressParameters = config.billingAddressConfig.convert(),
existingPaymentMethodRequired = config.existingPaymentMethodRequired,
allowCreditCards = config.allowCreditCards
allowCreditCards = config.allowCreditCards,
errorReporter = ErrorReporter.createFallbackInstance(
context = context,
productUsage = setOf(PRODUCT_USAGE_TOKEN),
)
)
},
)
Expand Down
17 changes: 15 additions & 2 deletions payments-core/src/main/java/com/stripe/android/googlepaylauncher/GooglePayRepository.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import com.google.android.gms.wallet.IsReadyToPayRequest
import com.google.android.gms.wallet.PaymentsClient
import com.stripe.android.GooglePayJsonFactory
import com.stripe.android.core.Logger
import com.stripe.android.core.exception.StripeException
import com.stripe.android.payments.core.analytics.ErrorReporter
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.flow
import kotlinx.coroutines.flow.flowOf
Expand Down Expand Up @@ -32,6 +34,7 @@ internal class DefaultGooglePayRepository(
private val existingPaymentMethodRequired: Boolean,
private val allowCreditCards: Boolean,
private val paymentsClientFactory: PaymentsClientFactory = DefaultPaymentsClientFactory(context),
private val errorReporter: ErrorReporter,
private val logger: Logger = Logger.noop(),
) : GooglePayRepository {

Expand All @@ -40,13 +43,15 @@ internal class DefaultGooglePayRepository(
context: Context,
googlePayConfig: GooglePayPaymentMethodLauncher.Config,
logger: Logger,
errorReporter: ErrorReporter,
) : this(
context.applicationContext,
googlePayConfig.environment,
googlePayConfig.billingAddressConfig.convert(),
googlePayConfig.existingPaymentMethodRequired,
googlePayConfig.allowCreditCards,
DefaultPaymentsClientFactory(context),
errorReporter,
logger
)

Expand Down Expand Up @@ -76,7 +81,11 @@ internal class DefaultGooglePayRepository(
).toString()
)
}.getOrElse {
// TODO (samer-stripe): Add unexpected error event here
errorReporter.report(
ErrorReporter.UnexpectedErrorEvent.GOOGLE_PAY_JSON_REQUEST_PARSING,
StripeException.create(it)
)

logger.error("Google Pay json parsing failed.", it)

return false
Expand All @@ -85,7 +94,11 @@ internal class DefaultGooglePayRepository(
val isReady = runCatching {
paymentsClient.isReadyToPay(request).await()
}.onFailure {
// TODO (samer-stripe): Add error event here
errorReporter.report(
ErrorReporter.ExpectedErrorEvent.GOOGLE_PAY_IS_READY_API_CALL,
StripeException.create(it)
)

logger.error("Google Pay check failed.", it)
}.getOrDefault(false)

Expand Down
5 changes: 4 additions & 1 deletion ...e/src/main/java/com/stripe/android/googlepaylauncher/injection/GooglePayLauncherModule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import com.stripe.android.core.Logger
import com.stripe.android.googlepaylauncher.DefaultGooglePayRepository
import com.stripe.android.googlepaylauncher.GooglePayEnvironment
import com.stripe.android.googlepaylauncher.GooglePayRepository
import com.stripe.android.payments.core.analytics.ErrorReporter
import dagger.Module
import dagger.Provides

Expand All @@ -19,14 +20,16 @@ class GooglePayLauncherModule {
@Provides
fun provideGooglePayRepositoryFactory(
appContext: Context,
logger: Logger
logger: Logger,
errorReporter: ErrorReporter,
): (GooglePayEnvironment) -> GooglePayRepository = { environment ->
DefaultGooglePayRepository(
appContext,
environment,
GooglePayJsonFactory.BillingAddressParameters(),
existingPaymentMethodRequired = true,
allowCreditCards = true,
errorReporter = errorReporter,
logger = logger
)
}
Expand Down
16 changes: 16 additions & 0 deletions ...va/com/stripe/android/googlepaylauncher/injection/GooglePayPaymentMethodLauncherModule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
package com.stripe.android.googlepaylauncher.injection

import com.stripe.android.core.networking.AnalyticsRequestFactory
import com.stripe.android.googlepaylauncher.DefaultGooglePayRepository
import com.stripe.android.googlepaylauncher.DefaultPaymentsClientFactory
import com.stripe.android.googlepaylauncher.GooglePayPaymentMethodLauncher
import com.stripe.android.googlepaylauncher.GooglePayRepository
import com.stripe.android.googlepaylauncher.PaymentsClientFactory
import com.stripe.android.networking.PaymentAnalyticsRequestFactory
import com.stripe.android.payments.core.analytics.ErrorReporter
import com.stripe.android.payments.core.analytics.RealErrorReporter
import dagger.Binds
import dagger.Module
import dagger.Provides
Expand All @@ -27,6 +31,18 @@ internal abstract class GooglePayPaymentMethodLauncherModule {
defaultPaymentsClientFactory: DefaultPaymentsClientFactory
): PaymentsClientFactory

@Binds
@Singleton
abstract fun bindsAnalyticsRequestFactory(
paymentAnalyticsRequestFactory: PaymentAnalyticsRequestFactory,
): AnalyticsRequestFactory

@Binds
@Singleton
abstract fun bindsErrorReporter(
realErrorReporter: RealErrorReporter
): ErrorReporter

companion object {
@Provides
@Singleton
Expand Down
6 changes: 6 additions & 0 deletions payments-core/src/main/java/com/stripe/android/payments/core/analytics/ErrorReporter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ interface ErrorReporter {
GET_SAVED_PAYMENT_METHODS_FAILURE(
eventName = "elements.customer_repository.get_saved_payment_methods_failure"
),
GOOGLE_PAY_IS_READY_API_CALL(
eventName = "elements.google_pay_repository.is_ready_request_api_call_failure"
),
LINK_CREATE_CARD_FAILURE(
eventName = "link.create_new_card.create_payment_details_failure"
),
Expand All @@ -108,6 +111,9 @@ interface ErrorReporter {
MISSING_HOSTED_VOUCHER_URL(
partialEventName = "payments.missing_hosted_voucher_url"
),
GOOGLE_PAY_JSON_REQUEST_PARSING(
partialEventName = "google_pay_repository.is_ready_request_json_parsing_failure"
),
LINK_INVALID_SESSION_STATE(
partialEventName = "link.signup.failure.invalidSessionState"
),
Expand Down
14 changes: 12 additions & 2 deletions payments-core/src/test/java/com/stripe/android/googlepaylauncher/GooglePayRepositoryTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import com.google.android.gms.wallet.PaymentsClient
import com.stripe.android.GooglePayJsonFactory
import com.stripe.android.PaymentConfiguration
import com.stripe.android.core.Logger
import com.stripe.android.payments.core.analytics.ErrorReporter
import com.stripe.android.testing.FakeErrorReporter
import kotlinx.coroutines.test.runTest
import org.junit.Before
import org.junit.Test
Expand Down Expand Up @@ -61,24 +63,31 @@ class GooglePayRepositoryTest {
}

@Test
fun `when google pay is ready request fails, 'isReady' should return false`() = runTest {
fun `when google pay is ready request fails, 'isReady' should be false & error should be reported`() = runTest {
val paymentsClient = mock<PaymentsClient>()
val errorReporter = FakeErrorReporter()

whenever(paymentsClient.isReadyToPay(any())) doReturn Tasks.forException(
ApiException(Status.RESULT_INTERNAL_ERROR)
)

val repository = createGooglePayRepository(paymentsClient)
val repository = createGooglePayRepository(paymentsClient, errorReporter)

repository.isReady().test {
assertEquals(false, awaitItem())

assertEquals(
ErrorReporter.ExpectedErrorEvent.GOOGLE_PAY_IS_READY_API_CALL.eventName,
errorReporter.getLoggedErrors().first(),
)

cancelAndIgnoreRemainingEvents()
}
}

private fun createGooglePayRepository(
paymentsClient: PaymentsClient,
errorReporter: ErrorReporter = FakeErrorReporter()
): DefaultGooglePayRepository {
return DefaultGooglePayRepository(
context = context,
Expand All @@ -87,6 +96,7 @@ class GooglePayRepositoryTest {
existingPaymentMethodRequired = true,
allowCreditCards = true,
paymentsClientFactory = { paymentsClient },
errorReporter = errorReporter,
logger = Logger.noop(),
)
}
Expand Down
2 changes: 2 additions & 0 deletions ...ntsheet/src/androidTest/java/com/stripe/android/paymentsheet/PaymentSheetAnalyticsTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ internal class PaymentSheetAnalyticsTest {
validateAnalyticsRequest(eventName = "mc_load_started")
validateAnalyticsRequest(eventName = "mc_load_succeeded")
validateAnalyticsRequest(eventName = "mc_complete_sheet_newpm_show")
validateAnalyticsRequest(eventName = "elements.google_pay_repository.is_ready_request_api_call_failure")
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could also turn off google pay for these tests.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We always run an operation to check if the device is supported by Google Pay irregardless of if Google Pay is enabled in the config which is why this failure happens. I could disable this check in debug 😬

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we check even if google pay is disabled coming from elements/sessions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup! iOS does this as well with Apple Pay

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we return a different mocked response so that we don't do this then?

validateAnalyticsRequest(eventName = "mc_form_shown")

testContext.presentPaymentSheet {
Expand Down Expand Up @@ -107,6 +108,7 @@ internal class PaymentSheetAnalyticsTest {
validateAnalyticsRequest(eventName = "mc_custom_init_default")
validateAnalyticsRequest(eventName = "mc_load_started")
validateAnalyticsRequest(eventName = "mc_load_succeeded")
validateAnalyticsRequest(eventName = "elements.google_pay_repository.is_ready_request_api_call_failure")
validateAnalyticsRequest(eventName = "mc_custom_sheet_newpm_show")
validateAnalyticsRequest(eventName = "mc_form_shown")

Expand Down
7 changes: 7 additions & 0 deletions .../src/main/java/com/stripe/android/customersheet/injection/CustomerSheetViewModelModule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import com.stripe.android.customersheet.CustomerSheetViewState
import com.stripe.android.customersheet.DefaultCustomerSheetLoader
import com.stripe.android.customersheet.analytics.CustomerSheetEventReporter
import com.stripe.android.customersheet.analytics.DefaultCustomerSheetEventReporter
import com.stripe.android.payments.core.analytics.ErrorReporter
import com.stripe.android.payments.core.analytics.RealErrorReporter
import com.stripe.android.payments.core.injection.PRODUCT_USAGE
import com.stripe.android.payments.financialconnections.DefaultIsFinancialConnectionsAvailable
import com.stripe.android.payments.financialconnections.IsFinancialConnectionsAvailable
Expand Down Expand Up @@ -58,6 +60,11 @@ internal interface CustomerSheetViewModelModule {
impl: DefaultCustomerSheetLoader
): CustomerSheetLoader

@Binds
fun bindsErrorReporter(
impl: RealErrorReporter
): ErrorReporter

@Binds
fun bindsStripeIntentRepository(
impl: RealElementsSessionRepository,
Expand Down