Skip to content

Commit 388e5ed

Browse files
authored
fix: update selectedSelectedInternalAccount to not capture exceptions where the selectedAccount is emtpy (#34301)
## **Description** This PR adds a check to make sure that there is a `selectedAccount` before capturing an exception, otherwise we often get misreporting during onboarding. This exception is most useful when we encounter scenarios where a `selectedAccount` exists but the corresponding account isn't in state. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: #29062 ## **Manual testing steps** N/A ## **Screenshots/Recordings** N/A ## **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. #### Performance checks (if applicable) - [x] 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](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93) to import wallets with many accounts and tokens - [ ] I've instrumented key operations with Sentry traces for production performance metrics - See [`trace()`](/app/util/trace.ts) for usage and [`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274) for an example For performance guidelines and tooling, see the [Performance Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers). ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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] > **Low Risk** > Narrow selector and error-reporting change with no auth or data handling impact; existing behavior for invalid non-empty IDs is unchanged. > > **Overview** > **`selectSelectedInternalAccount`** no longer reports to Sentry when the selected account id is an empty string and no matching account exists. That case is treated as “no account selected” (common during onboarding) instead of a missing-account error. > > Sentry **`captureException`** still runs when a **non-empty** selected id has no entry in the accounts map. Behavior for callers stays **`undefined`** when nothing is selected; a test covers the empty-id path. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit b1f95b5. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent e95d2e8 commit 388e5ed

2 files changed

Lines changed: 25 additions & 6 deletions

File tree

app/selectors/accountsController.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,24 @@ describe('Accounts Controller Selectors', () => {
137137
errorMessage,
138138
);
139139
});
140+
141+
it('does not throw an error if the selected account ID is empty', () => {
142+
const result = selectSelectedInternalAccount({
143+
engine: {
144+
backgroundState: {
145+
AccountsController: {
146+
...MOCK_ACCOUNTS_CONTROLLER_STATE,
147+
internalAccounts: {
148+
...MOCK_ACCOUNTS_CONTROLLER_STATE.internalAccounts,
149+
selectedAccount: '',
150+
},
151+
},
152+
},
153+
},
154+
} as RootState);
155+
expect(result).toBeUndefined();
156+
expect(mockedCaptureException).not.toHaveBeenCalled();
157+
});
140158
});
141159
describe('selectInternalAccounts', () => {
142160
it(`returns internal accounts of the accounts controller sorted by the keyring controller's accounts`, () => {

app/selectors/accountsController.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -118,13 +118,14 @@ export const selectSelectedInternalAccount = createSelector(
118118
const account = accounts[selectedAccountId];
119119

120120
if (!account) {
121-
const err = new Error(
122-
`selectSelectedInternalAccount: Account with ID ${selectedAccountId} not found.`,
123-
);
124-
captureException(err);
125-
return undefined;
121+
if (selectedAccountId !== '') {
122+
const err = new Error(
123+
`selectSelectedInternalAccount: Account with ID ${selectedAccountId} not found.`,
124+
);
125+
captureException(err);
126+
}
126127
}
127-
return account;
128+
return account ?? undefined;
128129
},
129130
);
130131

0 commit comments

Comments
 (0)