Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: safe waku #400

Merged
merged 3 commits into from
Sep 19, 2023
Merged

feat: safe waku #400

merged 3 commits into from
Sep 19, 2023

Conversation

agazso
Copy link
Contributor

@agazso agazso commented Sep 18, 2023

This PR adds a safe wrapper around sending and receiving waku messages. Also moves queueing and retry/resync logic to the safe wrapper from the adapter. The long term goal is to abstract away the connectivity related issues from the application.

Notes:

  • There are known issues with waku-js (such as this, this and this) that makes it very difficult or impossible to handle all errors gracefully in a wrapper at the moment.
  • The Svelte Readable subscribe function calls the callback function immediately, which is not needed, that's why there are the firstChatStoreSave and firstObjectStoreSave variables.
  • This safe wrapper approach does not work very well with the Svelte autoreload functionality. Editing and saving unrelated code may result in having multiple connection issues running in the background. In that case a refresh helps.
  • Currently the logging is turned on with debug loglevel for the wrapper. This helps building intuition about the connectivity issues and also help to see when to refresh the browser (see the previous point).
  • It would be good to disable the text inputs in the chat while the message is being sent and re-enable when it is sent. Currently only the send button is disabled but it is possible to send a message by pressing Enter. Maybe disabling sending with Enter can be a solution.
  • Added preliminary support for multiple peers, however it looks like that if any of the connections are not reliable then it makes the initial connection more flaky. The solution could be to only block until there is at least one peer that is connected, but at the moment it's not clear what is the best way to achieve it. That will be addressed in a separate PR. (Support multiple waku peers #404)
  • The disconnection flow is not heavily tested and there may be issues if there are queued messages. That will be addressed in a separate PR. (Safe waku disconnect flow #405)

@vercel
Copy link

vercel bot commented Sep 18, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
waku-objects-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 19, 2023 11:13am

@agazso
Copy link
Contributor Author

agazso commented Sep 19, 2023

@filoozom fixed the resubscribe and the callback, please re-review it.

Also added a safe wrapper around connect.

@agazso agazso requested a review from filoozom September 19, 2023 11:14
@agazso agazso merged commit dc74f35 into main Sep 19, 2023
@agazso agazso deleted the feat/safe-waku branch September 19, 2023 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

2 participants