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

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

Closed
1 of 6 tasks
kbecciv opened this issue Jan 31, 2024 · 40 comments
Closed
1 of 6 tasks

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

kbecciv opened this issue Jan 31, 2024 · 40 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor

Comments

@kbecciv
Copy link

kbecciv commented Jan 31, 2024

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?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6362892_1706735260543.Android-Onfido-Crash.mp4

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
@kbecciv kbecciv added the DeployBlockerCash This issue or pull request should block deployment label Jan 31, 2024
Copy link
Contributor

👋 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.

Copy link

melvin-bot bot commented Jan 31, 2024

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

@kbecciv
Copy link
Author

kbecciv commented Jan 31, 2024

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

@greg-schroeder
Copy link
Contributor

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
Copy link
Contributor

Wondering if it has to do anything with #35440, we merged that yesterday to fix another crash on Android. Will try to troubleshoot.

@francoisl
Copy link
Contributor

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.

@francoisl francoisl added Daily KSv2 and removed Hourly KSv2 DeployBlockerCash This issue or pull request should block deployment labels Jan 31, 2024
@mkhutornyi
Copy link
Contributor

mkhutornyi commented Feb 1, 2024

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
Copy link
Contributor

mountiny commented Feb 1, 2024

@mkhutornyi what is the changelog for the onfido bump?

@mkhutornyi
Copy link
Contributor

mkhutornyi commented Feb 1, 2024

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

Breaking changes:

  • breaking changes from version bump of native sdks
  • Added theme selection to the SDK configuration

Android sdk changelog is here (after 18.0.1)

Breaking changes:

  • The compile sdk version has been upgraded to 33 to meet Google Play's target API level requirement
  • Replaced com.google.android.play:core library with a more specific com.google.android.play:feature-delivery library for Dynamic Feature Module compatibility
  • Added Proguard rules for Retrofit to support R8 full mode according to the R8 FAQ (this fixes our issue)
  • Added support for the NFC required flow

iOS sdk changelog is here (after 29.0.0)

Breaking changes:

  • Due to a change introduced by Apple in March 2023 with iOS v16.0, PACE-only documents can no longer be read by iOS devices installed with iOS 16 (or newer). To minimize integration complexity and instability with future releases of iOS, Onfido is removing support for NFC-PACE.

@mountiny
Copy link
Contributor

mountiny commented Feb 1, 2024

The android compile sdk looks the most dangerous but we are already on 34

compileSdkVersion = 34
so maybe we should be good to upgrade this

@francoisl @hayata-suenaga what do you think?

@hayata-suenaga
Copy link
Contributor

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

@mkhutornyi
Copy link
Contributor

I explained detail in my proposal (alternative solution)

@francoisl
Copy link
Contributor

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

@mountiny
Copy link
Contributor

mountiny commented Feb 2, 2024

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 melvin-bot bot added the Overdue label Feb 7, 2024
@hayata-suenaga hayata-suenaga added the External Added to denote the issue can be worked on by a contributor label Feb 7, 2024
@melvin-bot melvin-bot bot changed the title Android - BA - Android App crashes when starting Onfido [$500] Android - BA - Android App crashes when starting Onfido Feb 7, 2024
Copy link

melvin-bot bot commented Feb 7, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 7, 2024
@mkhutornyi
Copy link
Contributor

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

@hayata-suenaga
Copy link
Contributor

building...

@mkhutornyi
Copy link
Contributor

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

@hayata-suenaga
Copy link
Contributor

ahhhh this is the issue! not PR 😭

@hayata-suenaga
Copy link
Contributor

building for real 😭

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Feb 19, 2024
@mkhutornyi
Copy link
Contributor

PR is ready for review

@melvin-bot melvin-bot bot added Monthly KSv2 and removed Weekly KSv2 labels Mar 13, 2024
Copy link

melvin-bot bot commented Mar 13, 2024

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!

@hayata-suenaga hayata-suenaga added Weekly KSv2 Bug Something is broken. Auto assigns a BugZero manager. and removed Reviewing Has a PR in review Monthly KSv2 labels Mar 13, 2024
Copy link

melvin-bot bot commented Mar 13, 2024

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

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Mar 13, 2024
@hayata-suenaga
Copy link
Contributor

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
Copy link
Contributor

lschurr commented Mar 13, 2024

Ah, got it!

@allroundexperts you're paid via newdot correct?

@lschurr
Copy link
Contributor

lschurr commented Mar 13, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Mar 15, 2024
@lschurr
Copy link
Contributor

lschurr commented Mar 15, 2024

Payment summary:

@melvin-bot melvin-bot bot removed the Overdue label Mar 15, 2024
@lschurr lschurr closed this as completed Mar 15, 2024
@JmillsExpensify
Copy link

$500 approved for @allroundexperts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests

9 participants