kweb-core
kweb-core copied to clipboard
Improve handling of WebSocket connection failures
related: #68
In the event of a websocket connection failure, Kweb should exponentially back-off in the event that it is unable to establish a websocket connection - and display a clear error if repeated attempts to establish the connection are unsuccessful.
Started working on this in issue-69-ws_error_handling. I copied a small js library that handles automatically reconnecting, and it seems to work fine. However, because the browser doesn't tell us why the connection failed (which is apparently by design) we don't know if the server is misconfigured, if we just lost our internet connection, or if the server has been restarted/redeployed.
If we simply lost connection, then it will reconnect and resend the hello message. Is that okay, or do we need to make sure we only send the hello on first connect?
If the server has been restarted, then when the client reconnects it will still have the client id from the old server instance. Currently I'm thinking that when the server encounters a client id it doesn't recognize it can just send a message back to the client asking it to reload the page. Does this approach make sense to you?
Yes @ethanmdavidson, I think that approach makes sense.
Oh, it possible it might be desirable to queue up any messages on the client while the connection is being reestablished to be sent to the server when it connects.
I had an adjacent problem (persisting client connections through restarts and offloading them to external sources), so I wrote an implementation of websocket reconnects with exponential backoff and a user facing message. #231
If the server has been restarted, then when the client reconnects it will still have the client id from the old server instance.
I've implemented a solution for this as well btw, but it's external to Kweb because the clientState cache is now publicly available. It's pretty dirty still (might end up pointing it to a db instead of local file), but It looks like this:
File("clients.txt")
.takeIf { it.exists() }
?.forEachLine { server.clientState.put(it, RemoteClientState(id = it, clientConnection = ClientConnection.Caching())) }
server.refreshAllPages()
Runtime.getRuntime().addShutdownHook(thread(start = false) {
File("clients.txt").printWriter().use { out ->
server.clientState.asMap().keys.filterNotNull().forEach(out::println)
}
})
The reload is required because you can pass the session data to the new instance, but it's pretty hard to reconstruct the entire state of the old instance regarding pages etc, so it's better just to force reload.
Currently I'm thinking that when the server encounters a client id it doesn't recognize it can just send a message back to the client asking it to reload the page.
I considered this as well - ended up steering clear of it. It smells like a security or practicality issue, but couldn't put my finger on why exactly.
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.
WebSocket backoff and connectivity was hardened.