MetaMask / MetaMask/metamask-mobile
Fix undefined accountId property that is causing an Engine failure during initialization
- Dominant language
- TypeScript
- Stars
- 3k
- Forks
- 1.7k
- Avg merge
- 1d 14h
- Merged PRs (30d)
- 669
Description
### What is this about?
We are continuing to see instances of Sentry errors where it shows that Engine is not found. There are currently two variants of the issue that we believe are caused by the same root issue. In one instance, Engine is not found when NetworkController is being accessed, while in the other, KeyringController is being accessed when Engine is not found:
- NetworkController being accessed - https://metamask.sentry.io/issues/6395530333/?environment=production&project=2299799&query=is%3Aunresolved%20issue.priority%3A%5Bhigh%2C%20medium%5D%20dist%3A1637%20engine%20does%20not%20exist&referrer=issue-stream&sort=date&stream_index=0
- This scenario is a by-product of the `App.tsx` rendering in the event of an Engine initialization failure. App needs to render since the app needs access to the vault recovery flow. In the future, the App.tsx file will not be rendered when Engine fails and vault recovery flow will be hoisted higher in the stack. The area where this error is happening does not affect user experience since they are on vault recovery.
- KeyringController being accessed - https://metamask.sentry.io/issues/6463179230/?environment=production&project=2299799&query=is%3Aunresolved%20issue.priority%3A%5Bhigh%2C%20medium%5D%20dist%3A1637%20engine%20does%20not%20exist&referrer=issue-stream&sort=date&stream_index=2
- This scenario happens when biometrics prompts while the engine fails to initialize. Upon success of biometrics, the app runs through auth logic but fails and attempts to lock the app, in which case it accesses KeyringController from engine. Since engine is not available, this error appears. Similar to the first issue, this error does not affect user experience since the user is on the vault recovery flow.
Diving into their stack traces. Ex: https://metamask.sentry.io/issues/trace/94c89e86313d4860971193ff0d352bb7/?environment=production&groupId=6463179230&node=error-21e301158800481db25188e99925cd3d&pageEnd=2025-03-28T05%3A21%3A22.734&pageStart=2025-03-27T05%3A21%3A22.734&project=2299799&query=is%3Aunresolved%20issue.priority%3A%5Bhigh%2C%20medium%5D%20dist%3A1637%20engine%20does%20not%20exist&referrer=issue-stream&sort=date&source=issue_details&stream_index=2×tamp=1743096082, we can see that in both cases, the underlying error being thrown during `Engine.init` is accountId being undefined. Furthermore, the error throws in the `AccountsController`. Lastly, there has been past suspicions that the error is thrown when another controller is using the `AccountsController` methods.
Some more investigative work is needed to get to the bottom of this issue. Below are some steps to take to get there:
- Collect and record all of the areas where accountId is set on the `AccountsController`. Record this in a document and attach the link to this issue
- Add logs to these areas whenever the value being set is undefined
- See if there are common errors occurring on Extension - Can indicate an issue related to controller interactions between controllers
- BONUS - Fix area if it's obvious and identified
### Scenario
_No response_
### Design
_No response_
### Technical Details
- Collect and record all of the areas where accountId is set on the `AccountsController`. Record this in a document and attach the link to this issue
- Add logs to these areas whenever the value being set is undefined
- See if there are common errors occurring on Extension - Can indicate an issue related to controller interactions between controllers
- BONUS - Fix area if it's obvious and identified
### Threat Modeling Framework
_No response_
### Acceptance Criteria
These are performed
- Collect and record all of the areas where accountId is set on the `AccountsController`. Record this in a document and attach the link to this issue
- Add logs to these areas whenever the value being set is undefined
- See if there are common errors occurring on Extension - Can indicate an issue related to controller interactions between controllers
- BONUS - Fix area if it's obvious and identified
### Stakeholder review needed before the work gets merged
- [ ] Engineering (needed in most cases)
- [ ] Design
- [ ] Product
- [ ] QA (automation tests are required to pass before merging PRs but not all changes are covered by automation tests - please review if QA is needed beyond automation tests)
- [ ] Security
- [ ] Legal
- [ ] Marketing
- [ ] Management (please specify)
- [ ] Other (please specify)
### References
_No response_
Contributor guide
Assessment
This issue has not been assessed yet.