Skip to content

Commit a6c46e6

Browse files
authored
fix: Add delay in checkPermissions for iOS to prevent UI crash (#14602)
## **Description** This PR addresses a UI crash on iOS in `AccountConnect.tsx` caused by a race condition in `permissionsController.getPermissions()`. - Added `await wait(100)` in `checkPermissions` for iOS to resolve the race condition. #### Debugging Notes This issue was tricky to debug as it occurred more frequently in live DEV environments. Eventually, I tracked it down by launching the app via Xcode instead of the regular React Native CLI, which provided access to all native logs and revealed the race condition. ## **Related issues** Fixes: ## **Manual testing steps** - Verified on iOS: UI crash no longer occurs. - Confirmed no impact on Android. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** ![image](https://github.com/user-attachments/assets/fb7f2a29-0422-4e9c-adfe-1eea2a611861) ### **After** - normal connection flow ## **Pre-merge author checklist** - [x] 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). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] 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.
1 parent 6ba5ae4 commit a6c46e6

File tree

2 files changed

+64
-2
lines changed

2 files changed

+64
-2
lines changed

app/core/SDKConnect/handlers/checkPermissions.test.ts

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,31 @@
11
/* eslint-disable @typescript-eslint/ban-ts-comment */
2+
3+
// Mock Platform first, before any imports
4+
jest.mock('react-native/Libraries/Utilities/Platform', () => {
5+
const Platform = {
6+
OS: 'ios',
7+
select: jest.fn()
8+
};
9+
return Platform;
10+
});
11+
12+
// Mock wait util - change this part
13+
jest.mock('../utils/wait.util', () => {
14+
const mockWait = jest.fn().mockResolvedValue(undefined);
15+
return {
16+
wait: mockWait,
17+
waitForCondition: jest.fn(),
18+
waitForKeychainUnlocked: jest.fn().mockResolvedValue(true),
19+
waitForConnectionReadiness: jest.fn(),
20+
waitForEmptyRPCQueue: jest.fn(),
21+
waitForUserLoggedIn: jest.fn(),
22+
waitForAndroidServiceBinding: jest.fn(),
23+
};
24+
});
25+
26+
// Import wait after mocking
27+
import { wait } from '../utils/wait.util';
28+
import { Platform } from 'react-native';
229
import { ApprovalController } from '@metamask/approval-controller';
330
import { PreferencesController } from '@metamask/preferences-controller';
431
import Engine from '../../Engine';
@@ -8,6 +35,7 @@ import { PermissionController } from '@metamask/permission-controller';
835
import { getPermittedAccounts } from '../../../core/Permissions';
936
import { KeyringController } from '@metamask/keyring-controller';
1037

38+
// Rest of the mocks
1139
jest.mock('../Connection', () => ({
1240
RPC_METHODS: jest.requireActual('../Connection').RPC_METHODS,
1341
}));
@@ -55,6 +83,8 @@ describe('checkPermissions', () => {
5583

5684
beforeEach(() => {
5785
jest.clearAllMocks();
86+
// Reset platform to iOS by default
87+
Platform.OS = 'ios';
5888
jest.useFakeTimers().setSystemTime(currentTime);
5989
mockGetPermittedAccounts.mockResolvedValue([]);
6090

@@ -108,15 +138,13 @@ describe('checkPermissions', () => {
108138

109139
it('should return true if permitted accounts exist', async () => {
110140
mockGetPermittedAccounts.mockResolvedValue(['0x123']);
111-
112141
const result = await checkPermissions({ connection, engine });
113142
expect(result).toBe(true);
114143
});
115144

116145
it('should return false if no permitted accounts exist and no approval promise', async () => {
117146
mockGetPermittedAccounts.mockResolvedValue([]);
118147
permissionController.getPermission = jest.fn().mockReturnValue(null);
119-
120148
const result = await checkPermissions({ connection, engine });
121149
expect(result).toBe(false);
122150
});
@@ -135,4 +163,28 @@ describe('checkPermissions', () => {
135163
);
136164
expect(result).toBe(true);
137165
});
166+
167+
describe('platform specific behavior', () => {
168+
it('should add delay on iOS after permission approval', async () => {
169+
Platform.OS = 'ios';
170+
mockGetPermittedAccounts.mockResolvedValue([]);
171+
permissionController.getPermission = jest.fn().mockReturnValue(null);
172+
requestPermissions.mockResolvedValue({});
173+
mockGetPermittedAccounts.mockResolvedValueOnce([]).mockResolvedValueOnce(['0x123']);
174+
175+
await checkPermissions({ connection, engine });
176+
expect(wait).toHaveBeenCalledWith(100);
177+
});
178+
179+
it('should not add delay on Android after permission approval', async () => {
180+
Platform.OS = 'android';
181+
mockGetPermittedAccounts.mockResolvedValue([]);
182+
permissionController.getPermission = jest.fn().mockReturnValue(null);
183+
requestPermissions.mockResolvedValue({});
184+
mockGetPermittedAccounts.mockResolvedValueOnce([]).mockResolvedValueOnce(['0x123']);
185+
186+
await checkPermissions({ connection, engine });
187+
expect(wait).not.toHaveBeenCalledWith(100);
188+
});
189+
});
138190
});

app/core/SDKConnect/handlers/checkPermissions.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
waitForCondition,
1212
waitForKeychainUnlocked,
1313
} from '../utils/wait.util';
14+
import { Platform } from 'react-native';
1415

1516
// TODO: should be more generic and be used in wallet connect and android service as well
1617
export const checkPermissions = async ({
@@ -111,6 +112,15 @@ export const checkPermissions = async ({
111112
}
112113

113114
const res = await connection.approvalPromise;
115+
if (Platform.OS === 'ios') {
116+
// A UI crash happening on ios from AccountConnect.tsx which is triggered by permissionsController.getPermission().
117+
// Seems to be a race condition between the connect being accepted and the approvalPromise being resolved.
118+
// Adding a small delay seems to fix the issue.
119+
// *** Assertion failure in -[RCTNativeAnimatedNodesManager disconnectAnimatedNodes:childTag:](), metamask-mobile/node_modules/react-native/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.m:142
120+
// Exception thrown while executing UI block: 'childNode' is a required parameter
121+
// 0x14146e100 - GPUProcessProxy::gpuProcessExited: reason=IdleExit
122+
await wait(100);
123+
}
114124
const accounts = await getPermittedAccounts(connection.channelId);
115125
DevLogger.log(`checkPermissions approvalPromise completed`, res);
116126
return accounts.length > 0;

0 commit comments

Comments
 (0)