Skip to content

Conversation

loneil
Copy link
Contributor

@loneil loneil commented Dec 20, 2024

Leaving for draft for now, want to double check all retention/removal stuff so there's no memory leak possibility.

Buffer up messages that would be sent to disconnected clients. On client reconnect (just "connect" server side still) check the buffer for the pid from the client and re-emit messages.
Only keep the messages in the buffer for a fixed amount of seconds (can adjust this maybe with config if needed? Or can use the same timeout as the UI maybe since the user would be prompted to try again anyways if they leave off into the wallet app too long)

Some details on Socket.io handling disconnections:
https://socket.io/docs/v4/tutorial/handling-disconnections

Socket.io does have a built in Connection State Recovery:
https://socket.io/docs/v4/connection-state-recovery
I think this would accomplish what this code does here, however it's not implemented in the Python library VCAuth uses
see: miguelgrinberg/python-socketio#1258

Maybe we shouldn't be using this library? It's a pretty simple wrapper though but could be considerations on handling web sockets in a different way. We do however need to consider that storing in-memory is not what we want forever on VCAuth and need to be able to hook into (DB likely) persistence for multi-pod deployment. The code here should be able to handle that need when we address it.

Update the python socket library too (and other python libraries), and socket.io client JS.

@loneil loneil force-pushed the feature/queueMessages branch 2 times, most recently from 1514a99 to a3b0302 Compare December 21, 2024 00:13
@coveralls
Copy link

coveralls commented Dec 21, 2024

Pull Request Test Coverage Report for Build 12440513774

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 85.254%

Totals Coverage Status
Change from base Build 12416479364: 0.0%
Covered Lines: 688
Relevant Lines: 807

💛 - Coveralls

@loneil loneil force-pushed the feature/queueMessages branch from 43fe8eb to 687279c Compare December 21, 2024 00:36
@loneil loneil requested a review from esune December 21, 2024 00:53
loneil added 3 commits April 3, 2025 22:17
Signed-off-by: Lucas ONeil <lucasoneil@gmail.com>
Signed-off-by: Lucas ONeil <lucasoneil@gmail.com>
Signed-off-by: Lucas ONeil <lucasoneil@gmail.com>
@loneil loneil force-pushed the feature/queueMessages branch from 687279c to daa74fa Compare April 4, 2025 05:19
@loneil
Copy link
Contributor Author

loneil commented Jul 22, 2025

Leaving this draft up for reference for now but we are looking into websocket refactoring as part of High-Availability drive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants