Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a critical error by adding undefined checks when accessing nested properties in the defiPositions controller's state. The fix prevents potential runtime errors when selectedAccountAddr or chain keys don't exist in the state object.
- Added optional chaining operators to safely access nested state properties
- Applied the fix to two locations where
providerErrorsis accessed from nested state
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
superKalo
left a comment
There was a problem hiding this comment.
I think the problem here is probably different - while #updatePositions is running, something (e.g. removeAccountData, account switch) can delete this.#state[selectedAccountAddr]. Later, updateSingleNetwork (or the catch {} path) tries to do:
this.#state[selectedAccountAddr][chain] = { ... }
... and blows up because the account bucket no longer exists.
I think this might be another scenario where we need a guard after every await in the updateSingleNetwork logic before writing and prevent writes after the account bucket has been deleted, rather than recreating state that was probably intentionally removed. Can you please check?
I think you can try to simulate this by artificially delaying await fetchCustomPositions logic and changing accounts in the meanwhile?
You can cross-check with @sonytooo who's more familiar with this defi positions error handling logic btw.
Fixes this issue from Sentry: https://monitor.ambire.com/organizations/ambire/issues/275/?environment=production&project=3&query=is%3Aunresolved%20error.unhandled%3Atrue&referrer=issue-stream&sort=date&stream_index=8