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.

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

EphemeralKeyProvider Called Twice #797

Closed
rayliverified opened this issue Feb 4, 2019 · 20 comments · Fixed by #1376 or #1394
Closed

EphemeralKeyProvider Called Twice #797

rayliverified opened this issue Feb 4, 2019 · 20 comments · Fixed by #1376 or #1394
Assignees
Labels
JIRA-ed Added to JIRA triaged Issue has been reviewed by Stripe and is being tracked internally

Comments

@rayliverified
Copy link

EphemeralKeyProvider is called twice, once upon attaching the KeyProvider and again when performing any getInstance() method.

Ephemeral Key is obtained here via this code:

CustomerSession.initCustomerSession(EphemeralKeyProvider(App.mInstance, premiumRepositoryImpl, object : EphemeralKeyProvider.ProgressListener {
            override fun onStringResponse(string: String) {
                if (!string.startsWith("Error: ")) {
                    stripeEphemeralKey = string
                } else {

                }
            }
        }))

And then again here:

CustomerSession.getInstance().retrieveCurrentCustomer(object: CustomerSession.CustomerRetrievalListener {
            override fun onCustomerRetrieved(customer: Customer) {
            }
            override fun onError(errorCode: Int, errorMessage: String?) {
            }
        })

Whenever any of the getInstance() methods are called, the ephemeral key is generated again.
maxthonsnap20190204115614

This means the first ephemeral key generated is redundant. Generating an ephemeral key takes some time and is slow on high latency networks. Minimizing the key generation calls especially if they are redundant should be considered?

@mshafrir-stripe
Copy link
Collaborator

@searchy2 thanks for filing. I've created an internal ticket and will add it to our team's backlog.

@mshafrir-stripe mshafrir-stripe self-assigned this Feb 4, 2019
@mshafrir-stripe mshafrir-stripe added the JIRA-ed Added to JIRA label Feb 4, 2019
@rayliverified
Copy link
Author

Thanks for taking a look and adding this to your backlog. Generating the ephm key twice doesn't cause problems as far as I can tell but it makes me somewhat nervous.

@mshafrir-stripe
Copy link
Collaborator

@searchy2 thinking about this some more, this is arguably a "feature" instead of a "bug". Making the request for a ephemeral key in advance so that it's ready to be used when needed may save perceived response time for the end user.

@rayliverified
Copy link
Author

Correct me if I'm wrong, my understanding is that the ephemeral key is used for initializing a customer session so it needs to be fetched in the initCustomerSession. Then, I'm guessing it's fetched again during an retrieveCurrentCustomer call because of staleness concerns?

For building out a shopping cart flow, it makes sense to initialize the customer and generate the ephemeral key in advance. The problem I see is that the key gets refreshed multiple times during the checkout flow which seems to defeat the purpose of fetching it in advance.

Stripe probably has dozens of payment flows to support so there's probably use cases where the same initCustomerSession generated ephemeral key is used throughout the entire flow. But what exactly is that flow?

My current flow is:

  1. initCustomerSession
  2. retrieveCurrentCustomer
  3. generate source token
  4. addCustomerSource
  5. setCustomerDefaultSource (set just added source as default, would be great to have an option to set newly added source as default to consolidate this step like exists in web)
  6. charge payment in backend

In this flow, the ephemeral key is fetched twice. The PaymentConfiguration has to be initialized twice too. Is the redundancy intentional?

@csabol-stripe csabol-stripe added the triaged Issue has been reviewed by Stripe and is being tracked internally label May 21, 2019
@nataliakuznetsova
Copy link

Hello, Stripe. I am experiencing the same issue with version 10.0.3 but in my case EphemeralKeyProvider is called 3 times within SCA 2 flow. Please provide an update. Thank you

@mshafrir-stripe
Copy link
Collaborator

mshafrir-stripe commented Aug 2, 2019

@nataliakuznetsova thanks for raising the issue. I'll hopefully investigate it this coming week.

@mshafrir-stripe
Copy link
Collaborator

@nataliakuznetsova can you provide me more details around your integration (e.g. which Stripe, PaymentSession, CustomerSession methods are you calling), and at what point do you see EphemeralKeyProvider called?

@talhaasim96
Copy link

@mshafrir-stripe I am experiencing the same issue with version 9.3.6 (also tried latest 10.2.1). In my case, I'm using CustomerSession.initCustomerSession() in onCreate and createEphemeralKey Api hits two times and returns me two customers. I'm not even using getInstance().

@mshafrir-stripe
Copy link
Collaborator

@talhaasim96 I just tested CustomerSession.initCustomerSession() with our sample store app, and I only see EphemeralKeyProvider#createEphemeralKey() called once. Can you share some code so that I can repro that method getting called twice?

@nataliakuznetsova
Copy link

@mshafrir-stripe I am using paymentSession in 3D Secure flow. I call presentPaymentMethodSelection on the payment session to select a payment methods. First I init CustomerSession with context and emphemeral key provider, then I create and init payment session, and in the end call presentPaymentMethodSelection. As a result I see that the end point is called 3 times. Please let me know if more details are needed

@mshafrir-stripe
Copy link
Collaborator

@nataliakuznetsova thanks for the details. To help me debug this issue, can you tell me if you are directly calling any methods on CustomerSession? Would you be able to share the code you're using to init CustomerSession and PaymentSession?

mshafrir-stripe added a commit that referenced this issue Aug 13, 2019
…sion()

If true, an ephemeral key will be fetched immediately. Otherwise, only
fetch an ephemeral key when needed.

Defaults to true to maintain existing behavior.

Fixes #797
@mshafrir-stripe
Copy link
Collaborator

mshafrir-stripe commented Aug 14, 2019

@nataliakuznetsova @talhaasim96 @searchy2

The reason that you're seeing superfluous ephemeral key requests:

  1. Calling CustomerSession#initCustomerSession() makes an initial ephemeral key request by default.
  2. Then when the PaymentSession is initialized, if an ephemeral key is not available, another request is made.

In your case, there is a race condition in which the request for the first key has not returned before we check if an ephemeral key is available in Step 2. The "prefetching" logic in Step 1 is an optimization to have an ephemeral key ready for some point in the near future, but unnecessary in the case where both CustomerSession and PaymentSession are initialized together.

I put up a PR (#1376) that allows you to skip prefetching by setting shouldPrefetchEphemeralKey to false in CustomerSession#initSession().

mshafrir-stripe added a commit that referenced this issue Aug 14, 2019
…sion() (#1376)

If true, an ephemeral key will be fetched immediately. Otherwise, only
fetch an ephemeral key when needed.

Defaults to true to maintain existing behavior.

Fixes #797
@nataliakuznetsova
Copy link

Hello @mshafrir thanks for your fix. Unfortunately, the end point is still triggered more than once in my case:

  • PaymentSession.init() triggers fetchCustomer that tiggers retrieveEphemeralKey
  • Then I can paymentSession.presentPaymentMethodSelection which will create PaymentMethodsActivity that also calls to retrieveEphemeralKey in getPaymentMethods

As a result the end point is triggered twice now.

@mshafrir-stripe
Copy link
Collaborator

@nataliakuznetsova are you are calling PaymentSession#presentPaymentMethodSelection() immediately after initializing the PaymentSession? If that's the case, can you initialize the PaymentSession before you need to present the payment methods screen?

For example, initialize the PaymentSession when the checkout Activity is created, then only call presentPaymentMethodSelection() when the user taps on "Add a payment method"?

@nataliakuznetsova
Copy link

nataliakuznetsova commented Aug 19, 2019

@msaffitz-stripe yes, this is how it is done in the check out flow. But besides the check out flow, the app lets user to manage the card via PaymentMethodActivity outside of payment. And in this flow I wouldn't want to initialize the Payment session too early

I am trying now to call presentPaymentMethodSelection when the callback onPaymentSessionDataChanged is invoked. Do you think it is a valid approach @mshafrir-stripe ?

@mshafrir-stripe
Copy link
Collaborator

@nataliakuznetsova that makes sense. I put up #1394 to let you customize the prefetch behavior.

@nataliakuznetsova
Copy link

@mshafrir-stripe thanks!

@mshafrir-stripe
Copy link
Collaborator

@nataliakuznetsova this is now available in 10.3.1

@talhaasim96
Copy link

@mshafrir-stripe

CustomerSession.initCustomerSession(getContext(),
new MyEphemeralKeyProvider(
new MyEphemeralKeyProvider.ProgressListener() {
@OverRide
public void onStringResponse(String string) {
Log.e(TAG, string);
if (string.startsWith("Error: ")) {
ToastUtils.showError(string);
}
}
}));

            PaymentConfiguration.init(Constants.STRIPE_LIVE_KEY);
            paymentSessionConfig = new PaymentSessionConfig.Builder().build();

            paymentSession = new PaymentSession(getActivity());
            paymentSession.init(this, paymentSessionConfig, savedInstanceState);

using above code, my api hits two times. this code is present in onCreate of activity

@mshafrir-stripe
Copy link
Collaborator

@talhaasim96 you can use one of the overloaded [CustomerSession#initCustomerSession][https://stripe.dev/stripe-android/com/stripe/android/CustomerSession.html#initCustomerSession-android.content.Context-com.stripe.android.EphemeralKeyProvider-boolean-) methods to specify whether the ephemeral key should be prefetched. In your case, you can disable it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JIRA-ed Added to JIRA triaged Issue has been reviewed by Stripe and is being tracked internally
Projects
None yet
5 participants