feat: integrate Sentry into QR account sync receive flow

[grvgoel81]

Description

Implements sentry integration for QR account sync failures, so sync receive-path errors are queryable in Sentry without leaking secrets (OTP, SRP, addresses, raw QR / deeplink payloads).

This change:

• Adds app/core/QrSync/qrSyncTelemetry.ts with feature:qr-sync tags, phase breadcrumbs, sensitive-field scrubbing, and reportQrSyncFailure (via Logger.error → Sentry)
• Reports scanner invalid/expired QR and Add Device submit failures.
• Adds an OTP grant-wait deadline watchdog + GRANT_WAIT_TIMEOUT, and reports terminate / import / provisioning failures.
• Emits scrubbed phase breadcrumbs on controller transitions.

Changelog

CHANGELOG entry: null

Related issues

Refs: https://consensyssoftware.atlassian.net/browse/TO-920

Manual testing steps

Feature: QR account sync Sentry integration

 Background:
    Given I am running a production-like build with metrics reporting enabled
    And I can open Sentry Issues filtered by feature:qr-sync

  Scenario: invalid or expired Add Device QR reports scanner failure
    Given I am on the QR scanner for Add Device / QR sync
    When user scans an invalid or expired sync QR code
    Then the app shows the existing invalid/expired sync error UI
    And a Sentry event is captured with feature:qr-sync and surface:scanner
    And the event extras do not contain OTP, SRP, addresses, or raw QR payloads

  Scenario: grant-wait timeout reports grant_wait failure
    Given a QR sync session is waiting for the extension grant / OTP confirmation
    And the grant-wait deadline elapses without completion
    When the OTP grant-wait watchdog fires
    Then the sync session terminates with GRANT_WAIT_TIMEOUT
    And a Sentry event is captured with feature:qr-sync and surface:grant_wait
    And breadcrumbs show phase transitions without secrets

  Scenario: import / provisioning failure reports import failure
    Given a QR sync session reaches password / import after a successful scan
    When user submits a password or completes import and import/provisioning fails
    Then the app shows the existing import failure UX
    And a Sentry event is captured with feature:qr-sync and surface:import
    And scrubbed extras do not contain mnemonic, OTP, or address values

  Scenario: happy-path sync does not spam failure events
    Given a valid QR sync session between mobile and extension
    When user completes Add Device sync successfully
    Then accounts sync without error UI
    And no QR sync failure Sentry events are created for that session

Screenshots/Recordings

Before

N/A - no UI change

After

N/A - no UI change

Pre-merge author checklist

• I've followed MetaMask Contributor Docs and MetaMask Mobile Coding Standards.
• I've completed the PR template to the best of my ability
• I've included tests if applicable
• I've documented my code using JSDoc format if applicable
• I've applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Performance checks (if applicable)

• I've tested on Android
  • Ideally on a mid-range device; emulator is acceptable
• I've tested with a power user scenario
  • Use these power-user SRPs to import wallets with many accounts and tokens
• I've instrumented key operations with Sentry traces for production performance metrics
  • See trace() for usage and addToken for an example

For performance guidelines and tooling, see the Performance Guide.

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

Medium Risk
New OTP grant-timeout termination changes session behavior when handshake stalls, and error reporting now runs across vault import and provisioning paths—though secrets are scrubbed before Sentry.

Overview
Adds qrSyncTelemetry so QR account sync receive-path failures are reported to Sentry with feature:qr-sync tags, surface/operation metadata, phase breadcrumbs, and scrubbed extras (no OTP, SRP, addresses, or raw QR/deeplink payloads).

reportQrSyncFailure replaces ad-hoc Logger.error calls across Add Device scan submit, the QR scanner (invalid/expired classification), QrSyncController, import/navigation hooks, provisioning, and onboarding provisionFromMetadata.

QrSyncController now schedules an OTP grant-wait watchdog from the display deadline, fails with new GRANT_WAIT_TIMEOUT, clears the timer on connect/completion/teardown, and emits phase breadcrumbs on key transitions; session termination also reports via telemetry.

Unit tests cover scrubbing, breadcrumbs, reportQrSyncFailure, and the grant-timeout behavior.

^{Reviewed by Cursor Bugbot for commit 83d5cd9. Configure here.}

[github-actions]

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.

[metamask-ci]

PR template — items to address before "Ready for review"

Warnings — informational, address before merging:

• Pre-merge author checklist has unchecked items (e.g. "I've tested on Android"). Every box must be consciously checked — see docs/readme/ready-for-review.md.

See docs/readme/ready-for-review.md for the full Definition of Ready for Review.

[cursor]

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

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 83d5cd9. Configure here.}

[github-actions]

🧪 Flaky unit test detection

Run history flaky detection

View recent run history

Historical failure rate is a hint, not proof — review each suggestion in context. See the flaky-test-detection skill for the full pattern reference and manual audit workflow.

Failures / runs sampled per window:

File	7d	15d	30d
app/components/UI/OptinMetrics/OptinMetrics.navigation.test.tsx	0/122	0/167	0/374
app/util/onboarding/finalizeOnboardingCompletion.test.ts	0/122	0/167	0/374

AI-detected flaky patterns

app/components/UI/OptinMetrics/OptinMetrics.navigation.test.tsx

• J1 — Missing act() on async state updates (critical)
  • This matches J1 (critical) because onComplete() is an async callback that triggers Redux dispatches (e.g. setWalletHomeOnboardingStepsEligibleAction) and other state updates inside the component's navigation flow. Calling it directly without act() can cause React to warn about updates outside of act() or lead to intermittent test races (TypeError: terminated). The waitFor after does not wrap the state update itself. No historical flakiness (0 failures). Fix by wrapping the async call in act() per the loaded skill example. Note: add import { act } from 'react-test-renderer'; if not present (common in RN tests).
  • Suggested fix in app/components/UI/OptinMetrics/OptinMetrics.navigation.test.tsx:

    -      mockReset.mockClear();
    -      await onComplete();
    -
    -      await waitFor(() => {
    -        expect(mockReset).toHaveBeenCalledWith(
    -          expect.objectContaining({
    -            routes: [{ name: Routes.ONBOARDING.HOME_NAV }],
    -          }),
    -        );
    -      });
    +      mockReset.mockClear();
    +      await act(async () => {
    +        await onComplete();
    +      });
    +
    +      await waitFor(() => {
    +        expect(mockReset).toHaveBeenCalledWith(
    +          expect.objectContaining({
    +            routes: [{ name: Routes.ONBOARDING.HOME_NAV }],
    +          }),
    +        );
    +      });

app/util/onboarding/finalizeOnboardingCompletion.test.ts

• J10 — jest.spyOn without jest.restoreAllMocks() in afterEach (medium)
  • This matches J10 (medium) because the test uses jest.spyOn(Logger, 'error') in multiple it() blocks (lines ~118 and ~155) but there is no afterEach(() => { jest.restoreAllMocks(); }) to clean up. The per-test mockRestore() is present but insufficient for isolation across tests; spies can leak. beforeEach only does clearAllMocks (good for J3 but not restore). History shows 0 failures but pattern is present per skill.
  • Suggested fix in app/util/onboarding/finalizeOnboardingCompletion.test.ts:

    -describe('finalizeOnboardingCompletion', () => {
    -  const mockDispatch = jest.fn();
    -
    -  beforeEach(() => {
    -    jest.clearAllMocks();
    -    mockProvisionFromMetadata.mockResolvedValue(undefined);
    -  });
    -
    -  it('logs when provisionFromMetadata rejects', async () => {
    -    const loggerSpy = jest.spyOn(Logger, 'error').mockImplementation(() => {
    -      // no-op
    -    });
    -    ...
    +describe('finalizeOnboardingCompletion', () => {
    +  const mockDispatch = jest.fn();
    +
    +  beforeEach(() => {
    +    jest.clearAllMocks();
    +    mockProvisionFromMetadata.mockResolvedValue(undefined);
    +  });
    +
    +  afterEach(() => {
    +    jest.restoreAllMocks();
    +  });
    +
    +  it('logs when provisionFromMetadata rejects', async () => {
    +    const loggerSpy = jest.spyOn(Logger, 'error').mockImplementation(() => {
    +      // no-op
    +    });
    +    ...

• J6 — Arbitrary setTimeout/sleep in test body (high)
  • This matches J6 (high) because await Promise.resolve() is used as a zero-delay synchronization barrier to let async error logging (from rejected mockProvisionFromMetadata or mockDiscoverAccounts) settle before assertions. This is non-deterministic under load/CI (similar to the setTimeout(0) anti-pattern called out in the skill with 321 occurrences). The same pattern appears in the discoverAccounts failure test. Replace with waitFor containing the assertion for deterministic polling. History shows no flakiness but this is a known source of intermittent CI failures.
  • Suggested fix in app/util/onboarding/finalizeOnboardingCompletion.test.ts:

    -    finalizeOnboardingCompletion({
    -      successFlow: ONBOARDING_SUCCESS_FLOW.IMPORT_FROM_SEED_PHRASE,
    -      accountType: AccountType.Imported,
    -      isBasicFunctionalityEnabled: true,
    -      walletSetupAttributionProps: {},
    -      dispatch: mockDispatch,
    -      needsQrProvisioning: true,
    -      discoverAccountsLogContext: 'TestContext',
    -    });
    -
    -    await Promise.resolve();
    -
    -    expect(loggerSpy).toHaveBeenCalledWith(
    -      expect.any(Error),
    -      expect.objectContaining({
    -        tags: expect.objectContaining({
    -          feature: 'qr-sync',
    -          surface: 'import',
    -          operation: 'provision_from_metadata',
    -          source: 'finalizeOnboardingCompletion',
    -          syncFlow: 'new_user',
    -        }),
    -      }),
    -    );
    +    finalizeOnboardingCompletion({
    +      successFlow: ONBOARDING_SUCCESS_FLOW.IMPORT_FROM_SEED_PHRASE,
    +      accountType: AccountType.Imported,
    +      isBasicFunctionalityEnabled: true,
    +      walletSetupAttributionProps: {},
    +      dispatch: mockDispatch,
    +      needsQrProvisioning: true,
    +      discoverAccountsLogContext: 'TestContext',
    +    });
    +
    +    await waitFor(() => {
    +      expect(loggerSpy).toHaveBeenCalledWith(
    +        expect.any(Error),
    +        expect.objectContaining({
    +          tags: expect.objectContaining({
    +            feature: 'qr-sync',
    +            surface: 'import',
    +            operation: 'provision_from_metadata',
    +            source: 'finalizeOnboardingCompletion',
    +            syncFlow: 'new_user',
    +          }),
    +        }),
    +      );
    +    });

This check is informational only and does not block merging.

[lwin-kyaw]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d5c2002587

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[vinnyhoward]

UX owned files look good

[sonarqubecloud]

Quality Gate passed

Issues
3 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

[github-actions]

🔍 Smart E2E Test Selection

• Selected E2E tags: None (no tests recommended)
• Selected Performance tags: None (no tests recommended)
• Risk Level: low
• AI Confidence: 88%

click to see 🤖 AI reasoning details

E2E Test Selection:
This PR makes the following changes to the QR sync feature:

1. New telemetry module (qrSyncTelemetry.ts): Adds Sentry observability helpers for the QR sync receive flow. Replaces Logger.error calls with structured reportQrSyncFailure calls across multiple files (QrSyncController, QrSyncProvisioningService, AddDeviceToWallet, QRScanner, completeExistingUserQrSyncImport, useQrSyncImportNavigation, finalizeOnboardingCompletion).

2. New syncFlow state field: Adds a non-persisted syncFlow field to QrSyncControllerState to distinguish new-user vs existing-user QR sync paths.

3. QrSyncSyncFlows and QrSyncErrorCodes constants: New constants for better runtime access.

4. Enhanced transitionTo method: Now accepts optional patches and error codes, and adds Sentry breadcrumbs on phase transitions.

5. initial-background-state.json update: Adds syncFlow: null to match the new state shape.

6. Test updates: OptinMetrics.navigation.test.tsx updated to match new telemetry format.

Why no E2E tags are needed:

• The changes are purely telemetry/observability improvements - no functional logic changes to QR sync flows
• No E2E smoke tests exist for QR sync or AddDeviceToWallet functionality (confirmed by searching tests/smoke/)
• The initial-background-state.json change only adds a new null field, which won't break existing tests
• No shared components (TabBar, Browser, Confirmations, navigation) are modified
• The finalizeOnboardingCompletion.ts change only replaces Logger.error with reportQrSyncFailure - same error handling behavior, just better structured telemetry
• The QRScanner change adds telemetry for invalid QR scans but doesn't change the scan classification or navigation logic

Performance Test Selection:
The changes are purely telemetry improvements (replacing Logger.error with structured Sentry reporting) and adding a non-persisted state field. There are no changes to rendering, data fetching, asset loading, onboarding flows, or any performance-sensitive code paths. No performance tests are warranted.

View GitHub Actions results