kweb-core icon indicating copy to clipboard operation
kweb-core copied to clipboard

Improve handling of WebSocket connection failures

Open sanity opened this issue 6 years ago • 4 comments

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.

sanity avatar Aug 03 '19 15:08 sanity

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?

ethanmdavidson avatar Apr 17 '20 20:04 ethanmdavidson

Yes @ethanmdavidson, I think that approach makes sense.

sanity avatar Apr 18 '20 15:04 sanity

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.

sanity avatar Apr 18 '20 15:04 sanity

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.

alkoclick avatar Aug 21 '21 03:08 alkoclick

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.

github-actions[bot] avatar Oct 15 '22 03:10 github-actions[bot]

WebSocket backoff and connectivity was hardened.

sanity avatar Oct 15 '22 15:10 sanity