MetaMask / metamask-extension

:globe_with_meridians: :electric_plug: The MetaMask browser extension enables browsing Ethereum blockchain enabled websites
https://metamask.io
Other
11.81k stars 4.82k forks source link

[Bug]: Notifications are not showing up as expected, errors appearing in the console, notification page is not loading #25877

Closed sleepytanya closed 1 month ago

sleepytanya commented 1 month ago

Describe the bug

I don't have exact steps to reproduce, seems like the bug is easy to repro, it happens after some interactions with the extension (probably I was interacting more with the notifications) . At some point notifications are not showing up as expected (red badge doesn't appear), alongside some errors appearing in the console and issues with the notification page loading properly. Issue resolves by reloading the extension.

Probably related: https://github.com/MetaMask/metamask-extension/issues/25748 https://github.com/MetaMask/metamask-extension/issues/25749 https://github.com/MetaMask/metamask-extension/issues/25754 https://github.com/MetaMask/metamask-extension/issues/25750

Expected behavior

Notifications work as expected.

Screenshots/Recordings

https://github.com/user-attachments/assets/a8c82c92-d4c5-4786-9acd-e33867e86448

https://github.com/user-attachments/assets/54526510-b680-4622-9bc9-7ff62f3bd287

Screenshot 2024-07-16 at 19 37 47 Screenshot 2024-07-16 at 19 31 26 Screenshot 2024-07-16 at 18 26 05 Screenshot 2024-07-16 at 18 26 20 Screenshot 2024-07-16 at 18 26 37

Steps to reproduce

  1. Send transaction
  2. Check for notifications

Error messages or log output

No response

Detection stage

In production (default)

Version

v12.0.0

Build type

Beta

Browser

Chrome

Operating system

MacOS

Hardware wallet

No response

Additional context

No response

Severity

No response

Prithpal-Sooriya commented 1 month ago

This has been noted and added to our teams board.

Posting for visibility. From some investigations, there seems to be a couple of things we can do to improve performance and also reliability.

Snap Issues: We have a pre-installed snap that we use for logging in and also for creating keys for profile syncing. We are doing this too much, which is slowing down and also throwing errors in the snap environment. We can cache the snap results and don't need to re-call as often.

Excessive API Calls: On the settings page, we are calling our APIs too many times. For each account, we re-call some APIs. We don't need to do this, so deduping and ensuring we call the minimal amount of APIs. Effects are ran at the account components, we should lift this to the single parent/root.

Storage Setup: Profile syncing secures user data by creating a client side encryption key. Generating this key (KDF) takes time (up to 5 seconds). This is in-memory cached, but does mean we need to eat slow initial loads. We can explore persisting this KDF Key, but will need additional eyes/investigations.

Prithpal-Sooriya commented 1 month ago

Errors have been fixed and cherry-picked into v12.0.0:

https://github.com/MetaMask/metamask-extension/pull/25946

https://github.com/MetaMask/metamask-extension/pull/25979

There is still some slowness in enabling/disabling notifications. This seems to be from the number of API calls required to setup notifications. Discussing with our backend team to see if we can alleviate some slowness.

Prithpal-Sooriya commented 1 month ago

I have tested this on v12. This has been fixed. Notifications are a little slow when enabling/disabling - this is being worked on in a separate issue https://github.com/MetaMask/metamask-extension/issues/25754