Skip to content

Commit

Permalink
fix: cp-7.41.0 Handle better errors by getting tags in pre-init (#13598)
Browse files Browse the repository at this point in the history
## **Description**

Handle errors in a more scalable way by adding a try catch to get tags
function. Also added test coverage

## **Related issues**

Fixes:

## **Manual testing steps**

1. Go to this page...
2.
3.

## **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.
  • Loading branch information
tommasini authored Feb 24, 2025
1 parent 1d6aeb9 commit 257c3ac
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 97 deletions.
80 changes: 20 additions & 60 deletions app/util/sentry/tags/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,22 +226,6 @@ describe('Tags Utils', () => {
expect(tags?.['wallet.transaction_count']).toStrictEqual(3);
});

it('returns undefined if ApprovalController is not defined', () => {
const state = {
...initialRootState,
engine: {
backgroundState: {
...backgroundState,
ApprovalController: undefined,
},
},
} as unknown as RootState;

const tags = getTraceTags(state);

expect(tags).toBeUndefined();
});

it('handles undefined pendingApprovals in ApprovalController', () => {
const state = {
...initialRootState,
Expand Down Expand Up @@ -278,77 +262,53 @@ describe('Tags Utils', () => {
expect(tags?.['wallet.pending_approval']).toBeUndefined();
});

it('returns undefined if NftController.allNfts is not defined', () => {
const state = {
...initialRootState,
engine: {
backgroundState: {
...backgroundState,
NftController: {
...backgroundState.NftController,
allNfts: undefined,
},
},
},
} as unknown as RootState;

const tags = getTraceTags(state);

expect(tags).toBeUndefined();
});

it('returns undefined if NotificationServicesController.metamaskNotificationsList is not defined', () => {
it('continues execution when individual tag collection fails', () => {
const state = {
...initialRootState,
engine: {
backgroundState: {
...backgroundState,
NotificationServicesController: {
metamaskNotificationsList: undefined,
AccountsController: {
get accounts() {
throw new Error('Test error');
},
},
},
},
} as unknown as RootState;

const tags = getTraceTags(state);

expect(tags).toBeUndefined();
});

it('returns undefined if TokensController.allTokens is not defined', () => {
const state = {
...initialRootState,
engine: {
backgroundState: {
...backgroundState,
TokensController: {
allTokens: undefined,
allTokens: {
'0x1': {
'0x1234': [{}, {}],
},
},
},
},
},
} as unknown as RootState;

const tags = getTraceTags(state);

expect(tags).toBeUndefined();
expect(tags?.['wallet.account_count']).toBeUndefined();
expect(tags?.['wallet.token_count']).toBeDefined();
});

it('returns undefined if TransactionController.transactions is not defined', () => {
it('handles missing controllers gracefully', () => {
const state = {
...initialRootState,
engine: {
backgroundState: {
...backgroundState,
TransactionController: {
transactions: undefined,
},
ApprovalController: undefined,
NftController: undefined,
TokensController: undefined,
},
},
} as unknown as RootState;

const tags = getTraceTags(state);

expect(tags).toBeUndefined();
expect(tags).toBeDefined();
expect(tags['wallet.pending_approval']).toBeUndefined();
expect(tags['wallet.nft_count']).toBeUndefined();
expect(tags['wallet.token_count']).toBeUndefined();
});
});
});
106 changes: 69 additions & 37 deletions app/util/sentry/tags/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,42 +7,74 @@ import { selectTransactions } from '../../../selectors/transactionController';
import { selectPendingApprovals } from '../../../selectors/approvalController';

export function getTraceTags(state: RootState) {
if (!state?.engine?.backgroundState?.AccountsController) return;
if (!state?.engine?.backgroundState?.NftController) return;
if (!state?.engine?.backgroundState?.NftController?.allNfts) return;
if (!state?.engine?.backgroundState?.NotificationServicesController) return;
const tags: Record<string, number | string | boolean> = {};

try {
tags['wallet.unlocked'] = state.user.userLoggedIn;
} catch (_) {
/* empty */
}

// Early return if core engine state is not available
if (
!state?.engine?.backgroundState?.NotificationServicesController
?.metamaskNotificationsList
)
return;
if (!state?.engine?.backgroundState?.TokensController) return;
if (!state?.engine?.backgroundState?.TokensController?.allTokens) return;
if (!state?.engine?.backgroundState?.TransactionController) return;
if (!state?.engine?.backgroundState?.TransactionController?.transactions)
return;
if (!state?.engine?.backgroundState?.ApprovalController) return;

if (!Object.keys(state?.engine?.backgroundState).length) return;

const unlocked = state.user.userLoggedIn;
const accountCount = selectInternalAccounts(state)?.length;
const nftCount = selectAllNftsFlat(state)?.length;
const notificationCount = getNotificationsList(state)?.length;
const tokenCount = selectAllTokensFlat(state)?.length;
const transactionCount = selectTransactions(state)?.length;
const pendingApprovals = selectPendingApprovals(state);
const pendingApprovalsValues = Object.values(pendingApprovals ?? {});

const firstApprovalType = pendingApprovalsValues?.[0]?.type;

return {
'wallet.account_count': accountCount,
'wallet.nft_count': nftCount,
'wallet.notification_count': notificationCount,
'wallet.pending_approval': firstApprovalType,
'wallet.token_count': tokenCount,
'wallet.transaction_count': transactionCount,
'wallet.unlocked': unlocked,
};
!state?.engine?.backgroundState ||
!Object.keys(state.engine.backgroundState).length
) {
return tags;
}

try {
if (state?.engine?.backgroundState?.AccountsController) {
tags['wallet.account_count'] = selectInternalAccounts(state)?.length;
}
} catch (_) {
/* empty */
}

try {
if (state?.engine?.backgroundState?.NftController?.allNfts) {
tags['wallet.nft_count'] = selectAllNftsFlat(state)?.length;
}
} catch (_) {
/* empty */
}

try {
if (
state?.engine?.backgroundState?.NotificationServicesController
?.metamaskNotificationsList
) {
tags['wallet.notification_count'] = getNotificationsList(state)?.length;
}
} catch (_) {
/* empty */
}

try {
if (state?.engine?.backgroundState?.TokensController?.allTokens) {
tags['wallet.token_count'] = selectAllTokensFlat(state)?.length;
}
} catch (_) {
/* empty */
}

try {
if (state?.engine?.backgroundState?.TransactionController?.transactions) {
tags['wallet.transaction_count'] = selectTransactions(state)?.length;
}
} catch (_) {
/* empty */
}

try {
if (state?.engine?.backgroundState?.ApprovalController) {
const pendingApprovals = selectPendingApprovals(state);
const pendingApprovalsValues = Object.values(pendingApprovals ?? {});
tags['wallet.pending_approval'] = pendingApprovalsValues?.[0]?.type;
}
} catch (_) {
/* empty */
}

return tags;
}

0 comments on commit 257c3ac

Please sign in to comment.