Skip to content

Commit b60cba2

Browse files
feat: Implement New migration failure detection cp-7.60.0 (#22757)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> Due to the previous implementation of detecting migration failures to prompt the Vault Recovery screen causing the Vault Recovery screen to show up on a fresh install to users who had previously had the MM app installed. This is due to on iOS due to keychain not being reset on app uninstall. This method uses a flag stored in FileSystem Storage to check instead of the presence of a backed up vault. Bug: #22567 Bitrise Build: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/3efcb785-454e-4fa6-8cd3-8b8b8b224999 ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin 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** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] 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](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Use a FilesystemStorage-backed migration error flag to trigger vault recovery from onboarding and clear it after successful restore; set the flag on migration failures and add corresponding tests. > > - **Onboarding**: > - Read `MIGRATION_ERROR_HAPPENED` from `FilesystemStorage`; if `'true'` and `getVaultFromBackup()` returns a vault, reset navigation to `Routes.VAULT_RECOVERY.RESTORE_WALLET`. > - Skip check in E2E or when `route.params.delete` is set; add error logging. > - **Vault Recovery (WalletRestored)**: > - On continue, remove `MIGRATION_ERROR_HAPPENED` via `FilesystemStorage.removeItem`; log failures but still navigate to `Routes.ONBOARDING.LOGIN` with `{ isVaultRecovery: true }`. > - **Migrations**: > - On any migration error, set `MIGRATION_ERROR_HAPPENED` in `FilesystemStorage` (uses `Device.isIos()` to avoid iCloud backup) before rethrowing. > - **Constants**: > - Add `MIGRATION_ERROR_HAPPENED` to `app/constants/storage.ts`. > - **Tests**: > - Update/add tests for onboarding migration check, recovery redirect conditions, error handling, and clearing the flag in `WalletRestored`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit e1156ba. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: metamaskbot <[email protected]>
1 parent 85f7a91 commit b60cba2

File tree

6 files changed

+352
-67
lines changed

6 files changed

+352
-67
lines changed

app/components/Views/Onboarding/index.js

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ import { MetaMetricsEvents } from '../../../core/Analytics';
3131
import { Authentication } from '../../../core';
3232
import { getVaultFromBackup } from '../../../core/BackupVault';
3333
import Logger from '../../../util/Logger';
34+
import FilesystemStorage from 'redux-persist-filesystem-storage';
35+
import { MIGRATION_ERROR_HAPPENED } from '../../../constants/storage';
3436
import { ThemeContext, mockTheme } from '../../../util/theme';
3537
import { isE2E } from '../../../util/test/utils';
3638
import { OnboardingSelectorIDs } from '../../../../e2e/selectors/Onboarding/Onboarding.selectors';
@@ -380,21 +382,23 @@ class Onboarding extends PureComponent {
380382
return;
381383
}
382384

383-
const { existingUser } = this.props;
384-
385385
try {
386-
const vaultBackupResult = await getVaultFromBackup();
386+
// Check for migration error flag
387+
// Using FilesystemStorage (excluded from iCloud backup) for reliability
388+
const migrationErrorFlag = await FilesystemStorage.getItem(
389+
MIGRATION_ERROR_HAPPENED,
390+
);
387391

388-
// Detect migration failure scenario:
389-
// - existingUser is false (Redux state was corrupted/reset)
390-
// - BUT vault backup exists (user previously had a wallet)
391-
const migrationFailureDetected =
392-
!existingUser && vaultBackupResult.success && vaultBackupResult.vault;
392+
if (migrationErrorFlag === 'true') {
393+
// Migration failed, check if vault backup exists
394+
const vaultBackupResult = await getVaultFromBackup();
393395

394-
if (migrationFailureDetected) {
395-
this.props.navigation.reset({
396-
routes: [{ name: Routes.VAULT_RECOVERY.RESTORE_WALLET }],
397-
});
396+
if (vaultBackupResult.success && vaultBackupResult.vault) {
397+
// Both migration error and vault backup exist - trigger recovery
398+
this.props.navigation.reset({
399+
routes: [{ name: Routes.VAULT_RECOVERY.RESTORE_WALLET }],
400+
});
401+
}
398402
}
399403
} catch (error) {
400404
Logger.error(

app/components/Views/Onboarding/index.test.tsx

Lines changed: 189 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,20 @@ jest.mock('../../../util/Logger', () => ({
44
log: jest.fn(),
55
}));
66

7+
// Mock FilesystemStorage
8+
jest.mock('redux-persist-filesystem-storage', () => ({
9+
getItem: jest.fn(() => Promise.resolve(null)),
10+
setItem: jest.fn(() => Promise.resolve()),
11+
removeItem: jest.fn(() => Promise.resolve()),
12+
}));
13+
14+
// Mock getVaultFromBackup
15+
jest.mock('../../../core/BackupVault', () => ({
16+
getVaultFromBackup: jest.fn(() =>
17+
Promise.resolve({ success: false, vault: null }),
18+
),
19+
}));
20+
721
// Mock animation components - using existing mocks
822
jest.mock('../../UI/FoxAnimation/FoxAnimation');
923
jest.mock('../../UI/OnboardingAnimation/OnboardingAnimation');
@@ -15,6 +29,8 @@ import {
1529
Animated,
1630
Platform,
1731
} from 'react-native';
32+
import FilesystemStorage from 'redux-persist-filesystem-storage';
33+
import { getVaultFromBackup } from '../../../core/BackupVault';
1834
import { renderScreen } from '../../../util/test/renderWithProvider';
1935
import Onboarding from './';
2036
import { backgroundState } from '../../../util/test/initial-root-state';
@@ -27,6 +43,8 @@ import Routes from '../../../constants/navigation/Routes';
2743
import { ONBOARDING, PREVIOUS_SCREEN } from '../../../constants/navigation';
2844
import { strings } from '../../../../locales/i18n';
2945
import { OAuthError, OAuthErrorType } from '../../../core/OAuthService/error';
46+
import Logger from '../../../util/Logger';
47+
import { MIGRATION_ERROR_HAPPENED } from '../../../constants/storage';
3048

3149
// Mock netinfo - using existing mock
3250
jest.mock('@react-native-community/netinfo');
@@ -1355,14 +1373,24 @@ describe('Onboarding', () => {
13551373
});
13561374

13571375
describe('checkForMigrationFailureAndVaultBackup', () => {
1376+
const mockGetVaultFromBackup = getVaultFromBackup as jest.Mock;
1377+
const mockFilesystemGetItem = FilesystemStorage.getItem as jest.Mock;
1378+
const mockNavReset = mockNav.reset as jest.Mock;
1379+
13581380
beforeEach(() => {
13591381
jest.clearAllMocks();
1360-
(StorageWrapper.getItem as jest.Mock).mockResolvedValue(null);
1382+
mockFilesystemGetItem.mockResolvedValue(null);
1383+
mockGetVaultFromBackup.mockResolvedValue({
1384+
success: false,
1385+
vault: null,
1386+
});
1387+
mockIsE2E = false;
13611388
});
13621389

13631390
it('returns early when route.params.delete is true', async () => {
13641391
// Arrange
1365-
const { toJSON } = renderScreen(
1392+
mockGetVaultFromBackup.mockClear();
1393+
renderScreen(
13661394
Onboarding,
13671395
{ name: 'Onboarding' },
13681396
{
@@ -1372,17 +1400,18 @@ describe('Onboarding', () => {
13721400
);
13731401

13741402
// Act - Component mounts and checkForMigrationFailureAndVaultBackup is called
1375-
await waitFor(() => {
1376-
expect(toJSON()).toBeDefined();
1403+
await act(async () => {
1404+
await new Promise((resolve) => setTimeout(resolve, 100));
13771405
});
13781406

1379-
// Assert - When delete param is true, vault backup check is skipped
1380-
expect(StorageWrapper.getItem).not.toHaveBeenCalled();
1407+
// Assert - When delete param is true, vault backup check is never reached
1408+
expect(mockGetVaultFromBackup).not.toHaveBeenCalled();
13811409
});
13821410

13831411
it('skips vault backup check when running in E2E test environment', async () => {
13841412
// Arrange
13851413
mockIsE2E = true;
1414+
mockGetVaultFromBackup.mockClear();
13861415

13871416
// Act
13881417
renderScreen(
@@ -1394,16 +1423,167 @@ describe('Onboarding', () => {
13941423
);
13951424

13961425
await act(async () => {
1397-
await new Promise((resolve) => setTimeout(resolve, 0));
1426+
await new Promise((resolve) => setTimeout(resolve, 100));
13981427
});
13991428

1400-
// Assert
1401-
expect(StorageWrapper.getItem).not.toHaveBeenCalled();
1429+
// Assert - When in E2E mode, vault backup check is never reached
1430+
expect(mockGetVaultFromBackup).not.toHaveBeenCalled();
14021431

14031432
// Cleanup
14041433
mockIsE2E = false;
14051434
});
14061435

1436+
it('checks migration error flag when not E2E and no delete param', async () => {
1437+
// Arrange
1438+
mockFilesystemGetItem.mockClear();
1439+
mockFilesystemGetItem.mockResolvedValue(null);
1440+
1441+
// Act
1442+
renderScreen(
1443+
Onboarding,
1444+
{ name: 'Onboarding' },
1445+
{
1446+
state: mockInitialState,
1447+
},
1448+
);
1449+
1450+
// Assert
1451+
await waitFor(() => {
1452+
expect(mockFilesystemGetItem).toHaveBeenCalledWith(
1453+
MIGRATION_ERROR_HAPPENED,
1454+
);
1455+
});
1456+
});
1457+
1458+
it('does not redirect when migration error flag is not set', async () => {
1459+
// Arrange
1460+
mockFilesystemGetItem.mockResolvedValue(null);
1461+
mockGetVaultFromBackup.mockResolvedValue({
1462+
success: true,
1463+
vault: 'mock-vault',
1464+
});
1465+
1466+
// Act
1467+
renderScreen(
1468+
Onboarding,
1469+
{ name: 'Onboarding' },
1470+
{
1471+
state: mockInitialState,
1472+
},
1473+
);
1474+
1475+
await act(async () => {
1476+
await new Promise((resolve) => setTimeout(resolve, 100));
1477+
});
1478+
1479+
// Assert
1480+
expect(mockNavReset).not.toHaveBeenCalled();
1481+
});
1482+
1483+
it('does not redirect when migration error flag is set but vault backup does not exist', async () => {
1484+
// Arrange
1485+
mockFilesystemGetItem.mockResolvedValue('true');
1486+
mockGetVaultFromBackup.mockResolvedValue({
1487+
success: false,
1488+
vault: null,
1489+
});
1490+
1491+
// Act
1492+
renderScreen(
1493+
Onboarding,
1494+
{ name: 'Onboarding' },
1495+
{
1496+
state: mockInitialState,
1497+
},
1498+
);
1499+
1500+
await act(async () => {
1501+
await new Promise((resolve) => setTimeout(resolve, 100));
1502+
});
1503+
1504+
// Assert
1505+
expect(mockGetVaultFromBackup).toHaveBeenCalled();
1506+
expect(mockNavReset).not.toHaveBeenCalled();
1507+
});
1508+
1509+
it('redirects to vault recovery when migration error flag is set and vault backup exists', async () => {
1510+
// Arrange
1511+
mockFilesystemGetItem.mockResolvedValue('true');
1512+
mockGetVaultFromBackup.mockResolvedValue({
1513+
success: true,
1514+
vault: 'mock-vault-data',
1515+
});
1516+
1517+
// Act
1518+
renderScreen(
1519+
Onboarding,
1520+
{ name: 'Onboarding' },
1521+
{
1522+
state: mockInitialState,
1523+
},
1524+
);
1525+
1526+
// Assert
1527+
await waitFor(() => {
1528+
expect(mockNavReset).toHaveBeenCalledWith({
1529+
routes: [{ name: Routes.VAULT_RECOVERY.RESTORE_WALLET }],
1530+
});
1531+
});
1532+
});
1533+
1534+
it('handles errors during vault backup check gracefully', async () => {
1535+
// Arrange
1536+
const mockError = new Error('Vault backup check failed');
1537+
mockFilesystemGetItem.mockResolvedValue('true');
1538+
mockGetVaultFromBackup.mockRejectedValue(mockError);
1539+
1540+
// Act
1541+
renderScreen(
1542+
Onboarding,
1543+
{ name: 'Onboarding' },
1544+
{
1545+
state: mockInitialState,
1546+
},
1547+
);
1548+
1549+
await act(async () => {
1550+
await new Promise((resolve) => setTimeout(resolve, 100));
1551+
});
1552+
1553+
// Assert
1554+
expect(Logger.error).toHaveBeenCalledWith(
1555+
mockError,
1556+
'Failed to check for migration failure and vault backup',
1557+
);
1558+
expect(mockNavReset).not.toHaveBeenCalled();
1559+
});
1560+
1561+
it('handles errors during FilesystemStorage read gracefully', async () => {
1562+
// Arrange
1563+
const mockError = new Error('FilesystemStorage read failed');
1564+
mockFilesystemGetItem.mockRejectedValue(mockError);
1565+
1566+
// Act
1567+
renderScreen(
1568+
Onboarding,
1569+
{ name: 'Onboarding' },
1570+
{
1571+
state: mockInitialState,
1572+
},
1573+
);
1574+
1575+
await act(async () => {
1576+
await new Promise((resolve) => setTimeout(resolve, 100));
1577+
});
1578+
1579+
// Assert
1580+
expect(Logger.error).toHaveBeenCalledWith(
1581+
mockError,
1582+
'Failed to check for migration failure and vault backup',
1583+
);
1584+
expect(mockNavReset).not.toHaveBeenCalled();
1585+
});
1586+
14071587
it('accesses existingUser prop from Redux state', async () => {
14081588
// Arrange
14091589
const { toJSON } = renderScreen(

0 commit comments

Comments
 (0)