Adyen / adyen-android

Adyen Android Drop-in and Components
https://docs.adyen.com/checkout/android
MIT License
119 stars 66 forks source link

[v5.3.0] `onAdditionalDetails` called twice with BLIK payment #1524

Closed wrozwad closed 3 weeks ago

wrozwad commented 1 month ago

Describe the bug I missed you 🥹. The overloaded callback onAdditionalDetails() in SessionDropInService is triggered twice during BLIK payment.

To Reproduce

  1. Start the DropIn payment process with passing a custom SessionDropInService in DropIn.startPayment() function
  2. Override onSubmit() and onAdditionalDetails() with custom API requests
  3. Complete a successful BLIK payment
  4. After a single onSubmit() trigger, you'll observe two invocations of onAdditionalDetails()
    override fun onAdditionalDetails(actionComponentData: ActionComponentData): Boolean {
    Log.d("TEST", "onAdditionalDetails: $actionComponentData")
    // do api call
    return true
    }

Expected behavior onAdditionalDetails() should be triggered only once during the BLIK payment.

SDK Version 2.3.0

OscarSpruit commented 1 month ago

Hi @wrozwad, good to have you back, we missed you too! We were not able to reproduce this behaviour. Could you add the following log line first thing to onSubmit and onAdditionalDetails and share the logs with us? Log.d("TEST", "Is flow taken over: $isFlowTakenOver") Could you also share the implementation of onSubmit and onAdditionalDetails?

wrozwad commented 1 month ago

The TEST logs for BLIK look as follows:

10:33:22.482  D  onSubmit: Is flow taken over: false
10:33:22.483  D  onSubmit, try setPaymentMethodAndPlaceOrder mutation
10:33:24.352  D  onSubmit, sendResult: Action
10:33:25.037  D  onAdditionalDetails: Is flow taken over: true
10:33:25.038  D  onAdditionalDetails, try adyenPaymentDetails mutation
10:33:25.111  D  onAdditionalDetails: Is flow taken over: true
10:33:25.112  D  onAdditionalDetails, try adyenPaymentDetails mutation
10:33:25.698  D  onAdditionalDetails, sendResult: Finished
10:33:25.967  D  onAdditionalDetails, sendResult: Error onFailure

In comparison, "onlineBanking_PL" looks as follows:

10:35:10.459  D  onSubmit: Is flow taken over: false
10:35:10.460  D  onSubmit, try setPaymentMethodAndPlaceOrder mutation
10:35:11.931  D  onSubmit, sendResult: Action
10:35:42.870  D  onAdditionalDetails: Is flow taken over: true
10:35:42.871  D  onAdditionalDetails, try adyenPaymentDetails mutation
10:35:44.432  D  onAdditionalDetails, sendResult: Finished

My "work in progress" implementation of onSubmit() and onAdditionalDetails(), essentially the entire SessionDropInService, looks like this:

@AndroidEntryPoint
class CheckoutSessionDropInService : SessionDropInService() {

    @Inject
    @IoScope
    internal lateinit var scope: CoroutineScope

    @Inject
    internal lateinit var repository: CheckoutRepository

    override fun onSubmit(state: PaymentComponentState<*>): Boolean {
        Log.d("TEST", "onSubmit: Is flow taken over: $isFlowTakenOver")
        if (state.isValid) {
            scope.launch {
                runCatching {
                    Log.d("TEST", "onSubmit, try setPaymentMethodAndPlaceOrder mutation")
                    repository.setPaymentMethodAndPlaceOrder(state)
                }.fold(
                    onSuccess = {
                        sendResult(
                            when (val paymentStatus = it.paymentStatus) {
                                PaymentStatus.Success -> {
                                    Log.d("TEST", "onSubmit, sendResult: Finished")
                                    DropInServiceResult.Finished(it.orderNumber.value)
                                }

                                is PaymentStatus.PendingAction -> {
                                    Log.d("TEST", "onSubmit, sendResult: Action")
                                    DropInServiceResult.Action(paymentStatus.action)
                                }

                                is PaymentStatus.Failure -> {
                                    Log.d("TEST", "onSubmit, sendResult: Error onSuccess")

                                    // The order has already been placed,
                                    // but there was an issue with the payment.
                                    // To distinguish this situation and redirect the user
                                    // to the payment status error screen,
                                    // we set an empty `reason`
                                    DropInServiceResult.Error(
                                        errorDialog = null,
                                        reason = null,
                                        dismissDropIn = true,
                                    )
                                }
                            }
                        )
                    },
                    onFailure = {
                        Log.d("TEST", "onSubmit, sendResult: Error onFailure")
                        sendResult(
                            // The order has still not been placed at this moment,
                            // so it's safe to dismiss DropIn and display a snack message
                            DropInServiceResult.Error(
                                errorDialog = null,
                                reason = it.localizedMessage,
                                dismissDropIn = true,
                            )
                        )
                    },
                )
            }
        }

        return true
    }

    override fun onAdditionalDetails(actionComponentData: ActionComponentData): Boolean {
        Log.d("TEST", "onAdditionalDetails: Is flow taken over: $isFlowTakenOver")
        scope.launch {
            runCatching {
                Log.d("TEST", "onAdditionalDetails, try adyenPaymentDetails mutation")
                repository.checkPaymentStatus(actionComponentData)
            }.fold(
                onSuccess = { (status, orderNumber) ->
                    sendResult(
                        // TODO map AdyenPaymentStatus to PaymentStatus
                        when {
                            status.isFinal == true && status.action == null -> {
                                Log.d("TEST", "onAdditionalDetails, sendResult: Finished")
                                DropInServiceResult.Finished(orderNumber.value)
                            }

                            status.action != null -> {
                                Log.d("TEST", "onAdditionalDetails, sendResult: Action")
                                DropInServiceResult.Action(
                                    Action.SERIALIZER.deserialize(
                                        JSONObject(status.action!!)
                                    ),
                                )
                            }

                            else -> {
                                Log.d("TEST", "onAdditionalDetails, sendResult: Error onSuccess")

                                // The order has already been placed,
                                // but there was an issue with the payment.
                                // To distinguish this situation and redirect the user
                                // to the payment status error screen,
                                // we set an empty `reason`
                                DropInServiceResult.Error(
                                    errorDialog = null,
                                    reason = null,
                                    dismissDropIn = true,
                                )
                            }
                        }
                    )
                },
                onFailure = {
                    Log.d("TEST", "onAdditionalDetails, sendResult: Error onFailure")
                    sendResult(
                        DropInServiceResult.Error(
                            errorDialog = ErrorDialog(it.localizedMessage),
                            reason = it.localizedMessage,
                            dismissDropIn = false,
                        )
                    )
                },
            )
        }

        return true
    }
}

The first and the second onAdditionalDetails for BLIK, and the single one invocation for "onlineBanking_PL", were called with the same stack:

onAdditionalDetails:88, CheckoutSessionDropInService (pl.superpharm.checkout)
invoke:119, SessionDropInService$requestDetailsCall$1$result$1 (com.adyen.checkout.dropin)
invoke:119, SessionDropInService$requestDetailsCall$1$result$1 (com.adyen.checkout.dropin)
invokeSuspend:97, SessionInteractor$onDetailsCallRequested$2 (com.adyen.checkout.sessions.core.internal)
checkIfCallWasHandled:259, SessionInteractor (com.adyen.checkout.sessions.core.internal)
invokeSuspend:117, SessionDropInService$requestDetailsCall$1 (com.adyen.checkout.dropin)
checkIfCallWasHandled:259, SessionInteractor (com.adyen.checkout.sessions.core.internal)
onDetailsCallRequested:96, SessionInteractor (com.adyen.checkout.sessions.core.internal)
invokeSuspend:117, SessionDropInService$requestDetailsCall$1 (com.adyen.checkout.dropin)
resumeWith:33, BaseContinuationImpl (kotlin.coroutines.jvm.internal)
run:108, DispatchedTask (kotlinx.coroutines)
handleCallback:938, Handler (android.os)
dispatchMessage:99, Handler (android.os)
loopOnce:226, Looper (android.os)
loop:313, Looper (android.os)
main:8751, ActivityThread (android.app)
run:571, RuntimeInit$MethodAndArgsCaller (com.android.internal.os)
main:1135, ZygoteInit (com.android.internal.os)

The only difference that I caught was with different ActionComponentData in SessionDropInService.requestDetailsCall(). The 1st one has this payload:

details={"payload":"Ab02b4c0!BQABAgCmqAx50Bks46GWFrXfSncKHXfKp+wv7B4VdeJstAmIOakZGNhsrhNVtpXPb8GGVPgOCfiPeYoybQDkbp7OuR9t0IiE35pU3e9vCemWXalIzUtcAFjdrQsMYg0WOw1q1rNL8RlsW1nrqw1iY\/hUDt2WKk7iE\/zG6LkYxsHPb5jMjc9xaXWwJ8uoff8l2Iim\/fdpoUZiC7DwD4fphPAIcTIyilY\/z7ZTXrzBWI24OimMB3I6etlz3UOvDiDenFxeyxfkR3cDsnx4B2WusWBF4kTsZAaKaJkwOwcq5\/En1WWWIUX\/3ZmzbhXoBait+KF\/\/qIruExOdQq5vXh+IckZQqgvVhRoStw1OWsakIu7\/wru37awOeeDEDXeOCSg2bQ0POe4uqtr8e2WEBkCLSRyHAhyDojmtpyW1\/dBpj5Npc5GPFHJ6OslrVlCCGzrg4+dWva80AhDYo2j75ILYXCfpfVNubWnyqKMEDR8XRHENXBaWf9mVDLDhUx6Wcc0RJr4JwzhVmeFiUALliwbkarXX597F5eoqlXlnlZtprCuag9VL5M9fyf+OTeksiYPQKVu1sIhliHHg3mkJHRWXs1\/UEzkHbJE0CU\/ELsvEeHbr8388j4BWcEFMntC0Q+LAMqVKvFNlB7Bco7rmLEjVKUSM8iA+9\/1vd50ZYh2xFO6eXXMcgzHiuMoaDltXu075UQASnsia2V5IjoiQUYwQUFBMTAzQ0E1MzdFQUVEODdDMjRERDUzOTA5QjgwQTc4QTkyM0UzODIzRDY4REFDQzk0QjlGRjgzMDVEQyJ9rDs51wZPva3JikyHUlw613wL3OWbCttsgJDVY00waen2QxEdwOKugp8tOyV0\/gXvNzzDLw81h0w7apZGxqjYsDUHREDVRmsWtZihNp3CltsK24lytN\/Vs6\/U3s\/i4YYXK+4Eh+0OmQDbrDzR2Zufig56W67qGJ0vohEEGYo8Eg0lkr0aM4vj06jqw\/qjvx7g8GU5tuLrqSHlt\/BK6KePEIZR0c\/6UE5ACSQUcfs+NAmblsQp4jRAU9j+qERVABzV3OuIukUs9rWUeIw3\/AGM+8F5cQ=="

And the 2nd one this:

details={"payload":"Ab02b4c0!BQABAgAQr6uwTrC6QasDwYAWZLILvc9ODxDth0SHj1n+MyAnHFISy\/ordiEEEjwzuONABpeyE+peIhoQoDzwlMsZx+S99QW+AlV433Z6Kr833ZRMEv3H3EhIyI\/9j0TO8WCFkkJh63qQmNlWGqSt6NsCLfwTUfIqSIJpjc9+WLK04l4xpSccQkuJdvibtF6eBJQnOXBmlz9K3QN99VJzyxGahUgrb2749DDTrpHGqK5mQ5FvvNtUKdYaYfef+LbeqK5\/D7MAqvyQAsD61Pls4kjWp6u1bbIhgOnwdfYTzoRRkTMdIPo6C496VtNuy+okD5e64RipTGcBxUd+hGM2FupegYjetzUnUrEqzyjO1lGG6oSUKgKCSjwwHuQPAoLXGQy8l+wDz+KddTlno70P7Z89tla2qwV6u6lCdy4Z3ky8JpSVoIr76XqwRAfIOPU5uHlAQj2NC1NCbS+sXVwo4qK8AJnqawG\/Sjx1D0LuM6Y03kPU+mm6UYpQ2RCvy2I\/oswP3v65g2Uq1BA5NsOq9JxsmJ6VtWD8G8vDk0du0cHzO\/vjR\/G42CEUSexHKV9YfJKHgppNj1xobVFdiNSMhU\/i5QBxz1ivSRismrdYqLenRYyPzX0XC7EWSw8c51X7Vtgc8Q9ytRo0IsV1qQfeuWM3dRmYgUluJR9oppsQGNWA94OeSwx+vcuB\/b8FvVNw6EEASnsia2V5IjoiQUYwQUFBMTAzQ0E1MzdFQUVEODdDMjRERDUzOTA5QjgwQTc4QTkyM0UzODIzRDY4REFDQzk0QjlGRjgzMDVEQyJ9tNY89kzVT5Xe8IXYsRTxL3HeUXDGhpc9ecHMERgv6w829a2BwOb8SW7HBer9mGbjkR0Jiw3NwsbprlX1it9dzrtojD2wR20bZAc5hDpAeCnFkGByHCljNhiK\/qwruqEe9669QiNAX6t8GTtcj8aeYeJm06023N3cFlF8ue9l0Lg7BxoZbfPlbJOo4+IzQP\/Hwsoz90XqyTZKJ1tGIuLxVIkEoIZmewBMYRBqIMXdUcWXVP6p8P81w6yYHf+V+9WYW0sGAQ4qHaE6p3xFJ4ozwxVL"

OscarSpruit commented 1 month ago

@wrozwad thank you for all the details! We managed to reproduce it and we are working on a fix.

We can only reproduce it when we add a delay in onSubmit before calling sendResult. Are you able to reproduce it consistently?

wrozwad commented 4 weeks ago

@OscarSpruit Our mutation repository.setPaymentMethodAndPlaceOrder(state) typically takes 1.5 seconds. This way, we achieve 100% reproducibility of the issue.

OscarSpruit commented 4 weeks ago

That makes sense then, just making sure we are not missing anything

OscarSpruit commented 3 weeks ago

Hi, we just released 5.3.1 in which this issue should be fixed.

wrozwad commented 1 week ago

Yes, I can confirm that the fix is working correctly.