Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.33k stars 2.76k forks source link

[$500] Android - BA - Android App crashes when starting Onfido #35519

Closed kbecciv closed 6 months ago

kbecciv commented 7 months ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 1.4.34-0 Reproducible in staging?: y Reproducible in production?: not able to check production If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: Applause- Internal Team Slack conversation:

Action Performed:

  1. Open Android App
  2. Login with Expensifail account
  3. Create workspace or go to existing one
  4. Select 'Bank Account' > Connect with Plaid
  5. Select Region Bank and follow the flow for Onfido (Company - Expensify, first name - First, last name - Last)

Expected Result:

Onfido started after Personal Information page completed

Actual Result:

Android App crashes when starting Onfido

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/93399543/3671be9a-37c0-40cb-a452-758243fd4ae6

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01dccccca284d16c78
  • Upwork Job ID: 1755346472180178944
  • Last Price Increase: 2024-02-07
  • Automatic offers:
    • mkhutornyi | Contributor | 0
github-actions[bot] commented 7 months ago

:wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.
melvin-bot[bot] commented 7 months ago

Triggered auto assignment to @hayata-suenaga (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

kbecciv commented 7 months ago

We think that this bug might be related to #wave6-collect-submitters CC @greg-schroeder

greg-schroeder commented 7 months ago

Hmm this might be handled by Callstack or should be - @mountiny can you confirm if this fits in the VBA refactor project or can be merged in?

francoisl commented 7 months ago

Wondering if it has to do anything with https://github.com/Expensify/App/pull/35440, we merged that yesterday to fix another crash on Android. Will try to troubleshoot.

francoisl commented 7 months ago

I can reproduce the crash on the current production version, using the VERIFYING flow from https://stackoverflowteams.com/c/expensify/questions/342. Going to remove the blocker label and keep this open so we can look and fix.

cc @mkhutornyi if you have an idea how to fix.

mkhutornyi commented 7 months ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

App crashes when start onfido flow on android release build

What is the root cause of that problem?

Crash log:

   Process: com.expensify.chat.adhoc, PID: 1047
java.lang.RuntimeException: Unable to start activity ComponentInfo{com.expensify.chat.adhoc/com.onfido.android.sdk.capture.ui.OnfidoActivity}: java.lang.IllegalArgumentException: Unable to create call adapter for class io.reactivex.rxjava3.core.Single
    for method h.b
    at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3449)
    at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3601)
    at android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:85)
    at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:135)
    at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:95)
    at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2066)
    at android.os.Handler.dispatchMessage(Handler.java:106)
    at android.os.Looper.loop(Looper.java:223)
    at android.app.ActivityThread.main(ActivityThread.java:7656)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947)
Caused by: java.lang.IllegalArgumentException: Unable to create call adapter for class io.reactivex.rxjava3.core.Single
    for method h.b
    at fd.f9.i(SourceFile:40)
    at mv.q.b(SourceFile:400)
    at retrofit2.Retrofit.c(SourceFile:25)
    at mv.r0.invoke(SourceFile:45)
    at java.lang.reflect.Proxy.invoke(Proxy.java:1006)
    at com.onfido.api.client.$Proxy18.b(Unknown Source)
    at com.onfido.api.client.f.d(SourceFile:17)
    at com.onfido.android.sdk.capture.network.OnfidoApiService.getSDKConfig$onfido_capture_sdk_core_release(Unknown Source:6)
    at com.onfido.android.sdk.capture.internal.config.SDKConfigRepository.<init>(SourceFile:28)
    at com.onfido.android.sdk.capture.internal.config.SDKConfigRepository_Factory.newInstance(Unknown Source:2)
    at com.onfido.android.sdk.capture.internal.config.SDKConfigRepository_Factory.get(SourceFile:1)
    at com.onfido.android.sdk.capture.internal.config.SDKConfigRepository_Factory.get(SourceFile:2)
    at mj.a.get(SourceFile:14)
    at com.onfido.android.sdk.capture.ui.OnfidoPresenterInitializer_Factory.get(SourceFile:1)
    at com.onfido.android.sdk.capture.ui.OnfidoPresenterInitializer_Factory.get(SourceFile:2)
    at com.onfido.android.sdk.capture.ui.OnfidoPresenter_Factory.get(Unknown Source:58)
    at com.onfido.android.sdk.capture.ui.OnfidoPresenter_PresenterAssistedFactory_Impl.create(SourceFile:1)
    at com.onfido.android.sdk.capture.ui.OnfidoActivity$presenter$2.invoke(SourceFile:1)
    at com.onfido.android.sdk.capture.ui.OnfidoActivity$presenter$2.invoke(SourceFile:2)
    at sq.l.getValue(SourceFile:21)
    at com.onfido.android.sdk.capture.ui.OnfidoActivity.getPresenter(Unknown Source:2)
    at com.onfido.android.sdk.capture.ui.OnfidoActivity.setupPresenterWith(Unknown Source:38)
    at com.onfido.android.sdk.capture.ui.OnfidoActivity.onCreate(SourceFile:60)
    at android.app.Activity.performCreate(Activity.java:8000)
    at android.app.Activity.performCreate(Activity.java:7984)
    at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1309)
    at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3422)
    ... 11 more
Caused by: java.lang.IllegalStateException: Single return type must be parameterized as Single<Foo> or Single<? extends Foo>
    at nv.i.get(SourceFile:117)
    at retrofit2.Retrofit.a(SourceFile:33)
    at mv.q.b(SourceFile:381)
    ... 36 more

This is another case of crash after enabling proguard

What changes do you think we should make in order to solve the problem?

# Keep generic signature of RxJava3 (R8 full mode strips signatures from non-kept items).
-keep,allowobfuscation,allowshrinking class io.reactivex.rxjava3.core.Flowable
-keep,allowobfuscation,allowshrinking class io.reactivex.rxjava3.core.Maybe
-keep,allowobfuscation,allowshrinking class io.reactivex.rxjava3.core.Observable
-keep,allowobfuscation,allowshrinking class io.reactivex.rxjava3.core.Single

add this to proguard-rules.pro

reference: https://github.com/square/retrofit/blob/3770704de89233c1240699392baa7198d8b63bff/retrofit-adapters/rxjava3/src/main/resources/META-INF/proguard/retrofit2-rxjava3-adapter.pro#L1-L5


Or this might be more general solution:

# R8 full mode strips generic signatures from return types if not kept.
-if interface * { @retrofit2.http.* public *** *(...); }
-keep,allowoptimization,allowshrinking,allowobfuscation class <3>

reference: https://github.com/square/retrofit/pull/3886/files

What alternative solutions did you explore? (Optional)

bump @onfido/react-native-sdk to latest version (10.6.0) Why? proguard issue fixed in onfido android sdk 19.5.0 (https://github.com/onfido/onfido-android-sdk/releases/tag/19.5.0) react-native sdk 10.6.0 is the one which bumped android sdk to 19.5.x

image image

mountiny commented 7 months ago

@mkhutornyi what is the changelog for the onfido bump?

mkhutornyi commented 7 months ago

RN sdk changelog can be found here (after 8.3.0), mainly version bump of native sdks

Breaking changes:

Android sdk changelog is here (after 18.0.1)

Breaking changes:

iOS sdk changelog is here (after 29.0.0)

Breaking changes:

mountiny commented 7 months ago

The android compile sdk looks the most dangerous but we are already on 34 https://github.com/Expensify/App/blob/b64b4080247db4e5e8fcd5dea6ada29f005ecaf7/android/build.gradle#L7 so maybe we should be good to upgrade this

@francoisl @hayata-suenaga what do you think?

hayata-suenaga commented 7 months ago

@mountiny could you explain why we need to upgrade the sdk version?

mkhutornyi commented 7 months ago

I explained detail in my proposal (alternative solution)

francoisl commented 7 months ago

Sounds good to me. The compile SDK version doesn't sound like a concern if we already target a higher version.

mountiny commented 7 months ago

Sorry my comment meant to say: we should be good to upgrade @onfido/react-native-sdk since the compile sdk does not have to be bumped.

@hayata-suenaga would you like to make this external and assign @mkhutornyi if you happy with their proposal?

melvin-bot[bot] commented 7 months ago

Job added to Upwork: https://www.upwork.com/jobs/~01dccccca284d16c78

melvin-bot[bot] commented 7 months ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @allroundexperts (External)

mkhutornyi commented 7 months ago

Ah I didn't know that I was assigned already. @hayata-suenaga please unassign and assign me back

melvin-bot[bot] commented 7 months ago

πŸ“£ @mkhutornyi πŸŽ‰ An offer has been automatically sent to your Upwork account for the Contributor role πŸŽ‰ Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review πŸ§‘β€πŸ’» Keep in mind: Code of Conduct | Contributing πŸ“–

mkhutornyi commented 7 months ago

Update: investigating android gradle build error after version bump, which came from conflict with another library

build error
melvin-bot[bot] commented 7 months ago

@allroundexperts, @mkhutornyi, @hayata-suenaga Whoops! This issue is 2 days overdue. Let's get this updated quick!

mkhutornyi commented 7 months ago

still working on fixing build errors

mkhutornyi commented 7 months ago

Looks like dupe issues are created:

mountiny commented 7 months ago

Thanks, put them all on hold. @mkhutornyi do you want to bringthe build issues to Slack? maybe we could get someone to help you

mkhutornyi commented 6 months ago

I finally managed to fix build error in gradle. @mountiny can you please generate adhoc build in https://github.com/Expensify/App/pull/36783?

mkhutornyi commented 6 months ago

Thanks for generating build.

proguard issue seems fixed but now another new crash (related to firebase) happening:

crash
hayata-suenaga commented 6 months ago

@mkhutornyi as Vit mentioned, I think the expensify-opens-source channel is the best place to ask help with this type of issue

I also rely on that channel for issues I can't handle myself πŸ‘

mkhutornyi commented 6 months ago

That fix should be quick. I can fix myself today πŸ™‚ (I am good at native app development)

mkhutornyi commented 6 months ago

@mountiny or @hayata-suenaga can you please generate build again? Just pushed fix

hayata-suenaga commented 6 months ago

building...

mkhutornyi commented 6 months ago

@hayata-suenaga I think you added label in the issue. Should be in PR

hayata-suenaga commented 6 months ago

ahhhh this is the issue! not PR 😭

hayata-suenaga commented 6 months ago

building for real 😭

mkhutornyi commented 6 months ago

PR is ready for review

melvin-bot[bot] commented 6 months ago

This issue has not been updated in over 15 days. @allroundexperts, @mkhutornyi, @hayata-suenaga eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

melvin-bot[bot] commented 6 months ago

Triggered auto assignment to @lschurr (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

hayata-suenaga commented 6 months ago

The PR was deployed to production two weeks ago.

Melvin forgot to remove the reviewing label so we missed this.

C+ payment for @allroundexperts and contributor payment for @mkhutornyi

lschurr commented 6 months ago

Ah, got it!

@allroundexperts you're paid via newdot correct?

lschurr commented 6 months ago

Payment for $500 has been sent via Upwork for @mkhutornyi

lschurr commented 6 months ago

Payment summary:

JmillsExpensify commented 5 months ago

$500 approved for @allroundexperts