effection icon indicating copy to clipboard operation
effection copied to clipboard

How to handle closing socket connections?

Open jnicklas opened this issue 4 years ago • 1 comments

When implementing the @effection/websocket-server, I implemented it as a Subscription<Connection> where each item in the connection represents a new connection. Unfortunately this doesn't really work for a few reasons:

  • There is currently no elegant way to iterate this and spawn a new task for each connection. See #332
  • It is unclear when a connection should be closed from the server end. We don't want websocket connections to dangle unless they are actually being processed. Once we are "done" with a connection on the server, we should close it. This could also occur if there is an error in the connection.
  • Error handling is unclear. What happens if there is an error in a connection, should it crash the entire server, or just this connection?

We should come up with a better API which solves these problems.

jnicklas avatar Jun 14 '21 09:06 jnicklas

I think that the part of the api that is right is modeling both sides of the connection as a finite subscription of messages.

  • if the client closes the connection, the subscription on the server finishes
  • if the server closes the connection, the subscription on the client finishes

On top of that, we need to be able to handle errors at each level of the lifecycle: tree server -> connection -> message

On the server:

  • if an error happens on the server, it can be mitigated without crashing the spawner of the resource.
    • might need to see all open connections in order to customize the shutdown of each?
  • if an error happens with a single connection, it can be handled without effecting the server.
  • if an error happens with a single message, it can be mitigated without effecting the connection.

Could we make it so that each connection visitor is automatically spawned in the server scope, and each message visitor is automatically spawned in the connection scope? E.g.

run(function* () {
  let server = yield createWebSocketServer();
  try {
    // each visitor is automatically spawned in the server scope.
    // errors are propagated to the calling scope.
    // operation blocks forever
    yield server.forEach(function*(connection) {
      try {
        // each visitor is automatically spawned in a connection scope
        // errors are propgated to connection.
        // operation blocks until connection close
        yield connection.forEach(function*(message) {
          try {
            // handle message
          } catch (error) {
            console.log('message level error');
          }
        });
      } catch (error) {
        console.log('connection level error')
      }
    });
  } catch (error) {
    console.log('server level error')
  }
});

We could do this with a forAll operation on subscription, but it's difficult for me to imagine ever wanting to not do this, so maybe it makes sense to just do this with forEach here. In other words, it's always invoked asynchronously.

cowboyd avatar Jun 17 '21 18:06 cowboyd

We need to re-think websocket connection in the context of v3 streams and iteration. Closing for now until some best practices start emerging.

cowboyd avatar Oct 12 '23 16:10 cowboyd