Skip to content

Commit b36e637

Browse files
authored
fix(perps): resolve CLIENT_NOT_INITIALIZED race condition during initialization (MetaMask#22178)
## **Description** This PR adds initialization state machine and guards to prevent CLIENT_NOT_INITIALIZED errors in the Perps feature. **Problem:** - Users experiencing CLIENT_NOT_INITIALIZED errors (1,251 events affecting 590+ users) - Errors occur during controller reinitialization when switching accounts/networks - `usePerpsPositionData` hook was attempting operations before controller completed initialization **Investigation Process:** - Sentry traces for CLIENT_NOT_INITIALIZED errors pointed to `usePerpsPositionData.ts` - Stack traces showed errors occurring during interval-based data fetching (REST API polling for historical candles) - Root cause identified: intervals firing during controller reinitialization - Reinitialization gap: `providers.clear()` → create new provider → 200ms delay → `isInitialized = true` - During this gap, `usePerpsPositionData` hook intervals continued executing, causing CLIENT_NOT_INITIALIZED errors **Solution:** 1. **PerpsController State Machine**: Added `InitializationState` enum tracking (UNINITIALIZED � INITIALIZING � INITIALIZED � FAILED) with retry logic (3 attempts, exponential backoff: 1s, 2s, 4s) 2. **Initialization Guards**: Added checks in `usePerpsPositionData` to block operations when `initializationState !== 'initialized'` 3. **WebSocket Ready Wait**: Added 500ms delay before marking controller as initialized to ensure WebSocket transport is ready 4. **Redux Selector**: Created `selectPerpsInitializationState` for UI components to check initialization status **Key Changes:** - `PerpsController.ts`: State machine, retry logic, initialization tracking - `usePerpsPositionData.ts`: Guards on historical data loading, price subscriptions, and interval setup - `selectors/perpsController/index.ts`: Selector for initialization state - `PerpsConnectionManager.ts`: Updated to use new `init()` method - `wait.ts`: Utility function for delays **Note on Impact Validation:** These changes target the identified reinitialization timing gaps that cause CLIENT_NOT_INITIALIZED errors. Production monitoring after deployment is required to validate the full impact on all Sentry issues listed below. Some issues may have multiple contributing factors beyond the reinitialization gap. **Android Fix:** Fixed dependency array issue that caused double-triggering of effects on Android by removing redundant `isControllerInitialized` from dependency arrays (kept only `initializationState` since `isControllerInitialized` is derived from it). ## **Changelog** CHANGELOG entry: Fixed Perps initialization errors causing CLIENT_NOT_INITIALIZED failures during account and network switches ## **Related issues** Expected to address: https://consensyssoftware.atlassian.net/browse/TAT-1932 **Sentry Issues Expected to Improve:** - [METAMASK-MOBILE-4RN7](https://metamask.sentry.io/issues/6255388807/) - CLIENT_NOT_INITIALIZED (933 events, 348 users) **[HIGH confidence]** - Stack trace directly matches reinitialization gap - [METAMASK-MOBILE-4QB2](https://metamask.sentry.io/issues/6248653802/) - getUserAccount failed (164 events, 164 users) **[MODERATE confidence]** - Same error type, may have multiple code paths - [METAMASK-MOBILE-4R5X](https://metamask.sentry.io/issues/6253182536/) - Missing activeProvider (97 events, 46 users) **[MODERATE confidence]** - Same error type, may have multiple code paths - [METAMASK-MOBILE-4RZD](https://metamask.sentry.io/issues/6259562071/) - fetchHistoricalCandles error (38 events, 31 users) **[MODERATE confidence]** - Same error type, may have multiple code paths - [METAMASK-MOBILE-4SW3](https://metamask.sentry.io/issues/6268066927/) - Empty account address (17 events, 5 users) **[MODERATE confidence]** - Same error type, may have multiple code paths - [METAMASK-MOBILE-4TR2](https://metamask.sentry.io/issues/6275226234/) - Empty orderbook (2 events, 2 users) **[MODERATE confidence]** - Same error type, may have multiple code paths **Validation Approach:** The initialization guards specifically target the ~200ms reinitialization gap that occurs during account/network switches. Post-deployment monitoring is essential to: 1. Confirm reduction in CLIENT_NOT_INITIALIZED error rates 2. Identify any remaining edge cases not covered by these changes 3. Validate that the improvements don't introduce new issues ## **Manual testing steps** ```gherkin Feature: Perps Controller Initialization Guards Scenario: User opens Perps feature on cold start Given the app is not running When user launches the app And navigates to Perps Then PerpsLoadingSkeleton should be displayed And controller should initialize within 3 attempts And user should see market data without CLIENT_NOT_INITIALIZED errors Scenario: User switches accounts while viewing Perps Given user is on Perps Trade screen with Account A When user switches to Account B via account selector Then loading state should appear during reinitialization And chart data should refresh for new account And no CLIENT_NOT_INITIALIZED errors should occur Scenario: User switches networks while viewing Perps Given user is on Perps with mainnet When user switches to testnet via network selector Then loading state should appear during reinitialization And provider should reinitialize with testnet configuration And no CLIENT_NOT_INITIALIZED errors should occur Scenario: Initialization fails due to network issues Given the device has poor network connectivity When user opens Perps feature Then controller should retry initialization 3 times with exponential backoff (1s, 2s, 4s) And appropriate error state should be shown if all attempts fail Scenario: Android chart rendering Given user is on Android device When user navigates to Perps Trade screen Then chart should render candles without showing skeleton indefinitely And candles should load correctly on first try ``` ## **Screenshots/Recordings** ### **Before** CLIENT_NOT_INITIALIZED errors occurring during: - Account switches - Network changes - Rapid navigation - Cold starts with slow connections See Sentry dashboard for error frequency (1,251 events affecting 590+ users). ### **After** - Controller initialization tracked via state machine - Retry logic handles transient failures - UI operations deferred until initialization complete - Existing PerpsLoadingSkeleton provides user feedback during initialization - Android charts render correctly without dependency array double-triggering ## **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 - [ ] 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] > Adds an initialization state machine with retry/backoff, gates UI work until initialized, updates connection manager to use new `init()`, and introduces a shared `wait` utility with corresponding tests and state fixtures. > > - **PerpsController**: > - Introduces `InitializationState` state machine with `initializationState/error/attempts` in Redux. > - Replaces `initializeProviders()` with `init()`; adds retry with exponential backoff and `wait` utility; defers `isInitialized` until WS ready; clearer errors in `getActiveProvider`. > - Network toggle now re-inits via `init()`; exports `InitializationState`. > - **Hooks/UI**: > - `usePerpsPositionData` defers historical fetch, subscriptions, and intervals until `initializationState === 'initialized'`. > - **Selectors**: > - Adds `selectPerpsInitializationState`. > - **Connection Manager**: > - Uses `PerpsController.init()` and shared `wait`; reconnection/connection flows updated accordingly. > - **Utilities**: > - Adds `utils/wait` with tests. > - **Tests/Fixtures**: > - Updates tests to new init flow and guards; mocks `wait`; adds init retry test; updates Engine tests, snapshots, and initial background state with new initialization fields. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 08f44da. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 955cd1e commit b36e637

15 files changed

Lines changed: 490 additions & 147 deletions

File tree

app/components/UI/Perps/controllers/PerpsController.test.ts

Lines changed: 139 additions & 57 deletions
Large diffs are not rendered by default.

app/components/UI/Perps/controllers/PerpsController.ts

Lines changed: 156 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -116,14 +116,21 @@ import {
116116
validatedVersionGatedFeatureFlag,
117117
} from '../../../../util/remoteFeatureFlag';
118118
import { parseCommaSeparatedString } from '../utils/stringParseUtils';
119-
120-
// Simple wait utility
121-
const wait = (ms: number): Promise<void> =>
122-
new Promise((resolve) => setTimeout(resolve, ms));
119+
import { wait } from '../utils/wait';
123120

124121
// Re-export error codes from separate file to avoid circular dependencies
125122
export { PERPS_ERROR_CODES, type PerpsErrorCode } from './perpsErrorCodes';
126123

124+
/**
125+
* Initialization state enum for state machine tracking
126+
*/
127+
export enum InitializationState {
128+
UNINITIALIZED = 'uninitialized',
129+
INITIALIZING = 'initializing',
130+
INITIALIZED = 'initialized',
131+
FAILED = 'failed',
132+
}
133+
127134
const ON_RAMP_GEO_BLOCKING_URLS = {
128135
// Use UAT endpoint since DEV endpoint is less reliable.
129136
DEV: 'https://on-ramp.uat-api.cx.metamask.io/geolocation',
@@ -143,6 +150,11 @@ export type PerpsControllerState = {
143150
isTestnet: boolean; // Dev toggle for testnet
144151
connectionStatus: 'disconnected' | 'connecting' | 'connected';
145152

153+
// Initialization state machine
154+
initializationState: InitializationState;
155+
initializationError: string | null;
156+
initializationAttempts: number;
157+
146158
// Account data (persisted) - using HyperLiquid property names
147159
accountState: AccountState | null;
148160

@@ -266,6 +278,9 @@ export const getDefaultPerpsControllerState = (): PerpsControllerState => ({
266278
activeProvider: 'hyperliquid',
267279
isTestnet: false, // Default to mainnet
268280
connectionStatus: 'disconnected',
281+
initializationState: InitializationState.UNINITIALIZED,
282+
initializationError: null,
283+
initializationAttempts: 0,
269284
accountState: null,
270285
positions: [],
271286
perpsBalances: {},
@@ -345,6 +360,24 @@ const metadata: StateMetadata<PerpsControllerState> = {
345360
includeInDebugSnapshot: false,
346361
usedInUi: true,
347362
},
363+
initializationState: {
364+
includeInStateLogs: true,
365+
persist: false,
366+
includeInDebugSnapshot: false,
367+
usedInUi: true,
368+
},
369+
initializationError: {
370+
includeInStateLogs: true,
371+
persist: false,
372+
includeInDebugSnapshot: false,
373+
usedInUi: true,
374+
},
375+
initializationAttempts: {
376+
includeInStateLogs: true,
377+
persist: false,
378+
includeInDebugSnapshot: false,
379+
usedInUi: false,
380+
},
348381
pendingOrders: {
349382
includeInStateLogs: true,
350383
persist: false,
@@ -706,10 +739,6 @@ export class PerpsController extends BaseController<
706739
);
707740

708741
this.providers = new Map();
709-
710-
this.initializeProviders().catch((error) => {
711-
Logger.error(ensureError(error), this.getErrorContext('constructor'));
712-
});
713742
}
714743

715744
private setBlockedRegionList(list: string[], source: 'remote' | 'fallback') {
@@ -1169,7 +1198,7 @@ export class PerpsController extends BaseController<
11691198
* Must be called before using any other methods
11701199
* Prevents double initialization with promise caching
11711200
*/
1172-
async initializeProviders(): Promise<void> {
1201+
async init(): Promise<void> {
11731202
if (this.isInitialized) {
11741203
return;
11751204
}
@@ -1183,61 +1212,125 @@ export class PerpsController extends BaseController<
11831212
}
11841213

11851214
/**
1186-
* Actual initialization implementation
1215+
* Actual initialization implementation with retry logic
11871216
*/
11881217
private async performInitialization(): Promise<void> {
1218+
const maxAttempts = 3;
1219+
const baseDelay = 1000;
1220+
1221+
this.update((state) => {
1222+
state.initializationState = InitializationState.INITIALIZING;
1223+
state.initializationError = null;
1224+
state.initializationAttempts = 0;
1225+
});
1226+
11891227
DevLogger.log('PerpsController: Initializing providers', {
11901228
currentNetwork: this.state.isTestnet ? 'testnet' : 'mainnet',
11911229
existingProviders: Array.from(this.providers.keys()),
11921230
timestamp: new Date().toISOString(),
11931231
});
11941232

1195-
// Disconnect existing providers to close WebSocket connections
1196-
const existingProviders = Array.from(this.providers.values());
1197-
if (existingProviders.length > 0) {
1198-
DevLogger.log('PerpsController: Disconnecting existing providers', {
1199-
count: existingProviders.length,
1200-
timestamp: new Date().toISOString(),
1201-
});
1202-
await Promise.all(
1203-
existingProviders.map((provider) => provider.disconnect()),
1204-
);
1205-
}
1206-
this.providers.clear();
1233+
let lastError: Error | null = null;
12071234

1208-
DevLogger.log(
1209-
'PerpsController: Creating provider with HIP-3 configuration',
1210-
{
1211-
hip3Enabled: this.hip3Enabled,
1212-
hip3AllowlistMarkets: this.hip3AllowlistMarkets,
1213-
hip3BlocklistMarkets: this.hip3BlocklistMarkets,
1214-
hip3ConfigSource: this.hip3ConfigSource,
1215-
isTestnet: this.state.isTestnet,
1216-
},
1217-
);
1235+
for (let attempt = 1; attempt <= maxAttempts; attempt++) {
1236+
try {
1237+
this.update((state) => {
1238+
state.initializationAttempts = attempt;
1239+
});
12181240

1219-
this.providers.set(
1220-
'hyperliquid',
1221-
new HyperLiquidProvider({
1222-
isTestnet: this.state.isTestnet,
1223-
hip3Enabled: this.hip3Enabled,
1224-
allowlistMarkets: this.hip3AllowlistMarkets,
1225-
blocklistMarkets: this.hip3BlocklistMarkets,
1226-
}),
1227-
);
1241+
// Disconnect existing providers to close WebSocket connections
1242+
const existingProviders = Array.from(this.providers.values());
1243+
if (existingProviders.length > 0) {
1244+
DevLogger.log('PerpsController: Disconnecting existing providers', {
1245+
count: existingProviders.length,
1246+
timestamp: new Date().toISOString(),
1247+
});
1248+
await Promise.all(
1249+
existingProviders.map((provider) => provider.disconnect()),
1250+
);
1251+
}
1252+
this.providers.clear();
1253+
1254+
DevLogger.log(
1255+
'PerpsController: Creating provider with HIP-3 configuration',
1256+
{
1257+
hip3Enabled: this.hip3Enabled,
1258+
hip3AllowlistMarkets: this.hip3AllowlistMarkets,
1259+
hip3BlocklistMarkets: this.hip3BlocklistMarkets,
1260+
hip3ConfigSource: this.hip3ConfigSource,
1261+
isTestnet: this.state.isTestnet,
1262+
},
1263+
);
1264+
1265+
this.providers.set(
1266+
'hyperliquid',
1267+
new HyperLiquidProvider({
1268+
isTestnet: this.state.isTestnet,
1269+
hip3Enabled: this.hip3Enabled,
1270+
allowlistMarkets: this.hip3AllowlistMarkets,
1271+
blocklistMarkets: this.hip3BlocklistMarkets,
1272+
}),
1273+
);
1274+
1275+
// Future providers can be added here with their own authentication patterns:
1276+
// - Some might use API keys: new BinanceProvider({ apiKey, apiSecret })
1277+
// - Some might use different wallet patterns: new GMXProvider({ signer })
1278+
// - Some might not need auth at all: new DydxProvider()
1279+
1280+
// Wait for WebSocket transport to be ready before marking as initialized
1281+
await wait(PERPS_CONSTANTS.RECONNECTION_CLEANUP_DELAY_MS);
1282+
1283+
this.isInitialized = true;
1284+
this.update((state) => {
1285+
state.initializationState = InitializationState.INITIALIZED;
1286+
state.initializationError = null;
1287+
});
1288+
1289+
DevLogger.log('PerpsController: Providers initialized successfully', {
1290+
providerCount: this.providers.size,
1291+
activeProvider: this.state.activeProvider,
1292+
timestamp: new Date().toISOString(),
1293+
attempts: attempt,
1294+
});
12281295

1229-
// Future providers can be added here with their own authentication patterns:
1230-
// - Some might use API keys: new BinanceProvider({ apiKey, apiSecret })
1231-
// - Some might use different wallet patterns: new GMXProvider({ signer })
1232-
// - Some might not need auth at all: new DydxProvider()
1296+
return; // Exit retry loop on success
1297+
} catch (error) {
1298+
lastError = ensureError(error);
12331299

1234-
// Wait for WebSocket transport to be ready before marking as initialized
1235-
await wait(PERPS_CONSTANTS.RECONNECTION_CLEANUP_DELAY_MS);
1300+
Logger.error(
1301+
lastError,
1302+
this.getErrorContext('performInitialization', {
1303+
attempt,
1304+
maxAttempts,
1305+
}),
1306+
);
12361307

1237-
this.isInitialized = true;
1238-
DevLogger.log('PerpsController: Providers initialized successfully', {
1239-
providerCount: this.providers.size,
1240-
activeProvider: this.state.activeProvider,
1308+
// If not the last attempt, wait before retrying (exponential backoff)
1309+
if (attempt < maxAttempts) {
1310+
const delay = baseDelay * Math.pow(2, attempt - 1); // 1s, 2s, 4s
1311+
DevLogger.log(
1312+
`PerpsController: Retrying initialization in ${delay}ms`,
1313+
{
1314+
attempt,
1315+
maxAttempts,
1316+
error: lastError.message,
1317+
},
1318+
);
1319+
await wait(delay);
1320+
}
1321+
}
1322+
}
1323+
1324+
this.isInitialized = false;
1325+
this.update((state) => {
1326+
state.initializationState = InitializationState.FAILED;
1327+
state.initializationError = lastError?.message ?? 'Unknown error';
1328+
});
1329+
this.initializationPromise = null; // Clear promise to allow retry
1330+
1331+
DevLogger.log('PerpsController: Initialization failed', {
1332+
error: lastError?.message,
1333+
attempts: maxAttempts,
12411334
timestamp: new Date().toISOString(),
12421335
});
12431336
}
@@ -1292,12 +1385,20 @@ export class PerpsController extends BaseController<
12921385
}
12931386

12941387
// Check if not initialized
1295-
if (!this.isInitialized) {
1388+
if (
1389+
this.state.initializationState !== InitializationState.INITIALIZED ||
1390+
!this.isInitialized
1391+
) {
1392+
const errorMessage =
1393+
this.state.initializationState === InitializationState.FAILED
1394+
? `${PERPS_ERROR_CODES.CLIENT_NOT_INITIALIZED}: ${this.state.initializationError || 'Initialization failed'}`
1395+
: PERPS_ERROR_CODES.CLIENT_NOT_INITIALIZED;
1396+
12961397
this.update((state) => {
1297-
state.lastError = PERPS_ERROR_CODES.CLIENT_NOT_INITIALIZED;
1398+
state.lastError = errorMessage;
12981399
state.lastUpdateTimestamp = Date.now();
12991400
});
1300-
throw new Error(PERPS_ERROR_CODES.CLIENT_NOT_INITIALIZED);
1401+
throw new Error(errorMessage);
13011402
}
13021403

13031404
const provider = this.providers.get(this.state.activeProvider);
@@ -3746,7 +3847,7 @@ export class PerpsController extends BaseController<
37463847
// Reset initialization state and reinitialize provider with new testnet setting
37473848
this.isInitialized = false;
37483849
this.initializationPromise = null;
3749-
await this.initializeProviders();
3850+
await this.init();
37503851

37513852
DevLogger.log('PerpsController: Network toggle completed', {
37523853
newNetwork,

app/components/UI/Perps/controllers/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
export {
3232
PerpsController,
3333
getDefaultPerpsControllerState,
34+
InitializationState,
3435
} from './PerpsController';
3536
export type {
3637
PerpsControllerState,

app/components/UI/Perps/hooks/usePerpsPositionData.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,15 @@ jest.mock('../../../../core/Engine', () => ({
3131
},
3232
}));
3333

34+
// Mock Redux selector
35+
jest.mock('react-redux', () => ({
36+
useSelector: jest.fn(() => 'initialized'), // Default to initialized state
37+
}));
38+
39+
jest.mock('../selectors/perpsController', () => ({
40+
selectPerpsInitializationState: jest.fn(),
41+
}));
42+
3443
describe('usePerpsPositionData', () => {
3544
const mockFetchHistoricalCandles = Engine.context.PerpsController
3645
.fetchHistoricalCandles as jest.Mock;

0 commit comments

Comments
 (0)