From 6fa293e42841f07e74f2ff055e141639ce4162a1 Mon Sep 17 00:00:00 2001 From: Samer Alabi <141707240+samer-stripe@users.noreply.github.com> Date: Fri, 17 Jan 2025 09:03:21 -0800 Subject: [PATCH] Attempt to wait for the intent callback to be available before failing next step retrieval. (#9927) --- .../payments/core/analytics/ErrorReporter.kt | 3 + .../intent/IntentConfirmationInterceptor.kt | 32 +++- ...efaultIntentConfirmationInterceptorTest.kt | 160 +++++++++++++----- 3 files changed, 153 insertions(+), 42 deletions(-) diff --git a/payments-core/src/main/java/com/stripe/android/payments/core/analytics/ErrorReporter.kt b/payments-core/src/main/java/com/stripe/android/payments/core/analytics/ErrorReporter.kt index eefb3306a74..57ac226f69e 100644 --- a/payments-core/src/main/java/com/stripe/android/payments/core/analytics/ErrorReporter.kt +++ b/payments-core/src/main/java/com/stripe/android/payments/core/analytics/ErrorReporter.kt @@ -268,6 +268,9 @@ interface ErrorReporter : FraudDetectionErrorReporter { EXTERNAL_PAYMENT_METHODS_LAUNCH_SUCCESS( eventName = "paymentsheet.external_payment_method.launch_success" ), + FOUND_CREATE_INTENT_CALLBACK_WHILE_POLLING( + eventName = "paymentsheet.polling_for_create_intent_callback.found" + ), } } diff --git a/paymentsheet/src/main/java/com/stripe/android/paymentelement/confirmation/intent/IntentConfirmationInterceptor.kt b/paymentsheet/src/main/java/com/stripe/android/paymentelement/confirmation/intent/IntentConfirmationInterceptor.kt index b9b65cd8a54..c7528e791f3 100644 --- a/paymentsheet/src/main/java/com/stripe/android/paymentelement/confirmation/intent/IntentConfirmationInterceptor.kt +++ b/paymentsheet/src/main/java/com/stripe/android/paymentelement/confirmation/intent/IntentConfirmationInterceptor.kt @@ -24,8 +24,11 @@ import com.stripe.android.paymentsheet.DeferredIntentValidator import com.stripe.android.paymentsheet.PaymentSheet import com.stripe.android.paymentsheet.R import com.stripe.android.paymentsheet.state.PaymentElementLoader +import kotlinx.coroutines.delay +import kotlinx.coroutines.withTimeoutOrNull import javax.inject.Inject import javax.inject.Named +import kotlin.time.Duration.Companion.seconds import com.stripe.android.R as PaymentsCoreR internal interface IntentConfirmationInterceptor { @@ -241,7 +244,7 @@ internal class DefaultIntentConfirmationInterceptor @Inject constructor( shippingValues: ConfirmPaymentIntentParams.Shipping?, shouldSavePaymentMethod: Boolean, ): NextStep { - return when (val callback = IntentConfirmationInterceptor.createIntentCallback) { + return when (val callback = waitForIntentCallback()) { is CreateIntentCallback -> { handleDeferredIntentCreationFromPaymentMethod( createIntentCallback = callback, @@ -280,6 +283,31 @@ internal class DefaultIntentConfirmationInterceptor @Inject constructor( ) } + private suspend fun waitForIntentCallback(): CreateIntentCallback? { + return retrieveCallback() ?: run { + val callback = withTimeoutOrNull(INTENT_CALLBACK_FETCH_TIMEOUT.seconds) { + var intentCallback: CreateIntentCallback? = null + + while (intentCallback == null) { + delay(INTENT_CALLBACK_FETCH_INTERVAL) + intentCallback = retrieveCallback() + } + + intentCallback + } + + if (callback != null) { + errorReporter.report(ErrorReporter.SuccessEvent.FOUND_CREATE_INTENT_CALLBACK_WHILE_POLLING) + } + + callback + } + } + + private fun retrieveCallback(): CreateIntentCallback? { + return IntentConfirmationInterceptor.createIntentCallback + } + private suspend fun handleDeferredIntentCreationFromPaymentMethod( createIntentCallback: CreateIntentCallback, intentConfiguration: PaymentSheet.IntentConfiguration, @@ -409,6 +437,8 @@ internal class DefaultIntentConfirmationInterceptor @Inject constructor( } private companion object { + private const val INTENT_CALLBACK_FETCH_TIMEOUT = 2 + private const val INTENT_CALLBACK_FETCH_INTERVAL = 5L private val GENERIC_STRIPE_MESSAGE = R.string.stripe_something_went_wrong } } diff --git a/paymentsheet/src/test/java/com/stripe/android/paymentelement/confirmation/DefaultIntentConfirmationInterceptorTest.kt b/paymentsheet/src/test/java/com/stripe/android/paymentelement/confirmation/DefaultIntentConfirmationInterceptorTest.kt index ba62a1dd647..0f8f62b16d5 100644 --- a/paymentsheet/src/test/java/com/stripe/android/paymentelement/confirmation/DefaultIntentConfirmationInterceptorTest.kt +++ b/paymentsheet/src/test/java/com/stripe/android/paymentelement/confirmation/DefaultIntentConfirmationInterceptorTest.kt @@ -3,6 +3,7 @@ package com.stripe.android.paymentelement.confirmation import com.google.common.truth.Truth.assertThat import com.stripe.android.core.exception.APIException import com.stripe.android.core.networking.ApiRequest +import com.stripe.android.core.strings.ResolvableString import com.stripe.android.core.strings.resolvableString import com.stripe.android.isInstanceOf import com.stripe.android.model.ConfirmPaymentIntentParams @@ -28,6 +29,8 @@ import com.stripe.android.testing.AbsFakeStripeRepository import com.stripe.android.testing.FakeErrorReporter import com.stripe.android.testing.PaymentMethodFactory import com.stripe.android.utils.IntentConfirmationInterceptorTestRule +import kotlinx.coroutines.async +import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.runTest import org.junit.Rule import org.junit.Test @@ -127,8 +130,9 @@ class DefaultIntentConfirmationInterceptorTest { } @Test - fun `Fails if invoked without a confirm callback for existing payment method`() = runTest { - val errorReporter = FakeErrorReporter() + fun `Fails if invoked without a confirm callback for existing payment method`() = testNoConfirmCallback( + userMessage = CREATE_INTENT_CALLBACK_MESSAGE.resolvableString, + ) { errorReporter -> val interceptor = DefaultIntentConfirmationInterceptor( stripeRepository = object : AbsFakeStripeRepository() {}, publishableKeyProvider = { "pk_test_123" }, @@ -137,7 +141,7 @@ class DefaultIntentConfirmationInterceptorTest { allowsManualConfirmation = false, ) - val nextStep = interceptor.intercept( + interceptor.intercept( initializationMode = InitializationMode.DeferredIntent( intentConfiguration = PaymentSheet.IntentConfiguration( mode = PaymentSheet.IntentConfiguration.Mode.Payment( @@ -150,23 +154,12 @@ class DefaultIntentConfirmationInterceptorTest { paymentMethodOptionsParams = null, shippingValues = null, ) - - assertThat(nextStep).isInstanceOf() - - val failedStep = nextStep.asFail() - - assertThat(failedStep.cause).isInstanceOf() - assertThat(failedStep.cause.message).isEqualTo(CREATE_INTENT_CALLBACK_MESSAGE) - assertThat(failedStep.message).isEqualTo(CREATE_INTENT_CALLBACK_MESSAGE.resolvableString) - - assertThat(errorReporter.getLoggedErrors()).containsExactly( - ErrorReporter.ExpectedErrorEvent.CREATE_INTENT_CALLBACK_NULL.eventName, - ) } @Test - fun `Fails if invoked without a confirm callback for new payment method`() = runTest { - val errorReporter = FakeErrorReporter() + fun `Fails if invoked without a confirm callback for new payment method`() = testNoConfirmCallback( + userMessage = CREATE_INTENT_CALLBACK_MESSAGE.resolvableString, + ) { errorReporter -> val interceptor = DefaultIntentConfirmationInterceptor( stripeRepository = object : AbsFakeStripeRepository() { override suspend fun createPaymentMethod( @@ -182,29 +175,18 @@ class DefaultIntentConfirmationInterceptorTest { allowsManualConfirmation = false, ) - val nextStep = interceptor.intercept( + interceptor.intercept( initializationMode = InitializationMode.DeferredIntent(mock()), paymentMethodCreateParams = PaymentMethodCreateParamsFixtures.DEFAULT_CARD, shippingValues = null, customerRequestedSave = false, ) - - assertThat(nextStep).isInstanceOf() - - val failedStep = nextStep.asFail() - - assertThat(failedStep.cause).isInstanceOf() - assertThat(failedStep.cause.message).isEqualTo(CREATE_INTENT_CALLBACK_MESSAGE) - assertThat(failedStep.message).isEqualTo(CREATE_INTENT_CALLBACK_MESSAGE.resolvableString) - - assertThat(errorReporter.getLoggedErrors()).containsExactly( - ErrorReporter.ExpectedErrorEvent.CREATE_INTENT_CALLBACK_NULL.eventName, - ) } @Test - fun `Message for live key when error without confirm callback is user friendly`() = runTest { - val errorReporter = FakeErrorReporter() + fun `Message for live key when error without confirm callback is user friendly`() = testNoConfirmCallback( + userMessage = PaymentsCoreR.string.stripe_internal_error.resolvableString + ) { errorReporter -> val interceptor = DefaultIntentConfirmationInterceptor( stripeRepository = object : AbsFakeStripeRepository() {}, publishableKeyProvider = { "pk_live_123" }, @@ -213,7 +195,7 @@ class DefaultIntentConfirmationInterceptorTest { allowsManualConfirmation = false, ) - val nextStep = interceptor.intercept( + interceptor.intercept( initializationMode = InitializationMode.DeferredIntent( intentConfiguration = PaymentSheet.IntentConfiguration( mode = PaymentSheet.IntentConfiguration.Mode.Payment( @@ -226,18 +208,72 @@ class DefaultIntentConfirmationInterceptorTest { paymentMethodOptionsParams = null, shippingValues = null, ) + } - assertThat(nextStep).isInstanceOf() + @Test + fun `Succeeds if callback is found before timeout time`() { + val dispatcher = StandardTestDispatcher() + + runTest(dispatcher) { + val errorReporter = FakeErrorReporter() + val paymentMethod = PaymentMethodFactory.card() + val interceptor = DefaultIntentConfirmationInterceptor( + stripeRepository = object : AbsFakeStripeRepository() { + override suspend fun createPaymentMethod( + paymentMethodCreateParams: PaymentMethodCreateParams, + options: ApiRequest.Options + ): Result { + return Result.success(paymentMethod) + } + + override suspend fun retrieveStripeIntent( + clientSecret: String, + options: ApiRequest.Options, + expandFields: List + ): Result { + return Result.success(PaymentIntentFixtures.PI_SUCCEEDED) + } + }, + publishableKeyProvider = { "pk_live_123" }, + stripeAccountIdProvider = { null }, + errorReporter = errorReporter, + allowsManualConfirmation = false, + ) - val failedStep = nextStep.asFail() + val interceptJob = async { + interceptor.intercept( + initializationMode = InitializationMode.DeferredIntent( + intentConfiguration = PaymentSheet.IntentConfiguration( + mode = PaymentSheet.IntentConfiguration.Mode.Payment( + amount = 1099L, + currency = "usd", + ), + ), + ), + paymentMethod = paymentMethod, + paymentMethodOptionsParams = null, + shippingValues = null, + ) + } - assertThat(failedStep.cause).isInstanceOf() - assertThat(failedStep.cause.message).isEqualTo(CREATE_INTENT_CALLBACK_MESSAGE) - assertThat(failedStep.message).isEqualTo(PaymentsCoreR.string.stripe_internal_error.resolvableString) + dispatcher.scheduler.advanceTimeBy(1000) + assertThat(interceptJob.isActive).isTrue() - assertThat(errorReporter.getLoggedErrors()).containsExactly( - ErrorReporter.ExpectedErrorEvent.CREATE_INTENT_CALLBACK_NULL.eventName, - ) + IntentConfirmationInterceptor.createIntentCallback = succeedingCreateIntentCallback(paymentMethod) + + dispatcher.scheduler.advanceTimeBy(1001) + + assertThat(interceptJob.isActive).isFalse() + assertThat(interceptJob.isCompleted).isTrue() + + val nextStep = interceptJob.await() + + assertThat(nextStep).isInstanceOf() + + assertThat(errorReporter.getLoggedErrors()).containsExactly( + ErrorReporter.SuccessEvent.FOUND_CREATE_INTENT_CALLBACK_WHILE_POLLING.eventName, + ) + } } @Test @@ -637,6 +673,48 @@ class DefaultIntentConfirmationInterceptorTest { ) } + private fun testNoConfirmCallback( + userMessage: ResolvableString, + interceptCall: suspend (errorReporter: ErrorReporter) -> IntentConfirmationInterceptor.NextStep + ) { + val errorReporter = FakeErrorReporter() + val dispatcher = StandardTestDispatcher() + + runTest(dispatcher) { + val interceptJob = async { + interceptCall(errorReporter) + } + + assertThat(interceptJob.isActive).isTrue() + + dispatcher.scheduler.advanceTimeBy(1000) + + assertThat(interceptJob.isActive).isTrue() + + dispatcher.scheduler.advanceTimeBy(1000) + + assertThat(interceptJob.isActive).isTrue() + + dispatcher.scheduler.advanceTimeBy(1) + + assertThat(interceptJob.isActive).isFalse() + + val nextStep = interceptJob.await() + + assertThat(nextStep).isInstanceOf() + + val failedStep = nextStep.asFail() + + assertThat(failedStep.cause).isInstanceOf() + assertThat(failedStep.cause.message).isEqualTo(CREATE_INTENT_CALLBACK_MESSAGE) + assertThat(failedStep.message).isEqualTo(userMessage) + + assertThat(errorReporter.getLoggedErrors()).containsExactly( + ErrorReporter.ExpectedErrorEvent.CREATE_INTENT_CALLBACK_NULL.eventName, + ) + } + } + private fun succeedingCreateIntentCallback( expectedPaymentMethod: PaymentMethod, ): CreateIntentCallback {