Optimise inbox to fix message reception issues and disconnection - #1481
Open
andy-verida wants to merge 15 commits into
Open
Optimise inbox to fix message reception issues and disconnection#1481andy-verida wants to merge 15 commits into
andy-verida wants to merge 15 commits into
Conversation
aurelticot
self-requested a review
July 3, 2024 05:10
7 tasks
andy-verida
marked this pull request as ready for review
July 8, 2024 10:37
andy-verida
commented
Jul 9, 2024
| healthCheck() | ||
| initInboxMessaging() | ||
| // Listen for the inbox event that failed to connect from the SDK, and active the health check | ||
| inbox.on('INBOX_FAILED_TO_CONNECT', healthCheck) |
Contributor
Author
There was a problem hiding this comment.
Listening for the event INBOX_FAILED_TO_CONNECT from the protocol
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
💭 What's changed?
Closes #1477
Notion: https://www.notion.so/verida/Wallet-Inbox-not-receiving-messages-ec2df6de2ffb4823bb7bcda06388c842
client-tsto notify when the inbox fails to connect and needs to do a heal checkreact-nativeto fix the issue that crashes the app when trying to restart the app (App crashes on reload in development mode with JSC Engine software-mansion/react-native-reanimated#4783 (comment))The problem and solution
Managing connections is more challenging on mobile devices as the app state can change and the OS can force disconnections. Mobile device internet connections change frequently.
The current way of managing connections from the protocol is quite good for dealing with desktop environments but not mobile devices. I think we need to add a
healthCheckmethod for any live connection DBs and constantly perform checks at intervals. It's cheap to call theDB.info()method.I'm unsure why the connections do not work in some cases, even when trying to reestablish them. We must re-initialize the DBs(Inbox), and in the worst case, soft restart the app to get the connections back.
Implementation:
healthCheckmethod: Perform health checks and reconnect attempts with a retry mechanism.Patch the
client-tsby emitting an eventINBOX_FAILED_TO_CONNECTAt this file https://github.com/verida/verida-js/blob/main/packages/client-ts/src/context/engines/verida/messaging/inbox.ts#L155-L158
This indicates the inbox is struggling to reconnect, especially on mobile devices where connections might be disconnected by the OS.
🧪 How to test these changes?
😨 Anything to be aware of?