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

PAY-2080: Retry pledge after error with PaymentSheet #1710

Merged
merged 10 commits into from Nov 29, 2022
Merged

Conversation

Arkariang
Copy link
Contributor

@Arkariang Arkariang commented Nov 28, 2022

📲 What

Several points in the PR:

  • 1 - Fix for crash with exception Fatal Exception: java.lang.IllegalArgumentException: Invalid Setup Intent client secret: ... discovered during QA party for Checkout flows with PaymentSheet. Firebase report
    Refined a bit the user facing error messages for this point with, instead of presenting the generic "Something went wrong" message, now the Stripe error localized message is displayed to the user.
  • 2 - Discovered TransactionTooLargeException for certain projects when triggering 3DS flow after adding payment method using PaymentSheet Firebase report
  • Updated Stripes library to latest version

🛠 How

⚠️ (use debug build to have complete logs).
This section is gonna described mainly those steps to identify the root cause behind #2. In order to debug an identify the TransactionTooLargeException a key tool has been https://github.com/guardian/toolargetool, is a debug tool so no trace of it on the PR.

First taking a closer look into the logcat trace of the error, on the warnings you can see :
Screenshot 2022-11-24 at 9 30 24 AM

This warning trace indicates fragments presented and their size, as you can see two of the are taking up much more memory than they should, and are considerably bigger that the rest one fragment with [size=143456] and [size=431628] those hexadecimals identifiers can be traced back to the KS fragments by filtering mode the onCreate logs (the screenshot is just to showcase how to filter in logcat, it does not match the with the execution of the previous exception)
Screenshot 2022-11-28 at 11 19 16 AM

At this point the guilty intents have been identified in your trace and those two were BackingAddOnsFragment and PledgeFragment, the intents for launching those two fragments were taking more than 100KB each ...

Now if you add toolargetool to your build, and have breakpoints on those functions starting the BackingAddOnsFragment and PledgeFragment you will be able to evaluate in real time the size of the intent by using toolargetool method bundleBreakdown

BEFORE: 140KB was the size for the the BackingAddOnsFragment bundle
Screenshot 2022-11-28 at 11 47 25 AM

NOW: 3.6 is the size for the BackingAddOnsFragment bundle
Screenshot 2022-11-28 at 11 41 59 AM

The solution has been reducing the project fields to only those mandatory to be able to operate the user journey from the moment a reward is picked, please take a look into Project.reduce() extension function to have more details about which ones are those mandatory fields.

Also taken the chance to review the RX subscriptions and tied those who weren't before to the lifecycle in all the user journey since the point a reward is picked, and reduced to only the project.slug the bundles for ProjectActivity, UpdateActivity.

👀 See

Screenshot 2022-11-28 at 12 00 11 PM

| Before 🐛 |

before.mp4

| After 🦋 |

after.mp4

| | |

📋 QA

For QA #1 :

  • Pledge to a project, adding a new payment method during checkout, introduce the payment details, and before hitting "Continue CTA" on the PaymentSheet turn on airplane mode, or loose connectivity, hit now the "Continue CTA" on the PaymentSheet. You should see an error message and the payment method not added to the UI. Once you recover connectivity, you should be able to add the payment method again and pledge.
  • Pledge to a project, adding a new payment method during checkout, a 3DS one, you can use for example 4000 0000 0000 3220, once the 3DS flow is presented fail it, you should see a message like the screenshot below, and the payment method not visible on the UI. After that you should be able to pledge or add a new payment method as usual.
    Screenshot 2022-11-28 at 1 59 19 PM

For QA #2 :

Story 📖

NTV-2080

@codecov
Copy link

codecov bot commented Nov 28, 2022

Codecov Report

Merging #1710 (864a90f) into master (c2c6986) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master    #1710      +/-   ##
============================================
+ Coverage     78.62%   78.64%   +0.02%     
- Complexity     1950     1952       +2     
============================================
  Files           359      359              
  Lines         18013    18031      +18     
  Branches       2146     2146              
============================================
+ Hits          14162    14180      +18     
  Misses         2603     2603              
  Partials       1248     1248              
Impacted Files Coverage Δ
...om/kickstarter/libs/utils/extensions/ProjectExt.kt 77.22% <100.00%> (+4.93%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@leighdouglas leighdouglas left a comment

Choose a reason for hiding this comment

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

great! Lgtm!

@Arkariang Arkariang merged commit a27a397 into master Nov 29, 2022
@Arkariang Arkariang deleted the imartin/PAY-2080 branch November 29, 2022 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants