Skip to content

[Identity] Fail out of verification flow on analyzer failure #12223

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.

Sign up for GitHub

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

Jump to bottom
Merged
Pearapps merged 8 commits into from
Feb 4, 2026
Merged

[Identity] Fail out of verification flow on analyzer failure #12223

Pearapps merged 8 commits into from
Feb 4, 2026

Conversation

@Pearapps
Copy link
Collaborator

@Pearapps Pearapps commented Jan 13, 2026

Simply fails out of the verification flow if we see a bug like we saw recently where R8 stripped out parts of Tensorflow lite.

I tested this out by removing the newly added Proguard consumer rules, and verified that trying to go through the verification flow will fail you out. I also made sure this does not trigger in any other circumstance.

@Pearapps Pearapps requested review from a team as code owners January 13, 2026 15:12
@Pearapps Pearapps requested a review from luisv-stripe January 13, 2026 15:12
@github-actions
Copy link
Contributor

github-actions bot commented Jan 13, 2026 •
edited
Loading

Diffuse output:

OLD: identity-example-release-base.apk (signature: V1, V2)
NEW: identity-example-release-pr.apk (signature: V1, V2)

          │          compressed           │         uncompressed          
          ├───────────┬───────────┬───────┼───────────┬───────────┬───────
 APK      │ old       │ new       │ diff  │ old       │ new       │ diff  
──────────┼───────────┼───────────┼───────┼───────────┼───────────┼───────
      dex │   2.2 MiB │   2.2 MiB │ -79 B │   4.5 MiB │   4.5 MiB │ +48 B 
     arsc │   1.1 MiB │   1.1 MiB │   0 B │   1.1 MiB │   1.1 MiB │   0 B 
 manifest │   2.3 KiB │   2.3 KiB │   0 B │     8 KiB │     8 KiB │   0 B 
      res │ 303.5 KiB │ 303.5 KiB │   0 B │ 457.7 KiB │ 457.7 KiB │   0 B 
   native │   7.9 MiB │   7.9 MiB │   0 B │  19.3 MiB │  19.3 MiB │   0 B 
    asset │   6.6 KiB │   6.6 KiB │   0 B │   6.3 KiB │   6.3 KiB │   0 B 
    other │ 107.1 KiB │   107 KiB │ -11 B │ 214.5 KiB │ 214.5 KiB │   0 B 
──────────┼───────────┼───────────┼───────┼───────────┼───────────┼───────
    total │  11.5 MiB │  11.5 MiB │ -90 B │  25.5 MiB │  25.5 MiB │ +48 B 

 DEX     │ old   │ new   │ diff       
─────────┼───────┼───────┼────────────
   files │     1 │     1 │  0         
 strings │ 21291 │ 21289 │ -2 (+2 -4) 
   types │  6505 │  6505 │  0 (+0 -0) 
 classes │  5216 │  5216 │  0 (+0 -0) 
 methods │ 31840 │ 31840 │  0 (+2 -2) 
  fields │ 18204 │ 18204 │  0 (+0 -0) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  163 │  163 │  0   
 entries │ 3719 │ 3719 │  0
APK 
    compressed    │   uncompressed   │                                           
──────────┬───────┼──────────┬───────┤                                           
 size     │ diff  │ size     │ diff  │ path                                      
──────────┼───────┼──────────┼───────┼───────────────────────────────────────────
  2.2 MiB │ -79 B │  4.5 MiB │ +48 B │ ∆ classes.dex                             
 29.4 KiB │  -8 B │ 65.3 KiB │   0 B │ ∆ META-INF/CERT.SF                        
 25.9 KiB │  -4 B │ 65.2 KiB │   0 B │ ∆ META-INF/MANIFEST.MF                    
  1.2 KiB │  +2 B │  1.2 KiB │   0 B │ ∆ META-INF/CERT.RSA                       
    271 B │  -1 B │    120 B │   0 B │ ∆ META-INF/version-control-info.textproto 
──────────┼───────┼──────────┼───────┼───────────────────────────────────────────
  2.2 MiB │ -90 B │  4.6 MiB │ +48 B │ (total)
DEX 
STRINGS:

   old   │ new   │ diff       
  ───────┼───────┼────────────
   21291 │ 21289 │ -2 (+2 -4) 
  
  + r8-map-id-de9cb19fb322718cfd9931a491303f24a3777e037c5d037fdfb7bd559d02c9eb
  + ~~R8{"backend":"dex","compilation-mode":"release","has-checksums":false,"min-api":21,"pg-map-id":"de9cb19fb322718cfd9931a491303f24a3777e037c5d037fdfb7bd559d02c9eb","r8-mode":"full","version":"8.13.17"}
  
  - , continue analyzing
  - Error executing analyzer : 
  - r8-map-id-360bc3230e54736203381a7c261204d0c0e78f69f94b8fe5aaa828b9d48bf40c
  - ~~R8{"backend":"dex","compilation-mode":"release","has-checksums":false,"min-api":21,"pg-map-id":"360bc3230e54736203381a7c261204d0c0e78f69f94b8fe5aaa828b9d48bf40c","r8-mode":"full","version":"8.13.17"}
  

METHODS:

   old   │ new   │ diff      
  ───────┼───────┼───────────
   31840 │ 31840 │ 0 (+2 -2) 
  
  + a9.e h(e, boolean, j, Boolean, Throwable, int)
  + m9.m i(Throwable) → boolean
  
  - a9.e h(e, boolean, Boolean, Throwable, int)
  - m9.m i(Throwable)

@Pearapps Pearapps requested a review from luisv-stripe January 14, 2026 18:30
luisv-stripe
luisv-stripe reviewed Jan 14, 2026
View reviewed changes
identity/src/main/java/com/stripe/android/identity/viewmodel/IdentityScanViewModel.kt
)
}

override fun onAnalyzerFailure(t: Throwable): Boolean {
Copy link
Contributor

@luisv-stripe luisv-stripe Jan 14, 2026

Choose a reason for hiding this comment

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

Why are we overriding here an not in CameraViewModel?
Also, are there any other places where we log an error (using the logger) but don't actually send it to the backend?

Copy link
Collaborator Author

@Pearapps Pearapps Jan 14, 2026

Choose a reason for hiding this comment

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

Why are we overriding here an not in CameraViewModel?

IdentityScanViewModel is where verificationFlowFinishable is, which is where we can react properly to the analyzer error.

Also, are there any other places where we log an error (using the logger) but don't actually send it to the backend?

Yes, likely many many dozens, unfortunately. We can tackle that, but like we chatted about yesterday in our sync that should be it's own effort and not tied to this if at all possible - that way we can make sure this is covered and schedule that work in line with other priorities if needed

Copy link
Contributor

@luisv-stripe luisv-stripe Jan 14, 2026

Choose a reason for hiding this comment

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

Could we just log in CameraViewModel and also add a timeout in case the user cannot complete document capture after a set time (20 seconds for example) and show the error screen after a timeout? I believe the timeout matches the web experience.

Copy link
Collaborator Author

@Pearapps Pearapps Jan 26, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I believe this is resolved after this discussion, but do let me know if there are still open questions https://hubble.corp.stripe.com/saved-queries/t/30457

@Pearapps Pearapps requested a review from luisv-stripe January 14, 2026 20:13
luisv-stripe
luisv-stripe approved these changes Feb 3, 2026
View reviewed changes
Copy link
Contributor

@luisv-stripe luisv-stripe left a comment

Choose a reason for hiding this comment

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

Thanks!

@Pearapps Pearapps merged commit 24060aa into master Feb 4, 2026
19 checks passed
@Pearapps Pearapps deleted the kpa/fail-on-analyzer-failure branch February 4, 2026 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@luisv-stripe luisv-stripe luisv-stripe approved these changes

@cjmisenas-stripe cjmisenas-stripe Awaiting requested review from cjmisenas-stripe

Assignees

No one assigned

Labels

identity

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Pearapps @luisv-stripe