Skip to content

fix: skip biometric cancellation error recording in mixpanel#24882

Closed
grvgoel81 wants to merge 5 commits into
mainfrom
fix-skip-biometric-error-recording
Closed

fix: skip biometric cancellation error recording in mixpanel#24882
grvgoel81 wants to merge 5 commits into
mainfrom
fix-skip-biometric-error-recording

Conversation

@grvgoel81
Copy link
Copy Markdown
Contributor

@grvgoel81 grvgoel81 commented Jan 19, 2026 •
edited
Loading

Description

Changelog

CHANGELOG entry: null

Related issues

Fixes:

  1. https://consensyssoftware.atlassian.net/browse/SL-460
  2. https://consensyssoftware.atlassian.net/browse/SL-461

Manual testing steps

Feature: my feature name

  Scenario: user [verb for user action]
    Given [describe expected initial app state]

    When user [verb for user action]
    Then [describe expected outcome]

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Avoids recording user-cancelled biometric auth as errors in both login and OAuth rehydration flows.

  • Treat Android DENY_PIN_ERROR_ANDROID and iOS UNLOCK_WALLET_ERROR_MESSAGES.IOS_USER_CANCELLED_BIOMETRICS as cancellations: disable biometry choice, stop loading, return early; no Logger.error or metrics tracking
  • Refactor error handling in Login and OAuthRehydration: early return for password errors; reorder vault corruption handling; add passcode_not_set categorization for rehydration failures
  • Add tests verifying no error logging or REHYDRATION_PASSWORD_FAILED tracking on biometric cancellation

Written by Cursor Bugbot for commit 87a692b. This will update automatically on new commits. Configure here.

@grvgoel81 grvgoel81 self-assigned this Jan 19, 2026
@grvgoel81 grvgoel81 added the team-onboarding Onboarding team label Jan 19, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 19, 2026

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

cursor[bot]
cursor Bot reviewed Jan 19, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Comment thread app/components/Views/OAuthRehydration/index.test.tsx
@grvgoel81 grvgoel81 marked this pull request as ready for review January 20, 2026 02:17
@grvgoel81 grvgoel81 requested a review from a team as a code owner January 20, 2026 02:17
himanshuchawla009
himanshuchawla009 requested changes Jan 20, 2026
View reviewed changes
Comment thread app/components/Views/OAuthRehydration/index.tsx
return;
}

if (loginErrorMessage === PASSCODE_NOT_SET_ERROR) {
Alert.alert(
strings('login.security_alert_title'),
strings('login.security_alert_desc'),
);
Copy link
Copy Markdown
Contributor

@himanshuchawla009 himanshuchawla009 Jan 20, 2026

Choose a reason for hiding this comment

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

PASSCODE_NOT_SET_ERROR shouldn't be recorded as unknown_error in line 438.

Copy link
Copy Markdown
Contributor Author

@grvgoel81 grvgoel81 Jan 20, 2026

Choose a reason for hiding this comment

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

resolved.

himanshuchawla009
himanshuchawla009 previously approved these changes Jan 20, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@himanshuchawla009 himanshuchawla009 left a comment

Choose a reason for hiding this comment

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

lgtm

@himanshuchawla009 himanshuchawla009 force-pushed the fix-skip-biometric-error-recording branch from ede6c09 to 87a692b Compare January 20, 2026 14:43
vinnyhoward
vinnyhoward previously approved these changes Jan 20, 2026
View reviewed changes
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jan 20, 2026

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
81.2% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@grvgoel81 grvgoel81 disabled auto-merge January 20, 2026 23:23
Cal-L
Cal-L previously approved these changes Jan 21, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@Cal-L Cal-L left a comment

Choose a reason for hiding this comment

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

LGTM

@grvgoel81 grvgoel81 force-pushed the fix-skip-biometric-error-recording branch from 668a6e2 to e8b36f6 Compare January 23, 2026 04:15
@github-actions github-actions Bot added size-L and removed size-M labels Jan 23, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 23, 2026

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeAccounts, SmokeWalletPlatform
  • Risk Level: medium
  • AI Confidence: 75%
click to see 🤖 AI reasoning details

The changes affect the Login screen (app/components/Views/Login/index.tsx) and OAuthRehydration screen (app/components/Views/OAuthRehydration/index.tsx), which are critical authentication components.

Key changes include:

  1. Login/index.tsx: Removed useAuthentication hook and replaced with direct Authentication class calls, added check for seedless password outdated state, changed biometric cancellation error handling (iOS and Android), added metrics opt-in check after login, and refactored login methods.

  2. OAuthRehydration/index.tsx: Added iOS biometric cancellation handling and improved error tracking.

These changes affect the core authentication flow that all users must go through. The E2E tests that use LoginView are tagged with RegressionAccounts and RegressionWalletPlatform. Since the changes:

  • Modify how users authenticate (password and biometric)
  • Change navigation flow after login (metrics opt-in check)
  • Affect seedless onboarding password detection
  • Change error handling for biometric cancellation

I recommend running SmokeAccounts (covers account security, login-related flows, and authentication) and SmokeWalletPlatform (covers wallet lifecycle and core platform features including delete wallet flow which uses LoginView).

The risk is medium because:

  • The changes are primarily refactoring (replacing hook with direct class calls)
  • Unit tests have been updated to cover the new implementation
  • The core functionality (login, biometric auth) should work the same way
  • However, authentication is critical and any regression would block all users

View GitHub Actions results

@grvgoel81
Copy link
Copy Markdown
Contributor Author

grvgoel81 commented Jan 23, 2026

Closing this as changes added in PR: #25087

@grvgoel81 grvgoel81 closed this Jan 23, 2026
@github-actions github-actions Bot locked and limited conversation to collaborators Jan 23, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@cursor cursor[bot] cursor[bot] left review comments

@Cal-L Cal-L Cal-L left review comments

@himanshuchawla009 himanshuchawla009 himanshuchawla009 left review comments

@vinnyhoward vinnyhoward vinnyhoward left review comments

Assignees

@grvgoel81 grvgoel81

Labels

size-L team-onboarding Onboarding team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@grvgoel81 @Cal-L @himanshuchawla009 @vinnyhoward