-
Notifications
You must be signed in to change notification settings - Fork 35
peer disconnections #46
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
Conversation
Mesa DescriptionTL;DRAddresses peer disconnections by upgrading to our custom Neko image, re-enabling double cursor for better latency, and enhancing Neko authentication and debugging. Why we made these changesTo fix ongoing peer disconnection issues, improve user experience by reducing perceived latency, and integrate Neko with our internal authentication and build processes. What changed?
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What Changed
This PR addresses peer disconnection issues by switching to a custom-built neko image with a reduced heartbeat interval. A key change involves modifying the NekoClient's heartbeat mechanism in neko/index.ts to send a fake chat message, ensuring the connection remains active. Additionally, the authentication provider in neko.yaml has been updated from noauth to multiuser, with corresponding credential updates in connect.vue. Finally, based on user feedback, the double cursor has been restored by removing the cursor: none style in index.html.
Risks / Concerns
This is a great update that directly tackles the disconnection problems. The use of a fake chat message for the heartbeat is a clever workaround. While effective, we should consider creating a dedicated, long-term solution to avoid potential side effects in the future. The auth system changes look solid. Well done!
5 files reviewed | 0 comments | Review on Mesa | Edit Reviewer Settings
| this.sendMessage(EVENT.CLIENT.HEARTBEAT) | ||
| this.emit('debug', `sending chat/message`) | ||
| this.sendMessage(EVENT.CHAT.MESSAGE, { content: `heartbeat/fake [${Date.now()}]` }) | ||
| }, heartbeat_interval * 1000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| this.emit('debug', `sending chat/message`) | ||
| this.sendMessage(EVENT.CHAT.MESSAGE, { content: `heartbeat/fake [${Date.now()}]` }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@raiden-staging - just for context what's the motivation of this additional message type?
I thought we would be sufficient with just the heartbeat + server side response you added!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
event.CLIENT_HEARTBEATtriggers newevent.SYSTEM_HEARTBEATreply from neko, but doesn't show up on websockets trace. will fix on neko/ws handler + new neko build in few.event.CHAT_MESSAGE → replyis handling keepalive for now.- unikernel deployment seems to work well so cleanup for the above incoming 🧹✨
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incredible - thank you again for digging into this issue!
#45 plus
thanks very much to @raiden-staging !